Repository navigation
Fix grid-vs-dispatch divergence for good — resolver layer + migrations - #107
Merged
Merged
Conversation
Introduces the canonical resolver layer for "which sessions live in this tab" — the question that PRs #37 / #39 / #44 / #45 / #46 / #58 / #59 / #69 / #83 + issue #104 all answered inconsistently. Pure addition. The existing `dispatchSessionIdsForTab` and `detachedDispatchSessionIdsForTab` selectors keep their current semantics (the former strips pinned for dispatch UI display reasons; the latter is detached-only for the bulk-attach ordering). They're not being replaced — `resolveTabSessions` is a new function for the common "all sessions in this tab, grid + detached" question. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces collectLeaves(tab.root) (and one manual grid+detached union) with resolveTabSessions(state, tabId) at every surface that asks "which sessions are in this tab?" and silently meant grid-only. Also fixes one focus-asymmetry site (detach-to-dispatch when-guard) and one buried-pane scope leak (palette shows other projects' buried entries). - TabBar.tsx — tab activity counter (#104) - PerformancePanel.tsx (x2) — visible IDs + render loop - AgentActivityModal.tsx — activity rows per session - ReaderView.tsx — non-dispatch fallback - SpotlightView.tsx — non-dispatch fallback - prompt-templates/templates.ts — replaces manual concat - worktrees/loadWorktreeDump.ts — live agents per worktree - App.tsx (path picker fallback) — active-tab scoped - App.tsx (pinned owner lookup) — uses resolver - App.tsx (pin modal grid pass) — uses resolver - CommandPalette.tsx — buried filter by sourceTabId - paneCommands.ts (detach-to-disp) — commandTargetSessionIdForState - paneCommands.ts (attach-to-grid) — isDetached() query After this commit: - tab counter reflects detached agents (closes #104) - perf telemetry covers them - activity modal lists them - spotlight surfaces them - worktree dump shows them per worktree - new-tab fallback no longer pre-fills a stale background cwd - palette buried list scopes to the active project - detach-to-dispatch shows correctly in dispatch mode - attach-detached-to-grid uses a named query, not subscript Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The grid-vs-dispatch session-set divergence kept recurring because
the broken pattern — collectLeaves(tab.root) for "sessions in this
tab" or state.detachedSessions[id] for "is this detached" — was
easy to write and nobody enforced the resolver layer. This check
is the lock: a grep-based CI test that fails the build if any of
the forbidden patterns appears outside an explicit allowlist of
resolver-layer + tree-mutation files.
Patterns flagged:
- collectLeaves(<expr>.root)
- (state|workspace.state).detachedSessions[<expr>]
- Object.{values,keys,entries}(<...detachedSessions)
Comment lines are stripped before matching so the explainer prose
can reference the literal pattern without tripping the check.
After this commit, the bug class is structurally closed: the next
contributor who writes the broken pattern hits a CI failure with a
message pointing them at the right resolver
(resolveTabSessions / isDetached in
src/renderer/src/workspace/queries.ts).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review of PR #107 surfaced three coupled issues — same root cause: the migrated views (ReaderView, SpotlightView, PerformancePanel) now show detached sessions, but downstream code that processes selections from those views still assumed grid-only. Fixes: 1. spotlight.ts / reader.ts — non-Dispatch setSpotlightSession / setReaderModeSession used to write the selected sessionId into Tab.focusedSessionId unconditionally. After the views started surfacing detached sessions, that could violate the grid-leaf invariant (`Tab.focusedSessionId` must be a leaf in `tab.root`). Gate the write on actual grid-leaf membership; for a detached selection only the Reader/Spotlight surface state is updated. 2. effects.ts — validFocusSessionIdsForMode's non-Dispatch path validated against `collectLeaves(tab.root)` while the views render `resolveTabSessions`. Mismatch would reset a detached Reader/Spotlight focus to the first grid pane. Validator now uses the same source as the view. 3. paneLabels.ts — paneLabelForSession only walked the grid tree, so detached rows in PerformancePanel rendered with "?". Switched to resolveTabSessions; grid leaves keep identical indices (resolver yields grid first in tile-tree order), TileTree/DispatchLayout call sites updated for the new state-shaped parameter. Detached sessions now get meaningful labels. CI lock tightening: - Added a destructuring pattern so `const { detachedSessions } = state` followed by `detachedSessions[id]` no longer slips through. Verified the regex fires on a contrived sample. - Removed hook/invalidation/ from the allowlist. The very directory hid the effects.ts bug above — narrowing the allowlist means the lock would have caught that mistake in this PR's CI run. - Removed tile-tree/paneLabels.ts from the allowlist (no longer walks the grid directly). - Documented the line-oriented-grep limitation in the header. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dropping the grep-based CI lock and the GitHub Actions workflow it
ran in. The check was over-engineering:
- The resolver itself (resolveTabSessions, isDetached) is now the
obvious way to ask "sessions in this tab"; new code copies from
neighbors, and every neighbor uses the resolver.
- The allowlist was a maintenance burden — the very PR that
introduced it had to revise the allowlist mid-review when
hook/invalidation/ turned out to hide a real bug.
- The check failed to catch any of the bugs the human review
caught. They were semantic, not syntactic; grep can't see them.
The closure of the bug class is the resolver API + the migrations,
not the CI grep. Keeping both was belt-and-suspenders for a single-
developer repo that doesn't need either.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Juliusolsson05
changed the title
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.
Summary
Closes the recurring grid-vs-dispatch session-set divergence bug class — PRs #37 / #39 / #44 / #45 / #46 / #58 / #59 / #69 / #83 + issue #104 — by introducing a canonical resolver and migrating every known broken site to it.
Branch history (5 commits):
Net effect
The branch's net diff vs `main` is: queries.ts + 13 migrations + the Reader/Spotlight focus fix. No CI workflow, no grep script, no allowlist.
What `resolveTabSessions` returns
For a tab id, the union of:
Excludes terminals from the detached side (Dispatch never holds terminals) and excludes `state.buried` entries (burying is the user's "put it away" signal). De-duped defensively.
Sites migrated (13)
Review fixes (commit #4)
Test plan
Closes
🤖 Generated with Claude Code