From 4da6aee660434fc7a1fa54804ce3ebd3e341587d Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 24 Oct 2026 14:05:05 +0200 Subject: [PATCH] fix: apply the review items for #682 at merge docs: architecture-invariants (arrow-key walk section) names the phone swipe beside the arrow keys as following the drawn order docs: root CLAUDE.md (Owner tab layouts) carves the phone swipe out of "every other order consumer reads sessionOrder" test: pin the grouped-tree branch of _stripSessionOrder() (sorted rail, header and web tab skipped, groups not interleaved) test: the step-in case uses a drawn order (homelab, w1, w2) that master's canonical walk and the old arithmetic cannot satisfy Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- test/session-swipe-gesture.test.ts | 73 +++++++++++++++++++++++++----- 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 02704d0f..ac38722e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -193,7 +193,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Unified session list**: `GET /api/sessions/unified` merges live sessions, persisted state, lifecycle-log history and transcript files into one deduped list (pure core `src/services/unified-session-service.ts`), backing the Cmd+K Session Manager, pinning and cross-device tab order (`PUT /api/session-order`, `src/session-order.ts`). ⚠️ Transcript history is THREE stores (`~/.claude/projects`, `~/.omp/agent/sessions`, `~/.codex/sessions`), folded via the `claudeSessionId → Codeman id` alias map (not Claude-only despite the name). ⚠️ `resumeId` is set by a SCANNER row only, never a live session; every surface that re-projects these rows (phone overview included) must carry it through, or a tap silently starts a second conversation. → [architecture-invariants#unified-session-list-and-session-manager](docs/architecture-invariants.md#unified-session-list-and-session-manager) -**Owner tab layouts** (`tab-layout*.ts` + `GET`/`PUT /api/tab-layout`): named tab GROUPS over the flat strip, scoped per owner (`@single` when multi-user is off), persisted as `tabLayouts` in state.json. The frontend reads AND edits it (`tab-layout-browser.js` + the grouped-rail block in app.js): the vertical rail draws the owner's groups as collapsible sections (collapse is per-device localStorage), and with no groups or a failed read the rail is the flat list. Groups are created, renamed, reordered and deleted, and rows moved between them, from the row/group menus (Shift+F10 on a header too) and by pointer drag in the grouped rail. A ref may carry `focus: true` ("Add to Focus" in the row/web-tab menu, the named `setFocus` operation): the rail then draws a SHORTCUT to it in a pinned, collapsible Focus section (`_renderTabFocus()`), its own element in `#tabRail` ABOVE `#sessionTabs`, so it shows on the flat rail too and is never a row, tree item, Alt+N slot, lineage anchor or drag target. ⚠️ Every copy of a ref (`copyRef` in tab-layout-browser.js, `parseRef`/`mapRef` in tab-layout.ts) must carry `focus` like `placement`: the browser PUTs whole layouts, so a dropped field is erased on the next save by whoever saves. ⚠️ Grouping is a render layer only: `sessionOrder`, Alt+N and every other order consumer still read the server-projected session order, and a grouped row's markup is the flat row's markup. ⚠️ Only the GROUPED rail is an ARIA tree (`role=tree`, headers owning `role=group`s, one roving `tabindex=0`); the strip, sidebar and flat rail stay `tablist`/`tab`. ⚠️ Every browser write is a named operation through ONE serialized `PUT /api/tab-layout` at a time (`createEditCoordinator`): a 409 replays the operations onto the server's layout and retries (bounded), and an SSE reload is deferred while a write is in flight. Never PUT the layout from anywhere else in the frontend (the `pagehide` keepalive in `_persistPendingTabLayoutEdits` is the one deliberate exception). ⚠️ Placing a NEW session in a group rides its create request (`tabGroupId` on `POST /api/sessions` / `/api/quick-start`) into `TabLayoutService.sessionCreated()`, which registers the session inside the owner lock and places it in the same write; never place a created session with a follow-up PUT. A spawned child lands after its parent; whether it keeps FOLLOWING it until moved by hand is the synced `spawnedTabsFollowParent` setting (default OFF: pinned `manual` in the creating write), read fresh in `sessionCreated()`. A case's **default session group** (Case settings; `caseDefaultGroups` in state.json, per owner and case) is the fallback placement in that same write, below `tabGroupId` and the parent, from the create request's `caseName` (quick-start, Ralph start, or inferred lexically from a `POST /api/sessions` `workingDir`); it is read and written ONLY through `TabLayoutService`. ⚠️ A legacy order PUT marks a child `manual` ONLY when the requested order splits it from its parent's block (`applyLegacySessionRank`), and the browser never echoes create/delete through `PUT /api/session-order`: the server already re-projected and broadcast it. Web-tab OPENNESS stays per device. ⚠️ `TabLayoutService` is the single mutation boundary (one completed server action = at most one versioned write); never write layout state from a route or manager directly. ⚠️ The layout PROJECTS onto `PUT /api/session-order` via `tab-layout-legacy-order.ts`; change both sides together. ⚠️ Reconciliation is gated on a SUCCESSFUL restore (`markRestorationComplete`/`assertDeletionReady()`): a failed restore must leave the layout untouched or live tabs get pruned. → [architecture-invariants#owner-tab-layouts](docs/architecture-invariants.md#owner-tab-layouts) +**Owner tab layouts** (`tab-layout*.ts` + `GET`/`PUT /api/tab-layout`): named tab GROUPS over the flat strip, scoped per owner (`@single` when multi-user is off), persisted as `tabLayouts` in state.json. The frontend reads AND edits it (`tab-layout-browser.js` + the grouped-rail block in app.js): the vertical rail draws the owner's groups as collapsible sections (collapse is per-device localStorage), and with no groups or a failed read the rail is the flat list. Groups are created, renamed, reordered and deleted, and rows moved between them, from the row/group menus (Shift+F10 on a header too) and by pointer drag in the grouped rail. A ref may carry `focus: true` ("Add to Focus" in the row/web-tab menu, the named `setFocus` operation): the rail then draws a SHORTCUT to it in a pinned, collapsible Focus section (`_renderTabFocus()`), its own element in `#tabRail` ABOVE `#sessionTabs`, so it shows on the flat rail too and is never a row, tree item, Alt+N slot, lineage anchor or drag target. ⚠️ Every copy of a ref (`copyRef` in tab-layout-browser.js, `parseRef`/`mapRef` in tab-layout.ts) must carry `focus` like `placement`: the browser PUTs whole layouts, so a dropped field is erased on the next save by whoever saves. ⚠️ Grouping is a render layer only: `sessionOrder`, Alt+N and every other order consumer still read the server-projected session order (Alt+[ / Alt+] and Ctrl+Tab included), except the phone swipe, which walks the drawn order like the strip's arrow keys (`nextSession({ visual: true })` through `_stripSessionOrder()`), and a grouped row's markup is the flat row's markup. ⚠️ Only the GROUPED rail is an ARIA tree (`role=tree`, headers owning `role=group`s, one roving `tabindex=0`); the strip, sidebar and flat rail stay `tablist`/`tab`. ⚠️ Every browser write is a named operation through ONE serialized `PUT /api/tab-layout` at a time (`createEditCoordinator`): a 409 replays the operations onto the server's layout and retries (bounded), and an SSE reload is deferred while a write is in flight. Never PUT the layout from anywhere else in the frontend (the `pagehide` keepalive in `_persistPendingTabLayoutEdits` is the one deliberate exception). ⚠️ Placing a NEW session in a group rides its create request (`tabGroupId` on `POST /api/sessions` / `/api/quick-start`) into `TabLayoutService.sessionCreated()`, which registers the session inside the owner lock and places it in the same write; never place a created session with a follow-up PUT. A spawned child lands after its parent; whether it keeps FOLLOWING it until moved by hand is the synced `spawnedTabsFollowParent` setting (default OFF: pinned `manual` in the creating write), read fresh in `sessionCreated()`. A case's **default session group** (Case settings; `caseDefaultGroups` in state.json, per owner and case) is the fallback placement in that same write, below `tabGroupId` and the parent, from the create request's `caseName` (quick-start, Ralph start, or inferred lexically from a `POST /api/sessions` `workingDir`); it is read and written ONLY through `TabLayoutService`. ⚠️ A legacy order PUT marks a child `manual` ONLY when the requested order splits it from its parent's block (`applyLegacySessionRank`), and the browser never echoes create/delete through `PUT /api/session-order`: the server already re-projected and broadcast it. Web-tab OPENNESS stays per device. ⚠️ `TabLayoutService` is the single mutation boundary (one completed server action = at most one versioned write); never write layout state from a route or manager directly. ⚠️ The layout PROJECTS onto `PUT /api/session-order` via `tab-layout-legacy-order.ts`; change both sides together. ⚠️ Reconciliation is gated on a SUCCESSFUL restore (`markRestorationComplete`/`assertDeletionReady()`): a failed restore must leave the layout untouched or live tabs get pruned. → [architecture-invariants#owner-tab-layouts](docs/architecture-invariants.md#owner-tab-layouts) **Hook events**: Claude Code hooks trigger via `/api/hook-event` (`permission_prompt`, `elicitation_dialog`, `elicitation_complete`, `elicitation_response`, `idle_prompt`, `stop`, `teammate_idle`, `task_completed`, `prompt_submitted`); see `src/hooks-config.ts` and `docs/claude-code-hooks-reference.md`. ⚠️ Every claude session installs the hooks block into its workspace (add-only merge) from every create path and from `restoreMuxSessions()`, gated by `workspaceHooksEnabled` (SYNCED, default ON). ⚠️ Route that decision through `applyWorkspaceHooks`, never call `ensureCodemanHooks` at a new site, or the setting silently stops applying. ⚠️ An AskUserQuestion / plan-selection dialog arrives as `permission_prompt` (RED alert), not `elicitation_dialog` (MCP elicitation). → [architecture-invariants#hook-events-and-workspace-hook-installation](docs/architecture-invariants.md#hook-events-and-workspace-hook-installation) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index f446f4a1..1f60d4d5 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -895,7 +895,7 @@ Tests: `test/tab-rail-search.test.ts` (gate) and `test/tab-rail-search.browser.t ⚠️ **Row ORDER is a third rail attribute** (`tabRailSort: 'activity'|'manual'`, `data-tab-rail-sort`, default **activity**): a sorted rail answers the home screens' question with the home screens' answer, `CodemanSessionOrder` over rows classified by `_mobileOverviewState` (`_tabRailSortOrder` in app.js). ⚠️ **It is applied as the flex `order` property, never by reordering the DOM**: `#sessionTabs` stays in `sessionOrder`, so the Alt+N badge (`_tabIdx`, which therefore does NOT run 1,2,3 down a sorted rail — it names a shortcut, not a position), drag-and-drop, the arrow-key walk, the sidebar filter and `_scrollActiveTabIntoView()` all keep reading the list they always read, and a session changing state moves ONE inline style instead of forcing the full rebuild that would restart every card's animation on every SSE tick. The incremental render path therefore has to re-apply it (a state flip adds no tab, so the full rebuild is never reached) and an empty string is what clears it when sorting stops. ⚠️ Web tabs are pinned past the cards by a CSS `order: 9999` rather than an inline one, since `renderWebviewTabs()` emits the same markup for every layout; the flex default of 0 would interleave them among the sorted sessions. ⚠️ `setupTabDragHandlers()` sets `draggable="false"` and returns while sorting is on: the drop rewrites `sessionOrder` correctly and the sort then puts the card straight back, so the affordance would be a lie — `'manual'` is the way back to drag-reordering. -⚠️ **The arrow-key walk is the one place that must follow the SORT rather than the DOM**: `_tabKeydownHandler` steps `querySelectorAll` order, which is `sessionOrder`, so on a sorted rail ArrowDown from the top card landed wherever that session sat in the tab order instead of on the card below it. It now sorts its node list by the COMPUTED `order` first (computed, not inline: web tabs get their `order: 9999` from CSS and would otherwise read as 0 and lead the walk). This is the exception that proves the DOM-stays-in-sessionOrder rule: the badge, drag model and Alt+N index all deliberately keep reading the DOM, and only the thing the user steps with their eyes follows the paint. +⚠️ **The arrow-key walk and the phone swipe are the two places that must follow the SORT rather than the DOM**: `_tabKeydownHandler` steps `querySelectorAll` order, which is `sessionOrder`, so on a sorted rail ArrowDown from the top card landed wherever that session sat in the tab order instead of on the card below it. It now sorts its node list by the COMPUTED `order` first (computed, not inline: web tabs get their `order: 9999` from CSS and would otherwise read as 0 and lead the walk). That walk is `_visibleTabRows()`, and the phone swipe shares it: `SwipeHandler` calls `nextSession({ visual: true })` / `prevSession({ visual: true })`, which step through `_stripSessionOrder()` (the grouped tree's session rows via `_tabTreeItems()`, otherwise `_visibleTabRows()`, web tabs skipped), so a swipe lands on the tab drawn next to the current one in the case and state layouts, a sorted rail and named groups; Alt+[ / Alt+], Ctrl+Tab and Alt+1..9 keep `sessionOrder`. An active session the list does not draw steps in at the first row (next) or the last (previous), and an empty drawn list goes nowhere rather than falling back to a hidden session (`test/session-swipe-gesture.test.ts`). This is the exception that proves the DOM-stays-in-sessionOrder rule: the badge, drag model and Alt+N index all deliberately keep reading the DOM, and only the things the user steps with their eyes (the arrow keys and the swipe) follow the paint. ### Split-pane sessions diff --git a/test/session-swipe-gesture.test.ts b/test/session-swipe-gesture.test.ts index 3e7086e9..2d4036cc 100644 --- a/test/session-swipe-gesture.test.ts +++ b/test/session-swipe-gesture.test.ts @@ -181,6 +181,8 @@ describe('the swipe walks the order the strip draws', () => { setTimeout, clearTimeout, requestAnimationFrame: vi.fn(), + // A row stub's `cssOrder` stands in for the flex `order` a sorted rail paints. + getComputedStyle: (el: { cssOrder?: number }) => ({ order: String(el.cssOrder ?? 0) }), HTMLCanvasElement: class HTMLCanvasElement {}, WebSocket: { OPEN: 1 }, fetch: vi.fn(), @@ -197,23 +199,32 @@ describe('the swipe walks the order the strip draws', () => { /** The real next/prev over a strip drawn as `rows` (ids; 'web' is a web tab) with `active` selected. */ function walk(rows: string[], active: string | null) { - const { proto, document } = loadAppProto(); const row = (id: string) => id === 'web' ? { dataset: { webviewId: 'web' } } : { dataset: { id }, classList: { contains: () => false } }; - const strip = { - getAttribute: () => 'tablist', - classList: { contains: () => false }, - querySelectorAll: () => rows.map(row), - }; + return drive( + { + getAttribute: () => 'tablist', + classList: { contains: () => false }, + querySelectorAll: () => rows.map(row), + }, + active + ); + } + + /** The real next/prev over `strip` (a stub of #sessionTabs) with `active` selected. */ + function drive(strip: unknown, active: string | null, sorted = false) { + const { proto, document } = loadAppProto(); const selected: string[] = []; const app: Record = { sessionOrder: ['w1', 'w2', 'homelab', 'w3'], sessions: new Map(['w1', 'w2', 'homelab', 'w3'].map((id) => [id, {}])), activeSessionId: active, - isTabRailSorted: () => false, + isTabRailSorted: () => sorted, selectSession: (id: string) => selected.push(id), }; - for (const name of ['_visibleTabRows', '_stripSessionOrder', 'nextSession', 'prevSession']) app[name] = proto[name]; + for (const name of ['_visibleTabRows', '_tabTreeItems', '_stripSessionOrder', 'nextSession', 'prevSession']) { + app[name] = proto[name]; + } document.getElementById = () => strip; const next = (o?: unknown) => (app.nextSession as (o?: unknown) => void).call(app, o); const prev = (o?: unknown) => (app.prevSession as (o?: unknown) => void).call(app, o); @@ -230,12 +241,50 @@ describe('the swipe walks the order the strip draws', () => { expect(selected).toEqual(['homelab', 'w2', 'w1']); }); - it('steps in at the first or last drawn row when the active session is not drawn', () => { - // A filter hid the active tab (or nothing is active on the home screen). - const { selected, next, prev } = walk(['w1', 'w2', 'homelab'], 'w3'); + it('walks the grouped tree: group by group, sorted inside each, headers and web tabs skipped', () => { + // A rail sorted by activity with a named group "ops" (homelab, a web tab, w3) above + // Ungrouped (w1, w2); the DOM keeps sessionOrder inside each group. The tree sorts + // INSIDE each group, so it draws w3, homelab, web | w1, w2, where one sort over the + // whole list (the flat walk) would interleave the groups as w3, w1, homelab, w2. + const tab = (id: string, cssOrder: number) => ({ + dataset: id === 'web' ? { webviewId: 'web' } : { id }, + cssOrder, + classList: { contains: (c: string) => c === 'session-tab' }, + closest: () => null, + }); + const header = { dataset: { tabGroupHeader: 'ops' }, classList: { contains: () => false } }; + const section = (head: unknown, rows: unknown[]) => ({ + classList: { contains: () => false }, + querySelector: () => head, + querySelectorAll: () => rows, + }); + const ops = [tab('homelab', 2), tab('web', 9999), tab('w3', 0)]; + // The Ungrouped heading is aria-hidden, not a treeitem, so the section has none. + const ungrouped = [tab('w1', 1), tab('w2', 3)]; + const sections = [section(header, ops), section(null, ungrouped)]; + const strip = { + getAttribute: (name: string) => (name === 'role' ? 'tree' : null), + classList: { contains: () => false }, + querySelectorAll: (sel: string) => + sel === '.tab-layout-group' ? sections : sel.includes('.tab-cluster') ? [] : [...ops, ...ungrouped], + }; + const { selected, next, prev } = drive(strip, 'homelab', true); next({ visual: true }); prev({ visual: true }); - expect(selected).toEqual(['w1', 'homelab']); + // Tree: homelab -> w1 (past the web tab, into the next group), homelab -> w3. + // The flat walk would give w2 / w1, the canonical order w3 / w2. + expect(selected).toEqual(['w1', 'w3']); + }); + + it('steps in at the first or last drawn row when the active session is not drawn', () => { + // A filter hid the active tab (or nothing is active on the home screen). The drawn + // order differs from sessionOrder's ends (w1 ... w3), so the canonical walk from w3 + // (w1 / homelab) cannot pass for this, and nor can the old arithmetic on the drawn + // order, which put previous on the second to last row (w1). + const { selected, next, prev } = walk(['homelab', 'w1', 'w2'], 'w3'); + next({ visual: true }); + prev({ visual: true }); + expect(selected).toEqual(['homelab', 'w2']); }); it('reaches a lone drawn row that is not the active one, and goes nowhere from a lone active one', () => {