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
Headline assessment
Section titled “Headline assessment”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:
| Symbol | Location (verified) |
|---|---|
void existsSync | lib/model/entities/task/ops/inflight.ts:116 |
void parsePath | lib/services/lease/runtime.ts:770 |
void tcpProbe | lib/services/dashboard/server.ts:1478 |
void inline | skills/task-work/dedup_search.ts:484 |
void topScore | skills/task-work/dedup_search.ts:632 |
tcpProbe | lib/services/dashboard/server.ts:1359 |
stopDashboard | lib/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.
Recurring themes
Section titled “Recurring themes”- 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.
- “Single home” headers that are false.
lib/util/fs.ts,strings.ts,date.ts,paths.ts,yaml.ts, andtable.tseach declare themselves canonical for a helper that is re-declared elsewhere 6–20 times. The consolidation landed; the sweep did not. - Python-port shims never retired.
pyTypeName(8 copies, 3 divergent variants),isPlainObject(8),pyRepr(2), and the{returncode, stdout, stderr}re-shim of the canonicalCommandResult(6 modules) all survive the TS port. - Regex grammar copied instead of shared. Two wikilink regexes appear 13 times across
11 entity schemas under four different names;
RELATED_PATTERNdenotes four different regexes depending on which schema you read. - Gates police spawns, not reuse.
scripts/check_command_seam.shchecks spawn shape only. Every easyGit straggler, hand-rolled entity read, and duplicated fs predicate passes it cleanly — which is precisely why these classes accumulated. - 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.
- 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 generatedsdlc.yaml.
PR 1 — Delete orphaned modules
Section titled “PR 1 — Delete orphaned modules”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.
Work items
Section titled “Work items”-
Delete
lib/services/quality/run-checks.ts(340 lines). Header namesapps/sdlc/scripts/run_quality_checks.tsas its only consumer; that shim was deleted by 0ee07a6f. Verified zero importers. The live replacement islib/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/configloadConfig— 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)nearops/run.ts:38. - Update the stale touchpoint rows naming
run-checks.tsindocs/planning/tasks/T-YF4K-*.md:21,T-BCNP-*.md:48,T-BQRU-*.md:63. - Do not touch
baseline.ts:findingsFromStdout,diff, andnormalizeFindingare all still live (ops/run.ts:310dynamic-imports them).
- Also dead with it:
-
Delete
lib/util/schema_patterns.ts(220 lines) andlib/util/tests/schema_patterns.test.ts. Header names thenew_*.tsscaffolding scripts as its consumers;find . -name 'new_*.ts'returns nothing. Sole importer is its own test. It is also the only module inlib/utilthat imports upward intolib/model, so deleting it removes the tier’s single layering inversion.- Before deleting, confirm the backlog
superRefinereject branches (lib/model/entities/backlog/schema.ts:109-140) are asserted somewhere. Adjacent coverage exists inlib/model/entities/backlog/ops/tests/update.test.tsandcli/backlog_cli/tests/create.test.tsbut does not enumerate reject cases. If absent, port those cases into a test besidebacklog/schema.ts. The task/milestone date and version cases are not worth porting. - Leave
arrayElementinlib/util/zod_introspect.ts:246in place even though this deletion orphans it — that module is a deliberate Zod-internals choke point whose value is centralizing every private_defread. Add a one-line note so a later dead-code pass does not re-report it.
- Before deleting, confirm the backlog
-
Delete
skills/task-close-out/verify_prs_against_pr.tsandskills/task-close-out/tests/prs_verify.test.ts.SKILL.md:284Step 4 now runssdlc task close-commit; the SKILL.md contains nobun runinvocation. The op reproduces the logic via the samereadPrsFrontmatter/computeAppendpair (close-commit.ts:134-139). Coverage is preserved bylib/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 atlib/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:1230claimsclose-commit.test.tscovers “create / append / noop”; it covers only create (:128) and append (:175). Correct the note or add the noop case.
- 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
-
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 inmoon.yml/justfile/lefthook.yml, not spawned by any test. Its usage line still prints the pre-move pathapps/sdlc/lib/lease/canonical_json_cli.ts.- Companion edit required:
lib/services/lease/tree.ts:4-6justifies thecanonical_json.tsfile 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 importscanonicalJsondirectly to avoid pulling intree.ts’sspawnRunnerdependency. The split must survive.
- Companion edit required:
-
Delete
cli/task_cli/_common.ts(10 lines).cli/task_cli/contains no verb module — only this file,README.md, andtests/. Both test files import from@lib/registry. ItsCYCLE_MARKER_PREFIXwas independently re-declared atlib/model/entities/task/ops/next.ts:58.- Companion edit required:
cli/task_cli/README.md:20-27Layout block lists both this file and anindex.tsthat already does not exist. Reduce the block toREADME.md+tests/. Do not fold the README intocli/README.md— it is 112 lines documenting the livesdlc task nextsurface; relocate it wholesale tolib/model/entities/task/ops/if you move it at all. cli/README.mdneeds no edit — its layout block already annotatesbacklog_clias the “sole remaining LEGACY_NOUNS entry” and never mentionstask_cli.- Leave
cli/task_cli/tests/in place (both suites spawncli/sdlc.tsand are live).
- Companion edit required:
-
Delete
lib/model/index.tsandlib/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-21already documents the deep-path convention, so the barrel headers are stale M-0002-port residue.- Cascade:
read.ts:195#frontmatterOfandcorpus/loader.ts:44-52#readFrontmatterare kept alive only by these barrels. Delete both, plus thedescribe("readFrontmatter", …)block atcorpus/tests/loader.test.ts:17-33and the symbol in that file’s import on line 10.loadCorpusdoes not usereadFrontmatter. - Do not generalize to
lib/model/ops/index.ts— that barrel is load-bearing (importing it registers ops viadefineOpat 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 exportextractTitlefromlib/model/index.ts— point it at the deep-path home or the barrel will be resurrected.
- Cascade:
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”.
Work items
Section titled “Work items”-
lib/model/entity.ts:132-247— deleteCOMMON_FRAGMENT_FILE,isCommonRef, andresolveSchema(~115 lines). No_common.jsonexists anywhere; zeroallOf/$refin any of the 21 on-diskschema.jsonfiles, soresolveSchemaunconditionally returns{ ...schema }at:188and everything below is unreachable.- Three stale claims must be corrected in the same change: the file header (
:25-31), theloadEntitySchemadoc comment (:254-265), and the rationale block at:140-142which asserts the fixtures “still ship” the$ref. Also refreshdocs/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).
- Three stale claims must be corrected in the same change: the file header (
-
lib/model/ops/audit.ts:679-694— deleterenderJsonand its privateensureAscii. Zero callers; the op’s JSON path returns the structured payload viabuildPayload:848. Note there is an unrelated liverenderJsonincleanup/index.ts:1364— do not touch it.- Rewrite the header at
:21-26: delete the wholeaudit_entities.tssentence (that file is gone) and droprenderJsonfrom the exported-functions list. Leave the adjacentcheck_entities.tsparagraph — that file still exists. - Keep
buildPayload(live callerops/migrate.ts:398) andrenderMarkdown(:849).
- Rewrite the header at
-
lib/model/identifier.ts— deletePREFIX_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-insertfrom the capability list at:4and “the corpus-migration script” from the adapter list at:8-10— that file does not exist.
- Header cleanup: drop
-
lib/config/schema.ts:79-108— deletevalidateSubkey. Zero production callers; not in thelib/config/index.tsbarrel. Delete thedescribeblock atlib/config/tests/schema.test.ts:102-126and 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), leavingvalidateLoaded’s as the single copy. -
cli/_common.ts— deleteemitGenericException:23-26(zero references; itserror: ${exc.constructor.name}: ${exc.message}format also diverges from the liveerror: ${message}convention, so it could not be adopted without breaking goldens),newUuid4:33-35(zero references; lease ops use their own atlib/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/tablevialib/services/lease/ops/_common.ts:315).- Update the header bullet list at
:4-8— after this, only exit codes andemitJsonremain. Also drop the staleTS port of apps/sdlc/cli/_common.pyline at:2. - Update
lib/util/table.ts:4-9to namelib/services/lease/ops/_common.tsas the sole re-exporting tier (the current text is accurate only whilecli/_common.ts:40exists). - Keep
EXIT_GENERIC— ~20 live consumers.
- Update the header bullet list at
-
lib/services/dashboard/server.ts:1469-1493— deletestopDashboard. Zero callers; orphaned by 0ee07a6f, which deletedplugin/scripts/status_dashboard.ts(its only consumer).lifecycleStop(dashboard-service.ts:145-167) is the live, statement-for- statement equivalent againstctx.io.- Update the
server.tsmodule header (:1-7), which advertises “serve/detach/stop/list”.
- Update the
-
cli/registry_adapter.ts:341-351— delete the unreachable invalid---outputbranch..choices(OUTPUT_MODES)at:266-269makes commander reject a bad value during parse, before the action fires (verified:bun cli/sdlc.ts task next --output bogusprints commander’s own message). Drop| nullfromresolveOutputMode’s return type and correct the doc comment at:84-89, whose closing sentence becomes false. Exit-code parity holds:CommanderErroris caught at:604-617and returns1, same asEXIT_GENERIC. -
lib/model/ops/validate.ts:96-101— delete thevalidateFrontmatterAgainstEntitypass-through and have its sole caller (_update.ts:164) callvalidateFrontmatterfrom../entity.tsdirectly (already imported at_update.ts:52-56). This closes the registration leakcli/sdlc.ts:51explicitly documents (“Without this, onlyvalidatewould leak in via a transitivetask updateimport”). Also drop the now-gratuitousexportondefaultEntitiesDiratvalidate.ts:128.- Do not pursue the follow-on split of
validate.ts:219-530(the.basevalidator) — the op descriptor at:623still dispatches to it, so the module graph is unchanged.
- Do not pursue the follow-on split of
-
lib/util/wikilinks.ts:47-53— deleteextractWikilinks(zero production callers) and update the module header at:9, which advertises “wrap/unwrap/extract”.lib/util/markdown_extract.ts:175-185— deleteheadingPlainTextand its privatenodeText. Narrow the mdast import at:29toimport type { Root } from "mdast";— bothHeadingandRootContentbecome unused, butRootis live at:46,:61,:78. -
lib/services/git/easy.ts— deletediffNames:166andremoteUrl:192. Zero references anywhere in the worktree, includingtests/easy.test.ts. Do this before easyGit is promoted topackages/ts/easy-git(PR #957) so unexercised surface is not exported into a package.watch.ts:239-243hand-rollsconfigGet("remote.origin.url")whereremoteUrlwould fit, but the swap is not semantics-preserving (git remote get-urlexpandsinsteadOfrewrites;git config --getdoes not). File that separately.skills/task-work/check_ancestry.ts:89-99is not a prospectivediffNamescaller — it needs a two-dot[baseRef, "HEAD"]argv form and throw-on-failure, neither of which the API models.
-
Dead export tails in
lib/model/entities/task/ops/:resolve-touchpoints.ts:294— dropresolveRow,globMatches,fileContainsSymbolfrom the export list (zero importers;globMatchesmoves in PR 8 anyway).parse-touchpoints.ts:487-488— dropdetectShape,parseLocationCellfrom the value exports. Keep the separateexport type { Section };line that follows.inflight.ts:179-180— dropreadFrontmatter,renderInflight(dashboard/server.ts:49imports onlyclassify). Delete the now-false back-compat justification at_probe_core.ts:14-16—tests/inflight.test.ts:19imports only the default export.gap-report.ts:153— delete the unreachableif (lc === "acceptance criteria") return "Acceptance criteria";; the loop at:150-152already matches that string via the aliases entry at:100and returns first.
-
lib/services/project/cleanup/index.ts— removeexportfrom the seven pipeline stages (discoverRepo:179,loadTasks:262,fetchPrs:335,gatherLocalBranches:394,gatherWorktrees:654,gatherRemoteBranches:945,gatherTaskLinkage:1122) and from the typesTaskRecordandPRInfo. The only two importers (ops/cleanup.ts:24-30,tests/cleanup.test.ts:31-37) name none of them.- Keep exported:
RepoInfo(transitively reachable viaCleanupResult.repoat:73-76),CleanupItem,SummaryBucket,CleanupResult,GatherInput,Scope,ALL_SCOPES, and the four consumed functions. Noteops/cleanup.tsimports 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 trailingexport { renderScanTodos };(already wired via thecli.renderdescriptor at:152).
- Keep exported:
-
lib/services/gate/ops/_skill_prose_contract.ts:128— delete theslugfield fromSkillProseSpecand 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 akey === spec.slugassertion test instead; that preserves the redundancy. -
lib/config/load.ts.describe()prose — four field descriptions name deleted Python scripts, andlib/config/scaffold.ts:104,143renders them as YAML comments into every consuming project’ssdlc.yaml. Fix all four::63quality_checks—apps/sdlc/scripts/run_quality_checks.py→sdlc quality run:118orchestrator—apps/sdlc/scripts/count_inflight_tasks.py→sdlc task inflightworktree_init— replace the Python idiomsubprocess.run(verb, shell=True, cwd=<worktree-root>)with the phrasing the siblingcommit_worktree_initdescription already uses correctly (a shell verb run in a shell (\sh -c`, cwd = the worktree root)`) :262lease_authority—apps/sdlc/scripts/sdlc_lease.py→ thesdlc 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.
Work items
Section titled “Work items”-
New
lib/util/guards.ts— promote the existing privateisMappingfromlib/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.
-
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) andNoneTypefor bothnullandundefined.lib/model/ops/migrate.ts:171’s"tuple"is the sole outlier and belongs inline at its one legitimate call site (see PR 6).isPlainObjectis not added here —isRecordfromguards.tscovers it. -
lib/util/strings.ts— addescapeRegExp:export function escapeRegExp(s: string): string {return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");}Six byte-identical copies exist today.
detect-runners.tsalready importscmpStrfrom this module, so adoption adds no new edge there. -
lib/util/fs.ts— add the write and walk helpers:export function ensureDir(dir: string): void; // mkdirSync recursiveexport function writeFileEnsuringDir(path: string, contents: string): void;export function rglob(root: string, keep: (relPath: string) => boolean): string[];rglobmust not filter onDirent.isFile()—scan-candidates.tspushes every non-directory entry and re-stats through symlinks at:328-329; filtering here would silently drop symlinked entries. Recurse one.isDirectory(), push any other entry satisfyingkeep, sort the result. -
lib/util/site_table.ts— add the scalar sibling tostringList:export function stringField(value: unknown): string; // "" when absent/non-string/emptySignature must be
(value: unknown), matchingstringList(value: unknown)— not(e: EntityView, key: string). The value form is the only one that can also absorb thestr()sites inservices/docs/site/projections.ts:264-265. -
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. -
lib/util/git.ts— add the worktree path helpers besideWORKTREES_DIR_REL:export function worktreeDir(root: string, basename: string): string; // absoluteexport function worktreeDirRel(basename: string): string; // repo-relativeUse
resolve()(notjoin()) internally to preserve current behavior for callers passing a relative root. Both forms are genuinely needed: the relative form backs thepreflight-worktree.ts:126output contract (asserted atpreflight-worktree.test.ts:147) and thegit worktree removecall atteardown-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.
Work items
Section titled “Work items”-
Delete every local
isFile/isDirand import from@lib/util/fs:File Symbol(s) lib/util/frontmatter.ts:157fileExists(sole use at:171)lib/model/entities/task/ops/resolve-touchpoints.ts:88,:96fileExists,dirExistslib/services/project/ops/scan-candidates.ts:103,:108isDir,isFilecli/backlog_cli/create.ts:526isDir(sole use at:540)skills/task-work/start_task.ts:663,:671isFile,isDirskills/task-work/append_pr_url.ts:113isFileskills/task-work/dedup_search.ts:156isDirskills/task-ensure-ready/ensure_ready_mutate.ts:605isFileskills/task-define/scan_validate_ac.ts:75isFileskills/migrate-runtime-state/migrate_runtime_state.ts:73,:81isDir,isFilePlus 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-215and:190-195, :408-414, :466-470,skills/task-work/start_task.ts:440-445,lib/model/identifier.ts:225-228(itscatch { continue; }is control-flow-equivalent toisFilereturning false, since the next line isif (!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, andstart_task.ts:440the local variable is namedisFile. Delete the local declaration outright and rewrite the guard asif (!isFile(path))— do not rename around it.Import style: use the
@lib/util/fsalias inlib/andcli/. Inskills/, match the file’s existing convention (most use relative../../lib/util/*.ts;preflight_permissions.ts:62-63already uses@lib/). -
Dead-catch removals (behavior-preserving, do these with the swap):
lib/model/entities/task/ops/resolve-touchpoints.ts:96-102—dirExists’s catch doesreturn existsSync(abs) && statSync(abs).isDirectory(), which can only fire becausestatSyncthrew; any throw reason makesexistsSyncfalse, so the&&always short-circuits. Replace the whole function withisDir.lib/model/entities/task/claims/paths.ts:132-140—existsUnderRoot’s catch returnsexistsSync(abs), which is itself a stat-in-a-try. Reduce the body toreturn existsSync(abs)(keep the sep/join absolutization at:133) and dropstatSyncfrom thenode:fsimport at:31. Do not switch toisFilehere — that is a behavior change (a directory named e.g.bar.tswould stop short-circuiting at:162), and it should be its own decision.
-
Import narrowing (required —
noUnusedLocalsis 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→ dropstatSynclib/services/project/ops/scan-candidates.ts— keepstatSyncand theStatstype:statOfstays (two live callers at:249and:328, the latter reusing theStatsobject forst.size).
-
Adopt
escapeRegExpfrom@lib/util/stringsat six sites, deleting the local copies:lib/services/quality/baseline.ts:44,lib/services/quality/detect-runners.ts:358(namedescapeRe),lib/util/markdown_extract.ts:187,lib/services/project/ops/scan-todos.ts:37(an arrowconst, not afunction),skills/task-work/dedup_search.ts:129,skills/task-work/preflight_permissions.ts:198. Leavelib/services/project/ops/scan-candidates.ts:218escapeRegexLiteralalone — it is a different, single-character function. -
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.
Work items
Section titled “Work items”-
rfc3339/nowUtcfrom@lib/util/date(header atdate.ts:5-10states these exist “so the stamps can never drift between entry points”):lib/services/quality/baseline.ts:180-183isoUtcNow→rfc3339(nowUtc())lib/services/pr/ops/classify.ts:460-462nowIso→rfc3339(nowUtc())(note its regex is/\.\d+Z$/, not/\.\d{3}Z$/— same output either way)lib/services/dashboard/server.ts:493-495isoZ(dt: Date)→rfc3339(dt)at its two call sites (:702,:1324). This is a formatter over a supplied Date, not anowstamp — do not substitutenowUtc().lib/services/orchestrator/ops/watch.ts:670-671andlib/services/orchestrator/ops/log-tick.ts:294-295— these areargs.X ?? <idiom>fallbacks; replace only the right-hand operand, preserving the deterministic-test override.
Trap: do not touch
lib/services/project/cleanup/index.ts:1343nowIso— it deliberately emits microsecond precision (pad(getUTCMilliseconds(),3) + "000"), andrfc3339would drop the fractional part fromgenerated_atat:1390. Likewise leavelib/services/lease/tree.ts:36-39(git-log+0000format) andlib/services/lease/runtime.ts:215-236(a deliberately distinct regex-validating parser, documented as such). -
posix()from@lib/util/paths(docstring: “Consolidated from inlined copies”) at three sites, allrelative(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))
-
cmpStrfrom@lib/util/strings(docstring: “Consolidated from inlined copies”) at 12 sites of(a, b) => (a < b ? -1 : a > b ? 1 : 0), all onstring[]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 siteslib/services/project/tests/scan.test.ts:137, :316. -
lib/services/dashboard/server.ts:368-375— delete the privateparseRfc3339(a verbatim copy oflib/util/date.ts:76-80plus a type-guard and NaN check) and import the shared one, keeping a thinunknown → Date | nullwrapper. While editing, tightendate.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.
Work items
Section titled “Work items”-
Adopt
pyTypeNamefrom@lib/util/pyand 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:209keeps 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.
-
Adopt
isRecordfrom@lib/util/guardsin place of the eightisPlainObject/isMappingcopies:lib/model/ops/migrate.ts:186,lib/model/ops/validate.ts:298(namedisMapping),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— itsisPlainObjecttakes aYamlValue, narrows to{[k:string]: YamlValue}, and additionally excludesPlainScalarinstances. Swapping it would treat wrapped scalars as mappings — a real bug in the emitter. - Do not convert
lib/services/harness/ops/install-claude.ts:78asObject— it returnsRecord<string, unknown> | nulland is consumed as a cast-helper at ~8 sites; converting rewrites each intoisRecord(x) ? x : null, a lateral move. Leave it, optionally re-implemented on top ofisRecord.
- Do not touch
-
Adopt
isRecordat the 13 inline guard sites (each currently followed by an uncheckedas 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:147casts toRecord<string, string>,server.ts:1216toRecord<string, Record<string, unknown>>,watch.ts:368/526index the narrowed value. watch.ts:316-322asObjListalready carries a correct(e): e is Record<string, unknown>predicate — replacing it withvalue.filter(isRecord)is a readability win, not a cast removal.
- Roughly 8 casts delete cleanly. The rest narrow to a different type and survive:
-
pyRepr— two byte-identical copies atlib/model/entities/task/migrations/v1-to-v2.ts:111andlib/model/entities/capability/migrations/v1-to-v2.ts:81. Adopt from@lib/util/py. Do not fold inv2-to-v3.ts:569pyReprStr— it is a narrower, different function (string-only, noNonebranch). -
Collapse
v3-to-v4.tsandv4-to-v5.ts— after the shim extraction these two modules are identical apart from the stamped version literal ("4"vs"5"). Extract astampOnlyMigration(version: string)factory in the migrations directory.- Each module must keep exporting
Frontmatter—tests/v3_to_v4.test.ts:8-12andtests/v4_to_v5.test.ts:8-12import the type, not justmigrateandMigrationError. - Preserve the
transformWantsBodyarity contract (lib/model/ops/migrate.ts:165-167).
- Each module must keep exporting
-
MigrationError—lib/model/entities/capability/migrations/errors.ts:10-15andlib/model/entities/task/migrations/errors.ts:10-15are identical 3-line classes. Folding them into one home is reasonable but low-value; keep the per-moduleexport { 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.
Work items
Section titled “Work items”-
lib/model/ops/_update.ts:93-97— delete the localparseFrontmatter, which shadowslib/util/frontmatter.ts:119while importingextractFrontmatterfrom that same module one line away (:60). Replace the call site at:146with 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 tonull, 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 reachesfm[key] = valueat:157and throws an uncaughtTypeError, escaping theOpErrorcontract. - Drop the now-unused
yamlParseimport at:48(closing a parse-seam bypass) andextractFrontmatterat:60. KeepFRONTMATTER_REin that import — thematchfrom:141is load-bearing at:189-192for the fence reconstruction. - Prune the grandfathered entry at
.claude/skills/project-check/util_redeclarations_baseline.json:30.
- Do not use
-
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, :195lib/model/ops/migrate.ts:271-272, :294(applyTransformChain)lib/model/ops/migrate.ts:335-336, :352(stampSchemaVersion)
Critical: in
applyTransformChainthe body is not constant —bodyis reassigned inside the chain loop by[fmObj, body] = callTransform(...)atmigrate.ts:285. The helper must take the body explicitly, never re-derive it from the originaltext, or every body reshape the transforms performed is silently discarded. Each site keeps its own parse, mapping guard, error type, and write decision (stampSchemaVersionkeeps itsnewText === textshort-circuit).- The cosmetic drift (
_update.ts:194appends.replace(/\n$/, ""), the migrate copies do not) is a no-op today becausedumpFrontmatterends inpyStrip— drop it.
-
lib/util/frontmatter.ts:268— routewritePrsFrontmatterthroughdumpFrontmatter. This is the medium-risk item. It currently emits via the npmyamlpackage’sstringifywhile every other frontmatter writer usesdumpFrontmatter(lib/util/yaml.ts:605), the PyYAML-byte-compatible emitter.yaml.ts:5-11explicitly documents that the npm package is not byte-compatible. Within the single task-work skill,start_task.ts:412writes a task file viadumpFrontmatterandappend_pr_url.ts:275rewrites the same file viawritePrsFrontmatter— the two steps of one workflow fight over the same bytes. 25 of 95prs:-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 byyaml.ts:5-11. - A coercion is required —
dumpFrontmatterthrowsscalar.startsWith is not a functionon numeric leaves. Promote the helper fromskills/task-ensure-ready/ensure_ready_mutate.ts:57-65, but fix its null branch during promotion: it mapsnull/undefined→"", whereasdumpFrontmatternatively emitsnull(asserted bylib/util/tests/yaml.test.ts:14,parent_key: null). - Decide the block-scalar question before swapping.
dumpFrontmatterhas no literal-block-scalar support, so the swap rewrites the ~120 task files carryingcompletion_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 inff6dca33under S-0007. Do not smear the churn across unrelated PR-append commits.
- Delete or correct the doc comment at
-
Replace hand-rolled
splitFrontmatter+ yaml-parse + guard with the existing seam:lib/services/dashboard/server.ts:344-356— exact match forparseFrontmatter’s semantics on all five branches. Becomesconst [, body] = splitFrontmatter(text); const frontmatter = parseFrontmatter(text) ?? {};Then check whetherparse as yamlParseat:35is still used.lib/services/project/cleanup/index.ts:286-295— not a straight swap. This site also doesif (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— useparseFrontmatterResult, notparseFrontmatter. The current code has no mapping guard (an array frontmatter reachesObject.entriesand yields index keys) but does surface theyamllibrary’s line/column diagnostic on malformed input.parseFrontmatterResultcloses the guard gap while preserving the diagnostic; a bareparseFrontmatter+ null-check would collapse three distinct failures into one misleadingno frontmatter in <path>message. This also routes the file through the parse chokepoint, fixing the directimport { parse } from "yaml"at:21.
-
lib/model/identifier.ts:259-267— replace the private frontmatter regex (/^---[ \t]*\n([\s\S]*?)\n---/, which rejects CRLF where the canonicalFRONTMATTER_REaccepts it) and theid:line scrape withreadRawFrontmatter/parseFrontmatter.- This is not behavior-preserving as written. Today an unreadable file hits
catch { continue; }and produces no violation; the read-layer primitive returnsnull, so the file would newly reportid-missing. If the skip is intentional, keep an explicit existence guard. - Two hazards the finding claimed are false: the
id:regex carries themflag, so a# id: T-BOGUScomment line and an indented nestedid: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
findIdentifierViolationsor theid-mismatch/id-missingkinds. Land this with a test. - Leave
lib/model/entities/capability/migrations/v1-to-v2.ts:115-119alone — all seven migration modules import zero@libby convention, the transform is dorment (the corpus is entirelyschema_version: "2"), and switchingderiveChildrentoparseFrontmatterwould make one unparseable sibling silently drop a child and turn a clean migration into a spurious contains-inconsistencyMigrationError.
- This is not behavior-preserving as written. Today an unreadable file hits
-
skills/task-work/preflight_permissions.ts:444-456— deletetaskBodyand replace its one call site at:572withconst [, 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 byFRONTMATTER_RE— immaterial for real task files, but be aware it is a behavior change, not purely a tightening. -
lib/util/frontmatter.ts:124-279— relocate the task-specificprs:block (PrsFieldError,readPrsFrontmatter,computeAppend,writePrsFrontmatter) out oflib/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, andreadPrsFrontmatter:170-214is precisely the hand-rolledreadFileSync+ regex +parseYaml+ field-index patternlib/CLAUDE.mdforbids.- This supersedes
T-JJZ3-lib-root-tidy.md:39-40,:48, which prescribed the merge on a “genericprs:helpers” premise the schema grep refutes; its cited authority (D-0007 §Migration step 2) actually routes generic PR helpers toutil/gh. lib/util/tests/frontmatter_prs.test.ts(34 references) moves with the code.- Preserve the exit-code contract deliberately:
PrsFieldErrorencodes 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).readTaskreturnsnullfor 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 theapplyFrontmatterUpdateswrite path second — that half adds schema gating, which is a real tightening forclose-commit.tsand would newly fail on a schema-drifted task.
- This supersedes
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.
Work items
Section titled “Work items”-
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 withtaskBranch,taskFilePath,worktreePathand replace::123const taskBranch = \task/${basename}`→const branch = taskBranch(basename)(rename the local; it currently shadows the helper's name — update its uses at:124,:136,:143`):126resolve(projectRoot, "docs", "planning", "tasks", \${basename}.md`)→taskFilePath(…)`:142resolve(projectRoot, ".sdlc", "worktrees", basename)→worktreePath(...)- the
typeof rawStatus === "string"dance at:128-132→frontmatterString(...)
probe-state.ts:149-155,dashboard/server.ts:541-544, andwatch.ts:83,:489all already use these; inflight is the lone outlier. Then drop the now-unusedresolveimport fromnode:path.- Converge
lib/services/orchestrator/ops/watch.ts’s localtaskStatushelper (~:487-493), which still inlinesconst s = fm["status"]; return typeof s === "string" ? s : null;.
-
Collapse the duplicated worktree-path constant.
lib/util/git.ts:13WORKTREES_DIR_RELandlib/services/project/ops/_worktree-common.ts:23WORKTREES_SUBDIRhold the identical literal.sdlc/worktrees, mutually unaware — andgit.ts’s header already claims to be “the single home”.- Delete
WORKTREES_SUBDIR; re-point_worktree-common.ts:26worktreePathForatworktreeDirRel(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 atlib/services/gate/ops/worktree-scope.ts:115. - Rename
skills/task-work/preflight_permissions.ts:666worktreePathFor(a different signature — takes a task path, returns absolute-or-null) to e.g.worktreeDirForTaskFile, with its body callingworktreeDir(root, basename). Three functions currently share two names across two return conventions.
- Delete
-
Extract the duplicated
globMatchesandtoAbs.lib/util/location.ts:111-123/:104-106andlib/model/entities/task/ops/resolve-touchpoints.ts:124-139/:84-86are identical statement-for-statement (the model copy carries a 3-line explanatory comment the util copy lacks).location.ts:109-110even documents the copy rather than removing it.- Keep
@lib/util/locationas the home; import both fromresolve-touchpoints.ts. - Drop
globMatchesfrom the export list atresolve-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"atresolve-touchpoints.ts:54. - Coverage note:
lib/util/tests/location.test.ts:153-160covers glob match/no-match but has no case for a glob matching only a directory. ThatonlyFiles: falseedge is pinned solely byresolve-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 atlocation.ts:6-8.
- Keep
-
easyGit adoption (
lib/CLAUDE.md: “Git operations prefer the typedservices/git/easy.tsclient over hand-built argv”):lib/services/lease/migration.ts:80-92localBranchExists→new Git(projectRoot).branchExists(\task/${taskId}`). DeleteRunResult:64-68,run():70-72, and thespawnRunnerimport at:29`.lib/services/project/ops/scan-candidates.ts:252-269gitCheckIgnoreFilter→Git.checkIgnore(easy.ts:230). Thread aGitintoscanCandidates— the handler at:419buildsnew 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 thespawnRunnerimport at:27.lib/services/plugin/ops/resolve.ts:38-68— deletewhich()(which shellssh -c "command -v git"purely to locate the binary, and interpolates its argument into a shell string unescaped),gitToplevel, andgitOriginUrl. The directory-source path currently spawns four processes;new Git(pluginDir).topLevel() ?? pluginDirat:116andnew Git(repoRoot).remoteUrl("origin")at:117take it to two with identical null-on-failure semantics. (Ordering note: PR 2 deletesGit.remoteUrlas unused — do this item or that one, not both. Recommended: keepremoteUrland adopt it here; drop onlydiffNamesin PR 2.)lib/services/plugin/info-report.ts:34-42, :52-64, :82-85— delete the module-levelGIT, thegit(args, cwd)helper (which passes cwd twice:["-C", cwd, ...]and{ cwd }), andcommitExists. Route throughnew Git(dir):topLevel(),revParse("--short","HEAD"),isDirty(),objectExists(\${sha}^{commit}`). **Trap:** userevParse(“—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”, flippinggitSummaryoutput fromHEAD@shato(detached)@sha. Unmodeled verbs (describe —tags,rev-list —left-right —count,tag —list) move tog.raw(…)`.lib/model/entities/task/ops/_lint_state_origin_core.ts:54-59— thread aGitthrough 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 atlint-state-origin.ts:101does not even acceptctx). Two traps: (a)listTaskBranchescurrently throwsLintErrorwhenfor-each-refexits non-zero (:110-112) whileGit#branchesreturns[]— a verbatim swap would silently degrade a git failure into “no task branches” and a green exit-0 pre-commit gate; useGit#rawhere or keep an explicit guard. (b)logSubjectreturnsnullwhere the current code produces a trimmed string, so the violation-line template needs?? "".
-
lib/services/project/cleanup/index.ts:106-121— deletemakeRunFn, which names its injected runner_gitRunnerand never reads it, constructing a freshspawnRunner(binary)per call. All three uses are git calls with"git"hand-prepended (:503, :766, :1005). Thread theGitinstance already built insidemakeGitFninstead.classifyRemoteBranch:978currently receives onlyrunRawand gains aGitparam rather than losing one.gitFncannot substitute at the--is-ancestorsites (it collapses non-zero exits to""), which is why theGitinstance is the right vehicle:git.at(cwd).raw([...]).exitCode === 0.:766→git.at(path).statusPorcelain()(dirtyFilesalready inits to[]at:755).- Delete the dead
_runRawparam atdiscoverRepo:184and its argument at:1438. - Narrow the
:21import to theCommandRunnertype. - 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 tospawnRunner("git")), and the cleanup tests deliberately exercise a real git binary against tmpdir fixtures. Production behavior is unchanged.
-
lib/services/project/cleanup/index.ts:263-295— rebuildloadTasksonscanEntities("task", repo.root, { withText: true }). It hand-rollsscanEntityDirstep for step, and is the only remainingsplitFrontmatter+yamlParseentity walk underlib/services/project/. Three traps:- Scan the headline over the body, not
row.text— keepconst [, body] = splitFrontmatter(row.text!), or a#comment inside frontmatter can become the headline. ThesplitFrontmatterimport stays. statSynccannot be dropped from the imports — still used at:759for the worktree on-disk check. OnlyreaddirSync/readFileSync/yamlParseleave.- Frontmatter handling differs:
loadTasksskips files with no frontmatter but keeps files whose YAML fails to parse (falling back to{});scanEntityDiremits both asfm: null. A faithful port needs an explicitif (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.
- Scan the headline over the body, not
-
lib/services/dashboard/server.ts:314-362readEntityDetail— keep the lazy two-branch structure (the readdir-first fast path avoids reading all 511 task files per detail request; do not replace it withscanEntities(..., {withText:true})). Two narrow fixes:- Replace the hand-rolled parse tail at
:344-355withparseFrontmatter(text) ?? {}(identical semantics), dropping ~12 lines and theparse as yamlParseimport at:35. - Fix the README/index inconsistency:
listGlobMd:161-169does not skipREADME.md/index.mdwhilescanEntityDirdoes (read.ts:262), so the basename branch can resolve a README the id branch never would. Apply the same skip.
- Replace the hand-rolled parse tail at
-
Promote the escape-aware table-cell split.
splitTableRow(parse-touchpoints.ts:225-231) and the inline copy insiderowHasEmptyCell(scan-placeholders.ts:123-131) share the same"\x00PIPE\x00"sentinel; a repo-wide grep forPIPEreturns exactly these two. Extract to an underscore-prefixedlib/model/entities/task/ops/_table_cells.ts(matching the directory’s_task_doc.ts/_probe_core.tsconvention — the underscore keeps the registry’s discovery walk from treating it as an op). Do not swap to markdown-contract’srawTableRow: the in-code note atparse-touchpoints.ts:260-264correctly documents that the library’ssplitRowis 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.
Work items
Section titled “Work items”-
Promote two wikilink constants into
lib/model/entities/_common.tsbeside the existingDATE_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 namedRELATED_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_PATTERNcollision. Verified safe: nothing outside theseschema.tsfiles imports any of the three names (schema_patterns.tsis deleted in PR 1). -
-
Rename the two surviving
RELATED_PATTERNvariants 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 specializerelated.items, and that design is live. -
Add a per-prefix factory to
_common.tsfor 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:62wikilinks at
decision:22, capability:50, driver:22, standard:22, backlog:37, backlog:39 (milestone variant with subId), backlog:41, backlog:43Hard constraint: the factories must emit
.sourcebyte-for-byte identical to today’s literals.lib/services/docs/data_model.ts:99-113derives each type’s prefix by string-matching the pattern source (/^\^([A-Z]{1,3})-/) and detects sub-ids viaidPattern.includes("(\\.\\d+)?")through thezodToJsonSchemaprojection. 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:56andFILENAME_RE:58cannot 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.
- Build the factories from new exported fragments in
-
Swap the inlined ISO-date regex for
DATE_PATTERNatmilestone/schema.ts:72,task/schema.ts:184, andlib/model/entities/task/ops/close-commit.ts:56. All three inline/^\d{4}-\d{2}-\d{2}$/— literallyDATE_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/reportreaching intolib/model/entities), forbidden by S-0008. skills/task-work/start_task.ts:75DATE_REmay stay (standalone skill glue, S-0001).
- Do not touch
-
Add a
requiredWhenhelper to_common.tsand collapse the 9 hand-written status-conditionalsuperRefinearms: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-118anddecision/schema.ts:88-105(byte-identical after comment strip — both encodeclosed/superseded ⇒ superseded_byandclosed/deprecated ⇒ deprecation_note; share one exportedSUPERSESSION_RULESconstant);milestone/schema.ts:105-121(two arms);task/schema.ts:214-227(a 10-line block);backlog/schema.ts:96-104(nested under apromoted/guard) and:145-152.Constraints:
- The predicate stays a caller-computed boolean — that is what makes this
behavior-preserving across
===,startsWith("closed/"), andstartsWith("promoted/"). - Message strings must stay byte-identical — asserted at
lib/model/tests/entity.test.ts:204,:220andcli/backlog_cli/tests/create.test.ts:122(an AJV-compat contract). - Do not add a
forbiddenWhensibling — 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 narrowedz.enumliteral typing onstatus, so aRecord<string, unknown>access cast is needed at the seam. Acceptable, but it is a real trade.
- The predicate stays a caller-computed boolean — that is what makes this
behavior-preserving across
-
Export
RESULT_PATTERN_BY_STATUSfrombacklog/schema.tsand collapse the 4-branchif/else-ifchain at:108-143(each branch repeating the same 6-linectx.addIssuebody 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.) -
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.jsonfiles 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.
Work items
Section titled “Work items”-
Extract the entrypoint preamble. A byte-identical 9-line block (
const projectRootOptsthrough the close ofresolveAuthority({...})) 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:fetchLeasederivesrefbut nobuildCwd;releaseToArchivenames itactiveRefand addsarchiveRef;acquireLeaseandreacquireLeasederiveme = opts.hostId ?? currentHostId(). Callers derive those in one extra local line each. - Optional: make
projectRootlazy. TodayfindProjectRootruns unconditionally even whenopts.authorityis supplied, sotransitionLease/heartbeatLease/reacquireLeaseeach pay a filesystem walk for aresolveAuthoritycall that short-circuits. - Delete the
parse as parsePathimport at:29and itsvoid parsePath;suppressor at:770if still present — verified absent in the current tree; skip if so.
- A private
-
Extract the expiry computation — the
Math.max(Math.floor(ttlSeconds), 1)clamp plusrfc3339formatting appears atruntime.ts:308-312, :455-461, :536-542, :610-616andmigration.ts:192-193.- Add
expiresAt(now: Date, ttlSeconds: number): stringtolib/util/date.ts, which already declares itself the single home for lease timestamp stamps and which both files already importrfc3339from. - Drop the second-truncation (
new Date(Math.floor(now.getTime()/1000)*1000)) as the no-op it is:rfc3339formats at second precision from UTC calendar fields, so truncated and untruncated bases provably yield identical strings (verified over 200k randomized inputs).migration.tsis 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.
- Add
-
Extract the CAS-conflict recovery at
runtime.ts:483-497,:626-640,:748-765. The current shape is actively misleading: thethrow new LeaseConflict(...)sits inside the innertry, so the conflict it just constructed is caught by its owncatch (e2)and only escapes via the trailingthrow 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/reacquireLeasebecomethrow new LeaseConflict(ref, freshOwnerOrNull(...) ?? "unknown");releaseToArchivebranches onowner === nullto emit itsRELEASE-NOOPline andreturn null. -
Scoping guard:
acquireLease’s CAS-CREATE recovery at:379-398looks similar but must not be folded in — it comparesexisting.owner === meand returns the existing lease on a self-owned ref, and throws a distinctLeaseError(notLeaseConflict) onRefNotFound.
-
-
Fix the missing
LeaseConflictarm — this is a real exit-code bug.leaseExceptionToOpError(ops/_common.ts:135-180) has no arm for the library-sideLeaseConflict, so four op handlers each hand-write the same three-line catch, and a fifth —ops/release.ts:46-51— omits it.releaseToArchivethrowsLeaseConflict(runtime.ts:755), which falls through to the genericLeaseErrorarm at_common.ts:174, so a cross-owner conflict onsdlc lease releaseexits 1 with a genericerror:instead of the LEASE-CONFLICT marker at exit 4, contradictingcli/README.md:138.-
Add the
LeaseConflictarm before theLeaseErrorarm at_common.ts:174, emittingmarkerError("LEASE_CONFLICT", formatMarker(MARKERS.LEASE_CONFLICT, { ref: exc.ref, reason: `owner=${exc.owner}` }))Import
LeaseConflictfrom../index.tsalongside 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 tocatch (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
CliLeaseConflictfortransition.ts:332, which synthesizes a conflict from aCASFailed.
-
-
Single-source the phase sets.
new Set(["claimed","working"])andnew Set(["awaiting-review","blocked"])are declared at bothruntime.ts:408-409andops/task/transition.ts:66-67, and both copies drive the same two protocol decisions (rotatelease_token; null outexpires_at). These are ADR rules, not local conveniences — adding a phase to one copy silently givessdlc lease task transitionand the librarytransitionLease()different semantics.- Export both from
runtime.ts, surface them in the lease index barrel, deletetransition.ts:66-67. - Constants only. Do not merge the two transition paths — the op gates on a 5-value
TRANSITION_PHASESenum (: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.
- Export both from
-
Stop redeclaring payload vocabulary at the op layer:
OWNER_KINDSis written out identically atops/task/claim.ts:39,ops/task/acquire.ts:24,ops/op/claim.ts:37whileschemas.ts:47-53already declares the authoritativeOWNER_KINDzod enum every payload validates against — it is simply not exported. Export it and delete the three copies.ops/op/claim.ts:38OP_STATESduplicates the authoritativeOPERATION_STATEenum atschemas.ts:65. Same fix — but note it is also interpolated into a.describe()string at:66viaOP_STATES.join(", "), which becomesOPERATION_STATE.options.join(", ").ops/_common.ts:58SDLC_VERSION_PAYLOAD = "1.0.0"restatesversion.ts:14SDLC_VERSION, whose own header says “Every consumer reads from this module rather than redefining a local constant” — and_common.tsalready imports from../index.ts, which re-exports it atindex.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.
Work items
Section titled “Work items”-
Delete the hand-rolled
parseFrontmatteratsite.ts:238-317(~75 lines) and useparseMarkdownFile/stripFrontmatter, already imported atsite.ts:41and used at:824. The header’s “verbatim port from regen.mjs — no third-party dep” rationale is stale:yamlis a direct dependency andmarkdown-contractis 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:3andskills/principle-review/SKILL.md:3use unquoted multi-linedescription: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## Descriptionblock and the Starlightdescription:frontmatter. - Folded scalars.
>is treated as literal (:280joins with\ninstead of folding to spaces), soskills/explore-codebase/SKILL.md:2yields embedded newlines;site.ts:362then doesdescription.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:353is/^#\s+\/sdlc:.../with nomflag, so it never matches — every generated skill page duplicates its title under## Source, exactly what the comment at:350-352claims to prevent.FRONTMATTER_REconsumes the trailing whitespace, so the swap makes it fire.
Also handle:
extractFrontmatterthrows 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-partySKILL.md— catch and report per-skill rather than aborting the whole build.Coverage gap:
generate_site.test.tscreates an empty skills dir at each fixture, so nothing in the suite covers this path. Verify withsdlc docs generate site --check, and add a fixture with a multi-linedescription:. - Truncation. The parser ignores plain-scalar continuation lines.
-
Extract
lib/services/docs/site/reference.ts. Every other generated family already has its own module undersite/(nav.ts,roadmap.ts,projections.ts+projections_render.ts,supplemental.ts,manifest.ts,routes.ts) withsite.tsorchestrating. The reference family is the sole exception. Move:OpEntry:198-203,loadRegistryOps:205-232(keep exported —computePageInventory:1150still 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 intosite.ts, exactly the patternsite/supplemental.ts:35already uses. Do not claim “no edge back into site.ts”. site.tskeeps the five orchestration calls at:995-999and drops to ~800 lines.cleanLegacyScripts:862-881is directory cleanup, not page rendering — moving it is optional and it may read better alongside the other sweep logic.
- The module imports the fs primitives and
-
Collapse the three inlined
escapeFmStringcopies atsite.ts:363, :443, :570— all.replace(/"/g, '\\"').slice(0, 200), duplicating the exported helper atsite/projections.ts:351(already used byprojections_render.ts:70-71), from a modulesite.tsalready imports five symbols from at:68-75. All three land in the newreference.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.
- Do not fold in
-
Unify the two
## Purposeextractors.entityOverview(site.ts:417-427) scans lines for/^##\s+Purpose\s*$/;data_model.ts:64-78runs a different regex over the same files.@lib/util/markdown_extract#sectionBodyalready does this via markdown-contract’s section projection.- Outputs are byte-identical across all 11 shipped
definition.mdfiles today — this forecloses drift, it does not fix a live bug. - Keep the existence guards.
parseMarkdownFilethrows on a missing file where both current functions return"". Insite.tstheexistsSync(defPath)guard survives naturally; indata_model.tsthetry/catchat:66-70must be widened to wrap theparseMarkdownFilecall, not deleted.data_model.ts’s first-paragraph/first-sentence narrowing at:73-77is genuinely distinct logic and stays.
- Outputs are byte-identical across all 11 shipped
-
Extract
writeIfChangedto a leaf module. It is implemented twice with equivalent behavior —site.ts:142-161andsite/supplemental.ts:116-134— and it encodes the--checkdrift-gate write/no-write contract (D-0010 §6), so a change applied to one copy makes--checkreport differently for mirrored supplemental pages than for generated ones.- Put it in a new
lib/services/docs/site/write.tsthat both depend on downward.site.ts:66importsmirrorSupplementalfromsupplemental.tsandsupplemental.ts:35imports 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-113comment 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, orlistMdFilesRecursive— each has exactly one caller and no duplicate undersite/.
- Put it in a new
-
Collapse the duplicated recursive
.mdwalk.rglobMd(lib/model/ops/audit.ts:147-167) andlistMdFilesRecursive(site.ts:1048-1060) are the same function — recursive descent,.mdleaves, absolute paths, sorted. Move tolib/util/fs.tsaslistMdFilesRecursive(root: string): string[], updatingaudit.ts:293(already isDir-guarded) andsite.ts:1085.- Decide deliberately whether the shared version tolerates or propagates a nested readdir
failure —
site.tscurrently propagates and its sweep is an integrity gate; propagating, withaudit.ts’sisDirguard 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.
- Decide deliberately whether the shared version tolerates or propagates a nested readdir
failure —
-
adrSectiondedup —lib/services/docs/generate.ts:332-337re-inlinessplitSection’s bucketing branch (:193-205) from 130 lines above it, because it needs to prepend folder-ADR rows.principleSection:446-461is 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/.
Dropped findings
Section titled “Dropped findings”| Finding id(s) | Reason for dropping |
|---|---|
xc-dead-void-silencer-residue | NOT 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-seams | L 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-duplication | The 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-shapes | The 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-literals | The 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-drifted | computePageInventory’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-message | A 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-conventions | Seven 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-boilerplate | Deleting 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-comment | Comment-only corrections with no code change. Worth doing, but not worth a review cycle each — fold opportunistically. |
misc-services-verb-list-op-boilerplate | Two 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-loops | The 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-enum | The 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-order | Deleting 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-elementary | A 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-walk | The 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-renderers | Sub-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-shim | The 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-path | Real (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-string | The 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-shellout | A 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-paths | Eight 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-dir | The 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-walker | Small 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-padding | Only 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-parsers | The 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-predicate | Three small cli/ dedups totalling ~40 lines. Worth one combined PR eventually; below the bar interleaved with this stack. |
Suggested merge order summary
Section titled “Suggested merge order summary”1. delete-orphaned-modules (deletions, ~900 lines)2. dead-exports-and-stale-headers (deletions, ~250 lines)3. util-shared-primitives (additions + tests) ← foundation4. 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-constants10. lease-runtime-dedup ← contains one exit-code fix11. 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.