feat(acp): wrap workers at the subprocess launch boundary by shellz-n-stuff · Pull Request #7985 · block/buzz

shellz-n-stuff · 2026-09-30T13:37:36Z

Summary

Replacing BUZZ_ACP_AGENT_COMMAND with a sandbox launcher hides the real adapter from Buzz: for example, Goose loses its default acp argument. Add optional BUZZ_ACP_LAUNCH_PREFIX, a JSON argument array applied at the shared subprocess boundary, after normal worker configuration.

On Unix, every spawn runs prefix... worker args... without shell interpolation. Worker identity, environment setup, stdio and existing process cleanup are preserved. Invalid or unavailable prefixes fail launch without falling back to the worker; unset keeps direct launch on every platform. Configured prefixes fail closed on non-Unix because per-worker process-tree cleanup is unavailable. Desktop reserves the key against saved user-environment overrides, and spawn failures name the executable.

Related issue

Prerequisite for Buzz-App #415. One commit directly on main, independent of #7942–#7944. No duplicate launch-prefix PR found. Policy enforcement, verified launcher staging, protected paths and the supporting runtime pin remain Buzz-App/plugin work.

Testing

Original head 9ae9d82f8: complete ACP suite (990 passed, three existing ignored), shipped local-task test, real-Goose probe, and full repository-wide just ci passed.
Review fixes: five launch tests and 44 Desktop environment-filter tests pass locally, covering the real production platform gate, save rejection, both merge layers, case variants, and executable error context.
Subprocess tests cover direct/wrapped argument defaults, Pi skills, Hermes/Codex environment, adapter identity, repeated spawns and invalid/missing prefix refusal.
Shipped buzz-acp run entrypoint completes a task through the wrapper and deterministic ACP peer.
Real Goose 1.52.0: direct and wrapped auth-methods return identical results; the wrapper receives goose acp with initially empty worker args.
Independent agent review: no blocking findings.
Required push checks passed on the updated head. The existing native Windows CI job now explicitly runs the platform-contract tests. Its result is pending; the non-Unix refusal branch was not exercised on this macOS host.

Repeated-spawn coverage is not a full lazy-pool wake/crash test. Protected Goose in the Buzz-App UI and human acceptance remain pending.

To try the hook after building buzz-acp, use an installed Goose path:

BUZZ_ACP_LAUNCH_PREFIX='["/usr/bin/env"]' target/debug/buzz-acp auth-methods \
  --agent-command /absolute/path/to/goose --agent-args '' --json

Expect the same authentication methods as without the prefix. Changing the prefix to ["/missing-launcher"] must fail. Draft pending human testing.

github-actions · 2026-09-30T13:37:52Z

🔐 Codex Security Review

Status: review required for the current range.

The current range is d7a35afa9859a0c666de93ab1cd4f8a433a1cfe8...7813e74f9c8a0fdf27437c4c5dafc0483179f4a0.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 7813e74f9c8a0fdf27437c4c5dafc0483179f4a0 to authorize a new review.
Any previous review applies only to its recorded range.

chatgpt-codex-connector · 2026-09-30T13:45:09Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-30T15:13:46.950765Z	`7813e74`	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ae9d82f8b

ℹ️ 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".

chatgpt-codex-connector · 2026-09-30T13:50:59Z

+            let mut command = Command::new(executable);
+            command.args(&prefix[1..]).arg(worker);


Contain supervised wrappers on Windows

When BUZZ_ACP_LAUNCH_PREFIX is used on Windows with a launcher that supervises rather than replaces the worker—a mode explicitly permitted by the README—AcpClient tracks only this launcher process. Since process-group creation and killing are Unix-only, both shutdown and Drop kill only the launcher, leaving the actual ACP worker and its MCP descendants running after shutdown or crash replacement. Place the wrapper and worker in a kill-on-close Windows Job Object, or require a launcher-provided equivalent instead of advertising generic supervision.

AGENTS.md reference: AGENTS.md:L240-L244

Useful? React with 👍 / 👎.

wesbillman

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

Changes requested: one P2 lifecycle blocker. Independently confirmed the existing Windows supervised-wrapper finding; keeping the detailed finding in that thread rather than duplicating it. This is an opt-in Windows regression, not a regression for unset/direct launches.

The smallest merge criterion is to reject configured prefixes on non-Unix without fallback, document that platform scope, and test refusal. Alternatively, provide per-worker process-tree containment and verify shutdown, Drop, and replacement cleanup. Desktop’s outer harness job does not protect individual worker replacement while the harness remains alive.

Reviewed head 9ae9d82f8bad58fb03c6fb11fa0e8674f66f3ff3 against base 2664d14316790a57ea44b3f97c57440b70e43436. Traced all production ACP launch callers, adapter identity/defaults, environment precedence, invalid-prefix handling, and Unix cleanup; no additional blockers found. Two independent review lanes covered input/test contracts and process lifetime.

Validation: source review and clean diff check; hosted Rust lint, unit tests, and Windows Rust checks passed. Other desktop/integration checks were still running at the snapshot. No local runtime tests or Windows reproduction performed. New wrapper tests are Unix-only and use exec-style wrappers, so green CI does not establish supervised-worker cleanup. Protected Goose UI/human acceptance and the linked Buzz-App integration remain separate validation gates.

wpfleger96

🤖 thanks for keeping this to the one spawn boundary. I traced every AcpClient::spawn caller, the Pi/Codex/Hermes identity paths and the invalid-prefix handling, and they hold up. two things beyond the Windows thread Carl already covered, both inline.

wpfleger96 · 2026-09-30T14:07:13Z

+use super::AcpError;
+use tokio::process::Command;
+
+pub(super) const PREFIX_ENV: &str = "BUZZ_ACP_LAUNCH_PREFIX";


🤖 Desktop doesn't reserve this key. RESERVED_ENV_KEYS in desktop/src-tauri/src/managed_agents/reserved_env_keys.rs lists BUZZ_ACP_AGENT_COMMAND, BUZZ_ACP_AGENT_ARGS and BUZZ_ACP_MCP_COMMAND as the code-execution surface, and merged_user_env only filters what's on that list. managed_agents/runtime.rs writes the layered user env (definition → global → persona → agent) onto the harness command last. So a saved persona or agent env var can set BUZZ_ACP_LAUNCH_PREFIX and choose the executable that every worker starts through. The remote deploy path (commands/agents_deploy.rs) puts the same merged env into launch.env.

this also works against the sandbox use case. if a host sets the prefix and then applies user env on top of it, the way Desktop orders things, a saved env var can replace the launcher with ["/usr/bin/env"] and the worker runs unprotected.

could we add it to RESERVED_ENV_KEYS, with the matching assertion in managed_agents/env_vars/tests.rs, and note in the README that hosts have to apply the prefix after any user-supplied env? I think this should land with this PR, since the key only becomes dangerous once this PR ships.

wpfleger96 · 2026-09-30T14:07:13Z

            };
        cmd.envs(launch_env.iter().cloned());
+        cmd.env_remove(launch::PREFIX_ENV);
        let mut child = cmd.spawn()?;


🤖 nit, nonblocking: with a prefix set, a missing or non-executable launcher surfaces here as a bare io error (No such file or directory), which looks identical to a missing worker. adding the launcher path to the error when the prefix is set would make the fail-closed case easier to diagnose from the harness log.

Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com>

wesbillman

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

Re-review: no remaining code blockers. The prior P2 Windows lifecycle finding is addressed. Reviewed head 7813e74f9c8a0fdf27437c4c5dafc0483179f4a0 against base 2664d14316790a57ea44b3f97c57440b70e43436, focusing on the delta from 9ae9d82f8.

Fix verified: every configured prefix now fails before command construction on non-Unix, including empty/non-Unicode values; unset preserves direct launch. Independent lifecycle re-review agrees. Desktop’s shared case-insensitive reservation reaches save validation and local/remote environment composition. Executable error context introduces no new defect found.
Existing CI verified: native Windows executed both platform-contract tests successfully and passed the new reservation regression in its full Desktop suite. Linux ACP suite: 992 passed, 3 skipped. These ran on synthetic merge 21033ec2e6beb7db9984b5f19821acdb1fade991, whose parents are the exact base/head above. No local runtime tests rerun.
Remaining gates, not code findings: Desktop Core CI was still running at closeout; protected-Goose UI and human acceptance remain pending. The previously noted Unix supervising-wrapper cleanup test gap remains optional. PR metadata is stale: Windows has passed, and the PR is non-draft despite its body saying “Draft pending human testing.” Reconcile readiness with the human acceptance checklist before merge.

This is a review comment, not approval.

…i-port * origin/main: fix(agents): stop built-in prompts from teaching sleep polling (#7992) feat(relay): add direct staff ban/timeout/delete with staff guard (#7883) fix(ci): gate security review on repo write access (#7986) feat(acp): wrap workers at the subprocess launch boundary (#7985) feat(buzz-relay): idempotent owner community deletion with quota reservation (#7969) feat(mobile): show contextual names in lists, Search and Pulse (#7896) Add Kimi Code's default install path to managed-agent binary discovery (#5997) Co-authored-by: Will Pfleger <wpfleger@block.xyz> Signed-off-by: Will Pfleger <wpfleger@block.xyz>

Brings in 31 upstream commits (639593b), including ACP native-steer frame-writer fixes (block#7568, block#8022), edited-message routing (block#4741), worker wrapping at launch (block#7985), BUZZ_GIT_IDENTITY (block#8024), thread roots in agent activity (block#8029) and built-in prompts without sleep polling (block#7992). Adaptations: - buzz-acp acp.rs: keep the fork's turn_output module alongside upstream's frame_writer module. - buzz-acp pool.rs: turn_started carries both the fork's triggeringRootEventIds and upstream's threadRootEventId. - buzz-acp queue.rs: drain_channel keeps upstream's withheld-steer reaction collection and still clears the cancelled-root tombstones. - buzz-acp queue.rs: task_root_event_id is now edit-aware, so the 1h cancelled-root tombstone also drops edits routed into a stopped tree. - buzz-acp base_prompt.md: keep the fork's empty-final-answer rule for bare acknowledgements, take upstream's handoff wording and no-sleep guidance. - buzz-acp tests: new `edit` field on fork-only QueuedEvent/BatchEvent tests. - desktop channels.rs: keep has_active_non_starter_channel guard inside upstream's ensure_starter_channels_inner. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Arnoldinh0 <arnaudlafosse92100@gmail.com>

shellz-n-stuff mentioned this pull request Sep 30, 2026

feat(agents): support native launch protection providers block/buzz-app#415

Merged

shellz-n-stuff marked this pull request as ready for review September 30, 2026 13:44

shellz-n-stuff requested a review from a team as a code owner September 30, 2026 13:44

chatgpt-codex-connector Bot reviewed Sep 30, 2026

View reviewed changes

wesbillman requested changes Sep 30, 2026

View reviewed changes

wpfleger96 reviewed Sep 30, 2026

View reviewed changes

shellz-n-stuff force-pushed the codex/acp-launch-prefix branch from 9ae9d82 to 90d759f Compare September 30, 2026 14:40

feat(acp): wrap workers at the subprocess launch boundary

7813e74

Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com>

shellz-n-stuff force-pushed the codex/acp-launch-prefix branch from 90d759f to 7813e74 Compare September 30, 2026 14:41

shellz-n-stuff marked this pull request as draft September 30, 2026 14:42

shellz-n-stuff marked this pull request as ready for review September 30, 2026 15:07

wesbillman reviewed Sep 30, 2026

View reviewed changes

wpfleger96 approved these changes Sep 30, 2026

View reviewed changes

wesbillman approved these changes Sep 30, 2026

View reviewed changes

shellz-n-stuff merged commit 95b018c into main Sep 30, 2026
94 checks passed

shellz-n-stuff deleted the codex/acp-launch-prefix branch September 30, 2026 15:52

Navigation Menu

feat(acp): wrap workers at the subprocess launch boundary - #7985

feat(acp): wrap workers at the subprocess launch boundary#7985
shellz-n-stuff merged 1 commit into
mainfrom
codex/acp-launch-prefix

shellz-n-stuff commented Sep 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 30, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Sep 30, 2026

Uh oh!

wesbillman left a comment

Uh oh!

wpfleger96 left a comment

Uh oh!

wpfleger96 Sep 30, 2026

Uh oh!

wpfleger96 Sep 30, 2026

Uh oh!

wesbillman left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		let mut command = Command::new(executable);
		command.args(&prefix[1..]).arg(worker);

Conversation

shellz-n-stuff commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related issue

Testing

Uh oh!

github-actions Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔐 Codex Security Review

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

wesbillman left a comment

Choose a reason for hiding this comment

Uh oh!

wpfleger96 left a comment

Choose a reason for hiding this comment

Uh oh!

wpfleger96 Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

wpfleger96 Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

wesbillman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shellz-n-stuff commented Sep 30, 2026 •

edited

Loading

github-actions Bot commented Sep 30, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 30, 2026 •

edited

Loading