Fix ACP intentional-stop write race - #2552
Merged
ymichael merged 1 commit intoAug 27, 2026
Merged
Conversation
ymichael
deleted the
bb/ci-flake-fix-acp-release-construction-epipe-thr_srertvqxjz
branch
August 27, 2026 18:30
ymichael
added a commit
that referenced
this pull request
Aug 27, 2026
## Human comments ## What was wrong On the independently verified fresh-main baseline [`54d6e3ee64f073872766a09a70a474999acb9f68`](54d6e3e), the ACP topology test treated existence of the fake agent's SIGTERM marker as completed publication and immediately read it. `writeFileSync(path, data)` creates or truncates the final pathname before writing its bytes, so scheduler contention could let the test observe that intermediate zero-length file. Tagged instrumentation reproduced the exact [main CI failure](https://github.com/get-bb/bb/actions/runs/33100377644/job/98616171627): every empty first read became `"SIGTERM\n"` shortly afterward, with exactly one fake-agent launch. The runtime timeout and release behavior were correct; the fixture exposed an incomplete marker as if it were complete. ## What changed The fake ACP agent now writes the SIGTERM marker to a same-directory, PID-specific staging file and atomically renames it to the observed path only after the marker bytes are complete. Existence of the final path is therefore a real completion boundary, while the existing topology test still covers the 300 ms construction timeout, best-effort release, real SIGTERM lifecycle, child exit, and live bridge. No test clock or assertion changed. [PR #2552](#2552) changes intentional-stop connection state but retains the same SIGTERM and does not touch this fake-marker seam, so the fixes do not conflict or duplicate one another. This changes no server/daemon wire data, CLI, configuration, or public plugin contract, so no `HOST_DAEMON_PROTOCOL_VERSION`, guide, or API documentation update is required. ## How you verified - Red/green exact stress harness on 16 logical CPUs, 64 focused runs at 16-way concurrency: before, 59/64 passed and 5/64 failed with `expected '' to contain 'SIGTERM'`; after, 64/64 passed with zero failures. A tagged diagnostic run reproduced 12/64 exact failures and proved each empty first read later contained `"SIGTERM\n"` with one child launch. - `pnpm exec turbo run test --filter=@bb/agent-runtime --force -- --run src/runtime.acp-topology.test.ts -t "releases the thread on the bridge when a construction times out on the runtime's side"` — 1 test passed. - `pnpm exec turbo run test --filter=@bb/provider-bridge-acp --force -- --run src/bridge/bridge.test.ts -t "releases a session still under construction: the agent is reaped and the pending thread/start fails"` — 1 focused lifecycle test passed. - `pnpm exec turbo run test --filter=@bb/agent-runtime --filter=@bb/provider-bridge-acp --force` — agent-runtime 22 files / 318 tests and ACP bridge 17 files / 295 tests passed. - `pnpm exec turbo run typecheck --filter=@bb/agent-runtime --filter=@bb/provider-bridge-acp --force` — passed. - `pnpm exec turbo run build --filter=@bb/agent-runtime --filter=@bb/provider-bridge-acp --force` — applicable Turbo build graph passed. - `pnpm exec prettier --check packages/provider-bridge-acp/src/bridge/fake-acp-agent.mjs` and `git diff --check` — passed; all temporary `[DEBUG-acp-marker]` instrumentation was removed. > AGENT GENERATED: by GPT-5.6-Sol
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human comments
What was wrong
On the independently verified fresh-main baseline
1d97c63ed13b5231c1051b7af7ac36661d1f65ab,AcpAgentConnection.kill()sent SIGTERM but left the connection writable and its requests pending until the asynchronous child exit event. When a release landed duringthread/startconstruction, a buffered or continuing write could therefore hit the child after stdin closed; the EPIPE handler then won the race and rejected the pending start with transport wording. That is the failure seen in main CI run 33029831856. The EPIPE was handled, but accepting traffic after an intentional stop was the underlying lifecycle bug, so expanding the bridge test's message regex would have hidden it.What changed
AcpAgentConnectionnow latches an intentional stop synchronously. It immediately makes the connection unavailable, rejects pending requests with the existingis not runninglifecycle error, and drops later request/notification responses before they can write to the terminating child. The existing SIGKILL backstop remains for a child that closes stdin during SIGTERM without exiting.A connection-level regression deterministically closes the fixture child's stdin during intentional stop, attempts the late write that previously produced EPIPE, and verifies the pending and future requests both observe the normalized stopped state while the child exits normally.
This changes no server/daemon wire data, CLI, configuration, or public plugin contract, so no
HOST_DAEMON_PROTOCOL_VERSION, guide, or API documentation update is required.How you verified
Received: ACP agent ".../node" stdin closed (EPIPE): write EPIPE; the same test passes in 77 ms with the stop latch.pnpm exec vitest run --root packages/provider-bridge-acp --config vitest.config.ts --project '@bb/provider-bridge-acp:isolated' src/bridge/bridge.test.ts -t 'releases a session still under construction: the agent is reaped and the pending thread/start fails'.pnpm exec turbo run test --filter=@bb/provider-bridge-acp --force- 17 files and 289 tests passed.pnpm exec turbo run typecheck --filter=@bb/provider-bridge-acp --filter=bb-plugin-provider-acp --force- both package typechecks and the SDK type-generation dependencies passed.pnpm exec turbo run build --filter=@get-bb/plugin-sdk --force- all 17 published runtime entries built, including the ACP bridge bundle.git diff --checkpassed; only the ACP connection and its lifecycle test changed.