Skip to content

fix(sessions): a reloaded child keeps its pointer to a closed, restorable parent - #1387

Merged
Juliusolsson05 merged 12 commits into
mainfrom
fix/replace-keeps-closed-parent-pointer
Sep 27, 2026
Merged

Juliusolsson05 merged 12 commits into
mainfrom
fix/replace-keeps-closed-parent-pointer

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Fixes #1379. Depends on #1374 (merged into this branch; merge #1374 first). Keeping the pointer only helps if restoring the parent relinks it, and that relink is #1374's.

Plan: docs/plans/2026-09-27-replace-keeps-closed-parent-pointer.md (first commit).

Problem

Replace (reload, provider switch, resume, rewind) and Reload Agents commit through remapSessionsRelationships(sessions, idMap). That drops any linkedParentId / orchestrationParentId / orchestrationRootId whose target is neither live nor in the idMap. A parent that is closed but still on the undo stack is neither. So reloading its live child erased the child's link, and undoing the parent later (#1374's relink) found nothing to relink: two live panes, permanently unlinked. Found by review c of #1374.

What merges

  • UndoCloseStack.restorableSessionIds(): every session id a waiting entry could bring back (session, project and group members). It is pruned first, like pop(), so an expired entry never counts.
  • Both commits (replace, Reload Agents) pass knownSessionIds = live ids ∪ restorable ids, through knownOrRestorable.
  • Unchanged: a pointer to a parent nothing can bring back (deleted, or its undo entry expired) is still dropped at the swap.
  • Kept pointers do not outlive the chance of a restore (review a). When the parent's entry leaves the stack WITHOUT being restored, dropPointersTo drops live pointers to exactly those ids, and never touches unrelated or cross-window pointers. That covers an entry consumed as stale or failed, one that expired, and one evicted past the cap. The stack reports expiry and eviction on a microtask, because prune can run inside a setState updater.

Tests

  • closedParentPointer.renderer.test.tsx:
    • a child replaced, and a child reloaded by Reload Agents, while its parent sits on the undo stack keeps all three pointers; both were red on main;
    • the control: a gone parent's pointers are still dropped.
  • lib/undoClose.test.ts: restorableSessionIds over session, tab and group entries, with expiry.
  • Group replay (undoCloseGhostPointers.renderer.test.tsx): a child restored after its parent was consumed, and a leftover pushed back for retry, both drop the dead pointer. An expiry notification reaches the workspace.
  • End to end (with fix(undo-close): a restored parent's live children follow it to its new id #1374): close P, reload its live child, undo P, and the child names P′. Red without fix(undo-close): a restored parent's live children follow it to its new id #1374's remap.
  • Ghost pointers: a consumed-as-stale parent entry drops the kept pointers. dropPointersTo and the stack's expiry and eviction notifications have unit tests.
  • Red checks: un-wiring either call site fails its test, and so does removing the consumed-entry drop, either notification, or the liveness guard.
  • sessionRecovery.renderer.test.tsx: one hand-built refs mock now carries undoStackRef. Production refs always do.
  • Gates: on 02e03d92, npx tsc -b is clean, and src/renderer/src/workspace/hook + src/renderer/src/lib pass (72 files, 476 tests).

Related

🤖 Generated with Claude Code

Juliusolsson05 and others added 6 commits September 26, 2026 23:52
…1373)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ew id

Undo Close remapped pointers only on the rows it restored, so a restored
parent's live orchestration and linked children kept the dead id and the
parent could not list, read or prompt them. Both restore commits now map
every live session through the restore's lineage (remapMetaLineage keeps
unmapped ids, so a child of another still-closed pane keeps its link).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…torable parent (#1379)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…able parent

#1379 (from #1374 review c): replace and Reload Agents dropped any
relationship pointer whose target was neither live nor in their idMap; a
parent closed but still on the undo stack is neither, so reloading its
live child erased the link and undoing the parent later had nothing to
relink. Both commits now treat the undo stack's restorable session ids
as known (UndoCloseStack.restorableSessionIds, pruned like pop). A pointer
to a parent nothing can restore is still dropped. Red before; each call
site pinned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace now consults refs.undoStackRef for restorable parents (#1379);
production refs always carry it, this test's partial mock did not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 Juliusolsson05 added type:bug Something works wrong class:C2-identity Identity/ownership across reload, replace, restore, wake sev:P3 Minor labels Sep 27, 2026
Juliusolsson05 and others added 6 commits September 27, 2026 01:44
…e the parent can never come back

#1387 review a:
- (major) keeping the pointer only helps if restoring the parent relinks
  it: #1374 is merged in (Depends on #1374) and an end-to-end test closes
  P, reloads its live child, undoes P and asserts the child names P'
  (red without #1374's remap).
- (minor) a kept pointer lingered as a ghost when P's entry left the stack
  without a restore. dropPointersTo drops pointers to exactly those ids
  (never unrelated or cross-window ones) when an entry is consumed as
  stale/failed, expires or is evicted; the stack notifies on a microtask
  because prune can run inside a setState updater. Each path is pinned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…members restored after them

#1387 review a, round 2: undo replays a group newest-first, so a parent
consumed as stale (folder gone) could precede its child in the same group;
the child then came back carrying pointers to a parent that can never
return. The group remembers consumed ids and drops pointers to them once it
has replayed. Review b, round 2: a hook-level test now proves the stack's
expiry notification reaches the workspace (red without the listener).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a member the group consumed

#1387 review a, round 3: when a group consumed parent P as stale and then
hit a transient failure on child C, C went back on the stack with its entry
metadata still naming P, and the retry restored that dead pointer. The
leftover's metadata now drops pointers to the replay's consumed ids
(dropEntryPointersTo). Red before, on the reviewer's exact sequence.

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

Copy link
Copy Markdown
Owner Author

Review disposition (head 02e03d92).

  • Round 1: a and b FIX-BEFORE-MERGE (the same major); c (pi) MERGE-READY with one minor.
  • Round 2: a FIX-BEFORE-MERGE (one new minor); b MERGE-READY.
  • Round 3: a FIX-BEFORE-MERGE (one more minor).
  • Round 4: a MERGE-READY.

Fixed

  • a + b (major): the kept pointer did not reconnect after Undo Close on its own. This PR now depends on fix(undo-close): a restored parent's live children follow it to its new id #1374, which is merged into this branch; merge fix(undo-close): a restored parent's live children follow it to its new id #1374 first. An end-to-end test closes P, reloads its live child, undoes P and asserts the child names P′. It is red without fix(undo-close): a restored parent's live children follow it to its new id #1374's remap.
  • a r1 / c (minor): the kept pointer lingered after P's entry left the stack unrestored, as a ghost that still classified the child as an orchestration worker. dropPointersTo drops pointers to exactly those ids, and never unrelated or cross-window ones, when:
    • an entry is consumed as stale or failed;
    • an entry expires or is evicted (the stack notifies on a microtask, because prune runs inside a setState updater).
  • b r2 (minor): the expiry → workspace wiring was untested. A hook-level test now makes removing the listener red.
  • a r2 (minor): a group replays newest-first, so a child restored after its parent was consumed carried the dead pointer back. The group now drops pointers to the members it consumed once it has replayed.
  • a r3 (minor): a leftover pushed back for retry still named the consumed parent, so the retry restored it. Its metadata is now cleaned before the push (dropEntryPointersTo).

Every fix was red before it landed, on the reviewer's own sequence. Each guard was removed on its own, and each removal fails a test.

Merge order: #1369, then #1374, then this PR.

🤖 Generated with Claude Code

@Juliusolsson05
Juliusolsson05 merged commit 1427037 into main Sep 27, 2026
2 checks passed
Juliusolsson05 added a commit that referenced this pull request Sep 27, 2026
@Juliusolsson05
Juliusolsson05 deleted the fix/replace-keeps-closed-parent-pointer branch September 27, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

class:C2-identity Identity/ownership across reload, replace, restore, wake sev:P3 Minor type:bug Something works wrong

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(sessions): reloading a live child while its parent is closed drops the child's parent pointer for good

1 participant