Skip to content

VC-778: Bound only deliberately stalled stages in the crash-recovery fixture - #929

Merged
hussainph merged 1 commit into
mainfrom
volli/VC-778-flake-crash-recovery-reconciles-an-accepted-comm
Oct 19, 2026
Merged

hussainph merged 1 commit into
mainfrom
volli/VC-778-flake-crash-recovery-reconciles-an-accepted-comm

Conversation

@hussainph

@hussainph hussainph commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Problem

crash-recovery.integration.test.ts failed in CI three times with timing-shaped errors (VC-778):

  • run 38902077773: start() line 197, the first fresh adapter.attach, received rejected
  • run 39082918262: line 681, a context_replay attach received rejected
  • run 39089674900: boot returned queue_admission_timeout: binding recovery did not finish

The cause is in the fixture, not the product. It set followUpAdmissionTimeoutMs: 50 for the whole runtime. In session-runtime.ts, that bound does not apply only to queued messages. #attach runs every attach's workspace and executor stages under follow-up:${commandId}, including a user's attach-* command. #rehydrateBinding runs every cold binding rebuild under follow-up:${attachmentId}:recovery. A real Pi attach or native_resume that took longer than 50 ms on a loaded runner therefore became a false queue_admission_timeout, and the attach was rejected. (An earlier note on the ticket said sighting 1 could not be this timer, because its id is attach-*. The follow-up: prefix inside #attach shows it can.)

Fix (test-only)

  • Genuine work keeps the production admission bound (30 s). The fixture now reads the bound through a getter. It returns the short 50 ms bound only after the fixture has entered one of its own gates (attachGate / admissionGate). The wrapper sets that flag synchronously, before the runtime starts the timer for that stage.
  • The stuck-stage tests now also assert which stage timed out (executor attach / message admission). Before, a slow real rebuild ahead of the gate could time out and still satisfy them.
  • toBe("accepted") assertions print the receipt, so any future refusal is named.

No retry was added, and no timeout was widened for the stalled stages.

Decisions

  • Kept the product untouched. The ticket note asks for a fixture/timing fix. The product question (all cold rehydrates share the 30 s follow-up bound) stays an open, unmeasured note on the ticket.
  • Used the production default (30 s) for genuine stages rather than a smaller "generous" number. That is the bound the product actually runs with.

Tests

  • New: accepts a slow but healthy attach and recovers its binding without a false admission timeout (VC-778). It uses a real 150 ms attach delay for the fresh attach and for the boot native_resume rebuild.
    • Without the fix (50 ms bound restored on every stage) it fails deterministically, with sighting 1's exact symptom: rejected / queue_admission_timeout: executor attach did not finish at start().
    • With the fix: the whole file passes, 15/15 (vp test run src/session-runtime/crash-recovery.integration.test.ts --maxWorkers=1).
  • vp check passes on the touched file.

Overlap: git merge-tree is clean against origin/main and against the heads of #909, #919, #920, #921 and #922. No open PR touches this file.

CI rerun disclosure

Run 39241933071, attempt 1: Test (packages) failed only on src/worktree/watch.test.ts:1111 ("500 Done tickets … cost 10 gh calls a cycle", 5 s timeout). That is the known flake VC-856, unrelated to this change. crash-recovery.integration.test.ts passed 15/15 in that same run. The failed jobs were rerun once, with the supervising owner's approval.

…very fixture (VC-778)

The fixture set followUpAdmissionTimeoutMs: 50 for the whole runtime, but
that bound covers every adapter.attach (workspace + executor stages) and
every cold binding rebuild, not only queued messages. A slow but healthy
real Pi attach under CI load then came back rejected with
queue_admission_timeout. Genuine work now keeps the production 30 s bound;
the 50 ms bound applies only once the fixture enters an attach/admission
gate. Adds a regression test with a real slow attach, and receipts in the
accepted-status assertion messages.
@coderabbitai

coderabbitai Bot commented Oct 19, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 6e5b8405-3ba2-417c-a1c7-896df28d225a
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant