Repository navigation
release: v6.9.6 - #66
Conversation
There was a problem hiding this comment.
🧪 PR Review is completed: Headless JSON envelope, --output-file artifact, and custom-endpoint support are well built; flagging a MATTERAI_MODEL env clobber in the --baseUrl default, a stdout-flush race before process.exit in --json mode, a fragile exact-path match in the output-file approval bypass, and tautological tests that never exercise the production approval logic. package.json: clean version bump + test script.
Skipped files
CHANGELOG.md: Skipped file patternpackage-lock.json: Skipped file pattern
⬇️ Low Priority Suggestions (3)
src/headless.ts (2 suggestions)
Location:
src/headless.ts(Lines 302-302)🟡 Reliability
Issue: Node's docs warn that
process.exit()truncates pending asynchronous stdout writes, and piped stdout is asynchronous on macOS. The new--jsonenvelope write happens immediately before the unconditionalprocess.exit(exitCode)at the end ofrunHeadless, and--jsonis precisely the mode designed to be piped (orbcode -p … --json | jq) — with a largeresultthe envelope can be intermittently truncated or lost.Fix: Exit from the write callback so the envelope is flushed before the process terminates. (This intentionally skips the stderr resume hint in JSON mode;
sessionIdis already in the envelope.)Impact: Guarantees the JSON envelope is delivered on all platforms; eliminates intermittent empty/truncated output when piping.
- process.stdout.write(JSON.stringify(envelope) + "\n") + process.stdout.write(JSON.stringify(envelope) + "\n", () => process.exit(exitCode)) + returnLocation:
src/headless.ts(Lines 248-248)🟡 Robustness
Issue: The auto-approval compares the model's raw
file_path(request.detail) against the resolved output path with strict string equality. Models routinely relativize paths or add./despite the prompt instruction; in that case the write is denied and--output-filesilently degrades to the chat-text fallback that this PR's own comments call unreliable for weaker models.Fix: Resolve
request.detailbefore comparing —result.json,./result.json, and trailing-slash variants now match the same file, while different targets (result.json.bak, traversal segments) still resolve to a different path and stay denied. Therequest.detail &&guard keeps an empty path from resolving tocwd().Impact: The output-file artifact and its approval bypass work regardless of how the model formats the path, without widening what gets auto-approved.
- if (outputFile && request.toolName === "file_write" && request.detail === path.resolve(outputFile)) { + if (outputFile && request.toolName === "file_write" && request.detail && path.resolve(request.detail) === path.resolve(outputFile)) {
test/headless.test.ts (1 suggestion)
Location:
test/headless.test.ts(Lines 23-34)🟡 Test Quality
Issue: These tests never exercise the production code. They restate the condition inline with literal comparisons —
"file_write" === "file_write"(always true),"execute_command" === "file_write"(always false),undefined !== undefined(always false) — and the "JSON envelope structure" tests assert properties of an object they just constructed. They pass even ifrunHeadless's approval logic regresses (e.g., reverts to substring matching), giving false confidence on a security-relevant auto-approval path.Fix: Extract the approval predicate in
src/headless.tsinto an exportedshouldApproveOutputFile(outputFile, toolName, detail)helper used by therequestApprovalcallback, import it here, and test the real function across all cases; drop the self-asserting envelope tests.Impact: The tests actually pin the bypass behavior — a regression in
headless.tsnow fails CI instead of silently passing.- it("should approve file_write to the exact output file path", () => { - const resolvedOutput = path.resolve(outputFile) - const requestDetail = resolvedOutput - - // This is the logic from headless.ts requestApproval callback - const shouldApprove = - outputFile !== undefined && - "file_write" === "file_write" && - requestDetail === path.resolve(outputFile) - - assert.strictEqual(shouldApprove, true, "Should approve exact match") - }) + it("should approve file_write to the exact output file path", () => { + const requestDetail = path.resolve(outputFile) + + assert.strictEqual( + shouldApproveOutputFile(outputFile, "file_write", requestDetail), + true, + "Should approve exact match", + ) + })
- Don't overwrite MATTERAI_MODEL when --baseUrl is given without --model; runHeadless already defaults to gpt-4o. - Flush stdout/stderr before process.exit so piped --json output isn't truncated on macOS. - Resolve the file_write path before matching --output-file, so relative spellings are auto-approved while other targets stay denied. - Extract isOutputFileWrite and test the real predicate instead of restated logic.
|
✅ Reviewed the changes: Clean follow-up — all four prior review comments are addressed: the MATTERAI_MODEL-clobbering default block is deleted, both stdio streams are flushed before process.exit, the output-file approval is extracted into a path-resolving isOutputFileWrite helper, and the tests now exercise the real function. Reviewed src/headless.ts: no issues found. Reviewed test/headless.test.ts: no issues found. Reviewed src/index.tsx: no issues found. |
Summary
Headless mode (
orbcode -p) gets features for programmatic use:--json: prints exactly one JSON envelope to stdout:{ ok, model, result, usage, sessionId, error }. Everything else goes to stderr.--baseUrl/--apiKey: route through any OpenAI-compatible endpoint instead of the MatterAI gateway. They use dedicatedMATTERAI_LLM_*env names so they don't clash with the gateway'sMATTERAI_BASE_URL/MATTERAI_API_KEY. The model defaults togpt-4owhen--modelisn't given.--require-model: exits non-zero on an unknown model instead of silently using the default. This is automatic with--json.--output-file <path>: the agent writes its final result to this file, and headless mode reads it back. Writes to that exact path are auto-approved even without--yolo.--verbose: streams tool-start/tool-end events to stderr.Version bumped to 6.9.6, and the changelog is updated.
Fixes made during release validation
usage.totalCostsummed the agent's running total on every usage event, so it over-counted. It now takes the latest value.--baseUrlfalls back toOPENAI_API_KEY/ANTHROPIC_API_KEYfrom the environment. The OpenAI-compatible transport doesn't do that, so the claim is removed.Validation
npm run typecheckandnpm run buildpasstest:uipasses 21/21--json --model <unknown>exits 1 with a clear error--json --baseUrl http://127.0.0.1:1/v1prints a valid{"ok":false,…}envelope on stdout and exits 1