[wpfleger96]

Moves the Goose plugin's ACP client into a shared module, src/features/agent-harness/acp/, and extends it so the generic ACP plugin can drive any ACP harness, including one with none of Goose's extensions. One Buzz-tool server pool now serves every ACP plugin.

Stack (merge order), 1 of 2: this PR → #1251 (generic ACP agent)

Includes #1359 (Goose stops posting text the agent didn't send), which was merged into this branch.

Layout

• types.ts: shared types and AcpHooks, where each harness says what it adds to plain ACP. Every hook is optional.
• process.ts: AcpProcess, which covers the transport, initialize, session/new, session/close, Buzz tool routing over HTTP MCP and auto-allowed permission prompts.
• session.ts: AcpSession, which covers the turn queue, settlement, activity, steering, stopping a turn from its activity (interrupt) and model and thinking-level selection.
• agent-sessions.ts: AgentSessions (each conversation's session, idle close and one-off turns) and ServerPool. The process comes from a { get() } source, so each plugin decides how many processes it runs.
• steps.ts: stepLabel, allow and errorMessage.
• src/bundled/goose/goose.ts holds Goose's hooks and ENV. sessions.ts keeps SharedProcess, Goose's single process, plus a thin AgentSessions subclass.

Harnesses without hooks

• System prompt: a systemPrompt hook is used first. Without one, ACP's systemPrompt (mode: "append") goes on session/new when sessionCapabilities.systemPrompt is advertised. Otherwise the instructions are placed once at the top of the session's first prompt.
• Steering: without a steering hook, a message sent mid-turn runs as the next turn. With one, the hook's report that no run is active clears the current run id.
• Model: without select, the harness keeps its own pick. Without newSessionMeta, no _meta is sent.
• Closing: session/close is sent only when sessionCapabilities.close is advertised, or when the hooks set closeSession: "always", as Goose's do.
• Buzz tools: a harness is refused unless mcpCapabilities.http is exactly true.

Startup and shutdown

• AcpProcess.start takes args, which go to spawn unchanged, and a signal. Aborting it rejects at once with the signal's reason and kills the process, whether or not the process ever exits.
• BUZZ_PRIVATE_KEY, BUZZ_AUTH_TAG and BUZZ_RELAY_URL are removed after the hooks' env and the launch's env (PROTECTED_ENV).
• With diagnoseStartup set on the hooks, for a harness launched through env, exit 126 or 127 before initialize answers maps to hooks.missing, and stderr written before the spawn resolves is kept for the error message. Goose doesn't set it, so its startup errors match main.
• AcpProcess.stop(timeout) stops the harness and resolves once it exits or the time limit passes.

Failed turns

• Settled carries the harness's JSON-RPC error code next to error when a turn fails on an error answer, so a plugin can recognise ACP's sign-in error (-32000) without matching text. The error and its code are held as one value, so a later refusal never keeps an earlier prompt's code. Goose's failure post is unchanged.

Goose replies

• Goose no longer posts text it wrote without calling send; that text stays in local activity. A failed turn still posts its error as before.

Shared server pool

• SHARED_POOL (48 servers) is used by every AgentSessions unless one is passed. Goose and generic sessions evict each other's least recently used idle session. Busy sessions are never evicted, so 48 is a soft limit that leaves headroom below the host's 64-server limit rather than guaranteeing it.

acp/client.test.ts covers the no-hook paths, steering across a slow model pick, startup, shutdown and the shared pool with a fake plain-ACP harness.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/user-flow) reviews.

No required changes found in the current Goose integration. Source assessment: 9/10 for the extraction’s minimalness, ownership, and preservation of current behavior. The shared session owner carries over the existing pool, idle-close, delivery, and disposal behavior; Goose retains its system-prompt, steering, provider/model/effort, and close-session hooks. The shared pool remains intentionally soft when all sessions are busy.

Two optional inline integration notes cover newly added generic-only paths. At this head, source searches find Goose as the only production consumer of the shared client; its wrapper neither uses fallback prompt injection nor passes a startup signal. These notes therefore do not establish a current Goose regression or require changing this extraction before merge. Reassess them when the generic harness consumer lands.

Reviewed head: 59030eb
Base: 330e60b

Scope: all nine changed files, relevant callers, test source, public PR description and commit messages. No description attachments; no disclosure findings in that inspected surface. No UI or focus implementation changes. Tests remain at the service/protocol layer; existing Goose tests are unchanged.

Validation: source-only; no PR code, tests, builds, apps or benchmarks executed. One head-bound hosted-check snapshot showed 30 successful checks and two skipped (Windows native validation and browser flake check). This does not establish native/live-provider behavior, generic-harness protocol interoperability, cancellation/retry acceptance, runtime performance, or human acceptance; test-cost timings were not assessed. This COMMENT is not an approval or merge authorization.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/user-flow) reviews.

No new actionable findings. Source assessment: 9/10. Both prior optional notes are addressed: fallback instructions survive a rejected first prompt, and cancellation covers a pending host spawn with late-process cleanup. The added regression cases cover those paths in source. The extracted session-store and last-used-time behavior preserves the current base implementation.

Reviewed head: 82f8fadeba1dfb9ab8490a054d9e8734af1ac62f; base: 2488d609f6f2da8a333ae6909aae901e3a492889. Follow-up compared with prior reviewed 59030eb5d7aff104f87f58721347b8ff106f4d48, including the current Goose integration. Public PR description inspected; no attached images or disclosure findings.

Source-only review: no PR code, tests, builds, apps or benchmarks executed. One hosted-check snapshot still had JavaScript, browser and Rust jobs running; Windows validation was skipped. Live provider behavior, generic-harness interoperability, native cleanup, rendered focus/accessibility and performance remain unvalidated. This COMMENT is not approval or merge authorization.

[wesbillman]

🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.

No blocking findings at 82f8fade (base main @ 69c644b7; merges cleanly). Reviewed security boundaries, session/process lifecycle, and Goose behavior preservation against 2488d609.

• Goose keeps unconditional session/close, the append system-prompt request, _meta tool lockdown, provider/model/default-model switching, and thinking-effort handling.
• Env handling is stricter than before: PROTECTED_ENV now wins over hooks and launch env, and native spawn already strips inherited BUZZ_*/NOSTR_*. The HTTP MCP gate is now === true. Permission answers and MCP routing are unchanged.
• Startup abort, late-spawn kill, and bounded stop() look correct. stop() bounds the wait; it doesn't confirm the process ended.

Validation gap: the no-hook paths have no production consumer yet. They're covered only by client.test.ts's fake harness, so real interop with a non-Goose ACP harness is unverified, and systemPrompt on session/new is still an unstable ACP field. Two non-blocking notes inline.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security), and Groot (product/failure-flow) reviews.

No required changes found in this follow-up. Source assessment: 9/10. The previous stderr-disclosure note is mitigated for the covered credential forms, and the no-hook MCP/permission policy is now documented. Exit settlement, cleanup, restart, and failure-message delivery remain unchanged. One optional inline note covers additional diagnostic quoting formats; pattern-based redaction is not an exhaustive guarantee that arbitrary stderr contains no secrets.

Reviewed head: 523724a41f27ce4ace3a6722512162ec1313eb8d; base: 2488d609f6f2da8a333ae6909aae901e3a492889. Scope: the five-file follow-up since reviewed 82f8fadeba1dfb9ab8490a054d9e8734af1ac62f, relevant consumers and test source. Public PR description and new commit inspected; no attachments or disclosure findings in that surface. No UI or focus implementation changes.

Source-only: no PR code, tests, builds, apps or benchmarks executed. One hosted-check snapshot showed 24 successful checks, six still running, and two skipped. Native/live-provider behavior, rendered accessibility/focus, performance, exhaustive redaction, and human acceptance remain unvalidated. This COMMENT is not approval or merge authorization.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/failure-flow) reviews.

No required changes found in this follow-up. Source assessment: 9/10. Two optional UX improvements are noted inline. The fixed ACP/goose failure messages keep raw harness diagnostics out of this channel-reporting path while preserving bounded details in device-local activity.

Reviewed head 3b654158bde45ad853ddff1fe94720850138df78 against base 2488d609f6f2da8a333ae6909aae901e3a492889, focusing on changes since reviewed head 523724a41f27ce4ace3a6722512162ec1313eb8d and affected callers. Inspected the public PR description (no attached images) and follow-up commit message; no disclosure finding on those surfaces.

Source-only: no tests, builds, PR code or app execution. Hosted checks were still running at the single snapshot; native/provider behavior, rendered focus/accessibility, and live recovery remain unverified. This is a COMMENT review, not approval or merge authorization.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/error/retry flows) reviews.

No new actionable findings. Source assessment: 9/10. The previous optional notes are addressed: failed model switches preserve their failure kind, and ambiguous diagnostic text no longer matches the earlier broad sign-in/model patterns. Goose’s removal of automatic unsent-reply posting follows the stated credential-disclosure fix; replies now require the send tool, while unsent text remains local. This deliberately means a turn that finishes without send supplies no channel reply or fallback notice.

Reviewed the seven-file follow-up from 3b654158bde45ad853ddff1fe94720850138df78, with related ACP settlement, process/session cleanup, tool publication, provider testing and standalone-wake callers. Public PR description and new commit messages inspected; no attached images or new disclosure finding.

Head: 1e9cd7f9df25f65942b09ddba35308b9aef16650
Base: 2488d60

Source only: tests, builds, app launches and live harness behavior were not executed. One hosted-check snapshot still had JavaScript, Rust and browser jobs running; Windows native validation was skipped. Native behavior, rendered focus/accessibility, performance and human acceptance remain unverified. This COMMENT review is not approval or merge authorization.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/error-recovery) reviews.

No actionable findings in this follow-up. Source assessment: 9/10. The narrower model-name heuristic avoids directing owners to change models for configuration, metadata, or service diagnostics. Quoted names and model-like IDs retain model-specific guidance; bare alphabetic IDs conservatively fall back to the generic failure message. The comment updates match the existing behavior: unsent harness text remains local, and channel failure reports use fixed wording.

Reviewed the five-file change since 1e9cd7f9df25f65942b09ddba35308b9aef16650 and its ACP/Goose error-reporting callers. This is a follow-up review, not a new exhaustive review of unchanged feature code. Public PR description (no attached images) and the new commit message inspected; no disclosure finding on those surfaces.

Head: 42746a037125ecc9df8e88d4b5f7c2a58b53d06c
Base: 2488d60

Source only: no tests, builds, app launches, or PR code execution. Hosted JavaScript, Rust and browser checks were still running at the single snapshot; Windows native validation was skipped. Live-provider behavior, rendered focus/accessibility, performance and human acceptance remain unverified. This COMMENT review is not approval or merge authorization.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/failure/retry) reviews.

No required changes found in this follow-up. Source assessment: 9/10. One optional error-code consistency improvement is noted inline.

The first five feature patches are unchanged by the rebase; the new sixth patch propagates JSON-RPC error codes. The fallback-instruction retry and pending-spawn cancellation repairs remain present. Earlier failure-classification and send-only changes are absent from this revision. The PR description was corrected during review and now accurately says Goose’s failure post is unchanged; the earlier description finding is resolved. Existing raw-error and unsent-text posting are not newly introduced by this extraction, and earlier reviews describing their removal do not apply to this head. Draft #1359 separately proposes removing the unsent-text fallback.

Reviewed the rebased nine-file ACP/Goose feature diff, new code-propagation patch, affected callers and prior feedback. This is follow-up review, not a new exhaustive review of unrelated base changes. Public description and feature commit messages inspected; no attached images or disclosure finding in that publication material.

Head: 3cfa0b7
Base: 4ad00a8

Source only: no PR code, tests, builds or apps executed. The single hosted-check snapshot still had JavaScript, Rust and browser jobs running; Windows native validation was skipped. Native cleanup, provider interoperability, runtime performance, rendered focus/accessibility and human acceptance remain unverified. This COMMENT review is not approval or merge authorization.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/failure/retry) reviews.

No changes requested in this follow-up. Source assessment: 9/10.

The previous optional stale-code finding is resolved: a later refusal replaces the whole failure record, so it cannot inherit an earlier prompt’s sign-in code. Goose’s startup errors now match the base implementation; extra early-stderr capture and exit-126/127 install hints require the new diagnoseStartup hook. Abort, transport and shutdown ownership remain unchanged by these fixes.

Stack integration note: #1251 at dfe68ff9b49ccc898cadb7ebded870a138ccc194 currently omits that hook in src/bundled/acp/harness.ts:23–32. When rebasing it onto this revision, set diagnoseStartup: true there to preserve its env-launched harness install hints and early stderr (harness.test.ts:57–75). That consumer is absent from this pinned PR; this is a downstream integration requirement, not a current Goose defect.

Scope: the four-file follow-up from 3cfa0b7, its callers and prior feedback. Public PR description and both follow-up commit messages inspected; no attached images or disclosure finding in that material.

Head: c8cbbbe
Base: 4ad00a8

Source only: no PR code, tests, builds or apps executed. The single hosted-check snapshot had JavaScript, Rust and browser jobs still running; Windows native validation was skipped. Live harness interoperability, native cleanup, rendered failure/retry focus, accessibility, performance and human acceptance remain unverified. This COMMENT review is not approval or merge authorization.

[salman1993]

🤖 Approved by Salman's AI agent at his explicit direction.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/error/retry flows) reviews.

No required changes found in this follow-up. Source assessment: 9/10. Removing automatic publication of unsent Goose text matches the shared prompt contract. Explicit tool sends, failure reporting, thread/Solo routing, activity settlement and session cleanup remain intact. The negative test covers narration and plain answers; crash-report routing remains covered in source.

Optional description update: #1359 is now merged into this head, so the stack paragraph should no longer list it as a future step. Mention that successful turns which never call send leave their text in local activity and produce no channel reply; runtime-generated failure reports remain unchanged.

Reviewed the two-file follow-up from c8cbbbe and affected callers, not a new exhaustive review of unchanged feature code. Public PR description, feedback and feature commit messages inspected; no attached images or disclosure findings in those surfaces.

Head: 926ba1c
Base: 4ad00a8

Source-only: no PR code, tests, builds, apps or benchmarks executed. The single hosted-check snapshot showed DCO passing and Semgrep/zizmor queued; CI completion is unverified. Live provider behavior, model adherence to send, native behavior, rendered focus/accessibility, performance and human acceptance remain unvalidated. This COMMENT is not approval or merge authorization.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/error/retry flows) reviews.

No required changes found in this follow-up. Source assessment: 9/10. The rebase preserves main’s Stop and host directory-resolution behavior in the extracted ACP owners. The latest test change waits for both prompts to start before interrupting, replacing a fixed microtask-flush count. Prior startup-cancellation, fallback-instruction retry, and error/code ownership repairs remain present.

One optional inline note covers a Stop/late-run-ID race inherited unchanged from the base. It is not an extraction regression. Optional wording cleanup: session.ts:6,32 and process.ts:18 still describe posting unsent text, although Goose now deliberately keeps it local. Removing automatic unsent-text publication is unchanged from the previously reviewed revision; actual provider failure transport and late-tool/cancel ordering remain runtime validation gaps, not demonstrated new defects.

Head: fff8f34
Base: a46ff1f

Scope: follow-up from 926ba1c, using range-diff plus the current ACP/Goose feature diff and affected callers. This does not re-review unrelated base changes. Public PR description and feature commit messages inspected; no attached images or disclosure findings on those surfaces.

Source-only: no PR code, tests, builds, apps or benchmarks executed. One head-bound hosted-check snapshot showed 35 successful checks and two skipped (Windows native validation and browser flake check). Native cleanup, live-provider interoperability, cancellation ordering, rendered focus/accessibility, performance and human acceptance remain unverified. Test-cost timing was not assessed. This COMMENT is not approval or merge authorization.

[wesbillman]

🤖 No changes requested in Star Lord’s automated source review via Wes’s account, integrating Rocket, Mantis and Groot. The rebased extraction preserves the current Stop, steering, prompt-refresh and session-lifecycle behavior; the added test covers a run ID arriving after Stop.

Reviewed head 3c6f0c57ab4a3c6623bc033be67c587a70aaa2b7 against base 9f60c3d1b4effef455840a9b1f3ede7d37cfc983, including the public description and feature commits. Source-only: no tests, builds or apps executed, and CI, native/provider behavior, performance and rendered focus/accessibility remain unverified.