Harden the subagent roster fold and close review gaps from #160 - #162
Merged
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
…, 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).
Collaborator
Author
|
Second commit (da363d6) folds in fixes for the three issues observed while watching the live demo through the new UI:
|
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.
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 laterwaititem 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_extractraises on malformed JSON rather than returning NULL, so one corrupt row aborted the whole migration before the JS-side try/catch could help.thread.revertedrowid-ordering caveat.Web polish from the parking audit
ownerAgentToolUseIdrows.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.