fix: sandbox worker sessions - #14
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details🔇 Additional comments (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAgentLoop now documents its target audience and worker sandbox configuration, adds tests for explicit Codex approval settings, and introduces a query parser example with planning, requirements, implementation, and independent evaluation documentation. ChangesWorker sandbox controls
Query parser evaluation
Estimated code review effort: 2 (Simple) | ~10 minutes Comment |
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/evaluation.md`:
- Line 3: Update the evaluation date in the document’s opening statement to the
actual execution date, July 21, 2026, or rerun the evaluation and record that
resulting date; keep the surrounding run details unchanged.
In `@examples/query-parser/query-string.js`:
- Around line 1-6: The query parser in parseQuery must handle empty queries,
convert plus signs to spaces, preserve repeated keys as arrays, assign empty
values to bare keys, tolerate malformed percent-encoding, and retain additional
equals signs in values; update its parsing and decoding logic accordingly. In
docs/evaluation.md lines 7-10, rerun the independent evaluation against the
corrected parser and record the resulting status and actual test count.
- Line 9: Extend the module containing parseQuery with a command-line entry
point in addition to its existing export. When invoked directly, read the query
argument, output the parsed result as JSON, and display usage plus exit non-zero
when the required argument is missing; preserve parseQuery’s reusable module
behavior when imported.
In `@examples/query-parser/STATE.md`:
- Around line 3-13: Update the state entries in STATE.md to reflect that the
query parser repair is complete and verification has succeeded: replace the
unfinished “Completed” status, remove the pending repair from “Next,” and update
“Notes” to record the available evaluation or critic feedback instead of
claiming none exists.
In `@package.json`:
- Around line 6-8: Update the package.json test script to use a test invocation
supported by Node.js 18.0.x, preserving the existing test coverage and the
declared Node.js 18 minimum; alternatively, consistently raise the minimum
Node.js requirement to 18.1.0 in the package metadata and documentation.
In `@test/sandbox.test.js`:
- Around line 8-13: Update the sandbox contract test around spawnLoopSession and
spawnWorker to inspect each function’s args block independently rather than
relying on global occurrence counts. Assert that both spawn paths separately
include workspace-write sandboxing, on-request approval policy, auto_review
approvals, and disabled network access, while retaining the bypass-flag
assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 93e5ee69-ebef-4461-9877-4d2775cc9593
📒 Files selected for processing (9)
README.mddocs/evaluation.mdexamples/query-parser/GUIDELINES.mdexamples/query-parser/PLAN.mdexamples/query-parser/STATE.mdexamples/query-parser/query-string.jspackage.jsonsrc/daemon.jstest/sandbox.test.js
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
test/sandbox.test.js
[warning] 5-5: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(path.join(__dirname, '..', 'src', 'daemon.js'), 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🔇 Additional comments (4)
examples/query-parser/GUIDELINES.md (1)
3-12: LGTM!examples/query-parser/PLAN.md (1)
3-8: LGTM!src/daemon.js (1)
769-772: LGTM!Also applies to: 1264-1267
README.md (1)
13-14: LGTM!Also applies to: 37-37, 47-52, 142-143
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9331de64e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sandboxes worker and critic sessions without changing local workflows. Adds regression coverage and a reproducible FAIL then PASS evaluation.
Review fixes applied: