Revert "fix(security): RQ-2426 — sandbox rule code (QuickJS-WASM)"#115
Conversation
…)" This reverts commit 67de108.
WalkthroughThe PR removes the QuickJS/WASM sandbox execution infrastructure entirely. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.0)src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.jsFile contains syntax errors that prevent linting: Line 1: Illegal use of an import declaration outside of a module; Line 3: Illegal use of an import declaration outside of a module; Line 6: Illegal use of an import declaration outside of a module; Line 7: Illegal use of an import declaration outside of a module; Line 8: Illegal use of an import declaration outside of a module; Line 9: Illegal use of an import declaration outside of a module; Line 10: Illegal use of an import declaration outside of a module; Line 185: Illegal use of an export declaration outside of a module src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.jsFile contains syntax errors that prevent linting: Line 1: Illegal use of an import declaration outside of a module; Line 3: Illegal use of an import declaration outside of a module; Line 6: Illegal use of an import declaration outside of a module; Line 7: Illegal use of an import declaration outside of a module; Line 8: Illegal use of an import declaration outside of a module; Line 91: Illegal use of an export declaration outside of a module Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js (1)
138-143: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse Error Code 7201 for parsed non-functions too.
This branch is still a parse/validation failure, so returning legacy 944 conflicts with the new error-code path.
Proposed fix
- "Can't parse Requestly function. Please recheck. Error Code 944." + "Can't parse Requestly function. Please recheck. Error Code 7201."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js` around lines 138 - 143, The invalid user function branch in modify_response_processor’s modify_response flow is still returning the legacy parse error, which should now use the new validation code path instead. Update the early return in the userFunction type check to surface Error Code 7201 instead of 944, keeping the same modify_response call and message shape so parsed non-functions are treated consistently with the new error-code handling.src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js (2)
46-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse Error Code 7201 for parsed non-functions too.
A value like
42parses but is still not a valid Requestly function; keep this on the new parse-error contract instead of falling back to legacy 944.Proposed fix
- "Can't parse Requestly function. Please recheck. Error Code 944." + "Can't parse Requestly function. Please recheck. Error Code 7201."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js` around lines 46 - 51, The invalid-function branch in modify_request_processor currently falls back to the legacy 944 message for values that parse but are not callable; update the check around userFunction and modify_request so parsed non-functions (for example, non-function values returned by parsing) use the new parse-error contract with Error Code 7201 instead of 944. Keep the existing legacy handling only for truly unparseable input, and distinguish that path from the parsed-but-not-a-function case in the same processor.
21-22: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winAwait the request-body rewrite before reporting success.
modify_request_using_codereturns a Promise, butprocess_modify_request_actionignores it, andRuleActionProcessor.process_actionalso does not await this branch. That can mark the action successful beforectx.rq_request_bodyis updated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js` around lines 21 - 22, `process_modify_request_action` is returning success before the request-body rewrite finishes because `modify_request_using_code` is asynchronous and its Promise is ignored. Update `process_modify_request_action` to await `modify_request_using_code(action, ctx)` before calling `build_action_processor_response`, and make sure the `RuleActionProcessor.process_action` path also awaits this branch so `ctx.rq_request_body` is updated before success is reported.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js`:
- Around line 35-37: Avoid executing untrusted request code during parse
validation in modify_request_processor.js: the try/catch around
getFunctionFromString(action.request) is currently evaluating user input too
early, so move parsing to a non-executing validation path and only
compile/execute inside modify_request_using_code when the request is actually
being modified. Also fix the async flow in modify_request_using_code by awaiting
the body-update path before continuing the proxy middleware, so the modification
completes before the request proceeds.
In
`@src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js`:
- Around line 127-129: Avoid evaluating action.response twice in
modify_response_processor.js: the current flow in the getFunctionFromString and
executeUserFunction path recompiles the same source after it has already been
parsed. Update the processor to compile the user function once from
action.response, then reuse that compiled function for execution instead of
passing the raw string again, so the modify response handling uses a single
compile/execute path.
In `@src/utils/index.ts`:
- Around line 6-8: The getFunctionFromString helper is executing user-controlled
rule strings with new Function, which runs code in the host realm. Replace this
with a parse-only validation path in getFunctionFromString and move actual rule
execution into the existing isolated VM/sandbox flow so untrusted rules never
get access to the host process/global environment. Keep the fix scoped around
getFunctionFromString and any callers that currently rely on it for execution.
- Around line 25-37: Move the console shutdown in the generated-function
execution path into a finally block so interception is always released. In the
logic around ConsoleCapture.start, generatedFunction(args), and
consoleCapture.stop, ensure stop() runs whether generatedFunction throws or the
awaited promise rejects, then keep collecting consoleCapture.getCaptures() and
pushing into ctx.rq.consoleLogs after cleanup.
- Around line 30-32: The finalResponse handling in the result flow should not
rely on util.types.isPromise, since it misses thenables from
generatedFunction(args). Update the logic around finalResponse to await it
directly so plain values, native promises, and thenables are all normalized
before the stringify/object path runs.
---
Outside diff comments:
In
`@src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js`:
- Around line 46-51: The invalid-function branch in modify_request_processor
currently falls back to the legacy 944 message for values that parse but are not
callable; update the check around userFunction and modify_request so parsed
non-functions (for example, non-function values returned by parsing) use the new
parse-error contract with Error Code 7201 instead of 944. Keep the existing
legacy handling only for truly unparseable input, and distinguish that path from
the parsed-but-not-a-function case in the same processor.
- Around line 21-22: `process_modify_request_action` is returning success before
the request-body rewrite finishes because `modify_request_using_code` is
asynchronous and its Promise is ignored. Update `process_modify_request_action`
to await `modify_request_using_code(action, ctx)` before calling
`build_action_processor_response`, and make sure the
`RuleActionProcessor.process_action` path also awaits this branch so
`ctx.rq_request_body` is updated before success is reported.
In
`@src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js`:
- Around line 138-143: The invalid user function branch in
modify_response_processor’s modify_response flow is still returning the legacy
parse error, which should now use the new validation code path instead. Update
the early return in the userFunction type check to surface Error Code 7201
instead of 944, keeping the same modify_response call and message shape so
parsed non-functions are treated consistently with the new error-code handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f65fd52-7b1c-42d2-b25c-d938ecde4d39
⛔ Files ignored due to path filters (7)
dist/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.jsis excluded by!**/dist/**dist/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.jsis excluded by!**/dist/**dist/utils/index.d.tsis excluded by!**/dist/**dist/utils/index.jsis excluded by!**/dist/**dist/utils/sandbox-globals.d.tsis excluded by!**/dist/**dist/utils/sandbox-globals.jsis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.jssrc/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.jssrc/utils/index.tssrc/utils/sandbox-globals.ts
💤 Files with no reviewable changes (2)
- src/utils/sandbox-globals.ts
- package.json
| try { | ||
| userFunction = getFunctionFromString(action.request); | ||
| } catch (error) { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file outline ==\n'
ast-grep outline src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js --view expanded || true
printf '\n== relevant file excerpt ==\n'
sed -n '1,220p' src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js | cat -n
printf '\n== locate helper definitions/usages ==\n'
rg -n "getFunctionFromString|executeUserFunction|modify_request_using_code|process_modify_request_action" src -g '!**/node_modules/**' || trueRepository: requestly/requestly-proxy
Length of output: 6058
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/utils/index.ts excerpt ==\n'
sed -n '1,220p' src/utils/index.ts | cat -n
printf '\n== rule action processor caller excerpt ==\n'
sed -n '1,180p' src/components/proxy-middleware/rule_action_processor/index.js | cat -n
printf '\n== response processor excerpt for comparison ==\n'
sed -n '1,240p' src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js | cat -nRepository: requestly/requestly-proxy
Length of output: 14673
🏁 Script executed:
node - <<'JS'
function getFunctionFromString(functionStringEscaped) {
return new Function(`return ${functionStringEscaped}`)();
}
console.log('== parse validation probe ==');
for (const src of [
'(() => 1)',
'(() => { console.log("side effect"); return () => 1; })()',
'((console.log("comma side effect"), () => 1))',
]) {
try {
const out = getFunctionFromString(src);
console.log(src, '=>', typeof out, out && out.name ? out.name : '');
} catch (e) {
console.log(src, '=> throws:', e.message);
}
}
console.log('\n== missing await probe ==');
async function modify_request_using_code() {
await new Promise((r) => setTimeout(r, 0));
globalThis.flag = true;
}
function process_modify_request_action() {
modify_request_using_code();
return 'returned immediately';
}
globalThis.flag = false;
console.log(process_modify_request_action(), 'flag=', globalThis.flag);
setTimeout(() => console.log('later flag=', globalThis.flag), 10);
JSRepository: requestly/requestly-proxy
Length of output: 452
Avoid evaluating user code during parse validation src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js:35-37
getFunctionFromString() runs new Function(... )() on untrusted input here, so top-level expressions can execute before the actual request-modification path. modify_request_using_code() is also fired without await, so the body update can land after the proxy flow has already moved on.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js`
around lines 35 - 37, Avoid executing untrusted request code during parse
validation in modify_request_processor.js: the try/catch around
getFunctionFromString(action.request) is currently evaluating user input too
early, so move parsing to a non-executing validation path and only
compile/execute inside modify_request_using_code when the request is actually
being modified. Also fix the async flow in modify_request_using_code by awaiting
the body-update path before continuing the proxy middleware, so the modification
completes before the request proceeds.
| try { | ||
| userFunction = getFunctionFromString(action.response); | ||
| } catch (error) { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js'
echo '--- outline ---'
ast-grep outline "$FILE" --view expanded || true
echo '--- relevant lines ---'
nl -ba "$FILE" | sed -n '1,260p'Repository: requestly/requestly-proxy
Length of output: 612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js'
python3 - <<'PY'
from pathlib import Path
p = Path("src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js")
text = p.read_text()
for start, end in [(1, 240)]:
for i, line in enumerate(text.splitlines(), 1):
if start <= i <= end:
print(f"{i:4d}: {line}")
PYRepository: requestly/requestly-proxy
Length of output: 7454
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "getFunctionFromString|executeUserFunction|modify_response_using_code|action.response" src/components/proxy-middleware -SRepository: requestly/requestly-proxy
Length of output: 3072
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
paths = [
"src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js",
"src/components/proxy-middleware",
]
p = Path(paths[0])
text = p.read_text().splitlines()
for start, end in [(1, 230)]:
for i in range(start, min(end, len(text)) + 1):
print(f"{i:4d}: {text[i-1]}")
PY
echo '--- search ---'
rg -n "getFunctionFromString|executeUserFunction|modify_response_using_code|modify_response_using_local" src/components/proxy-middleware -SRepository: requestly/requestly-proxy
Length of output: 9073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "function getFunctionFromString|const getFunctionFromString|executeUserFunction" src utils -SRepository: requestly/requestly-proxy
Length of output: 1234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the utility definitions precisely
fd -a 'utils' src . 2>/dev/null | head -n 20
rg -n "getFunctionFromString|executeUserFunction" . -g '!**/node_modules/**' -SRepository: requestly/requestly-proxy
Length of output: 3594
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("src/utils/index.ts")
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 220:
print(f"{i:4d}: {line}")
PYRepository: requestly/requestly-proxy
Length of output: 2554
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("src/components/proxy-middleware/rule_action_processor/processors/modify_request_processor.js")
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 180:
print(f"{i:4d}: {line}")
PYRepository: requestly/requestly-proxy
Length of output: 3420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("src/utils/index.ts")
lines = p.read_text().splitlines()
for i in range(1, len(lines)+1):
if i <= 120:
print(f"{i:4d}: {lines[i-1]}")
PYRepository: requestly/requestly-proxy
Length of output: 2554
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("src/utils/index.ts")
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
print(f"{i:4d}: {line}")
PYRepository: requestly/requestly-proxy
Length of output: 2554
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("src/utils/index.ts")
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
print(f"{i:4d}: {line}")
PYRepository: requestly/requestly-proxy
Length of output: 2554
Avoid evaluating action.response twice. getFunctionFromString(action.response) already evaluates the source, then executeUserFunction(ctx, action.response, args) rebuilds and evaluates it again from the raw string. That duplicates work in the proxy process and can replay expression-level side effects; use a single compile/execute path here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/components/proxy-middleware/rule_action_processor/processors/modify_response_processor.js`
around lines 127 - 129, Avoid evaluating action.response twice in
modify_response_processor.js: the current flow in the getFunctionFromString and
executeUserFunction path recompiles the same source after it has already been
parsed. Update the processor to compile the user function once from
action.response, then reuse that compiled function for execution instead of
passing the raw string again, so the modify response handling uses a single
compile/execute path.
| export const getFunctionFromString = function (functionStringEscaped) { | ||
| return new Function(`return ${functionStringEscaped}`)(); | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not execute rule code in the host realm.
Both validation and execution now evaluate user-controlled rule strings with new Function, giving rule code access to the proxy process/global environment. This reintroduces arbitrary code execution risk and bypasses the sandbox boundary this path needs. Keep execution inside an isolated VM/sandbox and make validation parse-only. As flagged by static analysis.
Also applies to: 20-20
🧰 Tools
🪛 OpenGrep (1.23.0)
[ERROR] 7-7: new Function() with dynamic input can execute arbitrary code. Avoid dynamic code evaluation entirely, or use a safe alternative.
(coderabbit.code-injection.new-function-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/index.ts` around lines 6 - 8, The getFunctionFromString helper is
executing user-controlled rule strings with new Function, which runs code in the
host realm. Replace this with a parse-only validation path in
getFunctionFromString and move actual rule execution into the existing isolated
VM/sandbox flow so untrusted rules never get access to the host process/global
environment. Keep the fix scoped around getFunctionFromString and any callers
that currently rely on it for execution.
Source: Linters/SAST tools
| const consoleCapture = new ConsoleCapture() | ||
| consoleCapture.start(true) | ||
|
|
||
| if (typeof output === "string") break; // settled | ||
| if (Date.now() > overallDeadline) break; // timed out | ||
| if (inflight.length === 0) break; // nothing pending → chain won't progress | ||
| const batch = inflight.splice(0); | ||
| await Promise.race([ | ||
| Promise.allSettled(batch), | ||
| new Promise((r) => setTimeout(r, Math.max(0, overallDeadline - Date.now()))), | ||
| ]); | ||
| } | ||
| let finalResponse = generatedFunction(args); | ||
|
|
||
| if (typeof output !== "string") { | ||
| // No __OUTPUT and nothing left to await → timed out / never settled. | ||
| reportSandboxError("timeout", "rule execution timed out or never settled"); | ||
| throw new SandboxError("Execution timed out", "timeout"); | ||
| if (types.isPromise(finalResponse)) { | ||
| finalResponse = await finalResponse; | ||
| } | ||
|
|
||
| let parsed: any; | ||
| try { | ||
| parsed = JSON.parse(output); | ||
| } catch { | ||
| // We control the marshaling, so malformed __OUTPUT is our bug. | ||
| const msg = "sandbox produced invalid output"; | ||
| reportSandboxError("prelude", msg); | ||
| throw new SandboxError(msg, "prelude"); | ||
| } | ||
| consoleCapture.stop() | ||
| const consoleLogs = consoleCapture.getCaptures() | ||
|
|
||
| ctx.rq.consoleLogs.push(...consoleLogs) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Stop console capture in a finally block.
If generatedFunction(args) throws or the awaited promise rejects, consoleCapture.stop() is skipped while downstream code catches the error and continues. That can leave console interception active after this rule execution.
Proposed fix
- const consoleCapture = new ConsoleCapture()
- consoleCapture.start(true)
-
- let finalResponse = generatedFunction(args);
-
- if (types.isPromise(finalResponse)) {
- finalResponse = await finalResponse;
- }
-
- consoleCapture.stop()
- const consoleLogs = consoleCapture.getCaptures()
-
- ctx.rq.consoleLogs.push(...consoleLogs)
+ const consoleCapture = new ConsoleCapture();
+ consoleCapture.start(true);
+
+ let finalResponse;
+ try {
+ finalResponse = generatedFunction(args);
+
+ if (types.isPromise(finalResponse)) {
+ finalResponse = await finalResponse;
+ }
+ } finally {
+ consoleCapture.stop();
+ const consoleLogs = consoleCapture.getCaptures();
+ ctx.rq.consoleLogs.push(...consoleLogs);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const consoleCapture = new ConsoleCapture() | |
| consoleCapture.start(true) | |
| if (typeof output === "string") break; // settled | |
| if (Date.now() > overallDeadline) break; // timed out | |
| if (inflight.length === 0) break; // nothing pending → chain won't progress | |
| const batch = inflight.splice(0); | |
| await Promise.race([ | |
| Promise.allSettled(batch), | |
| new Promise((r) => setTimeout(r, Math.max(0, overallDeadline - Date.now()))), | |
| ]); | |
| } | |
| let finalResponse = generatedFunction(args); | |
| if (typeof output !== "string") { | |
| // No __OUTPUT and nothing left to await → timed out / never settled. | |
| reportSandboxError("timeout", "rule execution timed out or never settled"); | |
| throw new SandboxError("Execution timed out", "timeout"); | |
| if (types.isPromise(finalResponse)) { | |
| finalResponse = await finalResponse; | |
| } | |
| let parsed: any; | |
| try { | |
| parsed = JSON.parse(output); | |
| } catch { | |
| // We control the marshaling, so malformed __OUTPUT is our bug. | |
| const msg = "sandbox produced invalid output"; | |
| reportSandboxError("prelude", msg); | |
| throw new SandboxError(msg, "prelude"); | |
| } | |
| consoleCapture.stop() | |
| const consoleLogs = consoleCapture.getCaptures() | |
| ctx.rq.consoleLogs.push(...consoleLogs) | |
| const consoleCapture = new ConsoleCapture(); | |
| consoleCapture.start(true); | |
| let finalResponse; | |
| try { | |
| finalResponse = generatedFunction(args); | |
| if (types.isPromise(finalResponse)) { | |
| finalResponse = await finalResponse; | |
| } | |
| } finally { | |
| consoleCapture.stop(); | |
| const consoleLogs = consoleCapture.getCaptures(); | |
| ctx.rq.consoleLogs.push(...consoleLogs) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/index.ts` around lines 25 - 37, Move the console shutdown in the
generated-function execution path into a finally block so interception is always
released. In the logic around ConsoleCapture.start, generatedFunction(args), and
consoleCapture.stop, ensure stop() runs whether generatedFunction throws or the
awaited promise rejects, then keep collecting consoleCapture.getCaptures() and
pushing into ctx.rq.consoleLogs after cleanup.
| if (types.isPromise(finalResponse)) { | ||
| finalResponse = await finalResponse; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Await finalResponse directly. util.types.isPromise only catches native Promise instances, so thenables returned from generatedFunction(args) can fall through to the object/stringify path. await finalResponse handles plain values, promises, and thenables here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/index.ts` around lines 30 - 32, The finalResponse handling in the
result flow should not rely on util.types.isPromise, since it misses thenables
from generatedFunction(args). Update the logic around finalResponse to await it
directly so plain values, native promises, and thenables are all normalized
before the stringify/object path runs.
2 participants
Reverts #112
Summary by CodeRabbit
New Features
Bug Fixes