stable
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Jakub Zych
|
a9a7a328e6 |
refactor: hard-fork GSD -> MSD (Make Software Done)
Mechanical rename produced by scripts/msd-rename.cjs: gsd/Gsd/GSD -> msd/Msd/MSD across contents and paths, upstream package/repo coordinates -> @golem15/msd-core and golem15com/msd-core. Deep links into upstream history, sibling upstream packages, the GSD-2 import feature, CHANGELOG.md and .changeset/ are kept as-is. Hand edits on top: MSD block-letter banner and logos, LICENSE copyright line, package/plugin identity, regenerated lockfile, install-tree fixtures, derived registries and benchmark baseline; migration checksum baseline re-locked (MSD keeps its own install state, so no install had applied the old sums); sort-order and regex-escaped expectations in tests adjusted. |
||
|
Tom Boucher
|
85545a77a5 |
fix(#4663): gate the canonicalization on the uat-passed predicate (#4809)
* test(#4663): add failing-first contract coverage for the blocked-uat canonicalization gate verify-work.md's complete_session step flips VERIFICATION.md to passed on 'zero issues' alone, so a session whose every UAT row is blocked (a session that observed nothing) canonicalizes the report. Pins the deployed contract the fix must satisfy: the flip runs the unflagged phase uat-passed predicate inside the human_needed branch, frontmatter.set sits inside a passed==true guard, a refusal message carries the blocker count and keeps human_needed, and an indeterminate pre-check fails closed. All four new assertions are RED until the workflow grows the guard. * fix(#4663): gate the canonicalization on the uat-passed predicate complete_session flipped VERIFICATION.md to passed whenever the session recorded zero issues and the status was human_needed — but blocked rows are not issues by this workflow's own rule, so a 0-passed / 0-issues / N-blocked session (one that observed nothing) rewrote the canonical report to passed. Every later reader (transition.md's preliminary check, resume paths, validate-phase, verification.status) then inherited the unearned pass while the phase-close predicate correctly refused it. The flip now runs the phase-close predicate in a new --uat-only form before canonicalizing: UAT rows evaluated (at least one pass, no pending/blocked/failed/unexplained-skip row), VERIFICATION-status blockers skipped — they must be, because the report still reads human_needed at pre-check time and that status is itself a blocking verification entry, so the full predicate could never pass there and the flip would deadlock (found by isolated review, probed). The --require-verification call stays the transition gate; the refusal branch reports the blocker count and keeps human_needed; an indeterminate pre-check fails closed. Emitted-Drift-Ack-Growth: verify-work.md — canonicalize block gains the uat-only pre-check and refusal branch (#4663) * test(#4663): align the canonicalize pre-check needles with the shipped line The workflow line carries a 2>/dev/null redirect the needles did not include, so both pre-check assertions fail against the committed fix (fixed-string grep verified). Reviewer-found; needle and message aligned. * fix(#4663): reword the canonicalize prose and refresh its size baseline The rationale paragraph mentioned the flagged transition-gate call by its flag, putting a --require-verification literal before the first phase uat-passed occurrence and breaking the existing ordering pin; the prose now describes it without the literal. verify-work.md's growth also drifted the committed compact-content baseline; regenerated via benchmark-compact-content.cjs --write (derived artifact, report-not-gate contract). Emitted-Drift-Ack-Growth: verify-work.md — canonicalize block gains the uat-only pre-check and refusal branch (#4663) * docs(#4663): backfill changeset PR number --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
5e0a7b1b56 |
fix(#4433,#4569,#4126): consolidate the phase-identity seam at name-validity, allocation, and branch-slug (#4640)
* fix(#4433): apply the name-validity guard symmetrically to every milestone-name capture extractMilestoneHeadingName already refused a punctuation-only captured name (#4134), but its two sibling capture sites in getMilestoneInfo — the STATE.md-anchored 🚧-bullet match and the no-STATE.md in-progress 🚧-bullet fallback — skipped straight to a bare truthiness check, so a malformed bullet whose only content past the version was punctuation passed through as a real milestone name. Extracts the existing inline /[\p{L}\p{N}]/u check into a single shared hasNameableContent predicate and applies it at all three capture sites, so the guard is one owner rather than a copy that happened to land at only one of them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(#4433): pin the name-validity guard at all three milestone-name capture sites Failing-first coverage for the hasNameableContent extraction: a punctuation-only 🚧-bullet name must not surface as a real milestone name, either on the STATE.md-anchored path or the no-STATE.md in-progress fallback, while a real name (including a digits-only one) still resolves COMPLETE exactly as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4569): consolidate decimal-phase-number allocation into one function cmdPhaseInsert allocated its next decimal sub-phase number by scanning only on-disk phases/ directories and ### Phase N.M: headings, never the roadmap summary checklist — so a decimal that existed only as a checklist bullet (no heading yet, no on-disk directory yet) was invisible, and phase insert could silently reallocate an already-used number. It also always nested one level deeper under afterPhase, with no way to request a sibling. cmdPhaseNextDecimal had its own separate, near-identical two-source scan (missing the checklist source too) — the exact "duplicate implementations kept in sync instead of deleted" pattern this issue exists to close. Extracts scanExistingDecimalPhaseNumbers (directories + headings + checklist bullets, in one place) and migrates both cmdPhaseInsert and cmdPhaseNextDecimal onto it — deleting cmdPhaseNextDecimal's own copy rather than patching it in parallel. Adds an allocation: 'nested' | 'sibling' argument to cmdPhaseInsert (default 'nested', matching every existing caller's behavior); a top-level phase with no existing decimal segment falls back to nested since there is no sibling level to join. No CLI flag wires 'sibling' yet — that is a separate, disclosed follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(#4569): pin decimal-allocation coverage across phase insert and next-decimal Failing-first coverage for scanExistingDecimalPhaseNumbers: a checklist-only decimal must not be reallocated by phase insert; a decimal present in heading, checklist, and on-disk directory simultaneously must count once; an unrelated phase family's checklist bullet must not cross-pollute; and phase next-decimal (migrated onto the same shared helper) must see a checklist-only decimal too, closing the same gap in a second command. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4634): extend the phase-id drift guard for name-validity and shell arithmetic The epic's ratchet requirement: lint-phase-id-drift.cjs must cover the two new predicates this PR introduces, and must also scan shell inside gsd-core/workflows/**/*.md and gsd-core/references/**/*.md for integer-coercing phase-number arithmetic ($((10#...)) and friends), which neither the canonical TypeScript module nor a source-only lint can reach. Adds findNameValidityDrift (bans re-deriving /[\p{L}\p{N}]/u outside hasNameableContent's owner file) and findShellPhaseArithDrift + scanMarkdownShellArith (bans $((10#...)) in workflow/reference markdown, sanctioned via <!-- phase-id-owner: --> on the preceding line). scanRepo keeps its existing, narrower contract (src/**/*.cts only) so the already-passing "the live repo is clean" test is untouched; a new scanAll merges both for the CLI's full report. Running the guard directly against this tree correctly reports the 7 pre-existing #4619 shell sites (workflows/execute-phase.md x4, workflows/execute-phase/steps/completion-reconciliation.md x2, references/tdd.md x1) as violations — demonstrating the ratchet works, not fixing them. #4619 is a live regression tracked and fixed separately; this PR does not touch those markdown files. A characterization test pins the current count of 7 so a future change to that number is investigated rather than silently absorbed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4569): wire --sibling through phase insert's CLI so the argument is reachable cmdPhaseInsert's allocation parameter had no CLI path to 'sibling' — shipped, untested, unreachable code (code-review finding: a guaranteed surviving mutant). Adds --sibling to phase insert's argument parsing, threads it through, and documents the flag in docs/CLI-TOOLS.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(#4569): exercise --sibling end-to-end through the real CLI Confirms --sibling joins afterPhase's parent decimal level rather than nesting, and falls back to nested when afterPhase has no existing decimal segment (no sibling level to join). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(#4634): demonstrate the two new drift detectors end-to-end via a planted violation The epic asks for the guard to be "demonstrated by watching it go red" on a reintroduced copy. The two new detectors (name-validity, shell-arith) had only unit-level fixture tests; mirrors the existing bracket-rule's planted-violation-in-a-temp-tree test for both, proving they're actually wired into scanRepo/scanMarkdownShellArith end-to-end, not just correct in isolation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4634): consolidate the drift guard's own owner-sanction-check logic Standards review flagged the "walk to nearest preceding non-blank line, check for a phase-id-owner comment" logic as duplicated across all four detector functions in a PR whose whole point is eliminating exactly that pattern. Extracts isSanctionedByPrecedingComment, shared by all four; behavior-preserving (verified: identical output before/after, same 7 known #4619 violations, zero token/bracket/name-validity). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4634): add Fixed changeset for the name-validity guard and allocation consolidation pr:0 placeholder — backfilled once the real PR number exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4126): consolidate branch-name slug substitution into one shared renderer cmdCommit (commands.cts) and cmdInitExecutePhase (init.cts) each independently implemented branch-name template substitution, and both substituted the literal string 'phase' when phase_slug was empty or undeliverable — producing a non-identifying branch name (gsd/phase-08-phase) that contradicted the honestly-reported phase_slug: null in the same payload. Same structural defect as the other three gaps in this epic: two consumers reimplementing one concept independently instead of sharing an owner. Adds renderPhaseBranchName (src/phase-id.cts) as the sole owner: a real slug substitutes normally; an empty/undeliverable one drops the {slug} token plus one adjacent separator (collapsing/trimming the result) rather than substituting a placeholder word, for the shipped default template and any user-configured shape alike. Both call sites now delegate to it; the old inline duplicates are deleted, not kept in sync. {project} substitution stays a separate step in init.cts, unchanged, since it is a config-level field with its own fallback contract. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(#4126): pin renderPhaseBranchName and both migrated call sites Property-based coverage for the shared renderer's degrade-path invariant (output, when non-null, never contains {slug} and never starts/ends with a separator), plus example coverage for real-slug substitution, empty/null/ non-string slug, token position at either edge, a doubled-separator template, and the only-{slug} -> null case. One regression test each in commands.test.cjs and init.test.cjs confirms a phase with no derivable slug no longer produces a branch name ending in the literal '-phase'. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: route scanExistingDecimalPhaseNumbers through the canonical enumeration owner Caught by an actual gsd-test run, not a hypothesis: the new decimal-scan helper (fix(#4569)) enumerated phases/ directories via a raw fs.readdirSync, which the pre-existing phase-enumeration drift guard (#3185/#3882) correctly flags as an unsanctioned re-derivation outside its canonical owner (listAllPhaseDirs / isSentinelPhaseId). Ironic given this epic's own thesis, and exactly why the guard exists: consolidating one seam can reintroduce drift in an adjacent one if the new code doesn't route through what's already there. Migrates the enumeration to listAllPhaseDirs; identical decimal-detection output for every existing case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4634): extend the drift guard for branch-slug fallback; fix a real regex bug Adds the fourth detector the epic's ratchet section names ("both branch-name sites"): bans a `.replace('{slug}', ... || 'phase')` call outright, sanctioned via renderPhaseBranchName or a dedicated comment. Wired into scanRepo (no per-file exemption — this is a banned anti-pattern everywhere, not a grammar with one legitimate owner). Now that #4126's fix (prior commit) has landed, scanRepo reports zero violations across all four .cts-scanning rules, restoring the simple "the live repo is clean" assertion instead of a pinned-known-count characterization. Also fixes a real bug an actual gsd-test run caught: findNameValidityDrift's regex didn't tolerate the doubled-backslash template-string form its own test claimed to cover (0 !== 1) — widened to \{1,2} matching TOKEN_DRIFT_RE's existing tolerance for the same two forms. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#4126): document the {slug} degrade behavior; update changeset for the full seam Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: detectPhaseNumberFromFiles wrongly rejected bare, slug-less phase directories Caught by an actual gsd-test run on the #4126 regression test, not a hypothesis: a bare phase directory with no slug remainder (e.g. .planning/phases/01/) has extractPhaseToken correctly return "01" — which is simply identical to the directory name in that case, not its no-match fallback. A stale `token !== phaseDir` check treated that equality as "no numeric token found" and rejected it regardless, leaving phaseNum null and silently skipping cmdCommit's phase-branching block entirely (the commit proceeded on whatever branch was already checked out instead of the phase branch). phaseTokenShape.test(normalized) already excludes every genuine non-phase case on its own: extractPhaseToken's real no-match fallback only fires for a dirName that doesn't start with a digit or short letter+digit prefix, and normalizePhaseName's leading-\d+ requirement rejects those regardless. The equality check was redundant for real rejections and actively wrong for bare-numeric directories. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: backfill changeset PR number to 4640 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Tom Boucher
|
75ee7b0214 |
enhance(#4273): add phase.tdd-applicable single-owner predicate (#4277)
* enhance(#4273): add phase.tdd-applicable single-owner predicate One query verb computes TDD-applicability for a plan (CLI flag, plan type: tdd frontmatter, a task's tdd="true" attribute, or the workflow.tdd_mode config default), mirroring phase.mvp-mode's precedence-cascade shape. Foundation for epic #4272 Phase 2, which wires both dispatch backends to consume it instead of restating the predicate independently. Also fixes workflow.tdd_mode, workflow.research, and workflow.nyquist_validation, which never reached cmdInitExecutePhase/cmdInitPlanPhase/cmdInitDebug/cmdInitNewMilestone because loadConfig() never populates config.workflow — a dead accessor found while wiring this verb's own config read, fixed inline per the no-defer rule rather than left alongside it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#4273): document phase.tdd-applicable's FEATURES.md entry Add a docs/features/ fragment for the new phase.tdd-applicable query verb and regenerate docs/FEATURES.md. docs/COMMANDS.md is left untouched: it documents /gsd-* slash commands only, and the sibling verb phase.tdd-applicable mirrors (phase.mvp-mode) has no formal CLI reference entry anywhere in docs/ either -- only inline prose mentions in docs/reference/workflow-fragments.md -- so there is no COMMANDS.md precedent to extend. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4273): use PHASE_NOT_FOUND reason code, remove try/finally from tests Two orthogonal code reviews flagged a mistyped error reason and a CONTRIBUTING.md-banned try/finally pattern in the phase.tdd-applicable change; both are corrected here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4273): stop whitelisting capability-owned config keys centrally workflow.tdd_mode, workflow.research, and workflow.nyquist_validation are each already owned by their own first-party capability's federated config schema (the tdd/research/nyquist capabilities declare them under their own capability.json `config`), resolved via isCapabilityConfigKey. Adding them to gsd-core/bin/shared/config-schema.manifest.json's central validKeys, as the prior commit in this branch did (mirroring workflow.mvp_mode, which genuinely is central-only), declares the same key in two places at once. That collision breaks capability-loader.cts's loadRegistry composition: gsd-test caught this as 84-85 unrelated failures across capability-cli/capability-command-dispatch/capability-lifecycle test files, every one showing "unknown capability: <id>" for a freshly-installed third-party capability that should have resolved fine. Verified directly (not asserted): reverting only this file, keeping the config-loader.cts tdd_mode/research/nyquist_validation flattening and the init.cts call-site fixes from the prior commit, and re-running the exact capability install + capability set repro from tests/capability-cli.test.cjs's "issue-2322" test locally reproduces the failure with the whitelist entries present and clears it without them. loadConfig() still surfaces all three flattened values correctly with no central whitelist entry (confirmed directly against the compiled module) — the whitelist additions were never required for the #4273 fix to work; they were an incorrect over-application of the mvp_mode precedent to keys that aren't central. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#4273): use getNested for tdd_mode (no legacy top-level fallback), allowlist new test file Both fixes address defects found by a gsd-test bench run: tdd_mode routed through get() invented an undocumented top-level alias that silently outranked the canonical workflow.tdd_mode key, and the new phase-tdd-applicable test file was missing from the file-count allowlist. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#4273): backfill changeset PR number Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Rezolv
|
0997d4f443 |
fix(#2620): inject the reference DispatchLogger on the live dispatch seam when observability is enabled (#2621)
* fix(#2620): inject the reference DispatchLogger on the live dispatch seam when observability is enabled The Command Routing Hub defaulted to createNoOpLogger and no caller ever injected createDefaultLogger, so GSD_AUDIT=1 wrote nothing and failed dispatches emitted no structured JSON to stderr — contradicting ADR-0174 §5/§6, CONTEXT.md's Dispatch Observability Module contract, and docs/CONFIGURATION.md. Inject the reference logger at both live createHub() sites, gated on the existing opt-in signal (newly exported isAuditEnabled). When observability is off no logger is injected, so the Hub keeps its no-op fallback and default output stays byte-for-byte identical. Enabling stderr-on-error unconditionally adds a second line to the --json-errors envelope that callers parse as exactly one JSON line, so that is deferred to its own increment under #2619. * chore(#2620): add changeset for the dispatch logger wiring fix * test(#2620): cover the phase seam and drop try/finally from the adapter test Two review findings from the #2621 round-1 review. The fix wires the logger at BOTH live createHub() seams, but only cjs-command-router-adapter was exercised. Adds a fail-first regression test for src/phase-command-router.cts:258 — verified RED against a tree with that hunk reverted (1 fail, exact assertion) and GREEN with it restored — plus a negative pin that no trace file appears when GSD_AUDIT is unset. The negative case passes pre-fix and is a pin, not fail-first. CONTRIBUTING.md:344 forbids try/finally inside test bodies; the new adapter test used it. Converted to the Pattern-2 t.after() form, switched to the centralized createTempDir helper, and removed the now-unused os require. * chore(#2620): scope the changeset to the activation path that actually ships The fragment claimed config.audit.enabled activates the audit trail. It cannot: both seams call isAuditEnabled() with zero arguments, so the config branch in _isAuditEnabled is unreachable from production, and src/config-schema.cts registers no audit key at all — a user setting it would be silently dropped. That string ships in the user-facing CHANGELOG. Scoped to GSD_AUDIT=1, which is what actually works. The missing schema key stays a disclosed deferred sub-defect on #2620. Also adds the (#2620) issue backlink the other fragments carry. * docs(#2620): correct the fork-leaked issue reference in the wiring comments Four files cited this fix as #26, the issue number from the fork where the change was first written. Upstream #26 is an unrelated closed SDK issue, and next already uses #26 with that meaning in src/validate.cts:17,29,42 and src/config.cts:474, so these references pointed somewhere real and wrong rather than merely dangling. Baked into permanent doc comments, they reach users compiled via the ADR-457 build-at-publish path. The changeset and tests/phase-command-router.test.cjs already cited #2620; this brings the remaining four files into line. Comment-only, no behaviour change. build:lib produces no generated drift. The rename is scoped to these four files so the pre-existing SDK #26 references in validate.cts, config.cts, health-validation.test.cjs and config.test.cjs are deliberately left untouched. --------- Co-authored-by: CI Rebase Check <ci@gsd-redux> |
||
|
Tom Boucher
|
0278329c3a |
fix(#2201): accept --phase N flag in the phase verb family (#2231)
* fix(#2201): accept --phase N flag in the phase verb family (complete, list-plans) The phase family router treated the first positional (args[2]) as the phase number, so `phase complete --phase 12` passed the literal '--phase' as the phase → 'Phase --phase not found'. The state family already accepted --phase N. Now complete and list-plans accept --phase N (and --phase=N) as well as the bare positional; unrecognized flags yield a usage error naming the accepted form. Closes #2201 * docs(#2201): add changeset fragment * docs(#2201): backfill PR number |
||
|
Tom Boucher
|
fa1ffb4824 |
fix(#1437): add phase.list-plans to gsd-tools (#1485)
* fix(#1437): add phase.list-plans to gsd-tools Register phase.list-plans in PHASE_COMMAND_ALIASES, implement cmdPhaseListPlans in src/phase.cts (uses findPhaseInternal + scanPhasePlans to return plan_count/has_plans/plans/phase_dir), and wire the handler in phase-command-router. Previously every call produced "Unknown phase subcommand". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(#1437): register new test file in lint-test-file-count allowlist Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(#1437): rename test to fix-NNN convention; update file-count allowlist Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
Tom Boucher
|
9223f2f4c8 |
feat(#247): runtime-neutral phase uat-passed predicate from HUMAN-UAT results (#1063)
* feat(#247): runtime-neutral phase uat-passed predicate from HUMAN-UAT results Wire the already-reserved `phase.uat-passed` alias (subcommand `uat-passed`, mutation:false) into the phase command router with a new markdown-aware predicate that evaluates HUMAN-UAT results and reports pass only when every required check passes. Post-SDK-retirement (ADR-0174/#174) successor to the SDK-framed #70, with no SDK-specific API surface. New pure module src/uat-predicate.cts: - stripFalsePositiveContexts: frontmatter -> HTML-comment -> CommonMark-style fenced-block state machine (tracks delimiter char+length) -> blockquote, each a small composable step, so a `result: passed` inside frontmatter, a fenced/~~~ block (incl. ~~~ nested in a ``` fence), a comment, or a blockquote is never counted. - parseUatResultItems: heading-block parser, column-0-anchored same-line result; a heading with no result -> `missing` (fail-closed). - analyzeMarkdown: unterminated fence/comment detection (malformed -> blocker). - evaluateUatPassed: allowlist pass/verification semantics; passed = no blockers && >=1 check && all passing; no_uat_artifacts discriminator (no vacuous pass); optional requireVerification policy hook. Thin cmdPhaseUatPassed handler in phase.cts; router closure rejects unknown flags via makeInvalidArgs. Hardened across two Codex adversarial passes (vacuous pass, dropped failing tests, permissive verification status, nested-fence escape, cross-line result value, masked unterminated comment) — all fixed fail-closed. New unit + CLI-integration suites incl. a fast-check property test; docs, CONTEXT glossary, inventory, and changeset updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#247): backfill changeset PR number (#1063) * fix(#247): indexOf paired-scan for unterminated-comment detection CodeQL js/incomplete-multi-character-sanitization (high) flagged the `raw.replace(/<!--[\s\S]*?-->/g,'')`-then-`.includes('<!--')` detection in analyzeMarkdown as incomplete sanitization (a single regex pass can leave a residual `<!--`). Replace it with a paired left-to-right indexOf scan that contains no `.replace()` of the comment token — CodeQL-clean and strictly more correct (a closed earlier comment can never mask a later unterminated one). Behaviour unchanged; 98 predicate tests + scoped docker run green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Tom Boucher
|
df04aae5e4 |
enhancement(#537): migrate all hand-written bin/lib/*.cjs to TypeScript source of truth (ADR-457) (#602)
* enhancement(#537): migrate code-review-flags to TS source of truth Collapse the hand-written get-shit-done/bin/lib/code-review-flags.cjs to a TypeScript source of truth (src/code-review-flags.cts), compiled by tsc to a gitignored .cjs build artifact at the same path, per ADR-457 (build-at-publish). Second module after the semver-compare pilot (#541). Behaviour is preserved byte-for-behaviour (characterization test added in tests/code-review-flags.test.cjs locks the parser quirks). Adds compile-time type checking: CodeReviewFlags interface + CodeReviewWorkflow literal union. The require() path is unchanged, so code-review.md and the bug-3727 test keep working. The emitted .cjs is gitignored and eslint-ignored, mirroring the pilot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 9 leaf bin/lib modules to TS source of truth ADR-457 build-at-publish, batch 1 (pure leaf modules, 0 sibling-deps): 001-legacy-orphan-files, context-utilization, redaction, artifacts, command-arg-projection, clock, ui-safety-gate, review-reviewer-selection, clusters. Each moves to src/*.cts (strict TS, typed), compiled by tsc to a gitignored .cjs at the same require() path; behaviour preserved byte-for- behaviour. Adds src/node-globals.d.ts (minimal ambient shim; "types":[]). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#537): add @types/node, drop hand-rolled node-globals shim ADR-457 migration infra: replace the temporary src/node-globals.d.ts ambient shim with @types/node@22 + "types":["node"] in tsconfig.build.json. Unblocks migrating the ~49 remaining bin/lib modules that use node:fs/path/os/ child_process. Build + full suite (3030 pass) + lint all green; no .cts type changes were needed (real Node types matched the shim). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 9 more bin/lib modules to TS (batch 2) ADR-457 build-at-publish. Clean leaves: installer-migration-report, prompt-budget. Type-error-prone leaves (were tsconfig.lint-excluded; now strict-typed and removed from that exclude list): secrets, phase-lifecycle, workstream-name-policy, decisions, validate, schema-detect. Plus runtime-name-policy. Strict type fixes narrow unknown->concrete domain types (no any/ts-ignore); behaviour preserved. Full suite green, lint 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate runtime-slash to TS (cross-import proof) ADR-457. First cross-module TS->TS import: src/runtime-slash.cts imports ./runtime-name-policy.cjs and tsc resolves the sibling .cts types under strict (no declaration files; NodeNext .cjs->.cts mapping), emitting a correct require("./runtime-name-policy.cjs"). Confirms the recipe for coupled modules, which must be migrated in dependency order (leaves-up). Suite green, lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 10 more bin/lib modules to TS (batch 3) ADR-457 build-at-publish, Wave-1 leaves: event, workstream-inventory-builder, plan-scan, fallow-runner, project-root, installer-migration-authoring, update-context, 000-first-time-baseline, runtime-homes, model-catalog. Strict typing fixed real issues (narrowing unknown, qualified fs/path calls, removed unnecessary casts); plan-scan/project-root/workstream-inventory-builder dropped from tsconfig.lint exclude. Behaviour preserved; suite green, lint 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 5 large Wave-1 leaves to TS (batch 4) ADR-457 build-at-publish: configuration, state-document, shell-command- projection (42 dependents), security, command-aliases. shell-command- projection keeps a namespace child_process import for mock-intercept testability. loadConfig/migrateOnDisk emit synchronously (every caller uses them sync; the one awaited migrateOnDisk caller tolerates a non-Promise) — full suite (3030 pass) confirms behaviour preserved. configuration/ state-document/command-aliases dropped from tsconfig.lint exclude. Also fixes the malformed batch-3 changeset frontmatter (type/pr) that failed lint:docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 6 Wave-2 modules to TS (batch 5) ADR-457 build-at-publish: config-schema, model-profiles, 002-codex-legacy-hooks-json, logger, active-workstream-store, adr-parser. First batch importing already-migrated siblings (configuration, model-catalog, shell-command-projection, redaction, security) via ./sibling.cjs specifiers. Strict type narrowing (typeof guards over String(unknown)); behaviour preserved; suite 3030 pass, lint 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 5 large Wave-2 modules to TS (batch 6) ADR-457 build-at-publish: graphify, install-profiles, intel, installer-migrations, worktree-safety. installer-migrations preserves its dynamic require() loader for numbered migration modules (scoped lint suppressions). Strict typing (typeof guards over String(unknown)); behaviour preserved; suite 3030 pass, lint 0 errors. Wave 2 complete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate Wave-3 modules to TS (batch 7) ADR-457 build-at-publish: planning-workspace, runtime-artifact-layout, command-routing-hub, drift. Uses `import x = require()` for export= siblings; drift's lazy require of runtime-slash hoisted to a top-level import (verified non-circular). Behaviour preserved; suite 3030 pass, lint 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate small Wave-4 modules to TS (batch 8) ADR-457 build-at-publish: cjs-command-router-adapter, phase-command-router, surface, roadmap-upgrade. Typed the hub router handler results as the HubResult discriminated union; surface drops 4 genuinely-unused imports. Behaviour preserved; suite 3030 pass, lint 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate core hub (2.5k LOC, 68 dependents) to TS (batch 9) ADR-457 build-at-publish: get-shit-done/bin/lib/core.cjs -> src/core.cts, preserving all 63 exports via export=. All sibling deps already migrated (shell-command-projection, model-profiles, model-catalog, worktree-safety, planning-workspace, project-root, configuration, config-schema). Strict types, no any/ts-ignore; config-schema lazy require hoisted (non-circular). Behaviour preserved (independently verified: core's shard 3030 pass / 0 fail). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#537): make ESLint-coverage + test-sprawl checks migration-aware #551 test hardcoded 12 now-migrated modules as "hand-written, must be linted"; that invariant is obsoleted by the ADR-457 migration. Rewrite it to a filesystem-driven invariant that holds at every stage: a bin/lib/*.cjs must be eslint-ignored IFF it has a src/*.cts source (tsc-generated), else linted (covers package-identity, which has no TS source). Also eslint-ignore config-types.cjs (has a src counterpart) and drop the redundant tests/clock.test.cjs (clock already covered by clock-seam + bug-474 tests), which tripped the lint-test-file-count ratchet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 9 Wave-5 router/inventory modules to TS (batch 10) ADR-457 build-at-publish: phases/verify/init/agent/task/validate/roadmap/state command routers + workstream-inventory. Router handler results typed against core's exported shapes; behaviour preserved (caught+fixed a --verify boolean flag regression mid-migration). Full suite green across all shards (only the 4 local gpg-env changeset-notes failures remain; CI passes them). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 7 Wave-5 modules to TS (batch 11) ADR-457 build-at-publish: gap-checker, docs, check-command-router, frontmatter, learnings, gsd2-import, profile-pipeline. Behaviour preserved; full suite green across all shards (only the 4 local gpg-env failures remain). Also broadens atomic-write-coverage.test.cjs to accept the tsc-compiled namespace-import form while still asserting platformWriteSync is called (safety guard intact). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate config + profile-output to TS (batch 12) ADR-457 build-at-publish: config (729 LOC), profile-output (1142 LOC). All exports preserved; cmdMigrateConfig de-asynced (migrateOnDisk is sync, awaited caller tolerates it). Behaviour preserved; suite green across all shards (only the 4 local gpg-env failures). Wave 5 complete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate 5 Wave-6 modules to TS (batch 13) ADR-457 build-at-publish: template, uat, workstream, roadmap, audit. Behaviour preserved (dead toPosixPath import dropped from audit; inline requires hoisted). Suite green across all shards (only the 4 local gpg-env failures). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate commands + state hubs to TS (batch 14) ADR-457 build-at-publish: commands (1305 LOC), state (2074 LOC, 17 dependents). All exports preserved; inner requires kept non-hoisted where load-order matters (install.js, per-call security); acquireStateLock cast inlined to preserve the err.code source token a structural test inspects. Behaviour preserved; suite green across all shards (only the 4 local gpg-env failures). Wave 6 complete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate milestone to TS (batch 15a, hand-authored) ADR-457 build-at-publish: milestone -> src/milestone.cts. Authored directly (subagent capacity was unavailable). Also relaxes core.output()'s 3rd param to optional, matching its real always-optional call contract (unblocks remaining 2-arg output callers). Behaviour preserved; suite green across all shards (only the 4 local gpg-env failures). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537): migrate phase, verify, init to TS (batch 15, final modules) ADR-457 build-at-publish, Wave 7 (the last hubs): phase (1608 LOC), verify (1615), init (2113). Adds src/package-identity.d.cts so verify can import the permanently value-baked package-identity.cjs under strict TS. Fixes two regressions the migration introduced in verify: restore cmdValidateHealth's `return result` (callers/tests read result.warnings — it is NOT side-effect-only), and make the bug-3384 source-pattern test tolerant of the tsc-compiled bracket-notation form of the git_list_failed->W020 branch (behaviour intact). Full suite green across all shards (only the 4 local gpg-env failures); lint 0 errors. All 86 migratable bin/lib modules are now TypeScript sources. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#537): finalize ADR-457 migration — retire tsconfig.lint.json All hand-written bin/lib/*.cjs are now src/*.cts sources, so the checkJs stopgap tsconfig.lint.json (unused; not wired into eslint, scripts, or CI) is deleted per ADR-457's final step. Also gitignore the tsc-generated config-types.cjs (was still committed) for consistency with every other emitted artifact. package-identity.cjs stays value-baked (declared via src/package-identity.d.cts). Suite green; #551 ESLint-coverage test green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#537): add prepare script so unpacked/git installs build bin/lib artifacts ADR-457 build-at-publish: bin/lib/*.cjs are now gitignored, built by tsc. The prepack/prepublishOnly hooks cover `npm pack`/publish, but `npm install -g <dir>` and git installs run the `prepare` lifecycle — which was missing — so the unpacked install shipped without the compiled .cjs and failed at startup with "Cannot find module './lib/core.cjs'" (caught by the smoke-unpacked CI job). Add `prepare` mirroring prepublishOnly (build:lib + build:hooks). prepare does NOT run for registry consumers (they get the pre-built tarball), only for source/local/pack installs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#537): make CI build/lockfile checks work with gitignored bin/lib artifacts ADR-457 build-at-publish exposed two CI assumptions that bin/lib/*.cjs are always present on disk: - check:env's lockfile-sync ran `npm ci --dry-run`, which now triggers the `prepare` build (tsc) — but it runs before deps are installed, so tsc is absent and it misreported the lockfile as out of sync. Add --ignore-scripts (a lockfile check must not build). - the lint-tests job installs with --ignore-scripts (no prepare build), but lint:skill-deps require()s the built install-profiles.cjs. Add an explicit `npm run build:lib` step after install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#537): narrow prepare to build:lib only (unbreak packed-smoke pack step) prepare running build:hooks emitted "✓ Copying ..." stdout during `npm pack`, which the install-smoke "Pack root tarball" step captures into $GITHUB_OUTPUT — breaking it with "Invalid format". build:lib (tsc) is silent on success and is all the unpacked/source install needs (the smoke-unpacked assertions exercise gsd-tools, i.e. bin/lib, and tolerate hook setup with `|| true`). Matches prepack. build:hooks still runs on prepublishOnly for real publishes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#537): wire Stryker mutation gate to build-at-publish layout The gate scored 0.00 because it mutated changed bin/lib/*.cjs that (a) were generated artifacts and (b) included modules with no coverage in the command's test set. Rework: mutation.yml now derives changed COVERED modules from src/*.cts and maps them to their built bin/lib/*.cjs; Stryker mutates those built artifacts with a no-rebuild command (mutating src/*.cts + per-mutant tsc was ~3x over the 30-min CI budget). NOTE: with the gate now correctly measuring the covered modules, their actual mutation score is 42.94% (< break 50) — a pre-existing test-coverage gap (adr-parser/prompt-budget/etc.), not introduced by this behaviour-preserving migration. Reaching 50 needs more tests, a threshold/scope change, or a waiver — a maintainer decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#537): raise mutation coverage of covered modules above the 50 gate Adds focused example-based unit tests that kill surviving mutants in the two lowest-scoring covered modules: - tests/prompt-budget.unit.test.cjs (112 tests): 17.9% -> 97.9% - tests/adr-parser.unit.test.cjs (205 tests): 44.7% -> 89.4% Both wired into stryker.config.mjs's command. Fresh full run over the 6 covered modules now scores 82.25% (>= break 50); every covered module is >= 68%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * enhancement(#537,#609): parallelize mutation gate via dynamic per-module matrix The serial Stryker run timed out at 30 min once the migration's added tests made every mutant re-run ~300 tests. Replace it with a dynamic matrix so the gate completes well under budget — folded into this PR (was tracked as #609) because it's a prerequisite for this PR's mutation gate to pass. - scripts/mutation-matrix.cjs: single source of truth (covered-module -> test files) computing changed covered modules from git diff -> {has_work, matrix}. - mutation.yml: detect -> dynamic `matrix: fromJSON(...)` mutate job (one parallel shard per changed module, scoped via MUTATION_TEST_CMD to only that module's tests, 15-min/shard) -> summary job that KEEPS the legacy check name "Stryker mutation score (changed files only)" so branch protection is unchanged. Per-shard jobs report as "Stryker (<module>)". - stryker.config.mjs: commandRunner.command reads MUTATION_TEST_CMD (falls back to the full command locally). Closes #609. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#537,#609): give each mutation shard ≥50% on its own tests; drop blacksmith note Per-module sharding revealed that active-workstream-store (46.5%) and frontmatter (7.4%) only cleared 50% in the old serial run via timeout-noise from the bloated 300-test command; on their own tests they were below the gate. Add focused unit tests: - tests/active-workstream-store.unit.test.cjs (115 tests): 46.5% -> 81.9% - tests/frontmatter.unit.test.cjs (165 tests): 7.4% -> 63.4% Both wired into scripts/mutation-matrix.cjs (per-module test map) and stryker.config.mjs DEFAULT_TEST_CMD. All 6 covered modules now clear break:50 with only their own tests (config-schema/context-utilization/prompt-budget/ adr-parser already did). Also removes the leftover blacksmith TODO comment — GitHub-hosted runners only; speed comes from parallel per-module shards. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#537,#609): strengthen prompt-budget tests to clear the gate on its own tests prompt-budget scored 39.58% when mutation-tested with ONLY its own tests (the way the per-module CI shard runs it) — an earlier ~98% reading was inflated by accidentally running the full multi-module command. Add 96 targeted tests to tests/prompt-budget.unit.test.cjs (exact note-template text, plan-truncation arithmetic/percentages, drop-block strings, noteInjected/hardFailed booleans): scoped score 39.58% -> 68.75% (>= break 50). All 6 covered modules now clear the gate on their own tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |