Sidebar: locked thread groups + harness sessions as children (0046) #119

Merged
chad merged 5 commits from glen/sidebar-thread-groups into main 2026-10-17 12:36:58 -04:00
Collaborator

Closes the two sidebar issues from Chad's report. Spec: PLAN.md (approved v1); design amendment docs/0046-thread-groups.md.

Change A — locked thread groups (store invariants)

  • Dragging a parent or any descendant now files the whole group: moveConversation normalizes to the topmost ancestor and writes folderId to root + every descendant. Same cascade for archive, trash, restore, and delete-forever (run-kind children keep their machine_runs teardown).
  • Both child kinds lock: start_thread chats (conversations.parent_conversation_id) and historical machine runs (machine_runs.parent_conversation_id — found as a gap in self-review; runs nested visually but didn't lock).
  • New children copy the parent's folder at creation (setConversationParent).
  • Migration 0053_group_locking.sql (data-only, cycle-safe recursive CTE): re-files already-diverged children — fixes the live DB where a parent had been moved away from its child.
  • Cycle safety on every walk (visited sets), cross-group rejection now checked against the root's project.

Change B — harness sessions + historical runs as children

  • loadSidebar injects one synthetic kind:"harness" row per harness_calls entry into its conversation's list — nesting, live spinners, and auto-expand come free from the existing tree builder. Read-model only: no rows created; folder counts, Unfiled counts, group totals, search, shelves, Inbox, and countLive all stay conversation-only.
  • Harness rows show a status glyph, open /h/<harness>/<conversation>, and are locked (no drag, no row menu).
  • Historical runs nest under the chat that started them via the machine_runs parent link.

Review

Fresh-eyes pass (review skill) verdict: one blocker, otherwise careful and well-tested — verified the invariant, no synthetic-row leakage, href semantics, migration safety, wake paths.

  • Fixed (red-checked regression test): purgeExpiredTrash threw not-found in the layout render path when an expired trashed group hit the cascade purge; it now tolerates ids already purged by an earlier group member.
  • Fixed nits: degenerate /h//… href fallback (now falls through to /c/), SidebarItem.kind doc names the "harness" value.
  • Declined (noted): cross-project historical run edges — historical-data-only, cosmetic; the root-project check is what the design specifies.

Gate

npm run typecheck ✓ · npm run lint ✓ (0 errors; 2 pre-existing warnings in untouched files) · npm run test ✓ 741 passed / 13 skipped (env-gated spikes). Known timing-flaky tests/x-chat.test.ts (untouched by this branch) green on re-run.

Commits: ea6f40d · 8ec4c76 · 1d65a71 · 9f57cad · e595ce6

Closes the two sidebar issues from Chad's report. Spec: PLAN.md (approved v1); design amendment docs/0046-thread-groups.md. ## Change A — locked thread groups (store invariants) - Dragging a parent **or any descendant** now files the whole group: `moveConversation` normalizes to the topmost ancestor and writes `folderId` to root + every descendant. Same cascade for archive, trash, restore, and delete-forever (run-kind children keep their machine_runs teardown). - Both child kinds lock: start_thread chats (`conversations.parent_conversation_id`) **and** historical machine runs (`machine_runs.parent_conversation_id` — found as a gap in self-review; runs nested visually but didn't lock). - New children copy the parent's folder at creation (`setConversationParent`). - Migration `0053_group_locking.sql` (data-only, cycle-safe recursive CTE): re-files already-diverged children — fixes the live DB where a parent had been moved away from its child. - Cycle safety on every walk (visited sets), cross-group rejection now checked against the root's project. ## Change B — harness sessions + historical runs as children - `loadSidebar` injects one synthetic `kind:"harness"` row per `harness_calls` entry into its conversation's list — nesting, live spinners, and auto-expand come free from the existing tree builder. Read-model only: no rows created; folder counts, Unfiled counts, group totals, search, shelves, Inbox, and `countLive` all stay conversation-only. - Harness rows show a status glyph, open `/h/<harness>/<conversation>`, and are locked (no drag, no row menu). - Historical runs nest under the chat that started them via the machine_runs parent link. ## Review Fresh-eyes pass (review skill) verdict: *one blocker, otherwise careful and well-tested* — verified the invariant, no synthetic-row leakage, href semantics, migration safety, wake paths. - **Fixed (red-checked regression test):** `purgeExpiredTrash` threw not-found in the layout render path when an expired trashed group hit the cascade purge; it now tolerates ids already purged by an earlier group member. - Fixed nits: degenerate `/h//…` href fallback (now falls through to `/c/`), `SidebarItem.kind` doc names the `"harness"` value. - **Declined (noted):** cross-project historical run edges — historical-data-only, cosmetic; the root-project check is what the design specifies. ## Gate `npm run typecheck` ✓ · `npm run lint` ✓ (0 errors; 2 pre-existing warnings in untouched files) · `npm run test` ✓ 741 passed / 13 skipped (env-gated spikes). Known timing-flaky `tests/x-chat.test.ts` (untouched by this branch) green on re-run. Commits: ea6f40d · 8ec4c76 · 1d65a71 · 9f57cad · e595ce6
0046 Change A: a thread group (root conversation + every descendant via
parent_conversation_id, both walks cycle-safe) moves, shelves, and dies
together. moveConversation normalizes the dragged id to its topmost
ancestor and files root + descendants in one transaction (cross-group
check against the ROOT's project); setArchived/setTrashed stamp the
group and restore clears it; deleteConversationForever purges the group
through the existing per-session teardown (run-kind sessions keep their
machine_runs path); setConversationParent copies the parent's folder at
creation. Migration 0053_group_locking.sql re-files already-diverged
children to their root's folder via recursive CTE (UNION keeps a
corrupt cycle finite).
0046 Change B (read model only, no schema change): loadSidebar injects
one synthetic kind:"harness" SidebarItem per harness_calls row into
its owning conversation's list (title = first line of the task clipped
to ~60 chars, runStatus = status, harness name for the href) so
buildSidebarTree nests it; historical kind:"run" rows get their tree
parent from machine_runs.parent_conversation_id via a read-time join.
conversationHref maps harness rows to /h/<harness>/<conversation>;
isItemRunning/isLiveRun treat a running call as live (parent spinner +
default-expand); ConversationRow renders the glyph and link but no row
menu and not draggable (sessions are locked to their conversation).
Counts stay conversation-only everywhere: folder count, Unfiled header,
group totals, countLive, search, shelves, inbox.
Review gap in 0046 Change A: the group walkers read only
conversations.parent_conversation_id, so a kind:"run" child (whose
parent link lives in machine_runs.parent_conversation_id — Change B
joined it for nesting but not for locking) stayed behind on
move/archive/trash/delete, and dragging a run did not bring its family.
Both walkers now share one parent-edge map built from BOTH sources
(conversations edge wins on conflict; visited-set cycle safety kept).
Migration 0053 gains a second data-only statement re-filing run
conversations to their machine-run parent's (already-rooted) folder —
one join, one level, no recursion. Tests: run locks for moves and
trash (from either side), and chat delete purges the run conversation
plus its machine_runs row. Docs + AGENTS.md say the invariant covers
both child kinds.
fix(organize): trash reaper tolerates group-cascade purge (+ href/kind-doc nits)
All checks were successful
deploy / checks (pull_request) Successful in 40s
deploy / deploy (pull_request) Has been skipped
e595ce6909
Review fixes on 0046:

- BLOCKER: setTrashed cascades trashedAt to the whole group, so an
  expired group lists parent AND children in purgeExpiredTrash's batch
  and the first deleteConversationForever purges the rest — the next id
  threw OrganizeError not-found out of the layout render path (a 500,
  aborting the remaining batch and all owned-sandbox destruction that
  pass). The sweep now catches not-found per id and continues;
  deleteConversationForever's not-found contract is unchanged. Red-checked:
  the new regression test fails with exactly that error on the old code.
- conversationHref: a kind:"harness" row without its harness name now
  falls through to the ordinary /c|/r mapping instead of emitting a
  broken /h//<id> URL.
- SidebarItem.kind doc comment now names the third emitted value,
  "harness" (synthetic read-model rows).
- docs/0046: the reaper interaction — a trashed group expires as a unit
  and the purge tolerates ids already purged by an earlier member.
chad approved these changes 2026-10-17 12:36:54 -04:00
chad merged commit d9404f0f9f into main 2026-10-17 12:36:58 -04:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
chad/sweatshop-legacy!119
No description provided.