Skip to content

apps/sdlc simplification audit — stacked PR plan

State: open/active · Genre: analysis

Scope: 176 verified code-quality findings across apps/sdlc, deduplicated to ~58 distinct issues and grouped into an 11-PR stack.

Date: 2026-07-19 Worktree: /Users/sksizer/Developer/dev/.claude/worktrees/sdlc-simplification


apps/sdlc is a well-factored codebase carrying the residue of three completed migrations — the Python→TypeScript port, the plugin/scripts deletion (0ee07a6f), and the op-substrate cutover — none of which swept their orphans. Roughly 1,000 lines are provably unreachable: five whole modules whose sole consumers were deleted, plus dead exports inside live files. The dominant live defect is not bugs but seam erosion: at least six lib/util modules carry headers asserting they are “the single home” for a helper that is in fact re-declared 14–20 times elsewhere (isFile/isDir, escapeRegExp, pyTypeName, rfc3339), and the YAML parse chokepoint’s stated invariant is bypassed by nine production modules. Two duplications have already produced observable wrong output: the docs-site hand-rolled YAML mini-parser truncates three real SKILL.md descriptions and silently disables the H1-strip on every generated skill page, and writePrsFrontmatter emits through a different YAML serializer than every other frontmatter writer, so appending one PR URL reformats a task file’s entire frontmatter block. Nothing here is architecturally wrong — the tiering, op substrate, and DI seams are sound — but the shared-utility layer needs a convergence pass before the next feature wave re-copies the same helpers a fifteenth time.

Verification note (corrected after synthesis): the synthesis pass reported xc-dead-void-silencer-residue as stale, on the grounds that tcpProbe, void existsSync in inflight.ts, void parsePath in runtime.ts, and void inline / void topScore in dedup_search.ts were absent from the tree. That conclusion was an artifact and has been reversed. Several audit sub-agents had write access and applied fixes to the worktree instead of only reporting; the synthesis agent then read the already-mutated tree. Those edits were reverted, and all five void X; silencers plus tcpProbe and stopDashboard are confirmed present:

SymbolLocation (verified)
void existsSynclib/model/entities/task/ops/inflight.ts:116
void parsePathlib/services/lease/runtime.ts:770
void tcpProbelib/services/dashboard/server.ts:1478
void inlineskills/task-work/dedup_search.ts:484
void topScoreskills/task-work/dedup_search.ts:632
tcpProbelib/services/dashboard/server.ts:1359
stopDashboardlib/services/dashboard/server.ts:1485

The finding therefore stands in full and is scheduled in PR 2.

Line numbers throughout the findings have drifted by 1–15 lines in several files; treat every citation below as approximate and re-grep before editing. All line numbers in the table above were re-verified against origin/main at a6aaf6dc.


  1. Migration orphans. Five modules (~900 lines) whose only consumer was deleted by 0ee07a6f or the op-substrate cutover, still compiling, still advertised in module headers.
  2. “Single home” headers that are false. lib/util/fs.ts, strings.ts, date.ts, paths.ts, yaml.ts, and table.ts each declare themselves canonical for a helper that is re-declared elsewhere 6–20 times. The consolidation landed; the sweep did not.
  3. Python-port shims never retired. pyTypeName (8 copies, 3 divergent variants), isPlainObject (8), pyRepr (2), and the {returncode, stdout, stderr} re-shim of the canonical CommandResult (6 modules) all survive the TS port.
  4. Regex grammar copied instead of shared. Two wikilink regexes appear 13 times across 11 entity schemas under four different names; RELATED_PATTERN denotes four different regexes depending on which schema you read.
  5. Gates police spawns, not reuse. scripts/check_command_seam.sh checks spawn shape only. Every easyGit straggler, hand-rolled entity read, and duplicated fs predicate passes it cleanly — which is precisely why these classes accumulated.
  6. Frontmatter has three write paths and four read paths. One byte-exact fence-preserving rewrite is copy-pasted three times; one writer uses a different serializer entirely.
  7. Stale comments actively misdirect. Multiple headers describe deleted files as live consumers, assert invariants that no longer hold, or (in lib/config/load.ts) ship dead script paths into every consuming project’s generated sdlc.yaml.

Branch: simplify/delete-orphaned-modules Effort: M · Risk: low · Depends on previous: no

Five modules and two barrels have zero importers. Each was orphaned by a specific commit; none is reachable from the CLI, the op manifest, any test, or any skill.

  1. Delete lib/services/quality/run-checks.ts (340 lines). Header names apps/sdlc/scripts/run_quality_checks.ts as its only consumer; that shim was deleted by 0ee07a6f. Verified zero importers. The live replacement is lib/services/quality/ops/run.ts, which reproduces the line/log/baseline-gated output verbatim (run-checks.ts:115/138/141/143 ≡ ops/run.ts:155/165/170/172).

    • Also dead with it: EXIT_OK:41, EXIT_FAILED:42, EXIT_CONFIG:43, RunIo:46, ConfigResult:51, loadConfig:65 (distinct from the widely-used @lib/config loadConfig — do not confuse them), VerbResult:69, runOneSilent:77, runOneStreamed:94, RunChecksArgs:215, runChecks:236.
    • Rewrite the stale block at lib/services/quality/ops/run.ts:18-22. It describes a deleted shim and calls the core “parallel” when it is sequential. Replace with a plain statement that this op owns the run logic. Also reword the section comment // Helpers (mirrored from the script) near ops/run.ts:38.
    • Update the stale touchpoint rows naming run-checks.ts in docs/planning/tasks/T-YF4K-*.md:21, T-BCNP-*.md:48, T-BQRU-*.md:63.
    • Do not touch baseline.ts: findingsFromStdout, diff, and normalizeFinding are all still live (ops/run.ts:310 dynamic-imports them).
  2. Delete lib/util/schema_patterns.ts (220 lines) and lib/util/tests/schema_patterns.test.ts. Header names the new_*.ts scaffolding scripts as its consumers; find . -name 'new_*.ts' returns nothing. Sole importer is its own test. It is also the only module in lib/util that imports upward into lib/model, so deleting it removes the tier’s single layering inversion.

    • Before deleting, confirm the backlog superRefine reject branches (lib/model/entities/backlog/schema.ts:109-140) are asserted somewhere. Adjacent coverage exists in lib/model/entities/backlog/ops/tests/update.test.ts and cli/backlog_cli/tests/create.test.ts but does not enumerate reject cases. If absent, port those cases into a test beside backlog/schema.ts. The task/milestone date and version cases are not worth porting.
    • Leave arrayElement in lib/util/zod_introspect.ts:246 in place even though this deletion orphans it — that module is a deliberate Zod-internals choke point whose value is centralizing every private _def read. Add a one-line note so a later dead-code pass does not re-report it.
  3. Delete skills/task-close-out/verify_prs_against_pr.ts and skills/task-close-out/tests/prs_verify.test.ts. SKILL.md:284 Step 4 now runs sdlc task close-commit; the SKILL.md contains no bun run invocation. The op reproduces the logic via the same readPrsFrontmatter/computeAppend pair (close-commit.ts:134-139). Coverage is preserved by lib/util/tests/frontmatter_prs.test.ts (noop/create/append, order preservation, exit-1/exit-2 paths).

    • Keeping the script would be actively harmful: it writes frontmatter directly in the shared primary checkout, whereas the op mutates inside an ephemeral worktree off origin/main — the parallel-WIP isolation guarantee pinned at lib/services/gate/ops/_skill_prose_contract.ts:1215.
    • Prune .claude/skills/project-check/util_redeclarations_baseline.json:45.
    • Note: the prose-contract note at _skill_prose_contract.ts:1230 claims close-commit.test.ts covers “create / append / noop”; it covers only create (:128) and append (:175). Correct the note or add the noop case.
  4. Delete lib/services/lease/canonical_json_cli.ts. Created by f41e6e09 as the shell half of the Python parity harness; a8a60a37 deleted that harness. Zero callers. Not an op, not in the manifest, not in moon.yml/justfile/ lefthook.yml, not spawned by any test. Its usage line still prints the pre-move path apps/sdlc/lib/lease/canonical_json_cli.ts.

    • Companion edit required: lib/services/lease/tree.ts:4-6 justifies the canonical_json.ts file split as “so its CLI wrapper can be invoked independently”. Re-point that comment at the real dependency-free consumer, lib/services/orchestrator/ops/watch.ts:69, which imports canonicalJson directly to avoid pulling in tree.ts’s spawnRunner dependency. The split must survive.
  5. Delete cli/task_cli/_common.ts (10 lines). cli/task_cli/ contains no verb module — only this file, README.md, and tests/. Both test files import from @lib/registry. Its CYCLE_MARKER_PREFIX was independently re-declared at lib/model/entities/task/ops/next.ts:58.

    • Companion edit required: cli/task_cli/README.md:20-27 Layout block lists both this file and an index.ts that already does not exist. Reduce the block to README.md + tests/. Do not fold the README into cli/README.md — it is 112 lines documenting the live sdlc task next surface; relocate it wholesale to lib/model/entities/task/ops/ if you move it at all.
    • cli/README.md needs no edit — its layout block already annotates backlog_cli as the “sole remaining LEGACY_NOUNS entry” and never mentions task_cli.
    • Leave cli/task_cli/tests/ in place (both suites spawn cli/sdlc.ts and are live).
  6. Delete lib/model/index.ts and lib/model/corpus/index.ts (both barrels, zero importers). Every consumer uses deep paths (@lib/model/read, @lib/model/corpus/loader, …). lib/model/README.md:20-21 already documents the deep-path convention, so the barrel headers are stale M-0002-port residue.

    • Cascade: read.ts:195#frontmatterOf and corpus/loader.ts:44-52#readFrontmatter are kept alive only by these barrels. Delete both, plus the describe("readFrontmatter", …) block at corpus/tests/loader.test.ts:17-33 and the symbol in that file’s import on line 10. loadCorpus does not use readFrontmatter.
    • Do not generalize to lib/model/ops/index.ts — that barrel is load-bearing (importing it registers ops via defineOp at import time) and is mandated by D-0007 / D-H7FS.
    • Update docs/planning/tasks/T-LWEH-entity-title-accessor.md:62,71, which instructs a future implementer to export extractTitle from lib/model/index.ts — point it at the deep-path home or the barrel will be resurrected.

Verification: bun test in apps/sdlc; bun run typecheck; sdlc quality run to confirm the quality op path is unaffected. Grep for each deleted symbol to confirm zero residual references.


PR 2 — Remove dead exports, unreachable branches, and stale headers

Section titled “PR 2 — Remove dead exports, unreachable branches, and stale headers”

Branch: simplify/dead-exports-and-stale-headers Effort: M · Risk: low · Depends on previous: no

Unreachable surface inside modules that stay. Each item is independently trivial; the coherent idea is “the exported surface should match what is actually consumed”.

  1. lib/model/entity.ts:132-247 — delete COMMON_FRAGMENT_FILE, isCommonRef, and resolveSchema (~115 lines). No _common.json exists anywhere; zero allOf/$ref in any of the 21 on-disk schema.json files, so resolveSchema unconditionally returns { ...schema } at :188 and everything below is unreachable.

    • Three stale claims must be corrected in the same change: the file header (:25-31), the loadEntitySchema doc comment (:254-265), and the rationale block at :140-142 which asserts the fixtures “still ship” the $ref. Also refresh docs/planning/references/RF-0004-json-schema-2020-12.md:25.
    • Remove the re-export at lib/model/index.ts:11 (that file is deleted in PR 1).
  2. lib/model/ops/audit.ts:679-694 — delete renderJson and its private ensureAscii. Zero callers; the op’s JSON path returns the structured payload via buildPayload:848. Note there is an unrelated live renderJson in cleanup/index.ts:1364 — do not touch it.

    • Rewrite the header at :21-26: delete the whole audit_entities.ts sentence (that file is gone) and drop renderJson from the exported-functions list. Leave the adjacent check_entities.ts paragraph — that file still exists.
    • Keep buildPayload (live caller ops/migrate.ts:398) and renderMarkdown (:849).
  3. lib/model/identifier.ts — delete PREFIX_BY_DIR:50, isCanonicalFilename:87, dashInsert:96, legacyIdChars:101. Verified zero references repo-wide (unfiltered grep). The migration script that consumed the last two, plugin/scripts/migrate_identifier_shape.ts, was deleted in 0ee07a6f and is absent from that commit’s list of deliberate retentions.

    • Header cleanup: drop dash-insert from the capability list at :4 and “the corpus-migration script” from the adapter list at :8-10 — that file does not exist.
  4. lib/config/schema.ts:79-108 — delete validateSubkey. Zero production callers; not in the lib/config/index.ts barrel. Delete the describe block at lib/config/tests/schema.test.ts:102-126 and the import at :13; trim the test file header at :4. This also removes one of the two byte-identical non-mapping guard blocks (schema.ts:66-72 / :95-101), leaving validateLoaded’s as the single copy.

  5. cli/_common.ts — delete emitGenericException:23-26 (zero references; its error: ${exc.constructor.name}: ${exc.message} format also diverges from the live error: ${message} convention, so it could not be adopted without breaking goldens), newUuid4:33-35 (zero references; lease ops use their own at lib/services/lease/ops/_common.ts:338), the date re-export at :31, and the table re-export at :37-40 (zero consumers; lease ops reach @lib/util/table via lib/services/lease/ops/_common.ts:315).

    • Update the header bullet list at :4-8 — after this, only exit codes and emitJson remain. Also drop the stale TS port of apps/sdlc/cli/_common.py line at :2.
    • Update lib/util/table.ts:4-9 to name lib/services/lease/ops/_common.ts as the sole re-exporting tier (the current text is accurate only while cli/_common.ts:40 exists).
    • Keep EXIT_GENERIC — ~20 live consumers.
  6. lib/services/dashboard/server.ts:1469-1493 — delete stopDashboard. Zero callers; orphaned by 0ee07a6f, which deleted plugin/scripts/status_dashboard.ts (its only consumer). lifecycleStop (dashboard-service.ts:145-167) is the live, statement-for- statement equivalent against ctx.io.

    • Update the server.ts module header (:1-7), which advertises “serve/detach/stop/list”.
  7. cli/registry_adapter.ts:341-351 — delete the unreachable invalid---output branch. .choices(OUTPUT_MODES) at :266-269 makes commander reject a bad value during parse, before the action fires (verified: bun cli/sdlc.ts task next --output bogus prints commander’s own message). Drop | null from resolveOutputMode’s return type and correct the doc comment at :84-89, whose closing sentence becomes false. Exit-code parity holds: CommanderError is caught at :604-617 and returns 1, same as EXIT_GENERIC.

  8. lib/model/ops/validate.ts:96-101 — delete the validateFrontmatterAgainstEntity pass-through and have its sole caller (_update.ts:164) call validateFrontmatter from ../entity.ts directly (already imported at _update.ts:52-56). This closes the registration leak cli/sdlc.ts:51 explicitly documents (“Without this, only validate would leak in via a transitive task update import”). Also drop the now-gratuitous export on defaultEntitiesDir at validate.ts:128.

    • Do not pursue the follow-on split of validate.ts:219-530 (the .base validator) — the op descriptor at :623 still dispatches to it, so the module graph is unchanged.
  9. lib/util/wikilinks.ts:47-53 — delete extractWikilinks (zero production callers) and update the module header at :9, which advertises “wrap/unwrap/extract”. lib/util/markdown_extract.ts:175-185 — delete headingPlainText and its private nodeText. Narrow the mdast import at :29 to import type { Root } from "mdast"; — both Heading and RootContent become unused, but Root is live at :46,:61,:78.

  10. lib/services/git/easy.ts — delete diffNames:166 and remoteUrl:192. Zero references anywhere in the worktree, including tests/easy.test.ts. Do this before easyGit is promoted to packages/ts/easy-git (PR #957) so unexercised surface is not exported into a package.

    • watch.ts:239-243 hand-rolls configGet("remote.origin.url") where remoteUrl would fit, but the swap is not semantics-preserving (git remote get-url expands insteadOf rewrites; git config --get does not). File that separately.
    • skills/task-work/check_ancestry.ts:89-99 is not a prospective diffNames caller — it needs a two-dot [baseRef, "HEAD"] argv form and throw-on-failure, neither of which the API models.
  11. Dead export tails in lib/model/entities/task/ops/:

    • resolve-touchpoints.ts:294 — drop resolveRow, globMatches, fileContainsSymbol from the export list (zero importers; globMatches moves in PR 8 anyway).
    • parse-touchpoints.ts:487-488 — drop detectShape, parseLocationCell from the value exports. Keep the separate export type { Section }; line that follows.
    • inflight.ts:179-180 — drop readFrontmatter, renderInflight (dashboard/server.ts:49 imports only classify). Delete the now-false back-compat justification at _probe_core.ts:14-16 — tests/inflight.test.ts:19 imports only the default export.
    • gap-report.ts:153 — delete the unreachable if (lc === "acceptance criteria") return "Acceptance criteria";; the loop at :150-152 already matches that string via the aliases entry at :100 and returns first.
  12. lib/services/project/cleanup/index.ts — remove export from the seven pipeline stages (discoverRepo:179, loadTasks:262, fetchPrs:335, gatherLocalBranches:394, gatherWorktrees:654, gatherRemoteBranches:945, gatherTaskLinkage:1122) and from the types TaskRecord and PRInfo. The only two importers (ops/cleanup.ts:24-30, tests/cleanup.test.ts:31-37) name none of them.

    • Keep exported: RepoInfo (transitively reachable via CleanupResult.repo at :73-76), CleanupItem, SummaryBucket, CleanupResult, GatherInput, Scope, ALL_SCOPES, and the four consumed functions. Note ops/cleanup.ts imports no types from this module — its zod schemas at :33+ are hand-written structural mirrors.
    • lib/services/project/ops/scan-todos.ts:213 — delete the trailing export { renderScanTodos }; (already wired via the cli.render descriptor at :152).
  13. lib/services/gate/ops/_skill_prose_contract.ts:128 — delete the slug field from SkillProseSpec and the 14 "slug": "..." lines that duplicate their own record key (:145, :179, :213, :299, :357, :585, :720, :878, :948, :982, :1094, :1306, :1340, :1521). Zero readers — the registry is only ever key-indexed. Do not add a key === spec.slug assertion test instead; that preserves the redundancy.

  14. lib/config/load.ts .describe() prose — four field descriptions name deleted Python scripts, and lib/config/scaffold.ts:104,143 renders them as YAML comments into every consuming project’s sdlc.yaml. Fix all four:

    • :63 quality_checks — apps/sdlc/scripts/run_quality_checks.py → sdlc quality run
    • :118 orchestrator — apps/sdlc/scripts/count_inflight_tasks.py → sdlc task inflight
    • worktree_init — replace the Python idiom subprocess.run(verb, shell=True, cwd=<worktree-root>) with the phrasing the sibling commit_worktree_init description already uses correctly (a shell verb run in a shell (\sh -c `, cwd = the worktree root)`)
    • :262 lease_authority — apps/sdlc/scripts/sdlc_lease.py → the sdlc lease * ops

Verification: bun test; bun run typecheck (noUnusedLocals is on, so orphaned imports fail the build); sdlc project setup in a scratch dir to eyeball the regenerated sdlc.yaml comments; sdlc entities audit and sdlc entities validate to confirm the entity plane is unaffected.


PR 3 — Add the shared lib/util primitives

Section titled “PR 3 — Add the shared lib/util primitives”

Branch: simplify/util-shared-primitives Effort: M · Risk: low · Depends on previous: no

Pure additions plus unit tests. No call sites change. This is the foundation for PRs 4–8; landing it separately keeps each adoption sweep a mechanical import swap.

  1. New lib/util/guards.ts — promote the existing private isMapping from lib/model/ops/validate.ts:298 (do not author a new one):

    export function isRecord(v: unknown): v is Record<string, unknown> {
    return v !== null && typeof v === "object" && !Array.isArray(v);
    }

    Leaf module, no imports, so no cycle or layering risk at any of the ~20 prospective sites.

  2. New lib/util/py.ts — the canonical Python-port shims:

    export function pyTypeName(value: unknown): string; // null|undefined -> "NoneType", array -> "list"
    export function pyRepr(value: unknown): string;

    Canonical semantics are "list" for arrays (7 of the 8 existing copies) and NoneType for both null and undefined. lib/model/ops/migrate.ts:171’s "tuple" is the sole outlier and belongs inline at its one legitimate call site (see PR 6). isPlainObject is not added here — isRecord from guards.ts covers it.

  3. lib/util/strings.ts — add escapeRegExp:

    export function escapeRegExp(s: string): string {
    return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
    }

    Six byte-identical copies exist today. detect-runners.ts already imports cmpStr from this module, so adoption adds no new edge there.

  4. lib/util/fs.ts — add the write and walk helpers:

    export function ensureDir(dir: string): void; // mkdirSync recursive
    export function writeFileEnsuringDir(path: string, contents: string): void;
    export function rglob(root: string, keep: (relPath: string) => boolean): string[];

    rglob must not filter on Dirent.isFile() — scan-candidates.ts pushes every non-directory entry and re-stats through symlinks at :328-329; filtering here would silently drop symlinked entries. Recurse on e.isDirectory(), push any other entry satisfying keep, sort the result.

  5. lib/util/site_table.ts — add the scalar sibling to stringList:

    export function stringField(value: unknown): string; // "" when absent/non-string/empty

    Signature must be (value: unknown), matching stringList(value: unknown) — not (e: EntityView, key: string). The value form is the only one that can also absorb the str() sites in services/docs/site/projections.ts:264-265.

  6. lib/util/frontmatter.ts — add the fence-preserving split/join pair:

    export function splitFrontmatterFences(text: string):
    { openFence: string; yaml: string; closeFence: string; body: string } | null;
    export function joinFrontmatterFences(
    parts: { openFence: string; closeFence: string; body: string }, newYaml: string): string;

    Purely lexical. It must not own parsing, mutation, error type, or the write decision — the three call sites (PR 7) differ in all four, and a mutate-callback engine cannot express applyTransformChain’s async body mutation.

  7. lib/util/git.ts — add the worktree path helpers beside WORKTREES_DIR_REL:

    export function worktreeDir(root: string, basename: string): string; // absolute
    export function worktreeDirRel(basename: string): string; // repo-relative

    Use resolve() (not join()) internally to preserve current behavior for callers passing a relative root. Both forms are genuinely needed: the relative form backs the preflight-worktree.ts:126 output contract (asserted at preflight-worktree.test.ts:147) and the git worktree remove call at teardown-worktree.ts:112.

Verification: new unit tests for each helper under lib/util/tests/. For stringField, assert parity with the four accessor bodies it replaces. For splitFrontmatterFences, assert byte-exact round-trip on a CRLF file, a file with a trailing-whitespace close fence, and a file with --- inside a body code fence.


PR 4 — Adopt @lib/util/fs predicates and escapeRegExp

Section titled “PR 4 — Adopt @lib/util/fs predicates and escapeRegExp”

Branch: simplify/adopt-fs-predicates-and-escape-regexp Effort: M · Risk: low · Depends on previous: yes (imports escapeRegExp added in PR 3)

lib/util/fs.ts’s header claims these predicates were consolidated from ~20 modules and that “this is their single home”. Verified: 14 production redeclarations survive.

  1. Delete every local isFile/isDir and import from @lib/util/fs:

    FileSymbol(s)
    lib/util/frontmatter.ts:157fileExists (sole use at :171)
    lib/model/entities/task/ops/resolve-touchpoints.ts:88,:96fileExists, dirExists
    lib/services/project/ops/scan-candidates.ts:103,:108isDir, isFile
    cli/backlog_cli/create.ts:526isDir (sole use at :540)
    skills/task-work/start_task.ts:663,:671isFile, isDir
    skills/task-work/append_pr_url.ts:113isFile
    skills/task-work/dedup_search.ts:156isDir
    skills/task-ensure-ready/ensure_ready_mutate.ts:605isFile
    skills/task-define/scan_validate_ac.ts:75isFile
    skills/migrate-runtime-state/migrate_runtime_state.ts:73,:81isDir, isFile

    Plus the anonymous inline guards at skills/task-work/preflight_permissions.ts:551-556, skills/task-work/summarize_dedup_telemetry.ts:281-286, skills/task-ensure-ready/scan_corpus_assumptions.ts:210-215 and :190-195, :408-414, :466-470, skills/task-work/start_task.ts:440-445, lib/model/identifier.ts:225-228 (its catch { continue; } is control-flow-equivalent to isFile returning false, since the next line is if (!isFile) continue;), lib/services/lease/ops/_common.ts:228-233, lib/services/lease/heartbeat-service.ts:140-145, lib/services/quality/baseline.ts:274-278.

    Shadowing trap: at preflight_permissions.ts:551, summarize_dedup_telemetry.ts:281, scan_corpus_assumptions.ts:210, and start_task.ts:440 the local variable is named isFile. Delete the local declaration outright and rewrite the guard as if (!isFile(path)) — do not rename around it.

    Import style: use the @lib/util/fs alias in lib/ and cli/. In skills/, match the file’s existing convention (most use relative ../../lib/util/*.ts; preflight_permissions.ts:62-63 already uses @lib/).

  2. Dead-catch removals (behavior-preserving, do these with the swap):

    • lib/model/entities/task/ops/resolve-touchpoints.ts:96-102 — dirExists’s catch does return existsSync(abs) && statSync(abs).isDirectory(), which can only fire because statSync threw; any throw reason makes existsSync false, so the && always short-circuits. Replace the whole function with isDir.
    • lib/model/entities/task/claims/paths.ts:132-140 — existsUnderRoot’s catch returns existsSync(abs), which is itself a stat-in-a-try. Reduce the body to return existsSync(abs) (keep the sep/join absolutization at :133) and drop statSync from the node:fs import at :31. Do not switch to isFile here — that is a behavior change (a directory named e.g. bar.ts would stop short-circuiting at :162), and it should be its own decision.
  3. Import narrowing (required — noUnusedLocals is on):

    • lib/util/frontmatter.ts:24 → import { readFileSync, writeFileSync } from "node:fs";
    • lib/model/entities/task/ops/resolve-touchpoints.ts:51 → import { readFileSync } from "node:fs";
    • cli/backlog_cli/create.ts:16 → drop statSync
    • lib/services/project/ops/scan-candidates.ts — keep statSync and the Stats type: statOf stays (two live callers at :249 and :328, the latter reusing the Stats object for st.size).
  4. Adopt escapeRegExp from @lib/util/strings at six sites, deleting the local copies: lib/services/quality/baseline.ts:44, lib/services/quality/detect-runners.ts:358 (named escapeRe), lib/util/markdown_extract.ts:187, lib/services/project/ops/scan-todos.ts:37 (an arrow const, not a function), skills/task-work/dedup_search.ts:129, skills/task-work/preflight_permissions.ts:198. Leave lib/services/project/ops/scan-candidates.ts:218 escapeRegexLiteral alone — it is a different, single-character function.

  5. Prune the now-stale entries from .claude/skills/project-check/util_redeclarations_baseline.json (the check treats stale entries as lenient, so this is cleanup, not a blocker).

Verification: bun test; bun run typecheck (catches every orphaned import); bun .claude/skills/project-check/check_util_redeclarations.ts to confirm the baseline shrinks rather than grows.


PR 5 — Adopt rfc3339, posix, and cmpStr at the remaining inline sites

Section titled “PR 5 — Adopt rfc3339, posix, and cmpStr at the remaining inline sites”

Branch: simplify/adopt-date-path-comparator-helpers Effort: M · Risk: low · Depends on previous: yes (file overlap with PR 4 in baseline.ts, scan-candidates.ts, detect-runners.ts)

Three more lib/util modules whose headers claim single-home status. All output is byte-identical after the swap — these are consistency changes, not fixes.

  1. rfc3339 / nowUtc from @lib/util/date (header at date.ts:5-10 states these exist “so the stamps can never drift between entry points”):

    • lib/services/quality/baseline.ts:180-183 isoUtcNow → rfc3339(nowUtc())
    • lib/services/pr/ops/classify.ts:460-462 nowIso → rfc3339(nowUtc()) (note its regex is /\.\d+Z$/, not /\.\d{3}Z$/ — same output either way)
    • lib/services/dashboard/server.ts:493-495 isoZ(dt: Date) → rfc3339(dt) at its two call sites (:702, :1324). This is a formatter over a supplied Date, not a now stamp — do not substitute nowUtc().
    • lib/services/orchestrator/ops/watch.ts:670-671 and lib/services/orchestrator/ops/log-tick.ts:294-295 — these are args.X ?? <idiom> fallbacks; replace only the right-hand operand, preserving the deterministic-test override.

    Trap: do not touch lib/services/project/cleanup/index.ts:1343 nowIso — it deliberately emits microsecond precision (pad(getUTCMilliseconds(),3) + "000"), and rfc3339 would drop the fractional part from generated_at at :1390. Likewise leave lib/services/lease/tree.ts:36-39 (git-log +0000 format) and lib/services/lease/runtime.ts:215-236 (a deliberately distinct regex-validating parser, documented as such).

  2. posix() from @lib/util/paths (docstring: “Consolidated from inlined copies”) at three sites, all relative(a, b).split(/[/\\]/).join("/"):

    • lib/services/docs/site.ts:190 → return posix(relative(contentRoot, absPath))
    • lib/services/docs/site/manifest.ts:387 → out.push(posix(relative(root, full)))
    • lib/services/docs/site/projections.ts:347 → return posix(relative(projectRoot, filePath))
  3. cmpStr from @lib/util/strings (docstring: “Consolidated from inlined copies”) at 12 sites of (a, b) => (a < b ? -1 : a > b ? 1 : 0), all on string[] and all collapsing to a bare .sort(cmpStr): lib/model/ops/audit.ts:166, :181, :246, :296, :537, :661, :741; lib/model/ops/migrate.ts:232; lib/model/corpus/graph.ts:106; lib/model/entities/capability/migrations/v1-to-v2.ts:169; lib/services/project/ops/scan-candidates.ts:124; and the two test sites lib/services/project/tests/scan.test.ts:137, :316.

  4. lib/services/dashboard/server.ts:368-375 — delete the private parseRfc3339 (a verbatim copy of lib/util/date.ts:76-80 plus a type-guard and NaN check) and import the shared one, keeping a thin unknown → Date | null wrapper. While editing, tighten date.ts:71-74’s carve-out sentence so it reads as “regex-guarded parsers whose accept/reject set differs”, not “any caller that returns null” — the current wording is loose enough to justify exactly this duplicate.

Verification: bun test; sdlc docs generate site --check (the posix swap touches route generation); sdlc quality baseline capture + diff round-trip to confirm the timestamp format is unchanged.


PR 6 — Consolidate the Python-port shims

Section titled “PR 6 — Consolidate the Python-port shims”

Branch: simplify/consolidate-python-port-shims Effort: M · Risk: low · Depends on previous: no (requires PR 3’s lib/util/py.ts)

pyTypeName has 8 copies in 3 divergent variants; isPlainObject has 8; pyRepr has 2. The divergence is real but currently harmless — migrate.ts:277,:341 gate on isPlainObject(fm) before invoking any transform, and undefined never reaches these paths (YAML parse yields null). The cost is maintenance, not wrong output.

  1. Adopt pyTypeName from @lib/util/py and delete the local copies at: lib/config/sdlc_yaml.ts:35, lib/model/ops/migrate.ts:169, lib/model/ops/validate.ts:278, lib/model/entities/capability/migrations/v1-to-v2.ts:60, lib/model/entities/task/migrations/v1-to-v2.ts:83, v2-to-v3.ts:65, v3-to-v4.ts:27, v4-to-v5.ts:29.

    • lib/model/ops/migrate.ts:209 keeps the literal word “tuple” inline in its message string (body-aware transform must return (dict, str); got ...) — that is the one call site where a Python-tuple-shaped return value is genuinely being described. The other three migrate.ts call sites (:217, :278, :342) render parsed YAML frontmatter, where "tuple" is already wrong today; the swap fixes them.
    • No test asserts the "tuple" or "is not a mapping" wording, so the message change is safe. The migration modules’ error-path tests assert the error type, not the string.
  2. Adopt isRecord from @lib/util/guards in place of the eight isPlainObject / isMapping copies: lib/model/ops/migrate.ts:186, lib/model/ops/validate.ts:298 (named isMapping), lib/model/entities/capability/migrations/v1-to-v2.ts:77, lib/model/entities/task/migrations/v1-to-v2.ts:102, v2-to-v3.ts:61, v3-to-v4.ts:44, v4-to-v5.ts:46, skills/task-work/preflight_permissions.ts:384.

    • Do not touch lib/util/yaml.ts:528 — its isPlainObject takes a YamlValue, narrows to {[k:string]: YamlValue}, and additionally excludes PlainScalar instances. Swapping it would treat wrapped scalars as mappings — a real bug in the emitter.
    • Do not convert lib/services/harness/ops/install-claude.ts:78 asObject — it returns Record<string, unknown> | null and is consumed as a cast-helper at ~8 sites; converting rewrites each into isRecord(x) ? x : null, a lateral move. Leave it, optionally re-implemented on top of isRecord.
  3. Adopt isRecord at the 13 inline guard sites (each currently followed by an unchecked as Record<string, unknown>): lib/util/frontmatter.ts:185, lib/model/read.ts:154, lib/model/entity.ts:468, lib/services/dashboard/server.ts:349, :1180, :1216, lib/services/orchestrator/ops/watch.ts:147, :153, :320, :368, :526, lib/services/pr/ops/classify.ts:150, :324, cli/_common.ts:52, cli/registry_adapter.ts:109.

    • Roughly 8 casts delete cleanly. The rest narrow to a different type and survive: watch.ts:147 casts to Record<string, string>, server.ts:1216 to Record<string, Record<string, unknown>>, watch.ts:368/526 index the narrowed value.
    • watch.ts:316-322 asObjList already carries a correct (e): e is Record<string, unknown> predicate — replacing it with value.filter(isRecord) is a readability win, not a cast removal.
  4. pyRepr — two byte-identical copies at lib/model/entities/task/migrations/v1-to-v2.ts:111 and lib/model/entities/capability/migrations/v1-to-v2.ts:81. Adopt from @lib/util/py. Do not fold in v2-to-v3.ts:569 pyReprStr — it is a narrower, different function (string-only, no None branch).

  5. Collapse v3-to-v4.ts and v4-to-v5.ts — after the shim extraction these two modules are identical apart from the stamped version literal ("4" vs "5"). Extract a stampOnlyMigration(version: string) factory in the migrations directory.

    • Each module must keep exporting Frontmatter — tests/v3_to_v4.test.ts:8-12 and tests/v4_to_v5.test.ts:8-12 import the type, not just migrate and MigrationError.
    • Preserve the transformWantsBody arity contract (lib/model/ops/migrate.ts:165-167).
  6. MigrationError — lib/model/entities/capability/migrations/errors.ts:10-15 and lib/model/entities/task/migrations/errors.ts:10-15 are identical 3-line classes. Folding them into one home is reasonable but low-value; keep the per-module export { MigrationError } re-exports intact either way.

Verification: bun test lib/model/entities/*/migrations/; sdlc entities audit; sdlc entities migrate --dry-run over the corpus to confirm zero body-output diff.


PR 7 — Converge the frontmatter read/write seams

Section titled “PR 7 — Converge the frontmatter read/write seams”

Branch: simplify/frontmatter-seam-convergence Effort: M · Risk: medium · Depends on previous: no (requires PR 3’s fence helpers)

Frontmatter currently has three write paths and four read paths. This PR collapses them. The writePrsFrontmatter half is the only medium-risk item in the stack and should be its own reviewable commit inside the PR — it reformats committed corpus files.

  1. lib/model/ops/_update.ts:93-97 — delete the local parseFrontmatter, which shadows lib/util/frontmatter.ts:119 while importing extractFrontmatter from that same module one line away (:60). Replace the call site at :146 with the seam’s result form:

    const { fm, parseError } = parseFrontmatterResult(text);
    if (parseError !== null || fm === null) {
    throw new OpError("INVALID_INPUT", `${parseError ?? "unparseable frontmatter"}: ${path}`);
    }
    • Do not use parseFrontmatter(text) ?? {} — util’s version collapses malformed and non-mapping input to null, so ?? {} would turn today’s hard throw into a silent empty-frontmatter write attempt.
    • This also closes a real edge: a scalar frontmatter (---\njust a string\n---) currently reaches fm[key] = value at :157 and throws an uncaught TypeError, escaping the OpError contract.
    • Drop the now-unused yamlParse import at :48 (closing a parse-seam bypass) and extractFrontmatter at :60. Keep FRONTMATTER_RE in that import — the match from :141 is load-bearing at :189-192 for the fence reconstruction.
    • Prune the grandfathered entry at .claude/skills/project-check/util_redeclarations_baseline.json:30.
  2. Adopt splitFrontmatterFences/joinFrontmatterFences (PR 3) at the three copy-pasted fence-preserving rewrites — identical down to the explanatory comment and the two derivation lines:

    • lib/model/ops/_update.ts:191-192, :195
    • lib/model/ops/migrate.ts:271-272, :294 (applyTransformChain)
    • lib/model/ops/migrate.ts:335-336, :352 (stampSchemaVersion)

    Critical: in applyTransformChain the body is not constant — body is reassigned inside the chain loop by [fmObj, body] = callTransform(...) at migrate.ts:285. The helper must take the body explicitly, never re-derive it from the original text, or every body reshape the transforms performed is silently discarded. Each site keeps its own parse, mapping guard, error type, and write decision (stampSchemaVersion keeps its newText === text short-circuit).

    • The cosmetic drift (_update.ts:194 appends .replace(/\n$/, ""), the migrate copies do not) is a no-op today because dumpFrontmatter ends in pyStrip — drop it.
  3. lib/util/frontmatter.ts:268 — route writePrsFrontmatter through dumpFrontmatter. This is the medium-risk item. It currently emits via the npm yaml package’s stringify while every other frontmatter writer uses dumpFrontmatter (lib/util/yaml.ts:605), the PyYAML-byte-compatible emitter. yaml.ts:5-11 explicitly documents that the npm package is not byte-compatible. Within the single task-work skill, start_task.ts:412 writes a task file via dumpFrontmatter and append_pr_url.ts:275 rewrites the same file via writePrsFrontmatter — the two steps of one workflow fight over the same bytes. 25 of 95 prs:-carrying task files are stranded in npm style against a 487/515 PyYAML-style corpus.

    Required decisions and steps:

    • Delete or correct the doc comment at :250-253, which asserts byte-parity with PyYAML that is false and contradicted by yaml.ts:5-11.
    • A coercion is required — dumpFrontmatter throws scalar.startsWith is not a function on numeric leaves. Promote the helper from skills/task-ensure-ready/ensure_ready_mutate.ts:57-65, but fix its null branch during promotion: it maps null/undefined → "", whereas dumpFrontmatter natively emits null (asserted by lib/util/tests/yaml.test.ts:14, parent_key: null).
    • Decide the block-scalar question before swapping. dumpFrontmatter has no literal-block-scalar support, so the swap rewrites the ~120 task files carrying completion_note: | into PyYAML single-quoted folded form with blank-line separators. That is round-trip-safe (verified) but a real readability regression. Either (a) add literal-style emission to the PyYAML emitter first, or (b) land the reformat as a deliberate one-time corpus sweep in its own commit, as was done for markdown in ff6dca33 under S-0007. Do not smear the churn across unrelated PR-append commits.
  4. Replace hand-rolled splitFrontmatter + yaml-parse + guard with the existing seam:

    • lib/services/dashboard/server.ts:344-356 — exact match for parseFrontmatter’s semantics on all five branches. Becomes const [, body] = splitFrontmatter(text); const frontmatter = parseFrontmatter(text) ?? {}; Then check whether parse as yamlParse at :35 is still used.
    • lib/services/project/cleanup/index.ts:286-295 — not a straight swap. This site also does if (fmRaw === null) continue, skipping files with no frontmatter. Keep that guard: const [fmRaw, body] = splitFrontmatter(text); if (fmRaw === null) continue; const fmObj = parseFrontmatter(text) ?? {};
    • skills/task-ensure-ready/ensure_ready_mutate.ts:66-79 — use parseFrontmatterResult, not parseFrontmatter. The current code has no mapping guard (an array frontmatter reaches Object.entries and yields index keys) but does surface the yaml library’s line/column diagnostic on malformed input. parseFrontmatterResult closes the guard gap while preserving the diagnostic; a bare parseFrontmatter + null-check would collapse three distinct failures into one misleading no frontmatter in <path> message. This also routes the file through the parse chokepoint, fixing the direct import { parse } from "yaml" at :21.
  5. lib/model/identifier.ts:259-267 — replace the private frontmatter regex (/^---[ \t]*\n([\s\S]*?)\n---/, which rejects CRLF where the canonical FRONTMATTER_RE accepts it) and the id: line scrape with readRawFrontmatter / parseFrontmatter.

    • This is not behavior-preserving as written. Today an unreadable file hits catch { continue; } and produces no violation; the read-layer primitive returns null, so the file would newly report id-missing. If the skip is intentional, keep an explicit existence guard.
    • Two hazards the finding claimed are false: the id: regex carries the m flag, so a # id: T-BOGUS comment line and an indented nested id: both correctly fail to match. Only the CRLF divergence is real, and it fails loudly.
    • Do not reach for scanEntityDir — it filters to *.md, which would drop the “entity file without .md extension” violation the walk exists to emit.
    • No test anywhere references findIdentifierViolations or the id-mismatch/id-missing kinds. Land this with a test.
    • Leave lib/model/entities/capability/migrations/v1-to-v2.ts:115-119 alone — all seven migration modules import zero @lib by convention, the transform is dorment (the corpus is entirely schema_version: "2"), and switching deriveChildren to parseFrontmatter would make one unparseable sibling silently drop a child and turn a clean migration into a spurious contains-inconsistency MigrationError.
  6. skills/task-work/preflight_permissions.ts:444-456 — delete taskBody and replace its one call site at :572 with const [, body] = splitFrontmatter(text);. The hand-roll accepts a bare leading --- with no newline and mis-slices a close fence carrying trailing content. Note one case goes the other way: "---\n---\nbody\n" (empty frontmatter) is stripped today and would not be by FRONTMATTER_RE — immaterial for real task files, but be aware it is a behavior change, not purely a tightening.

  7. lib/util/frontmatter.ts:124-279 — relocate the task-specific prs: block (PrsFieldError, readPrsFrontmatter, computeAppend, writePrsFrontmatter) out of lib/util. prs: is declared on exactly one entity schema (lib/model/entities/task/schema.ts:146), so this is task domain logic in util by construction, and readPrsFrontmatter:170-214 is precisely the hand-rolled readFileSync + regex + parseYaml + field-index pattern lib/CLAUDE.md forbids.

    • This supersedes T-JJZ3-lib-root-tidy.md:39-40,:48, which prescribed the merge on a “generic prs: helpers” premise the schema grep refutes; its cited authority (D-0007 §Migration step 2) actually routes generic PR helpers to util/gh.
    • lib/util/tests/frontmatter_prs.test.ts (34 references) moves with the code.
    • Preserve the exit-code contract deliberately: PrsFieldError encodes 2 = file-not-found, 1 = everything else, and both skill scripts translate it to their process exit code (append_pr_url.ts:251-255, verify_prs_against_pr.ts:94 — the latter is deleted in PR 1). readTask returns null for an absent file, so the distinction is preservable but must be kept on purpose.
    • Sequencing: land the move + readTask-based read first (pure win, no behavior change); land the applyFrontmatterUpdates write path second — that half adds schema gating, which is a real tightening for close-commit.ts and would newly fail on a schema-drifted task.

Verification: bun test; then the corpus check — run sdlc task update --set '{}' over a sample of task files and confirm the emitted bytes are stable; git diff --stat after a deliberate corpus reformat commit should touch only frontmatter blocks.


PR 8 — Converge task-ops path conventions and adopt the typed git client

Section titled “PR 8 — Converge task-ops path conventions and adopt the typed git client”

Branch: simplify/task-ops-paths-and-easygit Effort: M · Risk: low · Depends on previous: no (requires PR 3’s worktreeDir)

Two related convergences: the path conventions that _probe_core.ts and lib/util/git.ts already own, and the easyGit stragglers that pass the command-seam gate (because they do use spawnRunner) but violate the documented preference for the typed client.

  1. lib/model/entities/task/ops/inflight.ts — it already imports from ./_probe_core.ts (:33-37) yet re-spells all three conventions that module exports. Extend the import with taskBranch, taskFilePath, worktreePath and replace:

    • :123 const taskBranch = \task/${basename}`→const branch = taskBranch(basename)(rename the local; it currently shadows the helper's name — update its uses at:124, :136, :143`)
    • :126 resolve(projectRoot, "docs", "planning", "tasks", \${basename}.md`)→taskFilePath(…)`
    • :142 resolve(projectRoot, ".sdlc", "worktrees", basename) → worktreePath(...)
    • the typeof rawStatus === "string" dance at :128-132 → frontmatterString(...)

    probe-state.ts:149-155, dashboard/server.ts:541-544, and watch.ts:83,:489 all already use these; inflight is the lone outlier. Then drop the now-unused resolve import from node:path.

    • Converge lib/services/orchestrator/ops/watch.ts’s local taskStatus helper (~:487-493), which still inlines const s = fm["status"]; return typeof s === "string" ? s : null;.
  2. Collapse the duplicated worktree-path constant. lib/util/git.ts:13 WORKTREES_DIR_REL and lib/services/project/ops/_worktree-common.ts:23 WORKTREES_SUBDIR hold the identical literal .sdlc/worktrees, mutually unaware — and git.ts’s header already claims to be “the single home”.

    • Delete WORKTREES_SUBDIR; re-point _worktree-common.ts:26 worktreePathFor at worktreeDirRel (PR 3).
    • Replace the hand-joined derivations at lib/model/entities/task/ops/_probe_core.ts:44, lib/model/entities/task/ops/inflight.ts:142 (covered by item 1), skills/task-work/preflight_permissions.ts:671, skills/task-ensure-ready/ensure_ready_mutate.ts:429, and the display string at lib/services/gate/ops/worktree-scope.ts:115.
    • Rename skills/task-work/preflight_permissions.ts:666 worktreePathFor (a different signature — takes a task path, returns absolute-or-null) to e.g. worktreeDirForTaskFile, with its body calling worktreeDir(root, basename). Three functions currently share two names across two return conventions.
  3. Extract the duplicated globMatches and toAbs. lib/util/location.ts:111-123/:104-106 and lib/model/entities/task/ops/resolve-touchpoints.ts:124-139/:84-86 are identical statement-for-statement (the model copy carries a 3-line explanatory comment the util copy lacks). location.ts:109-110 even documents the copy rather than removing it.

    • Keep @lib/util/location as the home; import both from resolve-touchpoints.ts.
    • Drop globMatches from the export list at resolve-touchpoints.ts:294 — it has zero consumers repo-wide (covered by PR 2); do not re-export an import.
    • Delete the now-unused import { globSync } from "tinyglobby" at resolve-touchpoints.ts:54.
    • Coverage note: lib/util/tests/location.test.ts:153-160 covers glob match/no-match but has no case for a glob matching only a directory. That onlyFiles: false edge is pinned solely by resolve-touchpoints.test.ts:211-232, so the util copy is the less protected of the two. Add the directory case to the util test.
    • Do not fold in the grammar regexes or parseLocationCell — those are explicitly byte-pinned by the documented rationale at location.ts:6-8.
  4. easyGit adoption (lib/CLAUDE.md: “Git operations prefer the typed services/git/easy.ts client over hand-built argv”):

    • lib/services/lease/migration.ts:80-92 localBranchExists → new Git(projectRoot).branchExists(\task/${taskId}`). Delete RunResult:64-68, run():70-72, and the spawnRunnerimport at:29`.
    • lib/services/project/ops/scan-candidates.ts:252-269 gitCheckIgnoreFilter → Git.checkIgnore (easy.ts:230). Thread a Git into scanCandidates — the handler at :419 builds new Git(projectRoot, { runner: ctx.git }), matching _worktree-common.ts:32. This is the only git spawn in the project slice that cannot be driven by an injected runner. Delete the spawnRunner import at :27.
    • lib/services/plugin/ops/resolve.ts:38-68 — delete which() (which shells sh -c "command -v git" purely to locate the binary, and interpolates its argument into a shell string unescaped), gitToplevel, and gitOriginUrl. The directory-source path currently spawns four processes; new Git(pluginDir).topLevel() ?? pluginDir at :116 and new Git(repoRoot).remoteUrl("origin") at :117 take it to two with identical null-on-failure semantics. (Ordering note: PR 2 deletes Git.remoteUrl as unused — do this item or that one, not both. Recommended: keep remoteUrl and adopt it here; drop only diffNames in PR 2.)
    • lib/services/plugin/info-report.ts:34-42, :52-64, :82-85 — delete the module-level GIT, the git(args, cwd) helper (which passes cwd twice: ["-C", cwd, ...] and { cwd }), and commitExists. Route through new Git(dir): topLevel(), revParse("--short","HEAD"), isDirty(), objectExists(\${sha}^{commit}`). **Trap:** use revParse(“—abbrev-ref”,“HEAD”)for the branch read, **not**currentBranch()— the latter usesbranch —show-current, which returns ""on a detached HEAD where—abbrev-refreturns the literal”HEAD”, flipping gitSummaryoutput fromHEAD@shato(detached)@sha. Unmodeled verbs (describe —tags, rev-list —left-right —count, tag —list) move to g.raw(…)`.
    • lib/model/entities/task/ops/_lint_state_origin_core.ts:54-59 — thread a Git through the internal helpers instead of the module-level singleton (which makes this the one op that cannot be driven with a stubbed runner; its handler at lint-state-origin.ts:101 does not even accept ctx). Two traps: (a) listTaskBranches currently throws LintError when for-each-ref exits non-zero (:110-112) while Git#branches returns [] — a verbatim swap would silently degrade a git failure into “no task branches” and a green exit-0 pre-commit gate; use Git#raw here or keep an explicit guard. (b) logSubject returns null where the current code produces a trimmed string, so the violation-line template needs ?? "".
  5. lib/services/project/cleanup/index.ts:106-121 — delete makeRunFn, which names its injected runner _gitRunner and never reads it, constructing a fresh spawnRunner(binary) per call. All three uses are git calls with "git" hand-prepended (:503, :766, :1005). Thread the Git instance already built inside makeGitFn instead.

    • classifyRemoteBranch:978 currently receives only runRaw and gains a Git param rather than losing one. gitFn cannot substitute at the --is-ancestor sites (it collapses non-zero exits to ""), which is why the Git instance is the right vehicle: git.at(cwd).raw([...]).exitCode === 0.
    • :766 → git.at(path).statusPorcelain() (dirtyFiles already inits to [] at :755).
    • Delete the dead _runRaw param at discoverRepo:184 and its argument at :1438.
    • Narrow the :21 import to the CommandRunner type.
    • Framing: this is a DI-consistency cleanup, not a bug fix. No caller currently injects a non-spawn runner (the op and all seven tests pass ctx.git, which defaults to spawnRunner("git")), and the cleanup tests deliberately exercise a real git binary against tmpdir fixtures. Production behavior is unchanged.
  6. lib/services/project/cleanup/index.ts:263-295 — rebuild loadTasks on scanEntities("task", repo.root, { withText: true }). It hand-rolls scanEntityDir step for step, and is the only remaining splitFrontmatter + yamlParse entity walk under lib/services/project/. Three traps:

    • Scan the headline over the body, not row.text — keep const [, body] = splitFrontmatter(row.text!), or a # comment inside frontmatter can become the headline. The splitFrontmatter import stays.
    • statSync cannot be dropped from the imports — still used at :759 for the worktree on-disk check. Only readdirSync/readFileSync/yamlParse leave.
    • Frontmatter handling differs: loadTasks skips files with no frontmatter but keeps files whose YAML fails to parse (falling back to {}); scanEntityDir emits both as fm: null. A faithful port needs an explicit if (row.fm === null) continue;, which also drops parse-error files that are kept today. Latent (all 511 corpus files have frontmatter) but decide it deliberately.
  7. lib/services/dashboard/server.ts:314-362 readEntityDetail — keep the lazy two-branch structure (the readdir-first fast path avoids reading all 511 task files per detail request; do not replace it with scanEntities(..., {withText:true})). Two narrow fixes:

    • Replace the hand-rolled parse tail at :344-355 with parseFrontmatter(text) ?? {} (identical semantics), dropping ~12 lines and the parse as yamlParse import at :35.
    • Fix the README/index inconsistency: listGlobMd:161-169 does not skip README.md/index.md while scanEntityDir does (read.ts:262), so the basename branch can resolve a README the id branch never would. Apply the same skip.
  8. Promote the escape-aware table-cell split. splitTableRow (parse-touchpoints.ts:225-231) and the inline copy inside rowHasEmptyCell (scan-placeholders.ts:123-131) share the same "\x00PIPE\x00" sentinel; a repo-wide grep for PIPE returns exactly these two. Extract to an underscore-prefixed lib/model/entities/task/ops/_table_cells.ts (matching the directory’s _task_doc.ts / _probe_core.ts convention — the underscore keeps the registry’s discovery walk from treating it as an op). Do not swap to markdown-contract’s rawTableRow: the in-code note at parse-touchpoints.ts:260-264 correctly documents that the library’s splitRow is not escape-aware.

Verification: bun test lib/model/entities/task/ lib/services/project/ lib/services/plugin/; sdlc task inflight and sdlc task probe-state against a real worktree; sdlc project cleanup dry-run diffed against a pre-change run; sdlc plugin info output compared byte-for-byte.


PR 9 — Single-source entity schema patterns and the status-conditional helper

Section titled “PR 9 — Single-source entity schema patterns and the status-conditional helper”

Branch: simplify/entity-schema-pattern-constants Effort: M · Risk: low · Depends on previous: no

Two regex grammars and one superRefine shape are hand-typed across 11 entity schemas. The naming is the sharpest problem: RELATED_PATTERN currently denotes four different regexes across nine modules.

  1. Promote two wikilink constants into lib/model/entities/_common.ts beside the existing DATE_PATTERN:50 (already imported by capability, driver, product — the precedent is set):

    • ENTITY_WIKILINK_PATTERN = /^\[\[[A-Z]{1,3}-[0-9A-Z]{4}(\.\d+)?(-[a-z0-9]+(?:-[a-z0-9]+)*)?\]\]$/ — verified byte-identical at 5 sites under 3 names: task/schema.ts:59-60 (ENTITY_WIKILINK_PATTERN), milestone/schema.ts:26-27 (ENTITY_WIKILINK_PATTERN), capability/schema.ts:53-54 (RELATED_PATTERN), driver/schema.ts:25-26 (RELATED_PATTERN), backlog/schema.ts:32-33 (RESULT_BASE_PATTERN).

    • DOC_WIKILINK_PATTERN = /^\[\[[A-Za-z0-9][A-Za-z0-9._/-]*\]\]$/ — verified byte-identical at 5 sites, all named RELATED_PATTERN: decision/schema.ts:25, milestone/schema.ts:30, reference/schema.ts:18, standard/schema.ts:25, term/schema.ts:20.

    Import directly and delete the local aliases — do not re-export under the local name, which would deepen the RELATED_PATTERN collision. Verified safe: nothing outside these schema.ts files imports any of the three names (schema_patterns.ts is deleted in PR 1).

  2. Rename the two surviving RELATED_PATTERN variants so the name means one thing:

    • principle/schema.ts:28-29 → PRINCIPLE_RELATED_PATTERN (P-/S- ids only)
    • product/schema.ts:21 → PRODUCT_RELATED_PATTERN (/^\[\\+\]\]$/, any non-pipe target)

    Do not collapse these into the shared constants — they are deliberate per-type policies and merging them would widen validation. _common.ts’s header documents that per-type schemas specialize related.items, and that design is live.

  3. Add a per-prefix factory to _common.ts for the 8 single-prefix wikilinks and the 11 id regexes, which are the same template varying only by prefix:

    export function entityIdPattern(prefix: string, opts?: { subId?: boolean }): RegExp;
    export function entityWikilinkPattern(prefix: string | string[], opts?: { subId?: boolean }): RegExp;

    Sites: id regexes at

    reference:26, decision:33, driver:34, standard:33, product:29, principle:37, milestone:38, task:74, term:28, backlog:56, capability:62

    wikilinks at

    decision:22, capability:50, driver:22, standard:22, backlog:37, backlog:39 (milestone variant with subId), backlog:41, backlog:43

    Hard constraint: the factories must emit .source byte-for-byte identical to today’s literals. lib/services/docs/data_model.ts:99-113 derives each type’s prefix by string-matching the pattern source (/^\^([A-Z]{1,3})-/) and detects sub-ids via idPattern.includes("(\\.\\d+)?") through the zodToJsonSchema projection. Add a .source-equality test covering all 13 promoted constants.

    • Build the factories from new exported fragments in identifier.ts (ID_CHARS_SRC = "[0-9A-Z]{4}", SUB_ID_SRC = "(\\.\\d+)?"). ID_RE:56 and FILENAME_RE:58 cannot be reused directly — both are fully anchored with a generic [A-Z]{1,3} prefix and an always-optional .N.
    • SLUG_RE (lib/util/slug.ts:30) is anchored at both ends while the wikilink tail is the optional hyphen-prefixed group; the bodies coincide but it is not substitutable. Leave it.
  4. Swap the inlined ISO-date regex for DATE_PATTERN at milestone/schema.ts:72, task/schema.ts:184, and lib/model/entities/task/ops/close-commit.ts:56. All three inline /^\d{4}-\d{2}-\d{2}$/ — literally DATE_PATTERN’s value — while driver, capability, and product import the constant for the same job.

    • Do not touch lib/services/report/schema.ts:42 — that is a cross-tier import (lib/services/report reaching into lib/model/entities), forbidden by S-0008.
    • skills/task-work/start_task.ts:75 DATE_RE may stay (standalone skill glue, S-0001).
  5. Add a requiredWhen helper to _common.ts and collapse the 9 hand-written status-conditional superRefine arms:

    export function requiredWhen(ctx: z.RefinementCtx, when: boolean, field: string): void;
    // emits { code: z.ZodIssueCode.custom, path: [field], message: `'${field}' is a required property` }

    Sites: standard/schema.ts:103-118 and decision/schema.ts:88-105 (byte-identical after comment strip — both encode closed/superseded ⇒ superseded_by and closed/deprecated ⇒ deprecation_note; share one exported SUPERSESSION_RULES constant); milestone/schema.ts:105-121 (two arms); task/schema.ts:214-227 (a 10-line block); backlog/schema.ts:96-104 (nested under a promoted/ guard) and :145-152.

    Constraints:

    • The predicate stays a caller-computed boolean — that is what makes this behavior-preserving across ===, startsWith("closed/"), and startsWith("promoted/").
    • Message strings must stay byte-identical — asserted at lib/model/tests/entity.test.ts:204,:220 and cli/backlog_cli/tests/create.test.ts:122 (an AJV-compat contract).
    • Do not add a forbiddenWhen sibling — it would have exactly one call site (backlog/schema.ts:154-162) with a fully custom message, absorbing only two lines. Leave that rule inline.
    • Preserve each site’s existing // allOf if/then: comment — those document JSON-Schema provenance, not code shape.
    • A generic helper indexing fm[field] loses the narrowed z.enum literal typing on status, so a Record<string, unknown> access cast is needed at the seam. Acceptable, but it is a real trade.
  6. Export RESULT_PATTERN_BY_STATUS from backlog/schema.ts and collapse the 4-branch if/else-if chain at :108-143 (each branch repeating the same 6-line ctx.addIssue body with only the constant swapped) to:

    const want = RESULT_PATTERN_BY_STATUS[status ?? ""];
    if (want && !want.test(result)) ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["result"],
    message: `'${result}' does not match '${want.source}'` });

    (The second consumer of this map, lib/util/schema_patterns.ts:194-199, is deleted in PR 1, so this is now a pure intra-file simplification.)

  7. Stale headers — six schema files (backlog, decision, driver, milestone, reference, standard) still carry “The JSON stays on disk this slice”; the on-disk schema.json / _common.json files no longer exist. Correct them while editing.

Verification: bun test lib/model/; sdlc entities validate over the full corpus; diff the zodToJsonSchema projection before/after (it reads .source, so byte-identity is the gate); sdlc docs generate data-model and confirm zero diff.


PR 10 — Dedupe the lease runtime and op layer

Section titled “PR 10 — Dedupe the lease runtime and op layer”

Branch: simplify/lease-runtime-dedup Effort: M · Risk: low · Depends on previous: no

lib/services/lease/runtime.ts (770 lines) repeats four block shapes across its six public entrypoints, and the op layer redeclares vocabulary the schema module owns. One item is a genuine exit-code bug.

  1. Extract the entrypoint preamble. A byte-identical 9-line block (const projectRootOpts through the close of resolveAuthority({...})) opens all six entrypoints: fetchLease:256, acquireLease:350, transitionLease:425, heartbeatLease:517, reacquireLease:575, releaseToArchive:682 (~54 of 770 lines).

    • A private leaseTarget(taskId, opts) returning { projectRoot, auth, ref, buildCwd, cwdOpts } covers all six. The tails legitimately vary and must not be flattened: fetchLease derives ref but no buildCwd; releaseToArchive names it activeRef and adds archiveRef; acquireLease and reacquireLease derive me = opts.hostId ?? currentHostId(). Callers derive those in one extra local line each.
    • Optional: make projectRoot lazy. Today findProjectRoot runs unconditionally even when opts.authority is supplied, so transitionLease/heartbeatLease/reacquireLease each pay a filesystem walk for a resolveAuthority call that short-circuits.
    • Delete the parse as parsePath import at :29 and its void parsePath; suppressor at :770 if still present — verified absent in the current tree; skip if so.
  2. Extract the expiry computation — the Math.max(Math.floor(ttlSeconds), 1) clamp plus rfc3339 formatting appears at runtime.ts:308-312, :455-461, :536-542, :610-616 and migration.ts:192-193.

    • Add expiresAt(now: Date, ttlSeconds: number): string to lib/util/date.ts, which already declares itself the single home for lease timestamp stamps and which both files already import rfc3339 from.
    • Drop the second-truncation (new Date(Math.floor(now.getTime()/1000)*1000)) as the no-op it is: rfc3339 formats at second precision from UTC calendar fields, so truncated and untruncated bases provably yield identical strings (verified over 200k randomized inputs). migration.ts is already the correct form — it is the four runtime sites that change to match it, not the reverse.
    • The clamp is load-bearing: it prevents a zero/negative TTL minting an already-expired lease.
  3. Extract the CAS-conflict recovery at runtime.ts:483-497, :626-640, :748-765. The current shape is actively misleading: the throw new LeaseConflict(...) sits inside the inner try, so the conflict it just constructed is caught by its own catch (e2) and only escapes via the trailing throw e2.

    • One private helper suffices (not the two-function design):

      function freshOwnerOrNull(auth: string, ref: string, casOpts: { cwd?: string }): string | null {
      try { return validateDict(TaskLifecycleLeaseSchema, fetchRef(auth, ref, casOpts).payload).owner; }
      catch (e) { if (e instanceof RefNotFound) return null; throw e; }
      }

      transitionLease/reacquireLease become throw new LeaseConflict(ref, freshOwnerOrNull(...) ?? "unknown"); releaseToArchive branches on owner === null to emit its RELEASE-NOOP line and return null.

    • Scoping guard: acquireLease’s CAS-CREATE recovery at :379-398 looks similar but must not be folded in — it compares existing.owner === me and returns the existing lease on a self-owned ref, and throws a distinct LeaseError (not LeaseConflict) on RefNotFound.

  4. Fix the missing LeaseConflict arm — this is a real exit-code bug. leaseExceptionToOpError (ops/_common.ts:135-180) has no arm for the library-side LeaseConflict, so four op handlers each hand-write the same three-line catch, and a fifth — ops/release.ts:46-51 — omits it. releaseToArchive throws LeaseConflict (runtime.ts:755), which falls through to the generic LeaseError arm at _common.ts:174, so a cross-owner conflict on sdlc lease release exits 1 with a generic error: instead of the LEASE-CONFLICT marker at exit 4, contradicting cli/README.md:138.

    • Add the LeaseConflict arm before the LeaseError arm at _common.ts:174, emitting

      markerError("LEASE_CONFLICT", formatMarker(MARKERS.LEASE_CONFLICT, { ref: exc.ref, reason: `owner=${exc.owner}` }))

      Import LeaseConflict from ../index.ts alongside the existing imports at :39-46.

    • The four hand-written catches (task/acquire.ts:70, task/archive.ts:48, task/reacquire.ts:76, task/sweep.ts:172) then collapse to catch (e) { if (e instanceof LeaseError) throw leaseExceptionToOpError(e); throw e; } with byte-identical output.

    • Cover the release.ts exit-code change with a test — it shifts from GENERIC/1 to LEASE-CONFLICT/4. That is the fix, but it is a behavior change.

    • Keep CliLeaseConflict for transition.ts:332, which synthesizes a conflict from a CASFailed.

  5. Single-source the phase sets. new Set(["claimed","working"]) and new Set(["awaiting-review","blocked"]) are declared at both runtime.ts:408-409 and ops/task/transition.ts:66-67, and both copies drive the same two protocol decisions (rotate lease_token; null out expires_at). These are ADR rules, not local conveniences — adding a phase to one copy silently gives sdlc lease task transition and the library transitionLease() different semantics.

    • Export both from runtime.ts, surface them in the lease index barrel, delete transition.ts:66-67.
    • Constants only. Do not merge the two transition paths — the op gates on a 5-value TRANSITION_PHASES enum (:58-64), enforces the awaiting-review handoff-args gate (:233-244), and maps exceptions to exit-code markers. Only the Set declarations are equivalent.
    • Note for the implementer: T-KPVN intends to replace this phase-keyed rule with the ADR’s owner-keyed rotation matrix and its Today table references only runtime.ts — so the duplicate is a live trap for that task.
  6. Stop redeclaring payload vocabulary at the op layer:

    • OWNER_KINDS is written out identically at ops/task/claim.ts:39, ops/task/acquire.ts:24, ops/op/claim.ts:37 while schemas.ts:47-53 already declares the authoritative OWNER_KIND zod enum every payload validates against — it is simply not exported. Export it and delete the three copies.
    • ops/op/claim.ts:38 OP_STATES duplicates the authoritative OPERATION_STATE enum at schemas.ts:65. Same fix — but note it is also interpolated into a .describe() string at :66 via OP_STATES.join(", "), which becomes OPERATION_STATE.options.join(", ").
    • ops/_common.ts:58 SDLC_VERSION_PAYLOAD = "1.0.0" restates version.ts:14 SDLC_VERSION, whose own header says “Every consumer reads from this module rather than redefining a local constant” — and _common.ts already imports from ../index.ts, which re-exports it at index.ts:115.

Verification: bun test lib/services/lease/; exercise the exit-code contract explicitly — two concurrent sdlc lease release invocations on the same ref should now yield exit 4 with the LEASE-CONFLICT marker; sdlc lease list / inspect output unchanged.


PR 11 — Replace the docs-site YAML mini-parser and extract site/reference.ts

Section titled “PR 11 — Replace the docs-site YAML mini-parser and extract site/reference.ts”

Branch: simplify/docs-site-parser-and-reference-extraction Effort: M · Risk: medium · Depends on previous: no

lib/services/docs/site.ts is 1,202 lines — the only module in the slice over 525 — because the reference/ family never got its own module, and that is where a broken hand-rolled YAML parser has been hiding. This PR changes generated output on ~39 pages and must be reviewed as an intentional output change, not a refactor.

  1. Delete the hand-rolled parseFrontmatter at site.ts:238-317 (~75 lines) and use parseMarkdownFile / stripFrontmatter, already imported at site.ts:41 and used at :824. The header’s “verbatim port from regen.mjs — no third-party dep” rationale is stale: yaml is a direct dependency and markdown-contract is the house markdown front-end.

    Three live output bugs this fixes (verified against the real corpus):

    • Truncation. The parser ignores plain-scalar continuation lines. skills/api-enhancement-scan/SKILL.md:3 and skills/principle-review/SKILL.md:3 use unquoted multi-line description: values, so only the first physical line survives — api-enhancement-scan’s rendered description is 71 chars instead of 729, principle-review’s 65 instead of 354. Both ship today in the ## Description block and the Starlight description: frontmatter.
    • Folded scalars. > is treated as literal (:280 joins with \n instead of folding to spaces), so skills/explore-codebase/SKILL.md:2 yields embedded newlines; site.ts:362 then does description.split("\n")[0] and truncates it to ~85 chars.
    • Dead H1 strip. body = text.slice(end + 5) keeps the blank line after the closing fence, so all 39 skill bodies begin with \n. The H1-strip regex at :353 is /^#\s+\/sdlc:.../ with no m flag, so it never matches — every generated skill page duplicates its title under ## Source, exactly what the comment at :350-352 claims to prevent. FRONTMATTER_RE consumes the trailing whitespace, so the swap makes it fire.

    Also handle: extractFrontmatter throws on malformed YAML and on non-mapping frontmatter (markdown_extract.ts:78-88) where the old parser silently returned {}. None of the 36 current files throw, but the generator gains a hard-fail mode on a malformed third-party SKILL.md — catch and report per-skill rather than aborting the whole build.

    Coverage gap: generate_site.test.ts creates an empty skills dir at each fixture, so nothing in the suite covers this path. Verify with sdlc docs generate site --check, and add a fixture with a multi-line description:.

  2. Extract lib/services/docs/site/reference.ts. Every other generated family already has its own module under site/ (nav.ts, roadmap.ts, projections.ts + projections_render.ts, supplemental.ts, manifest.ts, routes.ts) with site.ts orchestrating. The reference family is the sole exception. Move: OpEntry:198-203, loadRegistryOps:205-232 (keep exported — computePageInventory:1150 still calls it), regenSkills:323-392, codeFmtWikilinks/entityOverview/ overviewToDescription:403-444, regenEntities:446-546, regenOps:552-598, writeRefIndex:604-609.

    • The module imports the fs primitives and type SitePage — a type-only edge back into site.ts, exactly the pattern site/supplemental.ts:35 already uses. Do not claim “no edge back into site.ts”.
    • site.ts keeps the five orchestration calls at :995-999 and drops to ~800 lines.
    • cleanLegacyScripts:862-881 is directory cleanup, not page rendering — moving it is optional and it may read better alongside the other sweep logic.
  3. Collapse the three inlined escapeFmString copies at site.ts:363, :443, :570 — all .replace(/"/g, '\\"').slice(0, 200), duplicating the exported helper at site/projections.ts:351 (already used by projections_render.ts:70-71), from a module site.ts already imports five symbols from at :68-75. All three land in the new reference.ts, where one local import collapses them.

    • Do not fold in skills/migrate-runtime-state/migrate_runtime_state.ts:105 — it escapes backslashes first and omits the 200-char cap; different semantics.
    • Note (pre-existing, now fixable in one place): none of the copies escape backslashes, so a description containing one can emit an invalid double-quoted YAML scalar.
  4. Unify the two ## Purpose extractors. entityOverview (site.ts:417-427) scans lines for /^##\s+Purpose\s*$/; data_model.ts:64-78 runs a different regex over the same files. @lib/util/markdown_extract#sectionBody already does this via markdown-contract’s section projection.

    • Outputs are byte-identical across all 11 shipped definition.md files today — this forecloses drift, it does not fix a live bug.
    • Keep the existence guards. parseMarkdownFile throws on a missing file where both current functions return "". In site.ts the existsSync(defPath) guard survives naturally; in data_model.ts the try/catch at :66-70 must be widened to wrap the parseMarkdownFile call, not deleted. data_model.ts’s first-paragraph/first-sentence narrowing at :73-77 is genuinely distinct logic and stays.
  5. Extract writeIfChanged to a leaf module. It is implemented twice with equivalent behavior — site.ts:142-161 and site/supplemental.ts:116-134 — and it encodes the --check drift-gate write/no-write contract (D-0010 §6), so a change applied to one copy makes --check report differently for mirrored supplemental pages than for generated ones.

    • Put it in a new lib/services/docs/site/write.ts that both depend on downward. site.ts:66 imports mirrorSupplemental from supplemental.ts and supplemental.ts:35 imports back from ../site.ts — the modules are already mutually dependent, and a direct import would deepen that cycle. A leaf module is the third option the :112-113 comment never considered.
    • Update or delete that comment as part of the move. Note: it is not stale as the finding claimed — “no import edge … beyond the banner constant + the SitePage type” accurately describes the current state.
    • Do not move deleteIfExists, ensureDir, listMdFiles, or listMdFilesRecursive — each has exactly one caller and no duplicate under site/.
  6. Collapse the duplicated recursive .md walk. rglobMd (lib/model/ops/audit.ts:147-167) and listMdFilesRecursive (site.ts:1048-1060) are the same function — recursive descent, .md leaves, absolute paths, sorted. Move to lib/util/fs.ts as listMdFilesRecursive(root: string): string[], updating audit.ts:293 (already isDir-guarded) and site.ts:1085.

    • Decide deliberately whether the shared version tolerates or propagates a nested readdir failure — site.ts currently propagates and its sweep is an integrity gate; propagating, with audit.ts’s isDir guard covering the missing-root case, is the safer default.
    • Leave the other three walks alone: listPluginFiles (_install.ts:133) excludes symlinks and emits root-relative POSIX paths in per-directory order; rglobFiles (scan-candidates.ts:113) emits every non-directory entry so its caller can statSync-filter through symlinks; indexBasenames (claims/paths.ts:105) is an unsorted stack walk returning a Map.
  7. adrSection dedup — lib/services/docs/generate.ts:332-337 re-inlines splitSection’s bucketing branch (:193-205) from 130 lines above it, because it needs to prepend folder-ADR rows. principleSection:446-461 is the in-file precedent for the “prep, then delegate” shape. Low value; fold in only if the diff stays small.

Verification: sdlc docs generate site --check (the load-bearing gate — the unit tests do not cover the skills path); bun run build:doc-site; visually diff the generated reference/skills/* pages before and after and confirm (a) the three truncated descriptions are now complete, (b) the duplicated H1 under ## Source is gone on all ~39 pages. Expect a diff on essentially every page under reference/.


Finding id(s)Reason for dropping
xc-dead-void-silencer-residueNOT dropped — this row is retracted. Synthesis marked it stale after reading a worktree that audit sub-agents had already mutated (several had write access and applied fixes rather than only reporting). Those edits were reverted; all five void X; silencers plus tcpProbe and stopDashboard are confirmed present against a6aaf6dc. See the corrected verification note at the top. The finding stands in full and is scheduled in PR 2.
dashboard-two-command-seamsL effort / medium risk for what reduces to a comment correction; asyncifying easyGit means a colored duplicate of the whole class — moves complexity rather than removing it.
project-cleanup-classify-ladder-duplicationThe remote-branch half has no golden assertions in cleanup.test.ts, so the refactor would be unverified on exactly the code being folded in. Needs test scaffolding first.
project-cleanup-hand-parsed-git-gh-shapesThe worktrees() swap needs a bare field added to easyGit’s Worktree first (cleanup surfaces bare in Zod-validated JSON); the gh third of the finding was refuted (hasGh/ghAuthenticated are two independently-consumed booleans that Gh.ready() cannot reproduce).
project-cleanup-repeated-literalsThe prefix-array hoist is safe but tiny; the facts-shape unification changes emitted JSON with zero test coverage in either direction. File separately with a deliberate include-headline/title decision.
docs-dup-page-inventory-driftedcomputePageInventory’s sole consumer uses it only as pages.length === 0; deleting it is right but tangles with --dry-run semantics (rerouting through the check pass would newly throw on namespace conflicts). Needs its own decision.
xc-errors-unknown-to-messageA 52-site mechanical sweep for one genuinely reachable TypeError (lease/ops/reconcile.ts:197, which formats (e as Error).constructor.name on arbitrary detector throws). Fix that one site opportunistically; the sweep is churn.
xc-errors-argparse-conventionsSeven sites across three conventions spanning the service and skill tiers, with two defineService sites deliberately returning 1 and a detached child returning 2. Needs an exit-code design decision, not a mechanical sweep.
gate-dup-registry-forbid-boilerplateDeleting the 56 redundant forbids is defensible (skill-md/legacy-script-path is a strict superset and already runs repo-wide via lefthook.yml:122) but retires 56 stable skill-prose/<slug>/f{N} finding ids. Product decision, not a cleanup.
gate-stale-regenerate-instruction, quality-stale-deliberation-commentComment-only corrections with no code change. Worth doing, but not worth a review cycle each — fold opportunistically.
misc-services-verb-list-op-boilerplateTwo independent 2-way pairs needing two separate factories (the get pair maps to SCHEMA_ERROR, the set pair re-throws OpError); net saving does not justify the abstraction.
cli-dup-option-build-readback-loopsThe claimed drift is unreachable — unwrapField sets isArray only when base === "ZodArray", so ZodBoolean and isArray are mutually exclusive. Medium risk for no live gain.
vcs-type-verdict-enumThe desktop half is refuted (generated/types.ts is ontogen-generated from the Rust crate, not the zod schemas, and has no @lib path). The TS-only half is a 3-line z.enum change — fold opportunistically.
entities-other-dead-product-key-orderDeleting HISTORICAL_KEY_ORDER.product removes an output-stability pin for zero gain; the real defect is the comment at _registry.ts:129-136 claiming three types need bespoke order when only two do. Comment fix only.
model-core-findcycles-overclaims-elementaryA contract/doc correction plus a characterization test. Real (the DFS emits one cycle per back edge, not all elementary cycles, so entities audit under-reports) but self-healing across re-runs. File separately.
task-rest-dup-fence-scanner, task-ops-dup-fence-walkThe v2-to-v3.ts copies are in a frozen migration whose header contracts byte-identical body output, and the fence branch has no test fixture. Extracting a stateful parser there is risk without a net.
task-ops-next-compare-dup, task-rest-dup-table-renderersSub-10-line intra-file dedups in one module each; below the threshold that justifies a review cycle. (The IMPACT_MISSING half of the former was refuted outright — the sign convention is internally coherent.)
scripts-skills-dup-run-wrapper, xc-errors-skill-run-helper-dup, xc-errors-returncode-shimThe two surviving run() wrappers differ materially in stdin handling (["ignore","inherit","inherit"] vs "inherit") and spawn-error fallback, and four call sites explicitly document relying on the thrown message to surface captured stdout — which CommandFailed drops. Needs a deliberate error-formatting decision.
scripts-skills-dup-sdlc-cli-pathReal (one variant is percent-encoding-buggy) but the proposed pluginRootDir() fix is wrong — build-binary.ts:124-150 deliberately excludes cli/**, so a compiled binary has no cli/sdlc.ts. Needs a module-relative helper and a D-0014 review.
scripts-skills-dup-pyjson-stringThe two ensure_ascii encoders are byte-identical (verified by exhaustive BMP sweep) and worth sharing, but neither has any test coverage today. Land with tests, separately.
scripts-skills-bootstrap-touch-shelloutA correct 2-line fix (spawnSync("touch") → writeFileSync) in scripts/, which is outside the audit’s scope and outside the command-seam gate’s scan roots. Fold opportunistically.
xc-orphan-conventions-dead-script-pathsEight prose sites across conventions/ and entity definition.md files. Real and worth fixing, but it is a docs sweep plus a guard-regex widening (LEGACY_DIRS_RE only matches the ${CLAUDE_PLUGIN_ROOT}-prefixed form), not a code simplification.
xc-fs-mkdir-write-pair, xc-fs-atomic-json-write, xc-fs-pr-cursors-dirThe writeFileEnsuringDir helper is added in PR 3; adoption is worth doing but four of the cited “write-pair” sites are not (atomic .tmp+rename, copyFileSync, appendFileSync, read-modify-write) and fusing them would change behavior. Adopt opportunistically rather than as a sweep.
gate-dup-staged-paths, gate-dup-citation-render, gate-shared-rglob-walkerSmall gate-layer dedups; the stagedPaths one is best solved by adding diffFilter? to Git.diffNames — which PR 2 deletes as unused. Revisit after the easyGit package extraction (PR #957) settles.
dashboard-* (arg parser, json response helper, pidfile record, entity detail)The dashboard is the highest-churn service and server.ts is 1,695 lines; these are all genuine but belong in a dedicated dashboard pass after the stack lands, not interleaved with it. The readEntityDetail parse fix is folded into PR 8.
xc-output-dup-column-paddingOnly two of four cited sites convert cleanly; scan-todos.ts right-aligns with padStart(4) and renderTable has no alignment concept. The -Infinity hazard was refuted (padEnd(-Infinity) does not throw and the width is never read on an empty array).
misc-services-* (plugin registry dup, plugin-root full report, claude settings dup, kind registry)A coherent lib/services/plugin + harness cleanup, but orthogonal to every theme in this stack. The resolvePluginRoot() → full buildReport() item (10–15 discarded subprocesses on a dev checkout, paid twice on the default harness install claude path) is the most valuable and should lead that separate pass.
registry-config-* (discovery register dup, withDerived, isModuleFile)discoverOps’s three repeated shapes are real, but the two reason strings deliberately mirror defineOp/defineService’s own messages and one is asserted by registry.test.ts:273 — the extraction must parameterize, not unify. Low risk, low value; separate pass.
cli-dup-backlog-argv-parsersThe valuable half is not the parser dedup but the drift it exposed: BACKLOG_STATUS_CHOICES omits promoted/decision, which backlog/schema.ts:63-70 accepts — so sdlc backlog create --status promoted/decision fails at parse time on a schema-valid value. Fix that bug directly; skip the loop extraction.
cli-dup-json-sort-replacer + dupes, cli-dup-ctx-resolution, cli-util-advanced-flag-predicateThree small cli/ dedups totalling ~40 lines. Worth one combined PR eventually; below the bar interleaved with this stack.

1. delete-orphaned-modules (deletions, ~900 lines)
2. dead-exports-and-stale-headers (deletions, ~250 lines)
3. util-shared-primitives (additions + tests) ← foundation
4. adopt-fs-predicates-and-escape-regexp [needs 3]
5. adopt-date-path-comparator-helpers [file overlap w/ 4]
6. consolidate-python-port-shims [needs 3]
7. frontmatter-seam-convergence [needs 3] ← medium risk (corpus reformat)
8. task-ops-paths-and-easygit [needs 3]
9. entity-schema-pattern-constants
10. lease-runtime-dedup ← contains one exit-code fix
11. docs-site-parser-and-reference-extraction ← medium risk (output change)

PRs 1–2 are pure deletions and can merge immediately. PR 3 is additive and unblocks 4–8. PRs 9–11 are independent of the util foundation and can be worked in parallel with 4–8 if capacity allows. PRs 7 and 11 are the only two that change observable output and should get the most review attention.


← Back to Notes