fix(workstation): close tabs at canonical resource owners - #727
Open
beruro wants to merge 4 commits into
Open
Conversation
Pre-commit hook ran. Total eslint: 0, total circular: 0
# Conflicts: # src/modules/index.tsx
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.
"## Problem\n\nClosing a WorkStation Browser tab removed only its visible workspace projection. The live Browser session and its localStorage record remained authoritative, so switching to another surface and back caused reverse synchronization to recreate the supposedly closed tab.\n\nThe same ownership ambiguity existed around Terminal teardown and entity-backed Session or Project Org tabs: presentation state could be removed in one surface while the underlying resource or another workspace reference survived.\n\n## Solution\n\n- Move Browser sessions into one canonical Jotai resource store and make explicit close commands remove the owner before clearing every WorkStation projection.\n- Define exhaustive tab ownership, shared-retention, and repository-affinity policies so resource tabs, ordinary shared presentation, and workspace-local tabs have distinct lifecycles.\n- Route single, other, saved, bulk, registry, Session deletion, and Project Org invalidation through symmetric lifecycle actions across every workspace.\n- Replace the shadow Terminal context/provider teardown path with the existing terminal owner store, generation-guarded asynchronous shutdown, immediate local ID rotation, and WorkStation-only PTY filtering.\n- Garbage-collect ownerless shared presentation during workspace disposal and persistence restore while retaining Browser and Terminal records until their owner closes.\n- Persist only changed WorkStation scopes and reconcile stale persisted Browser projections against canonical sessions.\n\nThe resulting invariant is: a user close destroys a resource at its canonical owner and then removes all projections; a mere workspace disposal removes only that workspace reference.\n\n## Potential risks\n\n- Browser and Terminal close actions are now global by design. Any caller that previously depended on closing only one visual reference must use workspace disposal or projection removal instead.\n- Terminal PTY shutdown is asynchronous. Generation checks and immediate local ID rotation protect rapid close/reopen, but the real Tauri PTY process path was not exercised manually in this isolated worktree.\n- Existing WorkStation v3 persistence remains format-compatible; stale ordinary shared records may be collected on load. Legacy v2 shared records are retained through the seed-claim path.\n- Rollback is a normal revert of this PR after its base lands; no schema, IPC, wire-format, dependency, or lockfile migration is involved.\n\n## Verification\n\n- Post-integration:
./node_modules/.bin/vitest run <the 13 lifecycle test files listed below>— PASS, 13 files / 134 tests after merging currentdevelop.\n- Post-integration:./node_modules/.bin/eslint src/modules/index.tsx src/store/chatPanel/chatPanelSessionInvalidationAtom.ts— PASS.\n- Post-integration:./node_modules/.bin/tsc --noEmit --pretty falseandgit diff --check— PASS.\n- Current-base compatibility: retained the root Browser owner while adding the new viewport-aware layout contract; removed the obsolete presentation-transition call because maximization is now derived at the layout boundary.\n\n-pnpm exec vitest run src/store/workstation/tabs/__tests__/workspaceState.test.ts src/store/workstation/tabs/__tests__/storage.test.ts src/store/workstation/browser/sessionState.test.ts src/store/workstation/browser/tabs/__tests__/sharedWorkspaceIntegration.test.ts src/store/workstation/codeEditor/terminal/__tests__/terminalAtoms.test.ts src/store/workstation/tabRegistry/atoms.test.ts src/store/session/sessionAtom/__tests__/mutations.test.ts src/store/projectOrgPresentationLifecycleAtom.test.ts src/store/chatPanel/chatPanelSessionInvalidationAtom.test.ts src/store/chatPanel/__tests__/chatPanelTabsAtom.test.ts src/store/chatPanel/__tests__/chatPanelChannelTabs.test.ts src/modules/WorkStation/AppShell/WorkstationTabBar.test.ts src/scaffold/NavigationSidebar/connectors/rustSessionDeleteReceipt.test.ts— 13 files and 133 tests passed after commit hooks.\n-pnpm exec tsc --noEmit --pretty false --incremental --tsBuildInfoFile /var/tmp/orgii-tab-lifecycle-pr.tsbuildinfo— passed.\n-git diff --name-only --diff-filter=ACM -z origin/develop -- '*.ts' '*.tsx' | xargs -0 pnpm exec eslint— passed for every changed TypeScript file.\n-env npm_config_cache=/var/tmp/orgii-codex-madge-cache pnpm check:circular— no circular dependencies across 6,321 modules.\n-git diff --checkand staged secret/personal-path/debug-log scan — passed.\n- Repository commit hooks ran lint-staged formatting, ESLint, scoped TypeScript checking, and circular-dependency statistics successfully.\n- The plainpnpm typecheckcommand was attempted twice but exceeded 120-second and 300-second wall-clock limits while several unrelated repository typechecks were running on the same host. The equivalent incrementaltsc --noEmitcommand completed successfully afterward.\n- Not run: live Tauri Browser/WebView and PTY manual testing; the isolated PR worktree did not launch a desktop runtime.\n\n## Architecture audit\n\nAll 10 layers were reviewed. Compilation passed; duplicate Browser/Terminal ownership paths were removed; ownership and lifecycle naming is explicit; Browser session, task workspace, and terminal session semantics are no longer overloaded; policy switches are exhaustive without a negative default; cross-surface invalidation is isolated in lifecycle aggregators; context documentation was updated; no wire or serialization contract changed; persistence restore and provider entry paths converge on the same owners; and single, bulk, delete, revoke, restart, and rapid reopen paths have symmetric coverage.\n\n## Performance guard\n\nVerdict: pass. The change adds no polling, timers, subscriptions, workers, scans, or hidden-state background work. Active close performs bounded in-memory filtering plus one owner teardown; hidden surfaces do no recurring work; repeated close/reopen uses terminal generations and fresh IDs; multi-workspace references share one owner; restart prunes stale projections. Incremental persistence avoids rewriting unchanged scopes. Unit coverage exercises repeated lifecycle transitions and cross-workspace cleanup; no real-surface profiling was available.\n\n## UI evidence\n\nNo screenshot is included because this changes lifecycle behavior and persistence only; it does not alter visual layout, styling, loading, empty, or error-state rendering."