Draft: Autonomous Orchestrator CLI#1026
Conversation
📝 WalkthroughWalkthroughThe PR adds an autonomous CLI profile with task orchestration, terminal-state reporting, cancellation, persistence, provider configuration, validation, documentation, and smoke tests. It also consolidates agent guidance and adds platform-specific ripgrep binary resolution with integration coverage. ChangesAutonomous CLI execution
Agent guidance consolidation
Platform-specific ripgrep resolution
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ExtensionHost
participant RooCodeAPI
participant JsonEventEmitter
CLI->>ExtensionHost: start autonomous task
ExtensionHost->>RooCodeAPI: dispatch task and monitor events
RooCodeAPI-->>ExtensionHost: completion, input, timeout, or cancellation event
ExtensionHost-->>CLI: task result or terminal error state
CLI->>JsonEventEmitter: emit authoritative terminal result
JsonEventEmitter-->>CLI: JSON or NDJSON output
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![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
🧹 Nitpick comments (6)
src/core/auto-approval/__tests__/autonomous-cli.spec.ts (1)
11-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider testing that the env var alone (without
alwaysAllowMcp) does not bypass approval.The current test only verifies the env var toggles approval when
alwaysAllowMcp: true. Given this is a security-sensitive auto-approval bypass, an additional case assertingROO_CLI_AUTONOMOUS=1withalwaysAllowMcp: falsestill returns{ decision: "ask" }would guard against a future regression weakening the AND-condition incheckAutoApproval.🤖 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/core/auto-approval/__tests__/autonomous-cli.spec.ts` around lines 11 - 22, Extend the test for checkAutoApproval to cover ROO_CLI_AUTONOMOUS=1 with alwaysAllowMcp set to false, asserting the result remains { decision: "ask" }; retain the existing enabled-case assertions and ensure the environment variable is cleaned up afterward.apps/cli/src/agent/json-event-emitter.ts (1)
215-256: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
resumableis hardcodedfalsefor every terminal state, includingneeds_input.
needs_inputconceptually represents a paused task awaiting more input, not necessarily an unresumable failure, and the payload already surfacesrootTaskIdexplicitly "for lineage-aware consumers." Hardcodingresumable: falseregardless ofstatemay under-report actual resumability once/if autonomous session resumption is wired up, or it may simply be an intentional placeholder for this draft. Worth confirming intended semantics and, ifneeds_inputsessions are in fact resumable via--session-id, derivingresumablefromstateinstead of a constant.♻️ Possible direction (pending confirmation of intended semantics)
- resumable: false, + resumable: event.state === "needs_input",🤖 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 `@apps/cli/src/agent/json-event-emitter.ts` around lines 215 - 256, Update emitTerminal so the terminal payload’s resumable value reflects the intended resumability of each AutonomousTerminalState, particularly treating needs_input as resumable when session continuation via --session-id is supported. Apply the same derived value to both the JsonEvent terminal object and JsonFinalOutput, preserving false for states that cannot be resumed.apps/cli/src/agent/__tests__/json-event-emitter-result.test.ts (1)
70-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a dedicated first-call "crashed" terminal test.
PR objectives flag missing explicit crash-outcome coverage. The only
state: "crashed"case exercised here is the second, suppressed call (used to prove the single-emission guard), so there's no assertion that a crash emitted as the first terminal event produces the correct payload (success: false,exitCode: 70, etc.). Based on learnings, pure-logic/serialization tests for terminal-state handling belong at this package-local unit-test layer rather than only in an e2e/smoke test.✅ Suggested additional test
+ it("serializes a crashed terminal outcome correctly", () => { + const { stdout, lines } = createMockStdout() + const emitter = new JsonEventEmitter({ mode: "stream-json", stdout, authoritativeCompletion: true }) + + emitter.emitTerminal({ state: "crashed", exitCode: 70, content: "boom" }) + + expect(lines()).toEqual([ + expect.objectContaining({ + type: "result", + subtype: "terminal", + state: "crashed", + exitCode: 70, + success: false, + done: true, + }), + ]) + })🤖 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 `@apps/cli/src/agent/__tests__/json-event-emitter-result.test.ts` around lines 70 - 88, Add a dedicated test alongside the existing JsonEventEmitter terminal tests that creates an authoritative stream-json emitter, emits a first terminal event with state "crashed" and exitCode 70, and asserts the single result payload includes subtype "terminal", done true, success false, and the crash exit code. Keep the existing duplicate-emission guard test unchanged.Source: Learnings
apps/cli/src/agent/__tests__/extension-host.test.ts (1)
713-726: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAttach the rejection assertion before triggering the ask.
taskPromisecan reject whileawait dispatcher.handleAsk(...)is in flight, i.e. beforeexpect(...).rejectssubscribes. Setting up the assertion first removes any unhandled-rejection flakiness.♻️ Suggested change
- const taskPromise = host.runTask("ask something") + const taskAssertion = expect(host.runTask("ask something")).rejects.toMatchObject({ state: "needs_input" }) const dispatcher = getPrivate<{ handleAsk: (message: ClineMessage) => Promise<unknown> }>( host, "askDispatcher", ) @@ - await expect(taskPromise).rejects.toMatchObject({ state: "needs_input" }) + await taskAssertion🤖 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 `@apps/cli/src/agent/__tests__/extension-host.test.ts` around lines 713 - 726, Attach the rejects assertion to taskPromise immediately after host.runTask("ask something") and before invoking dispatcher.handleAsk. Await that assertion after the ask completes, preserving the expected { state: "needs_input" } rejection while ensuring the rejection is observed during handleAsk.apps/cli/src/agent/extension-host.ts (1)
638-640: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid the
"unknown"sentinel leaking throughgetRootTaskId()/getLastTaskResult().For non-autonomous runs without an explicit
taskId, the host stores the literal string"unknown"as the root task id even though notaskIdis sent to the extension. Any consumer readinggetRootTaskId()in interactive/TUI mode gets a fake id rather than "not known yet".♻️ Suggested change
- const rootTaskId = taskId ?? (this.options.autonomous ? randomUUID() : "unknown") - this.rootTaskId = rootTaskId + const rootTaskId = taskId ?? (this.options.autonomous ? randomUUID() : undefined) + this.rootTaskId = rootTaskIdwith
waitForTaskCompletionacceptingstring | undefinedandTaskRunResult.rootTaskIdtyped accordingly (the autonomous path always has a concrete id).Also applies to: 662-664
🤖 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 `@apps/cli/src/agent/extension-host.ts` around lines 638 - 640, Update the root task initialization around waitForTaskCompletion to preserve undefined for non-autonomous runs without an explicit taskId instead of using the "unknown" sentinel. Allow waitForTaskCompletion and TaskRunResult.rootTaskId to use string | undefined, while retaining the concrete generated or supplied ID for autonomous and explicitly identified runs so getRootTaskId() and getLastTaskResult() report the correct unknown state.apps/cli/scripts/autonomous-smoke.ts (1)
168-180: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThrowing inside the async request handler hangs the request instead of failing the smoke test.
createServer(async (request, response) => ...)— Node does not await or catch the returned promise, so thethrowat Lines 176 and 179 produces an unhandled rejection while the CLI's HTTP request never gets a response and stalls until the 30s execa timeout. Respond with a diagnostic status and record the failure instead.♻️ Suggested change
- for (const marker of [ - "PROJECT_ORCHESTRATOR_OVERRIDE", - "PROJECT_MODE_RULE", - "PROJECT_AGENTS_INSTRUCTION", - "GLOBAL_PORTABLE_RULE", - ]) { - if (!body.includes(marker)) throw new Error(`provider request did not include ${marker}`) - } - if (body.includes("GLOBAL_ORCHESTRATOR_SHOULD_LOSE")) { - throw new Error("project orchestrator did not override the global mode") - } + const missing = [ + "PROJECT_ORCHESTRATOR_OVERRIDE", + "PROJECT_MODE_RULE", + "PROJECT_AGENTS_INSTRUCTION", + "GLOBAL_PORTABLE_RULE", + ].filter((marker) => !body.includes(marker)) + if (missing.length > 0 || body.includes("GLOBAL_ORCHESTRATOR_SHOULD_LOSE")) { + fixtureFailures.push( + missing.length > 0 + ? `provider request did not include ${missing.join(", ")}` + : "project orchestrator did not override the global mode", + ) + response.writeHead(400) + response.end("fixture precondition failed") + return + }Then assert
fixtureFailuresis empty alongside the other scenario checks.🤖 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 `@apps/cli/scripts/autonomous-smoke.ts` around lines 168 - 180, Replace the throws in the async request handler’s delegation-marker validation with a diagnostic HTTP response and recorded fixture failure, ensuring every failed validation ends the request without hanging. Update the smoke-test assertions alongside the existing scenario checks to require fixtureFailures to be empty.
🤖 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 `@apps/cli/scripts/autonomous-smoke.ts`:
- Around line 193-195: Ensure the fake provider created in the autonomous smoke
scenario is always closed by moving server cleanup into a finally block that
surrounds the assertion/scenario flow. Hoist the server and its listen setup
outside the try if necessary, while preserving the existing server.close
behavior and handling the intentionally hung request sockets during cleanup.
In `@apps/cli/src/agent/extension-host.ts`:
- Around line 490-495: Attach a rejection handler immediately when creating
initializationPromise in markWebviewReady, while preserving the promise for
activate() to await and surface the original failure. Ensure rejected
updateSettings or webviewDidLaunch dispatches are observed before isReady allows
activate() to continue.
- Around line 237-247: Update the autonomous-mode error handling around the
onInputRequired callback and waitForTaskCompletion lifecycle so the client’s
"error" event always has a listener before AutonomousRunError is emitted,
including before task activation and after cleanup. Add a persistent terminal
listener or otherwise route these errors safely without changing the existing
state and message construction.
In `@src/services/ripgrep/__tests__/index.spec.ts`:
- Around line 143-144: Update the fallback test around getBinPath to stop
mocking fileExistsAtPath and exercise the real filesystem probe through
resolvePlatformRipgrepPath. Preserve mocks only for candidate-order unit tests,
and keep the assertion that getBinPath(appRoot) resolves to resolvedRg using the
fixture’s actual filesystem state.
In `@src/services/ripgrep/index.ts`:
- Around line 109-117: The resolver in resolvePlatformRipgrepPath must use the
wrapper-exported rgPath from `@vscode/ripgrep` instead of constructing an optional
package name from process.arch; update the related test expectations and setup
in src/services/ripgrep/index.ts lines 109-117 and
src/services/ripgrep/__tests__/index.spec.ts lines 114-144 to cover the
wrapper’s platform-specific resolution behavior.
---
Nitpick comments:
In `@apps/cli/scripts/autonomous-smoke.ts`:
- Around line 168-180: Replace the throws in the async request handler’s
delegation-marker validation with a diagnostic HTTP response and recorded
fixture failure, ensuring every failed validation ends the request without
hanging. Update the smoke-test assertions alongside the existing scenario checks
to require fixtureFailures to be empty.
In `@apps/cli/src/agent/__tests__/extension-host.test.ts`:
- Around line 713-726: Attach the rejects assertion to taskPromise immediately
after host.runTask("ask something") and before invoking dispatcher.handleAsk.
Await that assertion after the ask completes, preserving the expected { state:
"needs_input" } rejection while ensuring the rejection is observed during
handleAsk.
In `@apps/cli/src/agent/__tests__/json-event-emitter-result.test.ts`:
- Around line 70-88: Add a dedicated test alongside the existing
JsonEventEmitter terminal tests that creates an authoritative stream-json
emitter, emits a first terminal event with state "crashed" and exitCode 70, and
asserts the single result payload includes subtype "terminal", done true,
success false, and the crash exit code. Keep the existing duplicate-emission
guard test unchanged.
In `@apps/cli/src/agent/extension-host.ts`:
- Around line 638-640: Update the root task initialization around
waitForTaskCompletion to preserve undefined for non-autonomous runs without an
explicit taskId instead of using the "unknown" sentinel. Allow
waitForTaskCompletion and TaskRunResult.rootTaskId to use string | undefined,
while retaining the concrete generated or supplied ID for autonomous and
explicitly identified runs so getRootTaskId() and getLastTaskResult() report the
correct unknown state.
In `@apps/cli/src/agent/json-event-emitter.ts`:
- Around line 215-256: Update emitTerminal so the terminal payload’s resumable
value reflects the intended resumability of each AutonomousTerminalState,
particularly treating needs_input as resumable when session continuation via
--session-id is supported. Apply the same derived value to both the JsonEvent
terminal object and JsonFinalOutput, preserving false for states that cannot be
resumed.
In `@src/core/auto-approval/__tests__/autonomous-cli.spec.ts`:
- Around line 11-22: Extend the test for checkAutoApproval to cover
ROO_CLI_AUTONOMOUS=1 with alwaysAllowMcp set to false, asserting the result
remains { decision: "ask" }; retain the existing enabled-case assertions and
ensure the environment variable is cleaned up afterward.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 70782dfe-ce10-440a-a7bf-4e532bb6abc8
📒 Files selected for processing (22)
AGENTS.mdCLAUDE.mdapps/cli/README.mdapps/cli/package.jsonapps/cli/scripts/autonomous-smoke.tsapps/cli/src/agent/__tests__/extension-host.test.tsapps/cli/src/agent/__tests__/json-event-emitter-result.test.tsapps/cli/src/agent/ask-dispatcher.tsapps/cli/src/agent/autonomous-run.tsapps/cli/src/agent/extension-host.tsapps/cli/src/agent/json-event-emitter.tsapps/cli/src/commands/cli/list.tsapps/cli/src/commands/cli/run.tsapps/cli/src/index.tsapps/cli/src/lib/utils/provider.tsapps/cli/src/types/json-events.tsapps/cli/src/types/types.tsapps/cli/src/ui/hooks/useExtensionHost.tssrc/core/auto-approval/__tests__/autonomous-cli.spec.tssrc/core/auto-approval/index.tssrc/services/ripgrep/__tests__/index.spec.tssrc/services/ripgrep/index.ts
| await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve)) | ||
| const address = server.address() | ||
| if (!address || typeof address === "string") throw new Error("failed to bind fake provider") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close the fake provider in finally, not only on the success path.
server.close() at Line 336 is skipped whenever any assertion between Lines 240–335 throws, so the listening socket (plus the intentionally hung request sockets) keeps the event loop alive and the smoke run hangs instead of failing fast in CI.
🛡️ Suggested fix
- server.close()
process.stdout.write("autonomous process smoke passed\n")
} finally {
+ server.closeAllConnections?.()
+ await new Promise<void>((resolve) => server.close(() => resolve()))
await rm(root, { recursive: true, force: true })
}This requires hoisting server (and its listen) outside the try, or moving the cleanup into a nested try/finally around the scenario block.
Also applies to: 336-340
🤖 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 `@apps/cli/scripts/autonomous-smoke.ts` around lines 193 - 195, Ensure the fake
provider created in the autonomous smoke scenario is always closed by moving
server cleanup into a finally block that surrounds the assertion/scenario flow.
Hoist the server and its listen setup outside the try if necessary, while
preserving the existing server.close behavior and handling the intentionally
hung request sockets during cleanup.
| onInputRequired: options.autonomous | ||
| ? (ask, text) => { | ||
| const state = ask === "api_req_failed" ? "provider_failed" : "needs_input" | ||
| this.client | ||
| .getEmitter() | ||
| .emit( | ||
| "error", | ||
| new AutonomousRunError(state, `${ask}: ${text || "human input is required"}`), | ||
| ) | ||
| } | ||
| : undefined, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the emitter used by ExtensionClient.getEmitter and its base class.
fd -t f 'extension-client.ts' apps/cli/src | head
ast-grep run --pattern 'getEmitter() { $$$ }' --lang typescript apps/cli/src || true
rg -nP -C4 'class ExtensionClient|getEmitter\s*\(' apps/cli/src --type=tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 5515
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate TypedEventEmitter and event emitter implementation =="
rg -n "class TypedEventEmitter|interface TypedEventEmitter|type TypedEventEmitter|EventEmitter|emit\\(|on\\(" apps/cli/src/agent apps/cli/src -g '*.ts' -g '!*.d.ts' | head -200
echo
echo "== extension-client relevant sections =="
sed -n '1,180p' apps/cli/src/agent/extension-client.ts
sed -n '480,545p' apps/cli/src/agent/extension-client.ts
echo
echo "== extension-host relevant cleanup/error handling sections =="
sed -n '220,260p' apps/cli/src/agent/extension-host.ts
rg -n -C4 "waitForTaskCompletion|cleanup\\(|on\\(\"error\"|on\\('error\"|useExtensionHost" apps/cli/src/agent/extension-host.ts
echo
echo "== dependency/event-emitter source if published inside repo =="
fd -t f 'event-emitter|emitter' apps . | head -50Repository: Zoo-Code-Org/Zoo-Code
Length of output: 32547
Ensure the client "error" event has listeners before emitting.
TypedEventEmitter delegates to Node’s EventEmitter, and emitting "error" with zero listeners throws. In autonomous runs the error handler is attached inside waitForTaskCompletion and removed by cleanup, so an ask arriving before runTask()/resumeTask() is active or after teardown will crash the CLI. Add a persistent/terminal "error" listener for autonomous mode, or avoid emitting directly; otherwise emit(new AutonomousRunError(...)) can bring down the process.
🤖 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 `@apps/cli/src/agent/extension-host.ts` around lines 237 - 247, Update the
autonomous-mode error handling around the onInputRequired callback and
waitForTaskCompletion lifecycle so the client’s "error" event always has a
listener before AutonomousRunError is emitted, including before task activation
and after cleanup. Add a persistent terminal listener or otherwise route these
errors safely without changing the existing state and message construction.
| // Serialize initial settings and launch. In particular, webviewDidLaunch | ||
| // loads project custom modes before activate() allows the first task. | ||
| this.initializationPromise = (async () => { | ||
| await this.dispatchToExtension({ type: "updateSettings", updatedSettings: this.initialSettings }) | ||
| await this.dispatchToExtension({ type: "webviewDidLaunch" }) | ||
| })() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Attach a rejection handler when initializationPromise is created.
markWebviewReady() sets isReady = true first, but activate() only awaits initializationPromise after pWaitFor(() => this.isReady, { interval: 100 }) resolves — up to ~100ms later. If updateSettings/webviewDidLaunch dispatch rejects in that window the promise is unhandled, and Node's default --unhandled-rejections=throw will terminate the process instead of surfacing the error through activate().
🛡️ Suggested fix
- this.initializationPromise = (async () => {
- await this.dispatchToExtension({ type: "updateSettings", updatedSettings: this.initialSettings })
- await this.dispatchToExtension({ type: "webviewDidLaunch" })
- })()
+ const initialization = (async () => {
+ await this.dispatchToExtension({ type: "updateSettings", updatedSettings: this.initialSettings })
+ await this.dispatchToExtension({ type: "webviewDidLaunch" })
+ })()
+ // Keep a settled handler attached so a rejection before activate() awaits
+ // it does not surface as an unhandled rejection.
+ initialization.catch(() => undefined)
+ this.initializationPromise = initialization📝 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.
| // Serialize initial settings and launch. In particular, webviewDidLaunch | |
| // loads project custom modes before activate() allows the first task. | |
| this.initializationPromise = (async () => { | |
| await this.dispatchToExtension({ type: "updateSettings", updatedSettings: this.initialSettings }) | |
| await this.dispatchToExtension({ type: "webviewDidLaunch" }) | |
| })() | |
| // Serialize initial settings and launch. In particular, webviewDidLaunch | |
| // loads project custom modes before activate() allows the first task. | |
| const initialization = (async () => { | |
| await this.dispatchToExtension({ type: "updateSettings", updatedSettings: this.initialSettings }) | |
| await this.dispatchToExtension({ type: "webviewDidLaunch" }) | |
| })() | |
| // Keep a settled handler attached so a rejection before activate() awaits | |
| // it does not surface as an unhandled rejection. | |
| initialization.catch(() => undefined) | |
| this.initializationPromise = initialization |
🤖 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 `@apps/cli/src/agent/extension-host.ts` around lines 490 - 495, Attach a
rejection handler immediately when creating initializationPromise in
markWebviewReady, while preserving the promise for activate() to await and
surface the original failure. Ensure rejected updateSettings or webviewDidLaunch
dispatches are observed before isReady allows activate() to continue.
| mockFileExists.mockImplementation(async (candidate: string) => candidate === resolvedRg) | ||
| await expect(getBinPath(appRoot)).resolves.toBe(resolvedRg) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Exercise the real filesystem probe here.
This behavior crosses getBinPath, resolvePlatformRipgrepPath, and fileExistsAtPath; mocking the latter makes the fallback assertion unit-style rather than filesystem-backed. Keep mocks for candidate-order unit tests, but use the real helper in this fixture.
As per coding guidelines, “Use integration tests when behavior depends on multiple internal modules but does not require the real VS Code extension host or browser/webview runtime.”
🤖 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/services/ripgrep/__tests__/index.spec.ts` around lines 143 - 144, Update
the fallback test around getBinPath to stop mocking fileExistsAtPath and
exercise the real filesystem probe through resolvePlatformRipgrepPath. Preserve
mocks only for candidate-order unit tests, and keep the assertion that
getBinPath(appRoot) resolves to resolvedRg using the fixture’s actual filesystem
state.
Source: Coding guidelines
| /** Resolve @vscode/ripgrep >=1.18, which ships rg in a platform-specific optional package. */ | ||
| export function resolvePlatformRipgrepPath(vscodeAppRoot: string): string | undefined { | ||
| try { | ||
| const wrapperManifest = path.join(vscodeAppRoot, "node_modules", "@vscode", "ripgrep", "package.json") | ||
| if (!fs.existsSync(wrapperManifest)) return undefined | ||
| const requireFromApp = createRequire(path.join(vscodeAppRoot, "package.json")) | ||
| const wrapperEntry = requireFromApp.resolve("@vscode/ripgrep") | ||
| const requireFromWrapper = createRequire(wrapperEntry) | ||
| return requireFromWrapper.resolve(`@vscode/ripgrep-${process.platform}-${process.arch}/bin/${binName}`) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'src/services/ripgrep/(index|.*spec)\.ts$|package\.json$|pnpm-lock|package-lock|yarn.lock' || true
echo "== ripgrep implementation outline =="
f="$(git ls-files | rg 'src/services/ripgrep/index.ts$' | head -n1 || true)"
if [ -n "${f:-}" ]; then
wc -l "$f"
sed -n '1,180p' "$f" | cat -n
fi
echo "== ripgrep tests relevant section =="
tf="$(git ls-files | rg 'src/services/ripgrep/__tests__/index.ser\.ts$|src/services/ripgrep/__tests__/index.spec.ts$' | head -n1 || true)"
if [ -n "${tf:-}" ]; then
wc -l "$tf"
sed -n '1,220p' "$tf" | cat -n
fi
echo "== package mentions =="
rg -n '"`@vscode/ripgrep`"|ripgrep|binName|resolvePlatformRipgrepPath|rgPath' . -S --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 44036
🌐 Web query:
npm @vscode/ripgrep 1.18.0 package files bin rgPath
💡 Result:
In version 1.18.0 of the @vscode/ripgrep npm package, the architecture and distribution model underwent a significant change from previous versions [1][2]. The package is now a lightweight wrapper that resolves the rgPath to a sibling package [3][2]. Specifically, the wrapper uses optionalDependencies to install the correct per-platform package (such as @vscode/ripgrep-linux-x64 or @vscode/ripgrep-win32-x64) based on the user's platform and architecture [3][2]. Regarding the bin file and rgPath: - Files are located within the platform-specific sub-packages [2]. When installed, these packages place the ripgrep binary at a path structured as packages/ripgrep-<os>-<arch>/bin/rg (or rg.exe on Windows) [4][3]. - At runtime, the main @vscode/ripgrep package exports the rgPath constant, which is resolved dynamically in lib/index.js to point to the correct binary location within the installed platform-specific sibling package [4][3][2]. Users should access the path using the standard exported constant: const { rgPath } = require('@vscode/ripgrep'); // child_process.spawn(rgPath,...); Note that older versions of the package (prior to the migration to per-platform packages) relied on a postinstall script to download binaries; this is no longer the case in 1.18.0, which ships the binaries directly within the npm tarball for each platform-specific package [4][3][5][6]. If you require access to binaries for multiple platforms from a single install, the @vscode/ripgrep-universal package is available, which provides both rgPath (for the current host) and a binPathFor({ os, arch }) function [3][7].
Citations:
- 1: microsoft/vscode-ripgrep@v1.17.1...v1.18.0
- 2: microsoft/vscode-ripgrep@62b2b26
- 3: https://p.rst.im/q/github.com/microsoft/vscode-ripgrep
- 4: https://www.npmjs.com/package/@vscode/ripgrep
- 5: https://github.com/microsoft/vscode-ripgrep/blob/master/README.md
- 6: https://npmx.dev/package/@vscode/ripgrep/v/1.17.1
- 7: https://github.com/microsoft/vscode-ripgrep
🌐 Web query:
npm @vscode/ripgrep-universal``
💡 Result:
@vscode/ripgrep-universal is an npm package that bundles ripgrep binaries for all supported platforms into a single package [1][2]. It was introduced in version 1.18.0 of the microsoft/vscode-ripgrep repository [3]. Key characteristics and usage: - Purpose: It is designed for use cases where access to binaries for multiple platforms is required from a single installation, such as when repackaging a Node.js application into cross-platform artifacts (e.g., installers or archives) from a single build host [1][2]. - Size: Because it includes binaries for all 12 supported platforms, the package is significantly larger than the standard @vscode/ripgrep package, approximately 60 MB [1][4]. - Comparison: For general application use, the standard @vscode/ripgrep package is preferred, as it only installs the binary for the current platform via optionalDependencies [1][2]. - API: The package exports rgPath (for the current platform) and a helper function binPathFor({ os, arch }) to retrieve the path to any platform's specific binary [1][4]. Example usage: import { rgPath, binPathFor } from '@vscode/ripgrep-universal'; // Path to the binary for the current platform console.log(rgPath); // Path to a specific platform binary const winPath = binPathFor({ os: 'win32', arch: 'x64' }); [1]
Citations:
- 1: https://www.pkgstats.com/pkg:`@vscode/ripgrep-universal`
- 2: https://p.rst.im/q/github.com/microsoft/vscode-ripgrep
- 3: https://github.com/microsoft/vscode-ripgrep/releases/tag/v1.18.0
- 4: https://github.com/microsoft/vscode-ripgrep
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dependency declarations =="
for f in package.json apps/cli/package.json apps/vscode-e2e/package.json apps/vscode-nightly/package.json; do
[ -f "$f" ] || continue
echo "-- $f --"
php -r "echo json_encode(json_decode(file_get_contents('$f'), true), JSON_PRETTY_PRINT)." 2>/dev/null | rg -n '"`@vscode/ripgrep`"|"`@vscode/ripgrep-universal`"|"vscode-ripgrep"|version|dependencies|devDependencies|optionalDependencies' -C 2 || true
done
echo "== lockfile entries around `@vscode/ripgrep` packages =="
python3 - <<'PY'
from pathlib import Path
p=Path('pnpm-lock.yaml')
lines=p.read_text().splitlines()
targets=['`@vscode/ripgrep`@1.18.0','`@vscode/ripgrep-universal`','`@vscode/ripgrep-darwin-arm64`@1.18.0']
for t in targets:
print(f'-- {t} --')
matches=[i for i,l in enumerate(lines) if l.strip().startswith(t + '@') or l.strip().startswith(t + ' ')]
for i in matches[:5]:
for j in range(i, min(len(lines), i+18)):
print(f'{j+1}: {lines[j]}')
print()
PY
echo "== inspect `@vscode/ripgrep` lib index in repository lock contents if present =="
tmpdir="$(mktemp -d)"
cd "$tmpdir"
if command -v npm >/dev/null 2>&1; then
npm pack `@vscode/ripgrep`@1.18.0 --silent
tar -tvf vscode-ripgrep-1.18.0.tgz | grep -E 'package/(lib/index|package/bin|packages/ripgrep-\w+-\w+/bin)' | head -n 40 || true
echo "-- package/index.js excerpt --"
tar -xzf vscode-ripgrep-1.18.0.tgz
sed -n '1,220p' package/index.js 2>/dev/null || true
else
echo "npm not available"
fi
rm -rf "$tmpdir"Repository: Zoo-Code-Org/Zoo-Code
Length of output: 744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
for p in ['package.json','apps/cli/package.json','apps/vscode-e2e/package.json','apps/vscode-nightly/package.json']:
try:
data=json.load(open(p))
except Exception as e:
print(f"### {p}: {type(e).__name__}: {e}")
continue
print(f"### {p}")
for sec in ["dependencies","devDependencies","peerDependencies","optionalDependencies"]:
if sec in data:
entries={k:v for k,v in data[sec].items() if "ripgrep" in k.lower()}
if entries:
print(sec, entries)
else:
print(sec, '<ripgrep entries>')
PY
echo "== package manifest from lock =="
python3 - <<'PY'
from pathlib import Path
text=Path('pnpm-lock.yaml').read_text()
for marker in ['`@vscode/ripgrep-universal`@', '`@vscode/ripgrep-darwin-arm64`@1.18.0']:
idx=text.find(marker)
print(f"\n-- {marker} at {idx} --")
if idx!=-1:
start=text.rfind("\n", 0, idx-50)
end=text.find("\n\n", idx+100)
print(text[start:end if end!=-1 else idx+500])
# find package.json of `@vscode/ripgrep`@1.18.0 in snapshot
idx=text.find("`@vscode/ripgrep`@1.18.0:")
if idx!=-1:
start=text.find("snapshot:", idx)
if start!=-1:
end=text.find("\n\n", start+8)
print("\n-- `@vscode/ripgrep`@1.18.0 snapshot --")
print(text[start:end])
PY
echo "== npm pack index/package paths =="
tmpdir="$(mktemp -d)"
cd "$tmpdir"
npm pack `@vscode/ripgrep`@1.18.0 --silent 2>/dev/null || true
tar -tvf vscode-ripgrep-1.18.0.tgz | rg -E 'package/(package\.json|lib/index\.js|bin/|packages/ripgrep-[^\s]+/package\.json|packages/ripgrep-[^\s]+/bin/)' | head -n 80
rm -rf "$tmpdir"Repository: Zoo-Code-Org/Zoo-Code
Length of output: 1363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
cd "$tmpdir"
npm pack `@vscode/ripgrep`@1.18.0 --silent
tar -tf vscode-ripgrep-1.18.0.tgz | sort | sed -n '1,120p'
echo "== package.json =="
tar -xzf vscode-ripgrep-1.18.0.tgz
cat package/package.json
echo "== lib/index.js =="
cat package/lib/index.js
echo "== platform package dirs/files excerpt =="
find package -maxdepth 4 -path '*/ripgrep-*-*/package.json' -o -path '*/ripgrep-*-*/lib/index.js' | sort | sed -n '1,80p'
for p in $(find package -path '*/ripgrep-*-*/package.json' | sort | head -n 4); do
echo "-- $p --"
cat "$p"
done
rm -rf "$tmpdir"Repository: Zoo-Code-Org/Zoo-Code
Length of output: 2036
Use rgPath from @vscode/ripgrep instead of constructing a package from process.arch.
@vscode/ripgrep 1.18.0 is a wrapper whose optional platform packages use the CPU target (darwin-x64, linux-arm64, etc.), not process.arch (x64, arm64). That means this resolver can construct names like @vscode/ripgrep-x64 instead of @vscode/ripgrep-darwin-x64 and return undefined; model the actual optional package dependency or use the wrapper-exported rgPath.
src/services/ripgrep/index.ts#L109-L120src/services/ripgrep/__tests__/index.spec.ts#L114-L125
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import * as childProcess from "child_process"
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
📍 Affects 2 files
src/services/ripgrep/index.ts#L109-L117(this comment)src/services/ripgrep/__tests__/index.spec.ts#L114-L144
🤖 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/services/ripgrep/index.ts` around lines 109 - 117, The resolver in
resolvePlatformRipgrepPath must use the wrapper-exported rgPath from
`@vscode/ripgrep` instead of constructing an optional package name from
process.arch; update the related test expectations and setup in
src/services/ripgrep/index.ts lines 109-117 and
src/services/ripgrep/__tests__/index.spec.ts lines 114-144 to cover the
wrapper’s platform-specific resolution behavior.
Source: Learnings
github-actions
Bot
added
the
awaiting-review
github-actions
Bot
removed
the
awaiting-review
taltas
changed the title
1 participant
Status
Implemented
roo --autonomous --print --workspace <path> --timeout <seconds>on the existingapps/cliplus@roo-code/vscode-shimpath. It runs the real bundled extension in one process, with no daemon and no second agent loop.orchestratorslug. Project.roomodesoverrides global custom modes and built-ins with normal Zoo Code precedence;--modeand--require-approvalare rejected..roomodes,.roo/mcp.json,.roo/rules*,AGENTS.md/AGENTS.local.md, and supported global equivalents. Credentials remain explicit flags or provider environment variables. Native VS Code settings databases, profiles, OAuth state, and SecretStorage are not read or imported.switch_modepreserves one conversation;new_taskcreates a fresh child, persists the parent, and resumes it with the child result. Child completion is progress; only accepted and persisted root completion exits 0.@vscode/ripgrep1.18 platform packages.Test Evidence
No real model credentials or billable requests were used.
pnpm --filter @roo-code/cli test: 575 tests passed after no-mistakes coverage fixes.pnpm --filter @roo-code/vscode-shim test: 407 tests passed.pnpm --filter @roo-code/cli test:autonomous-process: passed repeatedly against a local fake OpenRouter-compatible stream. It asserts project/global config precedence, root and child IDs, parent linkage, root Orchestrator and child mode, same-task mode switching, parent resumption, accepted root completion, input required, provider failure, timeout, SIGINT cancellation, second-signal force behavior, parseable output, and one terminal outcome. Six consecutive full runs were clean during implementation.pnpm --filter ./src bundle,pnpm --filter @roo-code/cli build, and./apps/cli/scripts/build.sh --skip-verify: passed; the release tarball found and packaged the 1.18 platform ripgrep binary.Unsupported Headless Capabilities
Known Risks
Risk: Medium. The mode intentionally grants unrestricted filesystem, command, and MCP authority, including outside-workspace and protected-file mutations. Run it only in a fully trusted environment.
run.tsis exercised by the built-process smoke rather than unit instrumentation; the no-mistakes CI fix excludes that process entrypoint from Codecov patch accounting while moving pure autonomous validation into a fully covered module.Follow-Up Extraction
Task/ClineProvider, retaining parity tests against this shim path.Validation Commands