[beruro]

Problem

Replay overrides keyed only by thread name leak between sessions. Cursor and playing writes can overwrite one another, queued updates can outlive a cell, and hidden documents retain playback timers. Removed sessions also leave replay overrides behind.

Solution

Key replay owners by session and thread, remount on identity changes, publish complete user replay transitions and cancel queued owner updates during cleanup. Pause timers while the document is hidden and resume without catch-up. Protect mounted cells from retention eviction, retain up to 256 inactive cell entries, and reapply that limit when owners unmount. Revoke mounted writers at the session-removal boundary before evicting their entries; removed owners cannot publish queued follow updates, timer ticks, or user-control writes, and their playback effects are disabled. The unused global replay command path (no production writer) is removed rather than hardened. Preserve follow, detached and synced replay modes.

Potential risks

Independent of #1760: stack #1783 was dissolved on 2026-09-15 and both commits were rebased onto develop. The replayed changes are identical to the reviewed stacked commits; only surrounding context lines differ. Replay state is memory-only, so no persisted migration is needed. Retention can evict an inactive cell after 256 inactive entries; mounted keys are retained in addition to that cap. Owner registrations are scoped to the Jotai store and released on unmount. Newly mounted owners are separate registrations; this is revocation of existing mounts, not a permanent session tombstone. Additionally, and document visibility does not detect every CSS-hidden panel; a mounted panel in a visible document can still run playback. Native desktop hidden/minimized behavior, screenshots, CPU/RSS measurements and long-session interaction remain unverified. Reverting restores prior ownership behavior without data recovery. An existing expand-button icon presentation issue is recorded in the UI audit and excluded from this state fix.

Verification

• pnpm exec vitest run --config config/vitest.config.ts --project app --maxWorkers=2 src/engines/Simulator/hooks/__tests__/useCellReplayState.test.ts src/engines/Simulator/components/SubagentPipCard.lifecycle.test.ts src/engines/Simulator/components/SubagentPipCard.state.test.ts src/engines/Simulator/hooks/__tests__/useCellReplayState.lifecycle.test.ts src/engines/Simulator/__tests__/ActivitySimulatorGrid.lifecycle.test.ts src/store/session/sessionAtom/__tests__/mutations.test.ts: 42 passed across six files.
• pnpm typecheck:fast: passed on the original stacked head, using the frozen-lockfile installation from refactor(workspace): extract private frontend core packages #1760.
• Tests cover same-name threads in different sessions, actual grid owner remounting, atomic play/reset behavior, StrictMode global commands, queued completion after unmount, hidden/resumed timers, bounded retention, session deletion and existing follow/detach/resync regressions.
• Standard staged lint, formatting and typecheck hooks passed. Changed production JSX introduces no raw-button/input bypass. Final diff contains only replay ownership, entry-point identity propagation and direct regressions/audit.
• Native visual evidence and runtime resource profiling were not collected; automated lifecycle tests are not a CPU/RSS benchmark.

Review follow-up verification

• pnpm exec vitest run --config config/vitest.config.ts --project app --maxWorkers=2 src/engines/Simulator/hooks/__tests__/useCellReplayState.test.ts src/engines/Simulator/hooks/__tests__/useCellReplayState.lifecycle.test.ts src/engines/Simulator/__tests__/ActivitySimulatorGrid.lifecycle.test.ts src/engines/Simulator/components/SubagentPipCard.lifecycle.test.ts src/engines/Simulator/components/SubagentPipCard.state.test.ts src/store/session/sessionAtom/__tests__/mutations.test.ts src/engines/SessionCore/replay/components/ShellReplayOutput/ShellReplayOutput.test.ts: 50 passed across seven files, including active-cell eviction, post-unmount retention, paused deletion, queued deletion, removal through removeSession, timer shutdown, rejected post-removal user/global writes, and store isolation.
• pnpm typecheck:fast: passed, including both package typechecks.
• pnpm exec eslint src/store/ui/simulatorAtom.ts src/engines/Simulator/hooks/useCellPersistence.ts src/engines/Simulator/hooks/useCellPlayback.ts src/engines/Simulator/hooks/useCellReplayState.ts src/engines/Simulator/hooks/__tests__/useCellReplayState.lifecycle.test.ts --max-warnings 0: passed.
• git diff --check: passed.
• Full app suite, native desktop interactions, and CPU/RSS profiling were not rerun for this follow-up. There are no new JSX/action presentation changes. No persistence migration or historical data cleanup is required.

Rebase onto develop verification

• Rebased onto develop at 4f7a624056. git range-diff 7c0bb11942..2cfca994e1 origin/develop..12317107be reports the fix commit as identical; the first commit differs only in one context line (the findIndexAtTime import it sits next to). The diff is still 16 files, +731/-51, and commit messages and authors are unchanged. The commits were replayed with git merge-tree/git commit-tree, so commit hooks did not run again; the checks below ran on the rebased head instead.
• pnpm install --frozen-lockfile --ignore-scripts: passed.
• pnpm typecheck:fast (tsgo): passed.
• pnpm exec vitest run --config config/vitest.config.ts --maxWorkers=2 src/engines/Simulator/hooks/__tests__/useCellReplayState.test.ts src/engines/Simulator/hooks/__tests__/useCellReplayState.lifecycle.test.ts src/engines/Simulator/__tests__/ActivitySimulatorGrid.lifecycle.test.ts src/engines/Simulator/components/SubagentPipCard.lifecycle.test.ts src/engines/Simulator/components/SubagentPipCard.state.test.ts src/store/session/sessionAtom/__tests__/mutations.test.ts: 46 passed across six files. The ShellReplayOutput suite listed above belongs to refactor(workspace): extract private frontend core packages #1760 and is no longer part of this diff.
• pnpm exec eslint --max-warnings 0 --report-unused-disable-directives on the 14 changed TypeScript files: passed.
• node scripts/quality/check-test-placement.mjs, scripts/ci/check-changed-file-length.cjs on the diff, and git diff --check origin/develop...HEAD: passed.
• Not rerun locally: the full app suite, typed lint and dependency boundaries (CI covers them), and native desktop behavior.

Second review follow-up verification

Commit 7c4a0372c2 removes globalReplayStateAtom and its cell effect. The only setter lived in .archive/, so the path ran only in tests. The command-only tests now cover the same behaviour through user controls and follow updates: end-of-playback stop, no publish after unmount, and revocation of a queued update. Releasing an owner under the retention cap no longer republishes the replay record. Earlier evidence above that mentions global commands predates this commit.

• pnpm exec vitest run --config config/vitest.config.ts --maxWorkers=2 on the six files listed in the rebase section: 47 passed. That includes a new test that unmount below the cap keeps the record identity.
• Sabotage checks on useCellReplayState.lifecycle.test.ts. Restoring the unconditional unmount republish fails the new identity test. Removing the owner write guard fails three removal/revocation tests, including the queued follow-update case. Both files were restored byte-for-byte.
• pnpm typecheck:fast, ESLint and oxlint on the four changed TypeScript files, node scripts/quality/check-test-placement.mjs, the file-length gate, and git diff --check: passed. Commit hooks (lint-staged, tsgo, commitlint) passed.
• Not rerun: the full app suite, typed lint (CI), and native desktop behaviour.

Develop integration verification (2026-09-22)

Merged origin/develop into the branch at ef782f36f2 rather than rebasing, so the
branch is not force-pushed under the author. Three files collided with the simulator
dead-state refactor 615d6915d8:

• src/store/ui/simulatorAtom.ts: kept clearCellReplaySessionAtom from this branch and
  took develop's deletion of simulatorDataSourceAtom, verified dead with no consumers in
  src/ or .archive/.
• src/engines/Simulator/hooks/useCellPlayback.ts: docblock only. Both sides stated a true
  fact, so the merged text keeps both — independent-mode only, and gated on document
  visibility.
• src/engines/Simulator/ActivitySimulatorGrid.tsx: dropped the sessionId prop from the
  SimpleGridCell call site. This one auto-merged without a conflict and only surfaced
  under tsgo. It type-checked on this branch because SimpleGridCell spread the shared
  GridCellProps, which this branch extended with sessionId; develop replaced that with a
  narrower own interface. SimpleGridCell renders from the shared cursor and never read the
  prop, so nothing is lost. IndependentGridCell, which does use it for cellReplayKey, is
  unchanged.

globalReplayStateAtom, removed here in 7c4a0372c2, was also removed independently on
develop, so that commit is now a no-op against the current base.

Net diff against develop is 16 files, +719/-49 (was +731/-51 before integration; the
difference is the dropped prop and the merged docblock).

• npx tsgo --noEmit --pretty false on the merged tree: passed, 0 errors. It failed with
  TS2322 before the SimpleGridCell resolution, which is how that silent collision was found.
• vitest run --config config/vitest.config.ts src/engines/Simulator src/store/session/sessionAtom src/store/ui: 445 passed across 61 files.
• The three lifecycle suites added by this PR: 31 passed across 3 files.
• prettier --check, oxlint -c src/.oxlintrc.json --max-warnings 0, and
  eslint --max-warnings 0 --report-unused-disable-directives on the 14 changed TypeScript
  files: passed.
• node scripts/quality/check-test-placement.mjs: passed. All changed files are under the
  700-line gate (largest: SubagentPipCard.tsx at 660).
• The merge commit was amended with hooks disabled, and the gates above were run by hand
  instead. Reason: on a merge commit lint-staged treats the whole incoming develop delta as
  staged, and its *.{css,scss,md,json} → prettier --write rule rewrote 11 unrelated docs
  (for example *agent* → _agent_ in evidence/resend-diagnosis.md). Those files already
  violate prettier on develop and CI does not prettier-check docs, so the churn was
  pre-existing and out of scope. They were restored to develop's content and the final diff
  contains only this PR's 16 files.
• Not rerun locally: the full app suite, typed lint and dependency boundaries (CI covers all
  three), and native desktop behaviour.

Audit

Architecture L1 tests/types; L2-L4 cell/session ownership and cache lifetime; L5 explicit replay transitions; L6-L7 complete state updates and identity keys; L8 memory-only state contract; L9 all cell entry points; L10 session removal at the owning mutation boundary. React review covers timer cleanup, canceled microtasks, keyed remounts and narrowly selected cell state. Performance verdict: source-level lifecycle regressions are fixed; mounted-owner protection, a 256-inactive-entry limit, removal revocation, and timer cleanup are verified in tests. Native resource-measurement verdict remains blocked because CPU/RSS and real Tauri behavior were not measured. See docs/org2-performance-guard-2026-09-14/SimulatorReplay.md. UI report: one existing fix candidate, four keep-with-reason, zero abstractions.

[Harry19081]

Review of 0b920240b725542013b90e4ff2c19a84db651efe: two P2 lifecycle issues reproduced in focused hook/atom tests.

1. Active replay is reset by retention eviction (src/store/ui/simulatorAtom.ts, lines 105–110). A mounted cell detached at index 1 loses its override after 256 other cell entries are written. Its mode changes to follow and its cursor snaps to the tail. Retention should evict inactive entries without changing an active owner's mode.
2. Session deletion does not invalidate mounted writers (src/store/ui/simulatorAtom.ts, lines 128–129). Clearing a detached cell's session while the cell remains mounted lets the follow effect recreate {currentIndex: 4, isPlaying: false}. Child views are not guaranteed to unmount immediately when sessionsAtom changes. Removal needs to revoke existing writers and stop playback, not just filter the retained record.

Existing verification passed: 46 app tests, 64 package tests, 25 workspace/tooling tests, package builds, full frontend typecheck, and targeted lint. Both added adversarial tests fail on this head. Native UI and CPU/RSS profiling were not run.

I am addressing both findings in this PR and will follow up with the fix and verification results.

[Harry19081]

Addressed both review findings in f4e2817.

• Mounted cell keys are protected from LRU eviction. The cache retains up to 256 inactive entries in addition to mounted keys, and owner cleanup reapplies the inactive cap.
• Session removal revokes existing mounted writers before deleting retained state. Removed cells cannot republish through queued commands, timer ticks, or user controls, and their playback/global-command effects stop. Registrations are scoped to the Jotai store and released on unmount.
• Added regressions for active eviction and post-unmount bounds, paused deletion, queued deletion, removal through the real removeSession boundary, timer shutdown, rejected later commands, and separate-store isolation.

Verification: 50 focused app tests passed across seven files; pnpm typecheck:fast, changed-file ESLint, git diff --check, and standard commit hooks passed. PR description now reflects the final retention policy and evidence. No persistence migration or historical cleanup is needed. Native Tauri interaction and CPU/RSS measurements remain unverified.

[Harry19081]

Second review, against the rebased head 12317107be. Fixes pushed as 7c4a0372c2.

Unstacked from #1760. This PR never needed #1760: its diff applies cleanly to develop, and typecheck plus the focused suites pass there.

• Stack #1783 was dissolved through GitHub's unstack API, and the base is now develop.
• Both commits were rebased onto develop 4f7a624056 with authors and messages kept. Range-diff shows them unchanged apart from one context line.
• @beruro the branch history was rewritten, so please git fetch and reset your local junyu/audit-simulator-replay-ownership to origin before pushing again.

Findings (fixed in 7c4a0372c2):

1. The global replay command path was dead code. Nothing in the app writes globalReplayStateAtom; its only setter lives in .archive/src/engines/Simulator/hooks/useGlobalReplay.ts. So the useCellPlayback effect this PR made StrictMode-safe and revocable, and its three tests, only ran in tests. The atom, the effect and the setLocalPlaybackSpeed plumbing are removed. The tests now cover the same behaviour through user controls and follow updates:
   • playback stops cleanly at the end;
   • a queued follow update is not published after unmount;
   • session removal revokes a queued follow update.
2. Every cell unmount republished the replay record (set(cellReplayStatesAtom, (s) => ({ ...s }))), even with nothing to evict. Eviction now runs through one helper that returns the same object when nothing is evicted, so an unmount writes only when it drops an entry. A new test checks that the record is unchanged after an unmount under the cap.

Otherwise the ownership and revocation design holds up. I found no problems with the order of mount-time writes, the removeSession callers, store scoping, or timer and visibility cleanup.

Verification

• The six focused test files pass (47 tests).
• Sabotage checks: restoring the old unmount write, or removing the owner write guard, makes the new tests fail.
• pnpm typecheck:fast, ESLint and oxlint, test placement, the file-length gate, git diff --check and the commit hooks all passed.
• Not rerun: the full app suite, typed lint (CI covers it), native desktop behaviour.