Skip to content

test(server): stop ACP soft-steer tests racing the prompt settle - #13201

Open
saphid wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:test/acp-settled-soft-steer-race
Open

saphid wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:test/acp-settled-soft-steer-race

Conversation

@saphid

@saphid saphid commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

What Changed

Added an afterPromptSettledWithBackgroundWork test hook to AcpAdapterV2. It fires once the adapter records a settled prompt while background work is still running. Three AcpAdapterV2 tests now wait on that hook before they send the soft-steer interrupt. Before, they waited for the raw stopReason frame and then did two Effect.yieldNow calls. Test-only change: the hook defaults to Effect.void.

Why

Stop after settled soft steer contains the orphan runtime and respawns without subagent carryover is an it.live test against the mock ACP agent subprocess. The raw stopReason frame is logged before the adapter records the settle. Two yields do not guarantee the adapter has caught up. When the interrupt arrives first, the adapter still treats the prompt as running and sends session/cancel, and the test fails:

AssertionError: settled soft steer must not send session/cancel before the later Stop: expected true to be false

This happened on 2 of 2 recent Linux CI runs of our fork's nightly build of this branch. On macOS it passed 14 of 14 runs, including 8 runs under CPU load. Two it.effect tests use the same wait pattern, so they get the same fix.

Verification

  • Reproduced the failure: added a scratch 100 ms delay before the adapter signals the settle. The unchanged test then fails every time with the assertion above.
  • Fix holds with the delay: with the fix and the same delay, the test passes.
  • Full file, delay removed: vp test run src/orchestration-v2/Adapters/AcpAdapterV2.test.ts passes 113/113 after restacking onto 9fcc9a4bdb (head 4f3c2042ed).
  • apps/server typecheck and vp fmt --check pass. vp lint reports no new warnings.

Verification caveat: the first new-tip run had two cancellation-acknowledgement failures in unchanged tests. Those two cases passed on the unmodified base, and the full candidate file passed 113/113 on retry without code changes.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (no UI changes)
  • I included a video for animation/interaction changes (no UI changes)

Fixed by Claude Opus 5.5 in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 23, 2026
saphid pushed a commit to saphid/t3code that referenced this pull request Sep 23, 2026
The previous patch fixed two AcpAdapterV2 tests but missed the it.live
"Stop after settled soft steer" test with the same race, which failed
the Test gate on the last two runs. The replacement commit covers all
three and is proposed upstream as pingdotgg#13201.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@macroscopeapp

macroscopeapp Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b0ad145

Macroscope's review found this PR approvable — This is a focused test-harness synchronization change: three ACP tests now await an explicit prompt-settlement signal instead of relying on scheduler yields. The optional adapter hook defaults to a no-op and is not provided on production paths, so no user-facing behavior or product defaults change.

No code changes detected at 4f3c204. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@saphid

saphid commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

@juliusmarminge could you merge this when you get a chance? It's test-only: three AcpAdapterV2 tests wait on a settle hook instead of yieldNow. The it.live one failed on Linux on its own.

All server test shards pass. The one red check, Check, fails the same way on t3code/codex-turn-mapping at 060756de5ad without this change: lint:restyle-ceiling reports 659 web className overrides against a ceiling of 628. It was still green on #13113, so the breach landed on the branch after that.

@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 6 times, most recently from 1bd44f2 to 3b9c885 Compare September 24, 2026 04:06
@saphid
saphid force-pushed the test/acp-settled-soft-steer-race branch from b0ad145 to 25e8a82 Compare September 24, 2026 12:23
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 3 times, most recently from fe4f6ad to 87c67bd Compare September 25, 2026 05:55
@saphid
saphid force-pushed the test/acp-settled-soft-steer-race branch 2 times, most recently from 70246f5 to ccab19d Compare September 27, 2026 00:25
Three AcpAdapterV2 tests waited for the raw `stopReason` frame from the
mock agent and then yielded twice, assuming the adapter had already
recorded the settled prompt. On slower runners the interrupt can win,
the adapter still sees a running prompt and sends `session/cancel`, and
"Stop after settled soft steer contains the orphan runtime" fails with
"settled soft steer must not send session/cancel before the later Stop".

Add an `afterPromptSettledWithBackgroundWork` test hook that fires once
the adapter records the settle, and have the tests await it instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@saphid
saphid force-pushed the test/acp-settled-soft-steer-race branch from ccab19d to 4f3c204 Compare September 27, 2026 01:16

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant