fix(host): rescue the edit barrier when an applyEditSettled transition throws - #406
Merged
Merged
Conversation
…n throws `commitTransition` runs outside the step's settle guard, which is the right default: settling a step whose transition never happened would hand the barrier a verdict for it. But an `applyEditSettled` transition unwinds before the panel commits the state it would have returned, so the write lock (`pendingApplyBaseVersion`) stays HELD with no replacement settlement ever coming — the deferred side channels (context-handoff / codex-context-handoff / switch-to-text) sat in the barrier forever and their at-receipt guards (the Codex single-flight) never released. Catch the transition throw and rescue with `settle(false)`, conditioned on the throwing event being the settlement itself: - Conditional, because an unconditional rescue is a different bug: `settle(false)` calls `dropAll()` before consulting `isLocked`, so dropping on a non-settlement throw would destroy thunks that the real settlement, still pending, would legitimately have drained. Only `applyEditSettled` ever releases the lock, so only it may rescue. - `false`, not `true`, because the state was never committed: the lock still reads held and `settle(true)` would take the barrier's WAIT arm and strand the thunks exactly as before. Only the failed verdict fires each `onDrop`, which is what frees the guards so the user can retry. The lock itself stays held — no queue or step policy can release state that was never committed. - The rescue does not step on the failure it recovers from: a throwing rescue settle is reported through the (isolated) reporter and the transition error propagates as the triage payload, matching the effect-throw ordering. The reporter isolation is now a shared helper used by both paths. Latent today — production's write validator is fail-closed, so the only throw sources are the reducer's own defensive exhaustive arms. Tests pin both directions and were measured non-vacuous: three go red against the unfixed source (drop, verdict, rescue-settle-throw), and relaxing the condition to always-rescue turns the two non-settlement tests red.
The claim that applyEditSettled is "the only event that ever releases the lock" was unqualified, but the core's disposed arm also clears pendingApplyBaseVersion (on teardown). Scope the claim to the live path and note why disposed needs no rescue: the barrier's own isDisposed() check already drops deferred thunks regardless of verdict there.
The catch block's bare event.type === "applyEditSettled" check gave no compile-time guard: a new HostSessionEvent member that also releases the write lock would silently skip the rescue and reintroduce the exact stranding this module fixes. Extract releasesWriteLockOnCommit(), an exhaustive switch with the same never-assignment idiom as isEditApplied's outer switch, and use it for the rescue condition. Not merged with isEditApplied's switch: the two answer different questions, and isEditApplied's own default message is pinned by an existing test. Pin the runtime default arm with a test mirroring isEditApplied's unknown-event test; the exhaustiveness itself is a tsc-time guard, the same split isEditApplied's own JSDoc already documents.
"keeps the transition error when the rescue settle AND the reporter
throw" only asserted toThrow(transitionErr), which does not tell apart
"the rescue ran and both its own throws were isolated" from "the
rescue never ran at all" — both reach the caller with just
transitionErr.
Capture settleEditBarrier / onSettleError calls and assert
settleEditBarrier ran once with false and onSettleError ran once with
the settle error. Revert-check: temporarily deleting the whole
`if (releasesWriteLockOnCommit(event)) { ... }` rescue block turns
this test red (settles/reported stay empty) along with four sibling
tests, confirming the strengthened assertions actually detect the
rescue's absence.
"Drop them so the guards are freed and the user can retry" over-claimed what settle(false) buys. Measured: a side channel that retries after the rescue lands in editSettledBarrier.run(), finds the write lock still held (the rescue never clears pendingApplyBaseVersion), and re-defers behind it -- stranded again until dispose. Reword to state the payoff is a one-shot guard/thunk release, and note that releasing the lock itself is a separate follow-up.
The JSDoc named only the pre-existing drain-settle call (after runEffects), but this PR added a second: the rescue settle in the transition-throw catch block, which can mask a transition error the same way the drain settle can mask an effect throw. Name both.
… a named helper The catch arm held the rescue inline — three nesting levels and the full rationale block — which buried the step's four-phase shape (transition → verdict → effects → settle) behind it. The rescue now lives in a rescueStrandedSideChannels closure next to reportSettleError, following the precedent this same change set already uses for reportSettleError and for hanging long why-blocks on a named helper's doc comment. No rationale was dropped: the paragraphs moved verbatim into the helper's doc. Two claims needed re-anchoring because the move invalidated their positional wording — onSettleError's doc now names the helper instead of pointing at 'the catch block below', and the settle-error note says 'the effect-throw path in the step below' plus a parenthetical that the caller rethrows immediately, which is what made the precedence self-evident while the code was inline.
The 3 comment sites explaining why a throw from the disposed transition needs no rescue said the barrier's isDisposed() check 'already' drops the deferred thunks, which reads as happening in this same step. It does not: releasesWriteLockOnCommit(disposed) is false, so settleEditBarrier is never called from this step at all. The actual drop rides an in-flight apply's own LATER applyEditSettled step (its dispatch fires post-dispose in every outcome arm, per effect-executor.ts's runApplyEdit header) finding isDisposed() already true (the panel sets its local disposed flag before dispatching the disposed event, per quoll-editor-panel.ts's onDidDispose) and taking the barrier's dropAll() branch there. If no apply was in flight, there was nothing deferred to strand in the first place. Also fixes an imprecise DRAIN/DROP conflation in the third site: the later settlement DRAINs (runs) the thunks on the live path, but DROPs them via isDisposed() if disposed won the race first — edit-settled-barrier.ts treats these as distinct outcomes of settle().
releasesWriteLockOnCommit's disposed case answers false, but no test drove a disposed event through step, so a mutation that moved disposed into the same rescue arm as applyEditSettled passed the whole suite unnoticed. Revert-check: reproducing that mutation (moving case "disposed" into the applyEditSettled true-arm) turns this new test red (settles === [false] instead of []) while every other test in the file stays green.
Chain a real editSettledBarrier through createHostSessionStep across two steps: a throwing `disposed` transition (which needs no rescue of its own), followed by the in-flight apply's own, independent applyEditSettled settlement. The mid-state assertions confirm nothing is dropped or run at the disposed step itself, and the follow-up step confirms the drop rides that later settlement's isDisposed() check. Neither existing test chains both steps through a real barrier: edit-settled-barrier.test.ts pins the isDisposed-drop in isolation, and the "does not attempt the rescue when a disposed transition throws" test uses a stub settleEditBarrier with no follow-up step.
The comments describing why a throwing settlement rescue stays conditioned on applyEditSettled characterized the barrier's later, independent settlement as a DRAIN/DROP binary (drain on the live path, drop if disposed won the race). edit-settled-barrier.ts's settle() has four outcomes: DRAIN, DROP via a failed apply, DROP via isDisposed() (the same `deps.isDisposed() || !applied` arm, two distinct causes), and WAIT when a stash-drain re-acquires the lock. Reworded the module header, the commitTransition dep doc, and rescueStrandedSideChannels's doc to name all three arms and, where DROP is discussed in detail, its two causes. releasesWriteLockOnCommit's doc makes no such enumeration claim, so it is unchanged.
…nsition doc The previous edit appended a sentence without re-wrapping, leaving a 90-column line in a block whose other prose lines wrap around 78-82. Words, punctuation, and line count are unchanged; only two line breaks moved.
mtskf
added a commit
that referenced
this pull request
Sep 10, 2026
…ransition (#409) * fix(host): add the settlementTransitionFailed recovery arm A throwing applyEditSettled transition unwinds before the panel commits the state that releases the write lock, so the lock stayed held for the rest of the panel's life and every later edit was stashed into a pendingEdit that could never drain. Add the reducer arm that releases the lock, drops the stash, and reposts the authoritative Document so the webview's single flight un-parks with its replay buffer intact. The throw also abandoned the settlement's own save-failure toast, so the arm always raises one outcome-blind internal-error toast in its place, adding a dropped-edit clause only post-dispose, where the replay buffer went with the iframe and the stash really was the only carrier. The arm is a pure field reset by design: decideEdit and the outcome switch are throw sources of the transition being recovered from, so the recovery must not re-enter them. Wiring follows. Also order the two withheld-ack effect lists so the user-visible showResyncFailure precedes the logWarn the executor runs unguarded. * fix(host): commit the write-lock recovery from the step's transition catch PR #406's rescue released the deferred side channels' at-receipt guards but never the write lock, so the payment was one-shot: a retried side channel re-deferred behind a lock nothing would release. The catch now commits the recovery transition first and drops the side channels second, so the drop is durable and a retry runs. The recovery re-bases on the throwing settlement's own observed version, so no second guarded version reader is introduced. Also re-anchor the release-site claims in this module and broaden the onSettleError channel, which now carries recovery failures too. * fix(host): wire the panel's write-lock recovery The panel hands the step its own state-committing lambda, so the recovery releases the lock THROUGH the reducer and the panel never patches reducer state itself. Pinned end to end against the real core + step + barrier + dispatcher composition, including that the next edit is accepted and persisted. Non-vacuity measured: stubbing commitWriteLockRecovery to return no effects reddens the lock-release assertion. * docs(host): re-anchor the write-lock release-site claim at every altitude The recovery arm makes "applyEditSettled is the only live release site" false. LEARNING 2026-09-09 recorded that this claim lives at several altitudes and that reviews twice caught one copy left stale, so all of them are updated together and the roster is pinned in the plan's inventory step. Comment-only, plus the stale "STAYS locked" narration on the barrier-drop test, which now says what the stub is isolating and points at the composition test for the actual release. * test(webview): name the same-epoch replay pin as the write-lock recovery's receiving side The recovery's data-preservation claim rests on this test, but nothing in this file said so — an unrelated webview-sync refactor could have deleted it and left the claim silently unverified. * fix(host): contain a throwing logWarn, and enforce three prose-only contracts Review-cycle findings on the write-lock recovery, in four groups. 1. runEffects' `logWarn` was the one effect case with no `try` of its own, and several reducer arms put a triage log AHEAD of the effect that pays the incident — the drain `accept` arm returns `[...staleReBaseWarn, applyEdit]`. A throw there unwound the loop and abandoned the applyEdit, and since the committed state had already re-acquired the write lock, no settlement was ever dispatched: the same stranded lock this branch exists to repair, except the recovery hangs off the TRANSITION catch and never sees a runEffects throw. Guarded at that one seam rather than by reordering each effect list. 2. The recovery's loss clause was gated on `disposed` alone, so the alive withheld-ack branch dropped the stash and told the user nothing was lost. There is no ack Document in that branch, so the webview's retained replay buffer is never replayed; the recovery then clears the lock, the next lock-free resync bumps `externalEpoch`, and the buffer is dropped on `recordedEpoch > buf.epoch`. The gate is now "will any ack go out?". 3. Three contracts that the comments declared and nothing enforced: - `releasesWriteLockOnCommit` returns the SETTLEMENT rather than a boolean, so the version reaches the recovery through the same decision that selected it. A future member mis-placed in that arm was silently handed the caller's fabricated `null`; it is now TS2322. - `HostSessionInputEvent` excludes `settlementTransitionFailed` from both dispatch surfaces. A dispatched recovery would queue behind a sibling whose lock-held stash arm then loses its stash to it; injecting that dispatch used to type-check clean. Derived with `Exclude` so a new member is carried automatically. commitTransition / commitWriteLockRecovery keep the wide type — committing the recovery is the legitimate path. - `commitWriteLockRecovery` staying REQUIRED is pinned in types-equality.test.ts, the one test program `pnpm compile` checks. 4. Comment accuracy at every altitude where this branch left a claim behind (LEARNING 2026-09-09). The rescue's `settle(false)` rationale said `true` would take the barrier's WAIT arm; since the recovery releases the lock first, `true` would DRAIN and RUN the thunks — the choice is DROP-vs-RUN. Also: the ack-site census (two, now three), the withhold pair's scope and post-dispose route count (three, now four), the accepted residual's throw source, `refused`'s effect order, "unbounded" recursion, and the test comment that presented its own stub as the production mechanism. Non-vacuity measured for every group. Reverting the logWarn guard reddens the new containment test; restoring the old `lostStash` reddens the new loss claim; gating the toast on `ackLabelObserved` reddens three core tests (it was green before, because the ordering test read presence through `indexOf` and absorbed absence as -1); gating the rescue on the recovery's success reddens exactly one of 335 step tests. For the two type mechanisms the probes were run BEFORE the narrowing as well as after: both were tsc-clean beforehand, which is the measurement that the contracts were prose-only. Adding `?` plus a `?? (() => [])` default to commitWriteLockRecovery leaves all 335 runtime tests passing and reddens only the type pin. * refactor(host): collapse the recovery arm's effect ternary and dedupe its test fixtures The recovery arm spelled `toast` and `triage` out on both branches of its return, so an ordering fix — the kind this PR made twice — could land on one branch and miss the other. Only the ack half is conditional now, and the order the surrounding comments justify lives in one list. Also: two comment corrections (SettlementEvent's doc claimed a nullability that belongs to the function's return type; a rationale restated 120 lines from the doc that owns it is now a pointer, keeping the clause that is not duplicated), three comment reflows left over 100 cols by earlier edits, and three local fixtures in the new describe block. `messageOf` deliberately keeps its cast so a missing effect still throws instead of reading as an empty string, which is what would vacate the not.toContain pins. * fix(host): contain every report that gates the lock-releasing dispatch The per-effect `logWarn` guard reported its own failure through a bare `console.error`, so a correlated console failure — VS Code patches the console as one IPC family — still unwound `runEffects` from inside the guard meant to contain it. On the drain `accept` arm that costs the WRITE and strands the write lock for the panel's life. Route every report made from a position where a throw must not unwind through one owner, `reportContained`. It takes a thunk so argument evaluation is contained too, and each site keeps its own console level and payload (the per-site triage tokens are unchanged). Five sites: the `showError` and `logWarn` per-effect guards, plus three that were never guarded and each needed only a SINGLE console failure to strand the lock — `readVersionGuarded`'s and `readCanWrite`'s catches (both evaluated while building the settlement event) and the rejection arm's leading log, which sat outside that arm's own `try`. Both guarded catches now carry `effect.message`, so triage keeps the identity of the toast or log line that was lost. Correct the `logWarn` header: the "ONLY effect with no try of its own" claim was written in the present tense in the hunk that added the try, and the "fixes the whole class at one seam" claim was wrong twice over — containment is best-effort, and three effects still evaluate their injected builder outside every try (`post` guards only `deps.send`). That gap is named where it stays findable. * fix(host): hedge the recovery arm's loss claim, and pin the gate it rests on The write-lock recovery arm is outcome-blind, so "A later unsaved edit was dropped." was an over-claim on the alive path: with no version advance the epoch does not move, the next same-epoch Document replays the retained buffer, and the bytes land. Split the clause by branch — definite post-dispose, where the stash really was the edit's only carrier, and hedged alive with a withheld ack, using the same "may not have been saved" phrase RESYNC_FAILURE_MESSAGE uses so the two toasts cannot disagree on certainty. The alive branch also names the remedy that preserves the bytes (copy before reloading), which composes with the resync toast's reload as an order rather than a conflict, and stands alone because that toast is latched per panel. Each branch carries its own closing instruction instead of sharing an appended one. Pin the clause on the SHARED `ackLabelObserved` gate: keying it on the raw `event.settledVersion` was undetectable by the whole session suite, because the two directions already covered keep the disjunction intact. The new test drives the state that separates them — alive, stash present, unobserved settled version, label already raised by a lock-held resync — and asserts BOTH clauses absent, since an absence check on the definite wording alone stays green against the hedge. Close the fail-OPEN derive: `HostSessionInputEvent`'s `Exclude` returns the original union when the excluded literal does not match, so a one-character typo re-admitted the forbidden dispatch with tsc silent, while its sibling `Extract` is fail-CLOSED. One pin in the only type-checked test location observes the exclusion expression itself. Comment corrections, all measured against the code: the effect ORDER is defence in depth, not a consequence of an unguarded `logWarn` (7 sites); the widened loss gate is NOT the settlement arm's gate, and the divergence is now stated outright with its reason; the four-step loss chain's third step is conditional; the alive stash's survival is bounded by the ack; `isEditApplied`'s heading is no longer a biconditional its own tail breaks; and a cross-reference drops a line number that went stale inside the PR that added it. * docs(host): reconcile two comments the hedged loss clause falsified Self-review of the previous commit: `withholdAckEffects`' header quoted the DEFINITE clause ("A later unsaved edit was dropped.") as what the recovery arm says on this same withheld-ack condition. That path is now the HEDGED branch — the definite wording belongs to post-dispose, where this pair is never built at all. Name the hedge, and say why it reuses RESYNC_FAILURE_MESSAGE's phrasing. The recovery arm's own note called both no-ack branches "losses" flatly, which is what the hedge exists to qualify: they differ in certainty, and that difference is exactly what the two wordings encode. * refactor(host): unnest the loss clause and share the fixtures that pin it The three-way loss clause was a nested ternary, which the house style forbids and which the same file already solves one arm up with an if/else chain. Unnesting it also puts each branch's commentary on its branch instead of in a header list the reader has to re-index. Tests: the two logWarn-containment cases carried a verbatim-duplicated 20-line fixture, so they could drift apart; `expectNoLossClaim` replaces three copies of the same not-dropped / not-hedge / Reopen triple, which makes this cycle's rule structural rather than remembered — checking only "dropped" reads the hedge as a pass, and a future no-loss site can no longer half-apply the check. Comments: two in-PR second derivations became pointers to the note that owns each argument. The standing "this does NOT close the class" guard stays; the history of how the claim was wrong belongs in the PR description, not the source. Non-vacuity re-measured after the refactor: reverting each fix still reddens the tests that were extracted, so the shared fixtures detect the original defects. * fix(host): route the remaining effect-executor reports through the contained owner `reportContained` already existed for reports made from a position where a throw must not unwind, but six such positions still reported through a bare console, so one console fault could unwind `runEffects` from inside the guard meant to contain it: - `sendEditRejected`'s three recovery dispatches. A throw here skipped `editRejectedDeliveryFailed` entirely (measured `dispatched: []`), leaving the rejection `pending` with the webview's single flight parked and visible-edge resync suppressed. The host makes no further attempt of its own; recovery waits on an external `documentChanged` or a webview reload. - `post`'s sync-throw catch, which `case "postRejectedDraft"` follows immediately with `sendEditRejected` — so an escape took the banner delivery, its recovery dispatch and the arm's `Cannot save:` toast with it. - the reseed build-failure guard, which additionally had the bare log AHEAD of the toast: with a throwing console it produced no toast attempt at all (`toastAttempts: 0`), making silent the exact state its own comment says must not be. The signal now goes first, matching the three reducer-side sites. - `reportResyncFailure`'s own fallback, the most correlated position of all since it runs only because `showError` just threw. The delivery-refused site also moves its two injected seam reads inside the thunk; argument evaluation happens before the call, so leaving them out contained only half the site. Each new wrap gets its own test: an unpinned guard is the one that regresses, measured on the `showError` fallback, which was the only `reportContained` site without a pin and survived reversion to a bare console with the file green. That gap is closed here too. Also makes this increment's own claims match what is measured: - the exclusion type pin was fail-OPEN on its own literal — `Extract<T, U>` answers `never` both when the union excludes the member and when the literal matches nothing. It now asks two questions of ONE shared literal, so the pair is a biconditional rather than a convention about keeping two copies in sync. - `reportContained`'s header conflated two different limits: the throw is fully contained, the report is not. Its list of calls that bypass the owner is now a rule rather than a roster. - the withhold pair's ordering rationale no longer rests on a failing fallback console call, which the guard absorbs; it rests on the guard being removable. - exports `RESYNC_FAILURE_MESSAGE` and pins the cross-module wording contract the recovery arm's hedge depends on. Rewording it previously left everything green. - corrects the `false`-arm count in `isEditApplied`'s contract, the ALIVE replay claim that a withheld ack falsifies, the "NO LOSS" branch that is really "no stash loss", and a toast-ordering claim that rested on a notification stack order this repo cannot verify. * docs(host): point each cross-reference at the claim it means `reportContained`'s header carries two standing claims — the best-effort limit and the effect-order rule — and two call sites pointed back at it by the same marker, so one of them resolved to the wrong paragraph. Naming the order rule gives each pointer a target it cannot miss, which is the failure mode that replacing duplication with pointers introduces. Also: a group comment counted three tests where there are four, and two headers were left mid-sentence by earlier edits. Comment-only — no statement, assertion, or test was added or removed. Measured while here: the header's roster of console calls that deliberately do NOT route through the helper matches a grep of every console call in the module, so that completeness claim is now verified rather than asserted. * fix(host): hedge the recovery loss clause for the alive withheld-ack case with no stash An alive `settlementTransitionFailed` recovery whose ack is withheld and whose `pendingEdit` is null fell into the no-loss branch, because `lostStash` requires a stash. That state told the user to "Reopen the file" — which destroys the webview buffer still holding the at-risk bytes. The recovery arm is outcome-blind, so those in-flight bytes sit under the same conditional loss as a stash: no ack means no replay, the next lock-free advance bumps the epoch, and edit-sync drops the buffer. Re-key the no-loss branch on `disposed || ackLabelObserved` so every alive recovery with an unobserved label takes the copy-first hedge, and generalise the hedge's wording so it no longer presupposes a stash that may not exist. Also correct three claims this branch introduced earlier and one type-level fail-open: - the `case "logWarn"` roster claimed ONE remaining unguarded position class "and it is not a console call". `case "openExternal"` is a second one, and its delegate's open positions ARE console calls (bare `console.warn` twice, plus bare `console.error` and `deps.showError` outside its own `try`). - the named ORDER RULE was stated as a universal that two reducer arms in this same repo contradict; restated in the count form the file already uses, with both exceptions named. - the withhold pair's order was justified partly as a defence against the unguarded builder positions. No effect list reaching that helper evaluates a builder, so the order cannot cover them; only the guard-removal half stands. - `RecoveryEventType = never` degenerated both event-union assertions to trivially true. One assertion asks the alias about itself, closing it. Pinned in both directions: restoring the old condition reddens the new no-stash hedge test, and inverting the new conjunct reddens all three no-loss sites plus the post-dispose definite claim. * docs(host): make the loss branch's state enumeration match its predicate Branch 1's header claimed exactly two states reach it, but its first bullet was keyed on `ackLabelObserved` alone — which is computed independently of `state.disposed`, so a disposed arm with an ack and a stash satisfied the bullet while landing in branch 2. The bullet's stated reason was also false post-dispose, where there is no replay buffer left to replay. Qualifying it as the ALIVE case makes the two bullets the branch predicate verbatim. Also rephrases "asserts no stash", which read as the opposite of its intent. Comment-only, zero line delta. * docs(host): de-quantify two signal-first census claims, narrow a settle claim - effect-executor.ts's ORDER RULE comment (and its second, duplicate reference to the same set later in the same file) asserted an exact count of reducer-side sites that put a user-visible signal ahead of a triage log. The count under-counted in both directions: a disposed signal-first arm was missing from it, and it omitted deliberate log-first exceptions. Replaced both with wording that names the closed set of sites citing the rule by name and explicitly disclaims closure in either direction, rather than asserting a count that goes stale on the next edit. - Narrowed two "the panel's `step` settles unconditionally" claims (also in effect-executor.ts) to name the one case where that does not hold: `isEditApplied` itself throwing before `runEffects` runs (host-session-step.ts:390). - test/extension/session/host-session-core.test.ts: replaced a comment claiming a specific wording assertion is uniquely the one that fails against the old condition with one describing which assertion actually distinguishes right from wrong, since `expect` throws at the first failure and the earlier framing no longer held once measured. Comment-only; no behaviour, assertion, or type change. * style(host): rewrap one comment line in the ORDER RULE block Pure reflow after the census de-quantification left one line at 93 columns while its neighbours sat at 65-85. No word, claim, or code changed.
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.
Summary
An
applyEditSettledtransition that throws unwinds before the panel commits the state it would have returned, so the write lock stays HELD with no replacement settlement ever coming — the deferred side channels (context-handoff / codex-context-handoff / switch-to-text) sat in the barrier forever and their at-receipt guards never released. The step now rescues that case with a barrier drop, conditioned on the throwing event being the settlement itself.Changes
host-session-step.ts: wrapdeps.commitTransition(event)in atry/catch. On a throw, callsettleEditBarrier(false)only whenevent.type === "applyEditSettled", then rethrow the transition error.settle(false)callsdropAll()before consultingisLocked, so dropping on a non-settlement throw would destroy thunks that the still-pending real settlement would legitimately have drained. OnlyapplyEditSettledever releases the lock, so only it may rescue.false, nottrue: the state was never committed, so the lock still reads held andsettle(true)would take the barrier's WAIT arm and strand the thunks exactly as before. Only the failed verdict fires eachonDrop, freeing the at-receipt guards so a retry is possible. The lock itself stays held — no step or queue policy can release state that was never committed.host-session-core.ts: correct the dispatcher's failure-policy comment — a throwingapplyEditSettledtransition still leaves the lock held, but the side channels behind it are no longer stranded.host-session-step.test.ts: 7 new tests pinning both directions.Latent today: production's write validator is fail-closed (
validate-for-write.tsturns parser throws into verdicts), so the only remaining throw sources are the reducer's own defensive exhaustive arms. No user-visible behaviour change, hence no CHANGELOG entry.Related
commitTransitionstrands the write lock, so deferred side channels are neither dropped nor drained" (out-of-scope finding from the PR fix(host): settle the edit barrier unconditionally so throwing effects cannot strand a deferred side channel #402 cycle-1 review). Follows fix(host): settle the edit barrier unconditionally so throwing effects cannot strand a deferred side channel #402 (unconditional settle) and fix(host): drain the dispatcher queue to empty when a step throws #405 (dispatcher drain policy).Test Plan
pnpm compile— green (5 tsconfigs)pnpm test:unit— 5373 passed / 279 filespnpm build+pnpm package(vsix audit: 22 entries, no violations)