refactor(install): registry-diff asset sweep + uninstall lifecycle parity - #282
Merged
Merged
Conversation
…istry-diff sweeps - Add shared `sweepOrphanedAssets` helper — single readdir/filter/unlink implementation reused by all three asset-type sweeps (PF-009 per-item isolation, PF-011 sweep-only/no-rewrite, missing-dir is a no-op). - Replace the LEGACY_AGENT_NAMES deletion loop with an ungated registry-diff sweep over ~/.claude/agents/devflow/ using getAllAgentNames(). Names that leave the registry are pruned automatically on every install shape. - Add equivalent registry-diff sweep for commands over ~/.claude/commands/devflow/ using getAllCommandNames() — the function existed but was previously unused by the installer. - Ungate the existing skills sweep: it previously ran only on full installs; it now runs on every install shape including --plugin partial installs. All three sweeps use getAllAgentNames/getAllCommandNames/getAllSkillNames which span ALL plugins — assets from uninstalled plugins survive a partial run. Tests: inverted the stale-agent/command partial-install test; added a comprehensive partial-install vacuity test (retired asset removed, known-asset from uninstalled plugin survives, corpus non-empty guard). Vacuity verified: test went RED with sweep disabled, GREEN with sweep restored. Co-Authored-By: Claude <noreply@anthropic.com>
…dule Move the registry-diff sweep helper from installer.ts (module-private) to src/core/orphan-sweep.ts (exported) so both the install pipeline and the uninstall pipeline can use the same implementation without duplication. installer.ts imports from core/orphan-sweep.js; the helper's semantics and per-item failure isolation contract are unchanged (avoids PF-009, PF-011). Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2 of the install/uninstall lifecycle wave. Closes the gap between what the install side cleans up and what the uninstall side actually removes. Exports (testability): - Export removeAllDevFlow, removeSelectedPlugins, isDevFlowInstalled so end-to-end tmpdir tests can drive them directly (PF-018 compliant — no CLI spawn needed, no ~/.claude guard required in these tests). Registry-diff sweep on selective uninstall: - removeSelectedPlugins now calls sweepOrphanedAssets (imported from core/orphan-sweep.ts) after its per-agent and per-command removal loops. Orphaned agents/commands whose names left the registry are swept away; knownNames spans ALL plugins so assets from other installed plugins survive. revertExternalAgents on selective uninstall path: - Mirrors the existing full-uninstall call. Runs BEFORE removeSelectedPlugins so agent .md files are still present when GPT model lines are stripped. Non-fatal: missing agents dir or revert errors are silently ignored. artifact list extended (removeDevFlowInstallArtifacts): - agent-models.json — cleared on every uninstall so reinstall starts with shipped defaults (was silently stranded, causing stale model assignments) - migrations.json — cleared so migrations run cleanly on reinstall - hud.json — HUD configuration - costs/sessions/ and costs/archive.jsonl — auto-generated cost history - logs/ — entire logs tree (project-slug dirs + global logs including proxy.log) - cache/ parent instead of cache/models only — covers any future sub-dirs - isDir === true discipline maintained throughout (avoids the silent TypeError gotcha in the per-item catch) keepDocs honored in resolveDevflowDirCleanup: - Adds keepDocs?: boolean to the function signature; returns 'artifacts-only' before reaching the precondition guard or isTTY check when keepDocs is true. Previously --keep-docs could still trigger the ~/.devflow wipe prompt via skill shadows or preference-profile.md — active data-loss path fixed. enumerateUserDevFlowContent extended: - Adds hud.json to the warning enumeration (user sets enable/disable via CLI) isDevFlowInstalled fixed: - Now checks agents/devflow and any devflow:* skill directory in addition to commands/devflow. A commandless plugin install (agents + skills only) no longer returns "No Devflow installation found" and exits 1. Dry run made faithful: - Selective path: unchanged (registry-computed, accurate) - Full path: scans actual on-disk directories and reports what would really be removed (including legacy/orphaned assets), rather than computing from the registry Tests: - 9a: removeAllDevFlow empties agents/devflow including retired + user-dropped files - 9b: removeSelectedPlugins sweep removes orphaned names not in any plugin - 9c: ~/.devflow residue EQUALS user-authored allow-list (equality, not subset); non-vacuousness proved — adding mystery-artifact.json caused RED, removing it restored GREEN - 9d: resolveDevflowDirCleanup returns 'artifacts-only' when keepDocs is true - 9e: isDevFlowInstalled returns true for agents/skills with no commands present - step 10: repoint devflow-audit-claude → devflow-bug-analysis/bug-analyzer + non-empty assertion to prevent vacuous pass on an empty selection avoids PF-009, PF-014, applies ADR-003 Co-Authored-By: Claude <noreply@anthropic.com>
- Stop deleting user-owned agent-models.json/hud.json on the artifact-only path. Both were enumerated by enumerateUserDevFlowContent (shown in the full-wipe confirm prompt) AND listed in removeDevFlowInstallArtifacts, which also runs on the decline, cancel, non-interactive and --keep-docs paths — so they were deleted regardless of the user's answer. - Derive the cache/ removal target from hudCacheDir() instead of path.dirname(modelCacheDir()); add a containment invariant so no artifact path can ever resolve to devflowDir itself (avoids PF-013). - Collapse costs/sessions + costs/archive.jsonl into the costs/ parent so the removal site cannot drift from src/hud/cost-history.ts. - Drop the unreachable 'Nothing to remove.' branch in the full dry-run plan (applies ADR-003) and correct the now-inaccurate 'scripts and manifest only' messages. - Use SKILL_NAMESPACE instead of a hardcoded 'devflow:' literal (2 sites). - Add tests/orphan-sweep.test.ts: direct coverage for the new shared sweep helper (missing dir, non-directory path, predicate-skip, recursive dir removal, EACCES isolation) with scanned-count non-vacuity assertions. - Add (9f) disjointness test and extend (9c) so the enumerate/artifact overlap regression is caught; both verified RED against the reintroduced bug.
…p hud.json as user state Resolves four misalignments from the self-review pass: - MISALIGNMENT 1: agent-models.json is added back to installArtifacts in removeDevFlowInstallArtifacts (AC-P1-F4) and removed from enumerateUserDevFlowContent. Stale per-agent model overrides keyed to renamed/deleted agents would silently re-apply on reinstall without this. - MISALIGNMENT 2: hud.json stays absent from installArtifacts and present in enumerateUserDevFlowContent — the reviewer's classification is correct. - MISALIGNMENT 3: the @d8 invariant comment now names agent-models.json as an artifact and hud.json as user state, matching the corrected code. The disjointness test (9f) proved real: adding agent-models.json back into enumerateUserDevFlowContent causes it to go RED (length 6 vs expected 5), removing it restores GREEN. - MISALIGNMENT 4: the comment in tests/plugins.test.ts (~:228) is rewritten to describe the registry-diff sweep mechanism (sweepOrphanedAssets) rather than the now-false statement that LEGACY_AGENT_NAMES drives stale-file cleanup. List contents are unchanged. After this fix a user who declines the full ~/.devflow wipe keeps: skills/ (shadows), rules/ (shadows), preference-profile.md, learning.json, hud.json And loses regardless of answer: agent-models.json, migrations.json, manifest.json, proxy artifacts, logs/, costs/, cache/
dean0x
added a commit
that referenced
this pull request
Aug 19, 2026
…ents TUI + HUD fixes (#287) > **Do not merge yet.** Two verification steps remain that cannot be run from an automated session — see [Before this can merge](#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 #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. --------- Co-authored-by: Claude <noreply@anthropic.com>
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
1 participant
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.
Summary
PR 1 of 4 in the
wave/agent-rosterintegration wave. This is the foundation PR: it replaces hand-maintained legacy-name cleanup with a self-maintaining registry-diff sweep, so the later PRs in this wave (removingaudit-claude, renaming 13 agents) need no legacy-list entries at all.Install side
sweepOrphanedAssetshelper insrc/core/orphan-sweep.ts; one compute site, consumed by both the installer and uninstaller.LEGACY_AGENT_NAMESdeletion loop with an ungated registry-diff sweep over~/.claude/agents/devflow/.getAllCommandNames()— which existed but was previously unused anywhere insrc/.--plugin) installs too.Safe because
getAllAgentNames()/getAllCommandNames()span all plugins regardless of selection: assets belonging to unselected plugins survive, and only names that left the registry entirely are removed.Uninstall side
removeAllDevFlow,removeSelectedPluginsandisDevFlowInstalledare now exported and therefore testable.revertExternalAgentsnow runs on the selective path, before agent files are removed.--keep-docsis honored inresolveDevflowDirCleanup— previously an active data-loss path that could prompt to wipe skill shadows andpreference-profile.md.isDevFlowInstalledno longer keys offcommands/devflowalone, so a commandless plugin set is detected instead of exiting 1.migrations.json,agent-models.json,costs/,logs/,cache/parent, proxy artifacts.~/.devflowitself or above it.Classification invariant
enumerateUserDevFlowContent(user state, survives unless explicitly confirmed) and the install-artifact list (removed on every path, including decline/cancel/--keep-docs) are now disjoint, enforced by a test.agent-models.jsonis an install artifact — leaving it behind meant a reinstall silently re-applied stale per-agent overrides.hud.jsonis user state.Testing
85 files / 2,716 tests / 0 failures (baseline 84 / 2,693; +23 fully accounted per-file). Build and
tsc --noEmitclean.New tests deliberately target the partial-install path — on a full install the pre-existing wholesale wipe deletes planted files anyway, so a green test there would prove nothing about the sweep. Each sweep test also asserts an unselected plugin's asset survived, which is what distinguishes "the sweep worked" from "the wipe ran". Non-vacuity was proven by red→green falsification for the sweep tests, the residue allow-list equality test, and the disjointness test.
Notes
LEGACY_SKILL_NAMESand theLEGACY_SKILLS_*lists are untouched and must stay so: they are deletion manifests for pre-namespace bare dirs outside the swept namespace.LEGACY_AGENT_NAMESnow has no production consumer. Left in place deliberately rather than churned in this PR.revertExternalAgentson the selective path reverts every installed agent rather than only those being removed, so surviving agents lose GPT frontmatter until the nextdevflow init. Fixing it is an API change toRevertOptions.