Repository navigation
Park and escalate on a recognized first-run gate - #155
Merged
Merged
Conversation
wait_for_readiness no longer fails a run outright the instant it
recognizes a vendor first-run gate (workspace trust, a theme picker,
sign-in). It now journals a new AdapterEventPayload::FirstRunGateDetected
(mapped to RuntimeEvent::FirstRunGateDetected { kind }, a new closed
FirstRunGateKind enum mirroring the TUI adapter's own internal GateKind),
raises the paired EscalationRaised { reason: "vendorFirstRunGate" } with
a runtime-authored question (Redacted::assert_runtime_authored -- every
word is a fixed template plus a closed-enum-derived phrase, never
vendor or captured content), and keeps polling classify_surface on no
deadline of its own until the surface resolves to PromptReady (resumes
the ordinary paste/Enter path, unmodified) or the process exits (the
existing exit watcher reports it, naming the gate that was blocking the
run). The escalation is journaled exactly once per distinct gate even
though the same Gate(kind) classifies on every tick until it resolves.
The escalation's DB/journal side effect -- inserting the escalations
row and raising EscalationRaised transactionally with the event append
-- is wired the same way the existing write-violation escalation
already is: a second mutation DomainAdapterEventSink::emit raises after
its main event commits, keyed off the just-built event's shape, rather
than a new bespoke call path. This is also the first production call
site to populate EscalationRaised.question; the redaction-guard
allowlist entry for EscalationRaised.reason is updated to name it.
packages/extension/src/milestones.ts's escalationRaised digest case
rendered only reason, never question -- a bug dormant since no
production call site had ever populated the field until now. It now
appends question after reason when present, and renders exactly
today's sentence when it is absent.
Tests: a fixture-driven replay of a real captured claude workspace-trust
gate proves wait_for_readiness escalates exactly once (naming the right
kind) and then fails naming the gate once the process is forced to
exit; a call-counting test vendor proves a gate that clears after a few
ticks resumes readiness normally; two new event_sink.rs tests prove the
full DB path -- FirstRunGateDetected and the paired EscalationRaised
both journal and broadcast, with a question naming the right vendor and
gate phrase, for both a claude and a codex gate kind. milestones.test.ts
gains both the question-present and question-absent digest cases.
bun run generate regenerates the FirstRunGateKind TS binding and the
RuntimeEvent union; packages/protocol-ts/src/index.ts's barrel export
is updated to match (caught by generate --check, not by hand).
Deliberately not yet fixed here: cancel() and dispose() both open on
self.run's async lock, which wait_for_readiness's new unbounded
gate-park poll now holds for as long as it runs -- a run parked at a
gate cannot yet be cancelled. That is a real, separate concern (a
lock-ordering fix, not an escalation-shape one) and lands as its own
following commit so it is reviewable on its own.
cargo fmt --all, cargo clippy --all-targets --all-features -D warnings,
CREW_DISABLE_VENDOR_CLI=1 cargo test --workspace (69 test binaries, all
passing), bun run typecheck, and bun test all verified clean.
The previous commit's gate-escalation poll waits on no deadline of its own, holding self.run's async lock for as long as it runs (wait_for_readiness is called with that lock already acquired and held for run_pipeline's whole duration). cancel()'s CancelScope::Worker/Subtree branch, and dispose(), both open by acquiring that same lock before they can send on the run's terminate_tx -- so a run parked at a gate could never actually be cancelled: the poll waits for an exit that only cancel can cause, and cancel waits for the poll. Under the previous bounded readiness_cap wait this was invisible; the new unbounded gate-park wait turns it into a genuine, permanent deadlock. Fixed with a lock-free CancellationToken, not a change to the lock itself: TuiAdapter grows a cancel_token field (a std::sync::Mutex around a tokio_util::sync::CancellationToken, never held across an await). start() and resume() each swap in a fresh, unfired token before entering the pipeline -- never a previous run's, so an earlier run's cancellation can never poison the next one's -- and thread it down through resume_from/run_pipeline into wait_for_readiness. cancel() and dispose() both fire the token first, before either touches self.run's lock; firing needs no lock of its own and always succeeds, and every existing path after that point is unchanged. The gate-park select! (and, for the same reason, the pre-existing bounded Undecided-branch select!) gains one new arm racing cancel_token.cancelled() alongside the existing pty.exit_watcher() arm. Test: a run parked at a gate that never clears on its own, cancelled from outside, ends well within a 2s outer timeout rather than hanging toward its 30s pty sleep or 30s readiness cap. Verified as a genuine regression control, not just a passing test: temporarily removed the new select arm and confirmed the test fails by timeout (not by a wrong assertion) before restoring it. cargo fmt --all, cargo clippy --all-targets --all-features -D warnings, and CREW_DISABLE_VENDOR_CLI=1 cargo test --workspace (69 test binaries, all passing) verified clean.
# Conflicts: # crates/runtime/src/adapter/event_sink.rs
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
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.
A recognized vendor first-run gate (workspace trust, a theme picker, sign-in) no longer fails a run
outright.
wait_for_readinessnow journals a typedFirstRunGateDetected { kind }event, raises thepaired
EscalationRaised { reason: "vendorFirstRunGate" }with a runtime-authoredquestionnaming the vendor and the specific gate, and keeps polling
classify_surfaceon no deadline of itsown until the surface resolves to
PromptReady(the ordinary paste/Enter path resumes unmodified) orthe process exits (the existing exit watcher reports it, naming the gate that was blocking the run).
The escalation journals exactly once per distinct gate even though the same gate classifies on every
poll tick until it resolves.
The escalation's database side effect -- inserting the
escalationsrow and raisingEscalationRaisedtransactionally with the event append -- reuses the exact second-mutation shapethe existing write-violation escalation already has inside the adapter event sink, rather than a new
bespoke call path. This is also the first production call site to populate
EscalationRaised.question.The milestone digest for
escalationRaisedpreviously rendered onlyreason, neverquestion-- abug dormant since no production call site had ever populated the field until now. It now appends
questionafterreasonwhen present, and renders exactly the previous sentence when it is absent.Parking a run at a gate on no deadline surfaced a real, separate defect:
cancel()anddispose()both open by acquiring the same lock the readiness poll now holds for as long as it runs, so a run
parked at a gate could never actually be cancelled -- the poll waits for an exit only cancel can
cause, and cancel waits for the poll. Fixed with a lock-free, per-run
CancellationToken:cancel()and
dispose()fire it first, before touching that lock; the poll gains one new arm racing italongside the existing exit-watcher arm. Firing the token unconditionally, for every cancel scope
including a plain turn-interrupt, is a deliberate widening of that scope's ordinary meaning in this
one window: parked at a gate there is no turn in progress to interrupt, so the only coherent reading
of "stop" is to stop waiting for the gate, and the alternative (a turn-interrupt cancel blocking
behind the same unbounded poll) is worse -- a caller that never returns.
Testing: a fixture-driven replay of a real captured gate proves the escalation fires exactly once and
that a forced process exit names the gate; a deterministic test vendor proves a gate that clears after
a few ticks resumes readiness normally; two database-level tests prove the full journal path for both
a claude and a codex gate, including the exact question wording; the cancellation fix is verified as a
genuine regression control by removing the new select arm and confirming the test fails by timeout
rather than trusting the code.