Skip to content

fix(workspace): seed pending conditions when a window adopts, restores or wakes a session - #1083

Merged
Juliusolsson05 merged 2 commits into
mainfrom
fix/adopt-conditions
Sep 20, 2026
Merged

Juliusolsson05 merged 2 commits into
mainfrom
fix/adopt-conditions

Conversation

@Juliusolsson05

Copy link
Copy Markdown
Owner

Fixes #895.

The bug

A permission or question pending when a window closes disappears from the adopting window. useWorkspaceAdoption seeds adopted runtimes from emptyRuntime() — conditions: null — and providers publish conditions only when they change (the OpenCode Terminal package and claude-code-headless both deduplicate), so nothing ever re-sends them. Dispatch loses ACTION/QUESTION and orchestration summaries stop naming the blocker; the raw TUI still shows the prompt.

Main already cached the snapshot in lastConditionsSnapshot. No renderer path read it.

The fix

conditions rides SessionBackendSnapshot — already the level-triggered backend fact a late renderer seeds from, and already fetched during adoption. One round trip, so readiness and conditions cannot come from two different instants.

The issue named adoption and recovery; there are three seeding sites with the same hole, and all three are fixed:

path why it had the hole
useWorkspaceAdoption seeds from emptyRuntime()
rehydrate base is emptyRuntime() on a cold restore
session.ts wake the spread predates the recovery

Two rules the tests pin

A snapshot the live channel already wrote wins on ts. Not hypothetical on the adoption path: session routing moves to this window before the offer arrives, and onSessionConditions creates a runtime for a session it has never seen (prev[sessionId] ?? emptyRuntime()). A prompt dismissed while main's cached snapshot is in flight is therefore already in the map, and seeding over it would put the dismissed prompt back — in the surface the user acts on. Ties go to what is already there.

The seed uses the same projection the live channel uses. conditions is not the only thing that projection owns — the composer picker is derived from the same snapshot. That is why applyConditionSnapshot moved out of useIpcSubscriptions into session-runtime/conditions.ts: there are two producers now, and a second copy is how one of them goes stale.

Tests — all fail first

One per path, plus the main-side source:

  • useWorkspaceAdoption.renderer.test.tsx — seed, in-flight clear, picker projection
  • rehydrate.renderer.test.ts — seed, in-flight clear
  • sessionRecovery.renderer.test.tsx — wake
  • sessionManager.wake.test.ts — the important one: every renderer test seeds a stubbed snapshot, so if the production snapshot omitted the field they would all still pass.

Mutation results

mutation result
adoption never seeds (pre-fix behaviour) 1 failed
rehydrate never seeds (pre-fix behaviour) 1 failed
wake never seeds (pre-fix behaviour) 1 failed
ignore what the live channel already wrote 2 failed
tie goes to the incoming snapshot 2 failed
drop the held snapshot when nothing is incoming 2 failed
invert the newer-than comparison 2 failed
seed skips the picker projection 1 failed
main omits conditions from the snapshot 1 failed

No survivors.

npx tsc -b clean on Node 24. --project unit --project renderer src/renderer/src/workspace/ src/renderer/src/session-runtime/ src/main/sessionManager.wake.test.ts: 1144 passed.

🤖 Generated with Claude Code

Juliusolsson05 and others added 2 commits September 20, 2026 07:47
…s or wakes a session

Fixes #895.

A permission or question pending when a window closed disappeared from
the adopting window. `useWorkspaceAdoption` seeded adopted runtimes from
`emptyRuntime()`, whose `conditions` is null, and providers publish
conditions only when they CHANGE — the OpenCode Terminal package and
claude-code-headless both deduplicate — so nothing ever re-sent them.
Dispatch lost ACTION/QUESTION and orchestration summaries stopped naming
the blocker, while the raw TUI still showed the prompt.

Main already cached the snapshot (`lastConditionsSnapshot`) and no
renderer path read it. It now rides `SessionBackendSnapshot`, which is
already the level-triggered fact a late renderer seeds from and which
adoption already fetches — one round trip, so readiness and conditions
cannot be seeded from two different instants.

The issue named adoption and recovery; there are THREE seeding sites with
the same hole, and all three are fixed:
- adoption (`useWorkspaceAdoption`),
- cold restore (`rehydrate`, where `base` is `emptyRuntime()`),
- waking a parked session (`session.ts`, where the spread predates the
  recovery).

Two rules the tests pin:

- A snapshot the LIVE channel has already written wins on `ts`. This is
  not hypothetical on the adoption path: session routing moves to this
  window before the offer arrives, and `onSessionConditions` creates a
  runtime for a session it has never seen, so a prompt dismissed while
  main's cached snapshot is in flight is already in the map. Seeding over
  it would put the dismissed prompt back in the surface the user acts on.
- The seed goes through the same projection the live channel uses, so the
  composer picker cannot disagree with the conditions beside it. That is
  why `applyConditionSnapshot` moved out of `useIpcSubscriptions` into
  `session-runtime/conditions.ts`: there are two producers now, and a
  second copy of the projection is how one of them would go stale.

Tests, all failing first, one per path plus the main-side source:
`useWorkspaceAdoption` (seed, in-flight clear, picker projection),
`rehydrate` (seed, in-flight clear), `sessionRecovery` (wake), and
`sessionManager.wake` — the last because every renderer test seeds a
STUBBED snapshot, so without it a production snapshot that omits the
field leaves all of them green.

Mutation-tested, 9 mutations, no survivors: each of the three seeding
sites reverted, ignoring what the live channel wrote, a tie going the
wrong way, dropping the held snapshot, inverting the comparison, skipping
the picker projection, and main omitting the field.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d of ordering a second producer

Review findings on #1083. The first attempt was built on the wrong
substrate, and this replaces it rather than patching it.

It carried the cached snapshot on `SessionBackendSnapshot` — an INVOKE
reply — and ordered it against live events by comparing `ts`. The review
showed that ordering cannot be made right:

- `ts` is `Date.now()` at 1 ms resolution, and the two snapshots being
  compared are two entries of ONE monotone sequence in main, so an equal
  `ts` means "we cannot tell", not "the same facts". OpenCode calls
  `emitConditionsSnapshot` from five sites with no dedupe latch, so a
  permission set and answered inside one millisecond produces exactly
  that tie — and the tie rule put the ANSWERED PROMPT BACK on screen,
  where pressing "1" writes a raw keystroke into a live agent's composer.
- The seed also diverged from the live fold on unread, attention and the
  feed-debug log, so an adopted blocked agent never showed NEW after its
  prompt was answered.
- And it was a second producer of `runtime.conditions`/`picker`, which is
  the shape the projection was extracted to prevent.

Main already had the answer, and `session:resync-routing` has been using
it: re-emit the cached snapshot on the ORDINARY `session:conditions`
channel. There is then nothing to order — main updates its cache before
forwarding, so the cache is never older than what the renderer has
folded, and the re-emit is just the next event in the same stream. The
renderer's one handler applies the projection, the unread mark and the
debug log exactly as it does for a live change.

`session:reseed-conditions` takes the session ids and checks ownership
per session. The renderer asks for it once its runtimes exist — after
adoption seeds, after rehydrate publishes, after a wake settles —
because every one of those paths rebuilds from `emptyRuntime()` and
would otherwise overwrite what it just received. It is a hint, not a
step: a preload without it still restores the workspace.

Also from the review:

- FINDING 2 (high): the adoption seed clobbered an EXIT observed while
  `getBackendSnapshot` was in flight, repainting a dead agent as
  `started` with an enabled composer. Both sibling seed sites already
  guarded this; adoption did not, and the first attempt had hardened only
  `conditions`.
- FINDING 5: the live `session:conditions` handler — the projection's
  only call site — had NO coverage, so dropping the picker projection
  there left the suite green. It has a test now, and so does the fold
  itself, including that the picker kind comes from provider policy.

Mutation-tested, 9 mutations, no survivors: the live channel dropping the
projection, a hardcoded picker kind, adoption never asking, adoption
asking BEFORE it seeds, the exit guard dropped, main skipping the
ownership check, and main sending an empty snapshot when it has none.

169 files / 1220 tests pass. `npx tsc -b` clean on Node 24.

Fixes #895

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

This review changed the design rather than the code. Finding 6's parenthetical — "main already has a seeding path for this cache… it reuses the one fold and needs no ordering key, because the re-emit is just another event in the same ordered stream" — is the whole answer, and I had walked past session:resync-routing doing exactly that.

The substrate was wrong (findings 1, 3, 4, 6, 7)

I was carrying the snapshot on an invoke reply and then hand-ordering it against live events. That cannot be made right:

  • ts is Date.now() at 1 ms, and the two snapshots being compared are two entries of one monotone sequence in main — so an equal ts means "we cannot tell", not "the same facts". My docstring said the opposite. Your OpenCode case is the proof: emitConditionsSnapshot from five sites with no dedupe latch, so a permission set and answered inside one millisecond produces exactly that tie — and the tie rule put the answered prompt back on screen, where TileLeaf.sendConditionKey will write a raw keystroke into a live agent's composer. That is the worst outcome in the whole feature and my rule caused it.
  • Switching to a revision, as bug(workspace): window adoption drops pending permission and question conditions #895 and you both suggest, would have fixed the ordering and left findings 4, 6 and 7 standing.

session:reseed-conditions re-emits the cached snapshot on the ordinary session:conditions channel. There is then nothing to order — main updates its cache before forwarding, so the cache is never older than what the renderer has folded, and the re-emit is just the next event in the same stream. The renderer's one handler applies the projection, the unread mark and the debug log exactly as for a live change.

That removes, rather than fixes:

finding now
1 — tie restores a dismissed prompt no ordering key exists
3 — ordering untested in the direction that matters no ordering rule
4 — held branch can skip the projection no held branch
6 — seed diverges on unread/attention/debug same fold, by construction
7 — new throw site inside setRuntimes the registry is not called from there
8 — "two producers" overstated back to one; the picker/screen point stands and the comment no longer claims otherwise

The renderer asks after its runtimes exist — adoption after its seed, rehydrate after it publishes, a wake after it settles — because all three rebuild from emptyRuntime() and would otherwise overwrite what they just received. A mutation that moves the call before the seed fails. It is a hint, not a step: a preload without it still restores the workspace, and that is a test too.

2. The adoption seed clobbered an observed exit (HIGH) — valid, fixed

Right, and the sharp part is that I had established the live channel writes during the fetch and then hardened exactly one field. Both sibling sites already had preserveObservedTerminalProcess; adoption did not. Your proof test is now in the suite — and my first fix for it was wrong in a way the test caught, because returning base erases the very thing being preserved (base starts from emptyRuntime()).

5. The live channel had zero coverage (MEDIUM) — valid, fixed

The most useful finding for the long run: the module was justified by "two producers must project identically", and the original producer's call site was untested, so dropping the projection there passed. There are tests now for the fold itself — including that the picker kind comes from conditionPolicy.composerPickerKind and not a hardcoded name — and for the live handler's use of it.

Mutation results

Your count was right and mine was wrong: I had reported "no survivors" for a battery of five mutations I chose, which is a statement about my test list, not about the code. Nine now, chosen from your findings:

mutation result
the live channel drops the picker projection 1 failed
hardcode the picker kind 1 failed
adoption never asks for a re-emit 1 failed
adoption asks BEFORE it seeds 1 failed
the exit guard is dropped 1 failed
main skips the per-session ownership check 2 failed
main sends an empty snapshot when it has none 1 failed
+ the two ordering mutants unrepresentable — the rule is gone

No survivors.

Also confirmed

Thank you for verifying the six rationale claims rather than taking them — transferSessions ordering, the edge-triggered providers, the cache's eviction, and that the WALL_CLOCK_MS_FLOOR hazard does not apply. Those stay true and the new design rests on the same facts. And for reproducing the control.system.test.ts timeout on origin/main before attributing it: that is the check that keeps a local-env failure from becoming a phantom finding.


169 files / 1220 tests pass across unit, renderer and system. npx tsc -b clean on Node 24.

@Juliusolsson05
Juliusolsson05 merged commit 50d3abc into main Sep 20, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the fix/adopt-conditions branch September 20, 2026 16:13
Juliusolsson05 added a commit that referenced this pull request Sep 20, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Juliusolsson05 added a commit that referenced this pull request Sep 20, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Juliusolsson05 added a commit that referenced this pull request Sep 20, 2026
…ened

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

bug(workspace): window adoption drops pending permission and question conditions

1 participant