Skip to content

Conversation

@MarceloRGonc
Copy link
Contributor

Part of OPS-2972.

Copilot AI review requested due to automatic review settings November 5, 2025 22:51
@linear
Copy link

linear bot commented Nov 5, 2025

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies the flow response handling for succeeded flows to support custom stop responses. When a flow completes successfully, it now checks for a custom stopResponse object and uses its status, body, and headers if present, otherwise returns a default success message.

Key Changes:

  • Added conditional handling for custom stop responses in succeeded flows
  • Changed default success response from NO_CONTENT to OK with a message body

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +325 to +327
status: result.stopResponse?.status ?? StatusCodes.OK,
body: result.stopResponse?.body,
headers: result.stopResponse?.headers ?? {},
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optional chaining on line 323 already checks if stopResponse exists, making the additional optional chaining (?.) on line 325 redundant. Since we're inside the if (result.stopResponse) block, you can safely access result.stopResponse.status directly.

Suggested change
status: result.stopResponse?.status ?? StatusCodes.OK,
body: result.stopResponse?.body,
headers: result.stopResponse?.headers ?? {},
status: result.stopResponse.status ?? StatusCodes.OK,
body: result.stopResponse.body,
headers: result.stopResponse.headers ?? {},

Copilot uses AI. Check for mistakes.
Comment on lines +325 to +327
status: result.stopResponse?.status ?? StatusCodes.OK,
body: result.stopResponse?.body,
headers: result.stopResponse?.headers ?? {},
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're inside the if (result.stopResponse) block, the optional chaining (?.) is unnecessary. You can access result.stopResponse.body directly.

Suggested change
status: result.stopResponse?.status ?? StatusCodes.OK,
body: result.stopResponse?.body,
headers: result.stopResponse?.headers ?? {},
status: result.stopResponse.status ?? StatusCodes.OK,
body: result.stopResponse.body,
headers: result.stopResponse.headers ?? {},

Copilot uses AI. Check for mistakes.
Comment on lines +325 to +327
status: result.stopResponse?.status ?? StatusCodes.OK,
body: result.stopResponse?.body,
headers: result.stopResponse?.headers ?? {},
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're inside the if (result.stopResponse) block, the optional chaining (?.) is redundant. You can access result.stopResponse.headers directly.

Suggested change
status: result.stopResponse?.status ?? StatusCodes.OK,
body: result.stopResponse?.body,
headers: result.stopResponse?.headers ?? {},
status: result.stopResponse.status ?? StatusCodes.OK,
body: result.stopResponse.body,
headers: result.stopResponse.headers ?? {},

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 5, 2025

@MarceloRGonc MarceloRGonc marked this pull request as draft November 6, 2025 12:58
@MarceloRGonc MarceloRGonc changed the title WIP Webhook trigger /sync should receive response from HTTP action Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants