refactor(agents)!: action-verb agent roster, audit-claude removal, agents TUI + HUD fixes

[dean0x]

Do not merge yet. Two verification steps remain that cannot be run from an automated session — see Before this can merge. Everything else is done and evidenced.

What this is

An integration branch carrying five related changes to devflow's agent system. Four were built as separate, individually-reviewable PRs (#282, #283, #284, #285) that each merged into this branch rather than into main, because they are individually reviewable but only jointly verifiable — PR 4's checks exercise a column PR 3 added, and PR 2's count changes ripple into PR 4's documentation. Reviewing them as one diff against main is the intended path.

A fifth change — a fix to the HUD status line — was found while reviewing this work and is merged in as fix/hud-base-branch. It is described below alongside the others.

main has been untouched throughout. If this branch is abandoned there is nothing to revert.

Why

Three unrelated problems, bundled because they all land in the same blast radius:

1. The agent roster read as actor-nouns (coder, designer, reviewer) where action-verbs were wanted (code, design, review). Cosmetic in isolation, but renaming an agent in this codebase is genuinely dangerous — see the next section.
2. The CLAUDE.md audit feature was unwanted and needed removing outright.
3. The devflow agents TUI had four defects, found by running it in a real terminal: aliases and full model IDs both appeared as separate picker stops for the same model; GPT selections were silently accepted while inert; there was no install-state column; and agent names rendered lowercase.

---

The one thing a reviewer must understand first

A devflow agent's identity is three independent strings, and nothing in the codebase links them.

Form	Example	Authoritative in
A — slug	bug-analyzer	filenames, DEVFLOW_PLUGINS[].agents[], ~/.claude/agents/devflow/{slug}.md, agent-models.json keys, devflow agents --set {slug}
B — spawn key	BugAnalyzer	the frontmatter name: on line 2 of each agent file
C — prose	"the Coder agent"	agent bodies, skills, the orchestrator charter, rosters, docs

Form B is not derived from form A, and it diverged in the shipped tree: bug-analyzer.md declared name: BugAnalyzer (hyphen dropped), with the slug appearing nowhere in the file.

The critical part: nothing in src/ ever parses that name: field. Only Claude Code does, at spawn time. So a rename that correctly updates the filename, the registry, and every subagent_type literal — but misses line 2 of one agent file — leaves every test green and every spawn broken at runtime. There was no guard for this; the equivalent guard existed for skills but had never been written for agents.

That risk is why PR 4 lands guards before it renames anything, and why the guard work is worth reviewing more carefully than the rename itself.

---

What's in it

PR #282 — install/uninstall lifecycle (9 files)

Foundation for the rest. Cleanup was previously asymmetric: skills got a registry-diff sweep (gated to full installs only), while agents and commands relied on hand-maintained legacy name lists.

• New shared src/core/orphan-sweep.ts; one compute site used by both the installer and the uninstaller.
• The sweep is now ungated and covers agents, commands, and skills — it runs on every install shape, including --plugin partial installs.
• Wired up getAllCommandNames(), which existed but was unused anywhere in src/.

The safety property to check: getAllAgentNames() / getAllCommandNames() / getAllSkillNames() span all plugins regardless of which are selected. That is what lets assets belonging to unselected plugins survive a partial install — the sweep deletes only names that left the registry entirely. If any code path intersects with the selected-plugin subset, that is a data-loss bug.

This is also why PRs 2 and 4 add zero legacy-list entries: retired names are swept automatically once they leave the registry.

Uninstall fixes in the same PR: removeAllDevFlow / removeSelectedPlugins / isDevFlowInstalled are now exported (previously unexported and therefore untestable); the selective path sweeps retired assets and calls revertExternalAgents; --keep-docs is honored in resolveDevflowDirCleanup (previously an active data-loss path that could prompt to wipe skill shadows and preference-profile.md); the artifact list is completed; and a containment precondition prevents any artifact path from resolving to ~/.devflow itself or above it.

A classification invariant was introduced and is enforced by a test: enumerateUserDevFlowContent (user state, survives unless explicitly confirmed) and the install-artifact list (removed on every path, including decline, cancel, and --keep-docs) must be disjoint. A name in both makes the confirmation prompt untruthful — it is listed as user content that removal would take, then deleted regardless of the answer. agent-models.json and migrations.json are artifacts; hud.json is user state.

PR #283 — remove the audit-claude feature (23 files)

Deletes the devflow-audit-claude plugin, the claude-md-auditor agent, the /audit-claude command, and the now-dead non-selectable-optional carry mechanism.

Why the carry was deleted rather than retargeted: devflow-audit-claude was the only plugin both optional: true and inside partitionSelectablePlugins's EXCLUDED, so once it is gone the helper's only reachable return value is [] — a guard for a now-impossible state. Keeping it would have meant retargeting its 17 it() blocks at synthetic fixtures, asserting the helper's algebra against plugins that do not exist. It is replaced by a recorded JSDoc invariant at the surviving seam: an optional plugin added to EXCLUDED needs a re-init carry.

Documentation counts were re-derived, not decremented — the previous "23 plugins (12 core + 10 optional language/ecosystem + 1 optional workflow)" was already wrong on both the split and the language-plugin count. Now 22 plugins (12 core + 10 optional) and 16 agents.

PR #284 — agents TUI (11 files)

• Aliases-only picker, rendered bare. Per model in registry order: contribute all of its aliases, and its canonical ID only if it has none. sol (gpt-5.6-sol) now renders as sol. renderModelCell drops from four branches to three. The other two parentheticals are deliberately kept — default (opus) reports the shipped default and (unavailable) is a status marker, not an identifier.
• The rule is entirely catalog-driven and requires no devflow-side maintenance. Nothing is hardcoded; new models from subswitch appear automatically, and a model that later gains an alias starts rendering as the alias with no code change.
• Inertness at selection time — the dormancy marker now appears on the keypress (and, after review, correctly clears on one).
• A fourth STATE column (budget 2+18+32+13+13 = 78 ≤ 80), plus orphan agent-models.json keys rendered as editable rows.
• Capitalized names in the TUI only. --list deliberately stays lowercase: its AGENT column is an identifier users copy into --set, which exact-matches.

src/core/model-discovery.ts is byte-identical on purpose — its selectableNames doubles as the --set validation allowlist, so narrowing it would have rejected --set coder --model gpt-5.6-sol.

PR #285 — the rename (97 files)

Old	New	Old	New
coder	code	simplifier	simplify
designer	design	skimmer	skim
evaluator	evaluate	synthesizer	synthesize
researcher	research	tester	test
reviewer	review	triager	triage
scrutinizer	scrutinize	validator	validate
bug-analyzer	diagnose

Unchanged: git, knowledge, learning.

/bug-analysis is NOT renamed — the plugin, the command, the skill, .devflow/docs/bug-analysis/ and tests/bug-analysis/ all stay. Only the agent moved. (bug-analyzer and bug-analysis share a prefix; the only safe discriminator is the trailing er/zer.)

Six guards were added, five of them before any rename, and each was individually falsified — mutated until it went red with the assertion captured, then restored:

• GAP-1 form A ↔ form B for every agent, via an exception map that shrank to empty and is now asserted empty.
• GAP-2 agentType: coverage over the dynamic-* commands — roughly 40 spawn sites that no test read at all before this. Tolerates the no-space agentType:"X" form.
• GAP-3 the orchestrator charter names only live agents, plus a size assertion: the hook that injects it fails open and silent past its cap, so an oversized charter is simply never injected with no error.
• GAP-4 roster model tiers vs frontmatter model:.
• GAP-5 a retired-name sweep using maximal recall (case-insensitive, no word boundary) over a corpus asserted non-empty at 248 files (src/assets + dist/commands + docs/reference + the git-tracked .devflow/features/ knowledge bases), with a bidirectional allowlist.
• GAP-6 pins the 13 shipped key mappings and their invariants.

All dist-reading guards now fail loudly naming npm run build when dist/ is absent, rather than skipping. Two pre-existing fail-open guards were fixed the same way.

Key migration: LEGACY_AGENT_KEYS maps the 13 old slugs to canonical names, applied by one pure canonicaliseAgentKeys used by both consumers — readAgentMapping and a scope:'global' migration entry — so they cannot drift. It parses raw JSON rather than routing through readAgentMapping, which drops invalid values and would silently delete user data on a round trip. Prototype-safe throughout (Object.create(null) + Object.hasOwn), which also closes an argv trap where --set constructor would otherwise return a function into path.join.

No LEGACY_AGENT_NAMES entries were added — PR #282's registry-diff sweep removes the old installed files automatically once the names leave the registry.

fix/hud-base-branch — HUD status-line base detection (2 files)

Found while reviewing the above: the status line reported 3 changed files on a branch that actually had 119.

detectBaseBranch() in src/hud/git.ts scanned the HEAD reflog for the branch most recently checked out from and treated that as the merge base. A reflog entry is not a merge base — here it selected a feature branch already fast-forwarded into the current one, so the "base" collapsed to roughly HEAD and the counter collapsed with it. A locally merged or deleted branch poisons it, and because logs/HEAD is per-worktree, the same branch reported different numbers from different checkouts.

Two further defects in the same file:

• Range semantics were mixed. The diff used git diff <ref> (working tree vs ref) while the ahead/behind arrow fifteen lines above used three-dot <ref>...HEAD. When the base carries commits not in HEAD, the diff bleeds in reverse changes.
• An uncached gh pr view network call sat on the render path, executed on every status-line render and bounded only by a 1s timeout. The docstring claimed that layer was cached; it was not.

Now the default branch resolves deterministically via origin/HEAD, falling back through origin/main|master|develop|trunk then local equivalents, with no network call. When nothing resolves the counter renders as absent rather than as a confidently wrong number. The reflog heuristic and the gh layer are deleted outright rather than kept as fallbacks — a fallback that fires in exactly the confusing cases is worse than none. --fork-point was deliberately avoided: it depends on reflog data and degrades in fresh clones and worktrees, the very fragility being removed.

The diff and the arrow now share a merge base. The diff still includes uncommitted work (useful in a status line); the arrow stays commits-only. That asymmetry is deliberate and commented.

Why its test lives in tests/integration/: src/hud/git.ts had zero coverage — every HUD test fed hand-written values into the rendering layer, and tests/hud-render.test.ts hardcoded ahead: 2, filesChanged: 3, which is precisely the wrong output this bug produced. The new test uses real temporary git repositories, so it belongs in the integration suite the repo already excludes from the default run. Placed in the unit suite it deterministically broke 14 unrelated timeout-prone tests through git-subprocess contention (measured: main green twice at ~21s; branch red 3/3 at ~58s with an identical failure set). Relocated, the unit suite returns to the main baseline exactly.

---

How to review this

Suggested order, roughly by risk:

1. src/core/orphan-sweep.ts and its two call sites. This is the only new deletion primitive. Confirm it never intersects with the selected-plugin subset.
2. The uninstall classification invariant in src/cli/commands/uninstall.ts — the disjointness of user-state vs install-artifact, and the containment precondition.
3. tests/agent-name-guards.test.ts. This is the safety net for the rename and the highest-value file in the branch. Worth asking of each guard: can this actually fail?
4. src/core/agent-models.ts + src/core/migrations.ts — the key migration, especially the raw-JSON parse and the prototype safety.
5. The rename diff itself. Large but mechanical. git diff main...wave/agent-roster -- src/assets/agents/ shows the 13 renames as git renames rather than delete+add.
6. src/cli/agents-view/ for the TUI changes.
7. src/hud/git.ts for the status-line fix — small, self-contained, and its test is in tests/integration/.

Two things that look wrong but are deliberate:

• LEGACY_SKILL_NAMES and the LEGACY_SKILLS_* lists are untouched and must stay so. They are deletion manifests for pre-namespace bare directories at ~/.claude/skills/{name}/, which sit outside the swept namespace. A prefixed entry there would delete a live install; a bare entry for a post-namespace skill would delete a same-named foreign directory.
• Reviewer Focus Areas (5 sites, 3 files) is intentionally NOT renamed. It is a cross-file contract — plan.mds emits the heading and code.md parses it — and also a PR-description section name. All five or none.

---

Verification

	main	this branch
Unit test files	84	85
Unit tests	2,693	2,767
Unit failures	0	0
Integration files	3	4
Integration tests	16	37
Integration failures	0	0

Post-review cleanup (4 commits, 9eaba10..0cd3922) adjusted two of these numbers: residual retired-name occurrences were purged from docs/reference/ and the tracked .devflow/features/ knowledge bases, the GAP-5 corpus was extended to cover both directories (233 → 248 files), and the dead LEGACY_AGENT_NAMES list was deleted — its two self-asserting tests account for the 2,769 → 2,767 unit-test delta. Suite re-run green at 85 files / 2,767 tests after the cleanup.

The integration suite was run twice at the end, both green. It is excluded from the default vitest run, so it is easy to forget — and it holds subagent-skill-preload, the only end-to-end exercise of the frontmatter name: field.

One test-harness fix was needed there: pack-install.test.ts's --version step had a 15s budget against an 0.08s true cost, and under parallel load had been measuring 2.9s–15.0s. It tipped over once. runSync also converted the resulting SIGTERM into a bare exit 1 with empty stderr, which made a timeout look exactly like a CLI crash. The budget is now 60s and the signal is surfaced. Packaging integrity was separately confirmed against the real tarball: 16 renamed agents present, deleted files absent, orphan-sweep.js compiled in, files[] and bin unchanged, and --version printing 2.0.0 on both main and this branch.

Build and tsc --noEmit clean. The integration suite — which the default vitest config excludes, and which nothing in this work had previously exercised — also passes (3 files / 16 tests), including subagent-skill-preload, the only end-to-end check of form B.

Behaviors verified end-to-end against a seeded temporary HOME:

• Disk migration rewrites keys with values byte-identical, records the migration id, and a second init is a no-op.
• The read-path alias resolves old keys with the file's sha256 unchanged, proving the read path and not the disk path produced the result.
• Malformed values survive verbatim — {"model":"not-a-real-model","effort":"bogus"} is preserved rather than dropped to {}. (Routing through readAgentMapping would have silently deleted them while a loose assertion still passed.)
• --set coder exits 0 and writes code.
• A partial install yields exactly 16 agent files, never 26.
• Spawn-key closure over freshly built dist/: 66 raw matches, all resolving to a live agent's form-B name or the Claude Code built-in Explore.

Provenance against main: .github/, .claude/, .devflow/docs/, .devflow/memory/ and .devflow/learning/ are byte-identical. The only change under scripts/ is the removal of one element from scripts/build-mds.ts for the audit-claude deletion.

Defects caught during review

Recorded because they are the kind a reviewer should look for elsewhere:

• The scripted prose pass corrupted a spawn contract. _partials/_preamble.mds enumerates the valid agentType string values; the PascalCase pass rewrote them to Code agent, Validate agent, …. That partial compiles into every dynamic command, so a workflow authored from it would have emitted agentType: "Code agent" and resolved to nothing.
• LEGACY_AGENT_KEYS briefly shipped empty because source was edited after the last build — the suite was green against stale compiled output while every dist-reading guard validated the old artifact.
• No test exercised the shipped key mappings. Every test emptied the production map first and injected synthetic entries; emptying the real map left all 92 related tests green. That is why the empty map above went unnoticed. GAP-6 closes it.
• A guard-export change silently disarmed three existing guards. Lifting EXCLUDED to a module export so an invariant could be asserted destroyed the test file's independent literal oracle, turning three working guards into tautologies that move with production.

---

Breaking changes

1. The 13 agent renames. Any subagent_type / agentType value in a user's own custom commands or agents must be updated by hand — devflow cannot migrate files it does not own.
2. --plugin=audit-claude is now rejected. Stale devflow-audit-claude entries in existing manifests are inert and pruned on the next partial reinstall.
3. Per-agent model overrides silently stop applying on downgrade. Retroactive version detection is impossible: readAgentMapping never reads the version field, and a test pins that it is ignored. There is no mechanism that could warn a user, so it is documented instead.
4. A session left open across the upgrade holds a stale orchestrator charter naming the old agents, and should be restarted.

---

Known gaps, deliberately out of scope

• revertExternalAgents on the selective uninstall path reverts every installed agent, not only those being removed, so surviving agents lose GPT frontmatter until the next devflow init. Fixing it is an API change to RevertOptions.
• devflow agents --list does not surface orphan agent-models.json keys; only the TUI gained them.
• An orphan holding a GPT model while routing is off cannot be cleared through the TUI, because dormancy suppresses the dirty flag at both default and the dormant value. Fixing it would change dormancy semantics, which this work deliberately holds constant.
• Header/footer lines other than the keybindings line remain unbounded below 60 columns (pre-existing).
• Test files are not typechecked — tsconfig.json scopes to src/**/* (pre-existing).
• A failed canonicalise-agent-keys-v1 migration is silently marked applied rather than retried, because runGlobalMigration marks any non-throwing return as applied and this migration returns I/O failures as warnings. Net impact is low: readAgentMapping re-canonicalises on every read and the file self-heals on the next write.

---

Before this can merge

Two checks cannot be performed from an automated session and are genuinely outstanding:

1. Live-spawn verification across all 8 workflows. This is the only check that catches a broken subagent_type at runtime, because nothing in src/ parses the frontmatter name: field. It cannot be run from a session that is itself using the old agents — installing the renamed agents mid-session breaks the running session. Sequence: node dist/cli.js init, then start a fresh session, then exercise the workflows. /dynamic-build is the only end-to-end check of agentType: spawns.
   "All N agents succeeded, 0 errors" is not evidence — agents can return with zero tool uses while an orchestrator counts them as succeeded. Check per-agent tool-use counts are greater than zero.
2. A TTY pass on the changes from fix(agents): aliases-only picker, install state, and orphan visibility in the agents TUI #284 — 9 picker stops, STATE flipping on keypress, no wrapped rows at 80 and 60 columns, Bug-Analyzer displayed while the JSON key stays lowercase. Preconditions, or the check is vacuous: warm model cache, proxy.json enabled: false, at least one registry agent absent from the install directory, and agent-models.json seeded with both a canonical-ID pin and an orphan key.

One further item is not objectively verifiable and needs a human judgment call rather than a test: whether the orchestrator, in practice, conflates Test the agent with "test" the noun, or Review the agent with "review" the verb. There is no ground truth for this; it wants a read of the live transcripts.

Built-in collision check: all 13 new names were verified clear against the Claude Code build current at the time of this work — the built-in agent types are Explore and Plan, and none of the new names collide (Plan is taken, but devflow has no plan agent). This must be re-checked on each major Claude Code upgrade: a future built-in could silently shadow a devflow agent, and no in-repo guard can detect that.

[dean0x]

Review cycle 2 (2026-08-19_1756) — incremental over 46 finding-resolution commits

Convergence: Zero cycle-1 findings re-introduced; all by-design deferrals held (per all 11 reviewers).

Resolved in follow-up commits a2e3c44..0c03f34 (pushed with this comment)

• readAgentMapping wired through shared envelope parser (BOM divergence closed); cast pair removed
• delete settings.hooks block removed (third-party hooks now survive uninstall)
• Managed proxy port captured before artifact removal (non-default ANTHROPIC_BASE_URL now stripped properly)
• Dry-run artifact list existence-filtered; test de-vacuized
• Full uninstall now sweeps orphaned devflow:* skill dirs
• Duplicate collision warning removed
• CHANGELOG Decisions-Citations record restored
• KB parser/type/char-count corrections (saved-inactive length corrected to 14 chars)
• recordSweep extraction and helper consolidation
• SweptOrphan kind tags added for agent/command/skill discrimination
• mdFileName adoption in rule paths
• Comment/doc corrections (resolveComparisonRef layer count, reference doc false pre-install claim)
• Skill-sweep test now verifies registry-skill survival
• 8 new discriminating test assertions added

Deferred for triage (pre-existing or policy — not introduced by branch)

• Bare-name skill deletion sourced from live registry vs frozen legacy list (PF-012 tension, 3 sites, pre-existing behavior)
• shellExec has no maxBuffer (for-each-ref/status overflow would silently blank HUD segments — pre-existing module)
• Three sequential git spawns at top of gatherGitStatus (~27ms wall time, parallelization deferred)
• TRUNK_BRANCHES↔SKILL.md sync guard hardcodes a third copy instead of parsing markdown
• readInstalledAgentNames swallows EACCES silently (deliberate plan decision — degrade-not-throw)
• runCleanupPhase further extraction needed (240 lines, 5 responsibilities, deferred as structural refactor)
• LEGACY_AGENT_KEYS identity-entry module-load assertion
• Low-severity test-shape items (export-shape tests, runDryRunPhase branch observation, installer sweep kind coverage)

Reports & Verdict

Full reports in .devflow/docs/reviews/wave-agent-roster/2026-08-19_1756/ — synthesis verdict was CHANGES_REQUESTED against 6e08ba5. The blocking subset is resolved by the pushed follow-up commits a2e3c44..0c03f34; remaining items are the deferred-triage list above (pre-existing patterns, low-touch structural improvements for follow-up).

---

[dean0x]

Three cycle-2 deferred-triage items resolved on this branch:

1. Bare-name skill deletion now sourced exclusively from frozen LEGACY_SKILL_NAMES (5562b13)

All four affected sites — installViaFileCopy, removeAllDevFlow, removeSelectedPlugins, and enumerateDryRunExtras — now sweep bare names from the frozen legacy list only. A fourth site (removeSelectedPlugins) was identified during validation beyond the three flagged in the review. The installer's own bare pass is deleted outright as redundant with init's legacy cleanup pass, which solely owns bare legacy removal. Discriminating tests seed a foreign skills/security/ dir (RED-proven against the old code); a non-vacuity anchor guards the hazard set. Avoids PF-012.

2. Explicit 16 MiB maxBuffer on HUD git subprocess (499aabe)

shellExec relied on Node's 1 MiB default; git for-each-ref output scales with ref count, and overflow was swallowed into '', silently blanking the ahead/behind and diff-stats segments. GIT_MAXBUFFER is now set at the single execFile choke point. Regression test simulates ERR_CHILD_PROCESS_STDIO_MAXBUFFER and pins graceful degradation.

3. TRUNK_BRANCHES guard now parses the SKILL.md canonical list (cf8c137)

The old guard test pinned its own hardcoded third copy of the branch list, so SKILL.md edits left CI green and the subset-only check missed extras. The test now parses the Protected Branches canonical list from worktree-support SKILL.md and asserts set-equality in both directions, including release/* → TRUNK_BRANCH_PREFIXES.

---

Still deferred (out of scope for this wave): runCleanupPhase extraction and low-severity test-shape items noted in the review.