Skip to content

Harden the subagent roster fold and close review gaps from #160 - #162

Merged
badcuban merged 2 commits into
mainfrom
subagent-review-followups
Aug 17, 2026
Merged

Harden the subagent roster fold and close review gaps from #160#162
badcuban merged 2 commits into
mainfrom
subagent-review-followups

Conversation

@badcuban

Copy link
Copy Markdown
Collaborator

Follow-ups from the multi-agent review pass run against the merged #160 (two Opus review agents + a sol second opinion — which doubled as the live verify of the new agent UX: auto-open, tracker, live status line, and codex-exec promotion all behaved as designed).

Critical: roster fold could brick server startup

The fold could emit two rows for one agent: a spawn's pending: placeholder and an id-keyed row learned from a later wait item are the same agent once the spawn completion carries both keys, but the patch only merged into the first match. The roster table is unique on id/agentThreadId/spawnCallId per thread, so the insert throws — and inside migration 048's transaction that error kills every subsequent boot with no recovery path. Patches now merge all matching rows into one (subagentProjection.ts), with a fold test reproducing the exact sequence.

Migration 048 hardening

  • json_valid() guards the candidate filter: json_extract raises on malformed JSON rather than returning NULL, so one corrupt row aborted the whole migration before the JS-side try/catch could help.
  • A thread whose replay fails is logged and skipped instead of failing the migration — an unbootable server is strictly worse than one stale roster.
  • Orphaned roster rows (threads with no roster-moving activities) are cleaned up, making re-runs a true rebuild.
  • Docblock notes the thread.reverted rowid-ordering caveat.
  • New migration test: both provider shapes, the collision sequence, a malformed payload row, and orphan cleanup.

Web polish from the parking audit

  • An agent's owned background tasks were parked from the feed but invisible in the Agents rail drill-in; the per-agent work filter now includes ownerAgentToolUseId rows.
  • All-anchor work groups with no resolvable tracker left phantom 16px gaps in the timeline; the row wrapper collapses its padding when its section renders nothing.

Shared sanitizer

The <usage> stripper re-lowercased the whole string per stacked trailing block (quadratic on hostile tails — sol's catch); it now lowercases once and walks an index backward, with a stacked-blocks regression test that would crawl on the old loop.

All suites green: server 1893, web 167 files + 471 browser, shared 295, typecheck and lint clean.

Follow-ups from two Opus review agents and a sol second-opinion run against
the merged #160 (which also served as the live verify of the new agent UX):

- The roster fold could emit two rows for one agent when a spawn's pending
  placeholder and an id-keyed row (learned from a later wait item) were
  bridged by a patch carrying both keys. The persisted table is unique on
  id/agentThreadId/spawnCallId per thread, so the write fails - and in
  migration 048 that failure aborted the transaction and left the server
  unbootable on every start. Patches now merge ALL matching rows into one.
- Migration 048: json_valid() guards the candidate filter (json_extract
  raises on malformed payloads rather than returning NULL), orphaned roster
  rows for threads with no roster-moving activities are cleaned up, a thread
  whose replay fails is logged and skipped instead of failing the migration,
  and the rowid-ordering docblock now notes the thread.reverted caveat. New
  migration test covers both provider shapes, the collision, a malformed
  payload row, and orphan cleanup.
- An agent's owned background tasks (ownerAgentToolUseId rows) were parked
  from the conversation but invisible in the Agents rail; the drill-in work
  view now includes them alongside sourceAgentThreadId rows.
- An all-anchor work group with no resolvable tracker rendered null inside
  a padded timeline row, leaving phantom 16px gaps; the row wrapper now
  collapses its padding when the section renders nothing.
- The <usage> stripper in the shared sanitizer re-lowercased the string per
  stacked block (quadratic on hostile tails); it now lowercases once and
  walks an index backward, with a stacked-blocks regression test.
@vercel

vercel Bot commented Aug 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
threadlines Skipped Skipped Aug 16, 2026 11:31pm

Request Review

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Aug 16, 2026
…, detection shielded

Fixes for three issues observed live after #160:

- Promoted codex exec runs never appeared in the panel. The server narrates
  a promoted run entirely through subagent.metadata activities, and the only
  thing the web did with that kind was filter it out of the work log - the
  durable roster row it fed only reached the client on a full detail
  snapshot, i.e. after a reload. The shared subagent record fold now applies
  metadata activities (spawn with callId, the later agent-id link, completion
  with the result body), so a promoted run shows up while it runs and
  settles in place. Tests use the three payloads a live run projected.
- "Detected agent process" rows surfaced the session's own transient
  children: any pid or command mentioned in chat prose (pgrep output, a
  quoted setup script) could match the provider's shell wrappers and search
  helpers, offering stop buttons for the harness's own processes.
  Process-only detection now shields the entire subtree under any live
  provider CLI process - that work is tracked, not lost - while genuinely
  orphaned processes (reparented to init/launchd) stay detectable.
- Background command runs (CI watchers, gate runs) crowded the top of the
  Agents tree while actual agents sat under "Earlier". The view now splits
  them into a Commands section below the agents, keeping their rows and stop
  handles. Consistently, a running command no longer counts as a running
  agent anywhere: not in the tab's live dot, not in the auto-open trigger,
  not in the launcher counts (which now name background commands as what
  they are).
@badcuban

Copy link
Copy Markdown
Collaborator Author

Second commit (da363d6) folds in fixes for the three issues observed while watching the live demo through the new UI:

  1. Promoted codex exec runs were invisible. The server narrates promotions through subagent.metadata activities; the web's only handling of that kind was a filter that discarded it, so the promoted agent only reached the panel via a full detail snapshot (a reload). The shared record fold now applies those activities — the run appears while running and settles in place with its result.
  2. "Detected agent process" rows matched the session's own child processes (shell wrappers, ugrep helpers), seeded from pids/commands mentioned in chat prose, with live stop buttons. Process-only detection now shields any live provider CLI's subtree; orphaned processes (reparented to launchd) stay detectable.
  3. Background command runs crowded the Agents tree above the actual agents. They now render in a Commands section below the agents, and a running command no longer counts as a running agent anywhere: tab live dot, auto-open trigger, launcher counts.

@badcuban
badcuban merged commit 7a8fe74 into main Aug 17, 2026
15 checks passed
@badcuban
badcuban deleted the subagent-review-followups branch August 17, 2026 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant