Repository navigation
fix(http): keep unwrapping legacy success envelopes for existing workflows - #9909
Merged
Merged
Conversation
…flows #9889 fixed JSend-style 200s ({status, data}) being reported as failures by deleting the old /api/proxy envelope branch outright. That also changed two behaviors existing workflows rely on: 200 bodies with success/status/data were unwrapped (<api.data.x> read the inner data) and success: false failed the block. Restore both through one shared toHttpToolResponse helper, gated on the body carrying a success field — the only shape the old route ever produced — so only the misclassified case changes. Non-string envelope errors are now serialized instead of surfacing as [object Object].
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
 [Medium impact] The PR appears safe to merge. SummaryRestores legacy response unwrapping for HTTP and webhook calls while keeping JSend-style bodies intact.
waleedlatif1 explicitly describes replacing HTTP fields with legacy envelope fields as intentional compatibility behavior. Reviews (1) · Last reviewed commit: "fix(http): keep unwrapping legacy succes..." · Reviewed by Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 3/5
apps/sim/tools/http/utils.tscan treat a legacy response withsuccess: "false"as a successful tool call. Requirebody.successto be a boolean before unwrapping the envelope.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/http/utils.ts">
<violation number="1" location="apps/sim/tools/http/utils.ts:172">
P2: The legacy-envelope check accepts non-boolean success values, so a response containing `success: "false"` is reported as a successful tool call. Require `body.success` to be a boolean before unwrapping the envelope.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/api/proxyenvelope branch fixed JSend-style 200s, but it also changed two behaviors existing API/Webhook blocks have had since that route was removed:success,status, anddatawas unwrapped, so<api.data.x>read the innerdatasuccess: falsefailed the block with the body'serror, so error branches rantoHttpToolResponsehelper intools/http/utils.ts, gated on the body carrying asuccessfield — the only shape the old route ever produced (success: response.ok). A body without it (JSend{status, data}) stays an ordinary response, which is the [BUG] http_request / webhook_request report a successful HTTP 200 as failed when the JSON body has top-level status and data keys #9838 fixsuccessfield change. Non-string envelope errors are now JSON-serialized instead of reaching the caller as an object ([object Object]in the API block)Type of Change
Testing
tools/index.test.ts: both real tools throughexecuteToolfor JSend,success: trueunwrap, andsuccess: falsewith string and object errors. Red on fix(http): stop treating 200 bodies with status and data keys as proxy responses #9889 as merged (6 compat cases), red on pre-fix(http): stop treating 200 bodies with status and data keys as proxy responses #9889 code only for the JSend case and the object-error serialization, green with this changebunx vitest run tools/index.test.ts tools/http executor/handlers/api(284 passed), biome,check:utils,check:test-patterns,check:comment-hygiene; full gate in CIChecklist
test-auditauthoring gate)