8fba67a674b3410fdd249d59f984bc9ade04e4e4
1243 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tom Boucher
|
8fba67a674 |
fix(#3303): correct code_review_depth doc; add agent_skills array form (#3449)
* fix(#3303): correct code_review_depth doc; add agent_skills array form * fix(#3303): backfill changeset pr field --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
f78a4b3b80 |
fix(#3275): resolve reviewer lane binaries via shared pathext scan (#3445)
* fix(#3275): resolve reviewer lane binaries via shared pathext scan * fix(#3275): pin changeset fragment to pr 3445 * fix(#3275): stage p4 resolver fixture in pathext casing --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
2537c286f9 |
refactor(#1762): clarify verification-missing/unknown routing text (#3439)
* test(#1762): assert reassuring verification-routing wording (RED) Regression test for #1762 — asserts readVerificationStatus's missing/ unknown next_action text reassures the user that execute-phase resumes at the verification gates without redoing work, instead of reading as a blind re-run instruction. Fails against current wording; the fix lands in the next commit. * fix(#1762): reassure verification-missing/unknown routing text is safe to run readVerificationStatus's 'missing' and 'unknown' next_action text read as "redo the implementation", when execute-phase's own discover_and_group_plans step (#2868) already resumes at the verification gates and skips execute_waves/checkpoint_handling entirely when every plan already has a SUMMARY.md. Reword both to say so explicitly, and soften the 'unknown' message to acknowledge a non-standard status may be an intentional marker rather than presuming re-verification is always the fix. Also updates progress.md's Route V.missing / V.unknown to consume the dynamic $VERIFICATION_NEXT_ACTION (matching V.gaps/V.human) instead of a hand-duplicated string, so the reassurance lands there too without a second copy to keep in sync. No change to next_command routing, status values, or the isPhaseComplete completeness predicate (still gated on status === 'passed' per the #2957 DISK-STRICT decision) — advisory text only. * fix(#1762): drop duplicated reassurance clause in progress.md routing text Review finding: the SUMMARY.md reassurance was stated once via the interpolated $VERIFICATION_NEXT_ACTION and again as a parenthetical on the command line, in both Route V.missing and Route V.unknown. Trimmed the parenthetical so $VERIFICATION_NEXT_ACTION stays the single source of truth. * docs(#1762): add changeset fragment Type Changed with a docs-exempt marker — advisory routing text only, no docs page documents this specific message today. * test(#1762): acknowledge progress.md emitted-content growth Route V.missing/V.unknown now render a small block referencing ${VERIFICATION_NEXT_ACTION} instead of a bare command line, growing progress.md by 376 bytes. Deliberate per the differential attribution check (ADR-2719). * chore: drop spent progress.md ack from #3218's emitted-drift fragment The #3218 growth it described is already on next (inert, per the lint's own note that spent acks 'can no longer clear anything'). Its presence collided with the new #1762 fragment naming the same bare filename, which lint-emitted-drift-ack rejects outright ('two ack sources may never name the same path'). #3218's other two acks (plan-phase.md, plan-review-convergence.md) are untouched. * docs(#1762): backfill changeset PR number --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
5fff839713 |
fix(#3258): honor all field-classification preservation rows (#3447)
* fix(#3258): honor all field-classification preservation rows * chore(#3258): set changeset pr to 3447 --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
fd4715f80f |
fix(#3262): guard phase writes against milestone-scope headings (#3446)
* fix(#3262): guard phase writes against milestone-scope headings * fix(#3262): fill changeset pr with 3446 --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
8757080676 |
fix(#3263): warn in roadmap validate on truncated milestone window (#3444)
* fix(#3263): warn in roadmap validate on truncated milestone window * chore(#3263): backfill pr number in changeset --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
8ecbac0350 |
fix(#3189): drop non-id prose from phase_req_ids before gap check (#3438)
* fix(#3189): drop non-id prose from phase_req_ids before gap check * chore(#3189): pin changeset fragment to pr 3438 * fix(#3189): use hyphenated IDs in e2e fixtures (parseRequirements requires hyphen) * fix(#3189): assert e2e item set via table, not rows (check query has no rows) * fix(#3189): check prose fragments against table rows, not the table heading --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
dbbcb8f736 |
fix(#3191): anchor remaining diff-base greps, portably (#3437)
* fix(#3191): anchor remaining diff-base greps, portably The #2989 fix anchored only the Tier-3 grep, and did so with \b — not a POSIX ERE token, so on macOS regex(3) it silently matches nothing and Tier 3 always fails closed. spawn_reviewer's agent-context DIFF_BASE and the fallow structural pre-pass's --changed-since base each still ran the original unanchored --grep="${PADDED_PHASE}", whose oldest substring match is routinely a version-string/date commit from months before the phase existed — feeding the reviewer agent a bogus diff_base exactly when files: is empty, and widening fallow's changed-files scope. All three derivations now use the same anchored, POSIX-portable '[Pp]hase N([^[:alnum:]_]|$)' with --extended-regexp; spawn_reviewer also gains Tier-3's parent-exists guard so the two computations are the same algorithm. Behavioral regression tests execute the shipped bash extracted from the workflow files against a git fixture on every platform, so the macOS \b hole is covered, not just the Linux CI view. * chore(#3191): backfill changeset PR number 3437 * fix(#3191): scope fallow test snippet past the gsd-tools resolver The CI runners have no installed gsd-tools, so executing the resolver line that precedes FALLOW_SCOPE_ARGS in the extracted fence exits 1 before the derivation under test ever runs. Slice the snippet to start at FALLOW_SCOPE_ARGS=() — the resolver is orthogonal to the base derivation the regression test binds. --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
483083aea6 |
fix(#3194): verify source-grounded lane evidence from review output (#3436)
* fix(#3194): verify source-grounded lane evidence from review output * chore(#3194): fill changeset pr number --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
1d5d77951c |
fix(#3190): commit review.md in --auto loop; fix report env var (#3434)
* fix(#3190): commit review.md in --auto loop; fix report env var Three coupled defects in gsd-core/workflows/code-review-fix.md: - The --auto re-review loop overwrote REVIEW.md each iteration but the single docs commit staged only REVIEW-FIX.md, so the committed REVIEW.md stayed at iteration 1 and contradicted the committed REVIEW-FIX.md. The --auto commit now stages the converged REVIEW.md alongside REVIEW-FIX.md (guarded on AUTO_MODE; non-auto single-pass runs unchanged). - The two inline frontmatter validators (HAS_STATUS, FIX_FRONTMATTER) exported REVIEW_PATH into a node -e body that reads process.env. FIX_REPORT_PATH, so the status check was always empty and REVIEW-FIX.md was never committed. Both now export FIX_REPORT_PATH. - On successful convergence the spent .iterN.md backups are removed so the phase directory is clean; they are retained on degradation for post-mortem. Regression test: tests/code-review-fix-pipeline-regression.test.cjs. * chore(#3190): set changeset pr to 3434 --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
ff2d08d453 |
fix(#3193): tolerate attributes on plan-task child tags (#3433)
* fix(#3193): tolerate attributes on plan-task child tags * chore(#3193): add changeset * chore(#3193): set changeset pr to 3433 --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
0d4d78550f |
fix(#3188): null absent planning-doc paths in init phase queries (#3430)
* fix(#3188): null absent planning-doc paths in init phase queries The init phase-op / plan-phase / execute-phase queries emitted a non-null absolute requirements_path / state_path / roadmap_path even when the named file did not exist — built with a bare path.join and no existence check, unlike the conditional sibling fields (patterns_path, context_path, ...) in the same payload. Consumers (e.g. ultraplan-phase.md:104 'requirements_path is not null') therefore read missing files. Each of the three reading sites now returns null when the file is absent and its absolute path when present. The project/milestone-bootstrap and doc-ingest emitters that use these paths as write-targets for not-yet-created files are intentionally unchanged. * chore(#3188): backfill changeset pr: 3430 --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
a3b72d9071 |
fix(#3171): init execute-phase emits display name, not directory slug (#3429)
* fix(#3171): init execute-phase emits display name, not directory slug When a phase directory already exists on disk, the disk-lookup path (searchPhaseInDir) derived phase_name from the directory-name remainder -- itself an already-slugified value (phase.add writes ${num}-${slug} dirs) -- so phase_name and phase_slug came out byte-identical. The execute-phase workflow forwards phase_name into 'state begin-phase --name', which wrote that raw slug into STATE.md's current_phase_name on every phase start. cmdInitExecutePhase now prefers the ROADMAP's curated display name ('### Phase N: <Name>') for phase_name, matching the no-disk fallback path that already did this correctly. phase_slug is unchanged (it feeds branch-name construction). The state.begin-phase authoritativeFm override (#2821/#2736) is untouched; the correction is in the value fed into --name. The milestone_name half of #3171 was subsumed by #3216 / PR #3226; this fixes the remaining current_phase_name half. * docs(changeset): backfill pr 3429 for #3171 --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
4792bc02f5 |
fix(#3165): recover phase_count when scoped window is truncated (#3428)
* fix(#3165): recover phase_count when scoped window is truncated * chore(#3165): add changeset for roadmap.analyze phase_count recovery * chore(#3165): backfill PR number in changeset --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
7976b1ca0d |
feat(#1689): per-plan agent_hint executor routing (#3417)
* feat(#1689): per-plan agent_hint executor routing Option A per-plan specialist routing: a plan with an `agent_hint:` frontmatter field is dispatched to that subagent instead of gsd-executor when it resolves on the active runtime; absent/unresolved/disabled falls back to gsd-executor (byte-identical). Default-on via workflow.agent_hint_routing. - src/phase.cts: parse agent_hint into the plan-index JSON (plan_json.agent_hint) - agent-install-check.cts: resolveAgentHint() reuses getAgentsDir + runtime filename variants; probes project + global agent dirs; fails closed; rejects path-traversing names - gsd-tools.cjs: 'resolve-agent' query route (fail-closed to gsd-executor; --raw/--json) - execute-phase.md: lean per-plan reference + {EXECUTOR_TYPE} placeholder (host stays under the ADR-857 Phase 6 byte ceiling) - execute-phase/steps/per-plan-executor-routing.md: resolution logic (Agent()-based dispatch; advisory on orchestrator-worktree) - config: workflow.agent_hint_routing (validKey, default-on via SCHEMA_DEFAULTS, boolean validator) - docs (CONFIGURATION.md, plan-md.md), changeset, tests/agent-hint-routing-1689.test.cjs (17 tests) * chore(#1689): backfill changeset PR number (#3417) * chore(#1689): regenerate install-tree fixtures for new workflow fragment * chore(#1689): ack deliberate execute-phase.md growth (agent_hint routing) * test(#1689): SPAWN contract allows parameterized subagent_type placeholder agent-frontmatter's spawn-type checks scanned subagent_type="..." as a concrete agent name. execute-phase now uses subagent_type="{EXECUTOR_TYPE}" (a runtime placeholder resolved via resolve-agent, default gsd-executor). Skip {TOKEN} placeholders in both the known-type and <available_agent_types> checks; execute-phase still lists the built-in roster incl. gsd-executor. * fix(#1689): CI conformance for the routing fragment - per-plan-executor-routing.md: add the canonical runtime-launcher preamble to its gsd_run block (runtime-launcher-parity #373), matching sibling step fragments. - agent-install-check.cts: drop a literal ~/.claude/agents path from the resolveAgentHint JSDoc so it does not leak into the compiled engine .cjs (cline install leak guard). --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
470389f3a2 |
chore(#3212): tokenizer-first for stateful grammars — a shared scanner — Phase 3 (#3424)
* feat(#3414): promote git-cmd.js token-walk into a shared scanner, fix #3169 Phase 3 of epic #3212 (ADR-3212 §4). New src/token-scanner.cts generalizes hooks/lib/git-cmd.js's proven token-walk (#3129 — "has not re-opened"): tokenizeShellLike (quote-aware shell tokenizer, byte-identical port) and indentWidth (bullet-nesting depth). git-cmd.js migrates onto tokenizeShellLike with zero behavior change (parity-asserted against every existing #3129 fixture in tests/worktree-safety.test.cjs's folded block); isGitSubcommand's phases 1-3 (env-prefix skip, executable check, global-option consume) extracted into skipToSubcommand, shared with the new extractBranchArgument (git checkout -b / git branch <name>) — a new capability exercising the seam on the domain the ADR names, not a migration of existing duplicated logic (none existed). Fixes #3169: src/decisions.cts's parseDecisionLines couldn't distinguish a cross-reference bullet nested under an open decision from a fresh malformed declaration attempt. An earlier bold-run-content-classification design was tried and disproven against the repo's own existing FIX-B fixtures (D-02, "no colon no dash") before being adopted — both have identical shape under any content-only rule. Nesting depth (via indentWidth) is the actual distinguishing signal: a bullet indented deeper than the currently-open decision's own bullet is elaboration, folded into its text like a continuation line, never tested against the parse-miss guard. A bullet at the same-or-shallower indent is unchanged. Scope-narrowing disclosed, not silent: of the ADR's four named bugs (#3197, #3169, #2570, #2528), three no longer need this phase's work. were independently fixed and closed since the ADR was authored — #2570's fix is already a correctly-bounded regex per the ADR's own decidability test (no scanner needed); #2528's fix is a deliberate, twice-reviewed non-scanner design (its own code comment records a scanner-based attempt that regressed a symmetric case and was reverted) that this phase does not disturb. Only #3169 required new work. get_impact: isGitSubcommand CRITICAL/196 affected symbols, parseDecisionLines CRITICAL/164 affected symbols (ADR §6 due diligence). Six-gate ripple: .gitignore, eslint.config.mjs, docs/INVENTORY.md, docs/INVENTORY-MANIFEST.json (regenerated), CONTEXT.md glossary. Design: .gsd/phase/chore-3414-tokenizer-first-seam/40-design.md Test matrix: .gsd/phase/chore-3414-tokenizer-first-seam/50-test-matrix.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3414): add required fast-check property tests per code review TESTING-STANDARDS.md:169 requires at least one fast-check property test for any module that implements parsing — src/token-scanner.cts had none, an orthogonal Standards-axis review finding. Adds two seeded property tests (mirroring Phase 1/2's fast-check-setup.cjs convention): indentWidth counts exactly a generated leading-space run; tokenizeShellLike round-trips a generated array of whitespace/quote-free words joined with single spaces. The design doc's own "no property test needed" rationale was wrong — it argued no algebraic law applied, but the standard is unconditional for parsing modules regardless of whether one "feels" applicable. Corrected in .gsd/phase/chore-3414-tokenizer-first-seam/50-test-matrix.md. Also fixes two Spec-axis wording drifts the same review found between the design doc and the shipped code (doc-only, no behavior change): extractBranchArgument's documented signature dropped an unused subVariants parameter that was never implemented, and the #3169 fail-first fixture description corrected from "15-decision plan via cmdDecisionCoverageVerify" to the actual compact 3-decision analog via the real blocking gate, check.decision-coverage-plan. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#3414): add changeset for #3169 fix Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#3414): backfill changeset pr number to 3424 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
|
b9adedbc86 |
fix(#3151): stop emitting effort: into skill frontmatter (cache invalidation) (#3425)
Claude Code applies SKILL.md effort: as output_config.effort; any change from the session baseline invalidates the prompt cache at BOTH scope boundaries (entry + exit, the latter often machine-fired via subagent-completion notification). The reporter's owned measurement confirms it: /gsd-progress (effort:low) in a medium session → cache_creation 63,404 (entry) + 18,589 (exit), while a no-effort skill shows none. ~76% of invocations paid in full. Fix (trek-e AC#2/AC#4): convertClaudeCommandToClaudeSkill no longer emits effort: into Claude-runtime skill frontmatter (src/runtime-artifact-conversion.cts + duplicated bin/install.js). normalizeClaudeSkillEffort removed (dead). The six declaring skills (plan-phase/execute-phase/autonomous/next/progress/stats) no longer carry effort. Source command files keep effort (input, used elsewhere); the separate agent-effort surface (#3160) is untouched. Tests: install-runtime-artifacts #769 block flipped to assert effort is ABSENT from installed SKILL.md + converter output (the AC#4 behavioral coverage). Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
d30c99bc92 |
chore(#3421): delete orphan verify-phase workflow, migrate live gates to verifier (#3422)
* chore(#1892): delete orphan verify-phase workflow, migrate live gates to verifier reference * test(#1892): retarget structural suites from verify-phase.md to verifier-phase-gates.md * chore(#1892): reword retired-workflow mentions for removed-but-needed lint * test(#1892): correct stale surface labels in retargeted suites * docs(#1892): add verifier-phase-gates row to locale inventories * chore(#3421): backfill changeset pr number --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
b77b7f8e56 |
fix(#1526): delegate auto-chain post-completion to transition workflow (#3419)
* fix(#1526): delegate auto-chain post-completion to transition workflow execute-phase's auto-chain completion called phase.complete then a light inline set (partial PROJECT.md update + offer-next) and never invoked the transition workflow, silently skipping graduation scan, session-continuity, project-reference, accumulated-context, and current-position updates — so a phase completed via auto-chain left different project state than a normal transition. Fix (delegate, user decision 2026-08-13): replace execute-phase's update_project_md + offer_next with a delegation step that @-includes transition.md in post-completion mode. Add a post_completion_mode step to transition.md that skips verify_completion + update_roadmap_and_state (phase.complete already ran; avoids double-write) and begins at evolve_project. Standalone transition (mode 1) is unchanged. Regression: tests/auto-chain-transition-delegation.test.cjs (source-text-is-the- product) asserts the delegation, the skip-set, the removed inline step, and the mode. Ack fragment 1526 covers execute-phase.md + transition.md growth (spent 2930 fragment removed — same-path owner conflict, like #3025/#3024). * docs(#1526): backfill changeset PR number (#3419) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
0624c5da6f |
chore(#3212): src/text-lines.cts is the sole owner of line-terminator handling — Phase 2 (#3420)
* test(#3413): failing-first suite for the line-terminator seam Phase 2 of epic #3212 (ADR-3212 §3/§6/§7). Tests only — src/text-lines.cts does not exist yet, so tests/text-lines.test.cjs fails with MODULE_NOT_FOUND at its require line, which is the intended RED. The frontmatter.test.cjs additions drive #3360 (confirmed-bug) fail-first: parseMustHavesBlock currently returns [] for every must_haves block on a CRLF-authored plan file, because \r is its own LineTerminator in ECMAScript and two /m-anchored \s* patterns can absorb it, inflating a captured indent by one character and tripping the "not nested under must_haves" guard. Verified locally against the current (unfixed) compiled module: both the direct repro and the silent-exit "blank line before must_haves:" variant return [] today. A parity property test (crlf vs lf must deep-equal for every block name) matches a pattern this maintainer has required repeatedly for prior CRLF fixes in this codebase (Cortex-recorded, verify_intent=held). The no-crlf-fragile-split.rule.test.cjs additions lock the eslint rule's future fix-hint text (pointing at splitLines()) and its self-reference non-violation (the seam's own correct \r?\n split must never flag itself). Design: .gsd/phase/chore-3413-text-lines-seam/40-design.md Test matrix: .gsd/phase/chore-3413-text-lines-seam/50-test-matrix.md * chore(#3413): src/text-lines.cts owns line-terminator handling Phase 2 of epic #3212 (ADR-3212 §3/§6/§7). Adds splitLines/normalizeEol/ detectEol/joinLines and migrates frontmatter.cts onto it. parseMustHavesBlock (#3360, confirmed-bug) returned [] for every must_haves block on a CRLF plan file. Root cause: \r is its own LineTerminator in ECMAScript, so under /m two \s*-anchored indentation lookups could match at the position INSIDE a \r\n pair and absorb the terminator, inflating the captured indent by one character and tripping the "not nested under must_haves" guard. Two silent exits, one with a diagnostic and one without (a blank line before must_haves: hits the silent path). Fixed by converting both lookups from a whole-string /m match to split-then-scan — splitLines first, then a per-line, non-/m match — the same structural pattern parseYamlRegion (30 lines away in the same file) already used safely. Nothing downstream of the two lookups changed; blockLines is now sliced from the already-split array instead of re-splitting a substring, but its contents are unchanged for LF input, and the per-line dash/kv parsing loop is untouched. A parity property test (CRLF and LF plans parse to identical must_haves for every block name) matches a pattern this maintainer has required repeatedly for prior CRLF fixes in this file's neighborhood (Cortex: 7 recorded decisions, verify_intent -> held). frontmatter.cts's other .split(/\r?\n/) call sites (parseYamlRegion, isFrontmatterShaped, sliceTopLevelFrontmatterSegments, spliceFrontmatter) are rerouted onto splitLines — a literal 1:1 substitution, zero behavior change, since splitLines IS that same regex plus a type guard. The 4 scripts/normalizeLineEndings copies (gen-registry, gen-loop-host- contract, gen-capability-registry, gen-context-index) are deleted and rerouted onto normalizeEol, which strips a bare unpaired \r exactly like the deleted copies did (not just \r\n pairs) -- verified against each script's own --check mode against its real generated output. local/no-crlf-fragile-split widens from tests/ to src/**/*.cts, with its fix-hint message now naming splitLines() instead of the raw regex -- the prohibition finally has a primitive to point at. Detection logic unchanged in this phase (deliberate scope limit, see design doc Known limits: the rule doesn't yet recognize safeReadFile/platformReadSync as a content source, and has no detector for the \s-adjacent-to-anchor shape that is #3360's actual mechanism -- the CLASS is converged by the direct fix + regression test regardless). joinLines/detectEol are NOT wired into frontmatter.cts's own write path (cmdFrontmatterSet/Merge -> platformWriteSync) -- verified that platformWriteSync already, unconditionally converts CRLF->LF on every .md write today as a pre-existing policy owned by a different module, and ADR-3212's backward-compatibility clause rules out a file-format change in any phase. Stated explicitly in Known limits rather than left for a reader to discover. Six-gate ripple: .gitignore, eslint.config.mjs (src/**/*.cts block), docs/INVENTORY.md + INVENTORY-MANIFEST.json (regenerated), CONTEXT.md glossary (Text Lines Module, mirroring Phase 1's Pattern Module entry). Design: .gsd/phase/chore-3413-text-lines-seam/40-design.md Test matrix: .gsd/phase/chore-3413-text-lines-seam/50-test-matrix.md * fix(#3413): fix 13 pre-existing CRLF-fragile splits the widened rule found Widening local/no-crlf-fragile-split from tests/ to src/**/*.cts (the previous commit) immediately surfaced 13 real, pre-existing violations across 10 files -- undetected until now because the rule never scanned src/. This is the exact defect class ADR-3212 exists to close, playing out again one phase after Phase 1 hit the same shape ("the new lint rule -- once live -- found 27 more"). Per CLAUDE.md's no-defer rule, fixed inline rather than deferred or suppressed; there is no established suppression convention for this rule in src/ and inventing one now would undermine the point of widening it. audit.cts, broken-windows.cts, core-utils.cts, init.cts, milestone.cts, phase.cts (x3), profile-output.cts, roadmap.cts (x2): bare-\n splits or regex character classes widened to \r?\n / [^\r\n], each following the same pattern already established migrating frontmatter.cts. phase-estimation.cts: `\r?(?:\n|$)` restructured to `(?:\r?\n|\r?$)` -- already semantically CRLF-safe, but the rule's lexical scanner doesn't recognize \r? guarding a group (only \r? immediately before a literal \n). Verified the two forms are equivalent across all four EOL/EOF cases before restructuring, not assumed. roadmap-upgrade.cts needed two coupled sites, not the one flagged line: computeMigrationPlan and applyMigration must agree on line representation for the lines[edit.lineIndex] === edit.from equality check to hold, and the write-back needed joinLines + detectEol -- a plain lines.join('\n') was silently flattening a CRLF ROADMAP.md to LF wholesale on every migration. This is the first real production consumer of joinLines/detectEol in this epic (frontmatter.cts's own write path doesn't use them -- see the previous commit's Known limits). Fixing the 13 flagged sites surfaced 4 more adjacent same-shape sites the rule doesn't track (.search() and new RegExp(dynamicString) aren't in its tracked call/construction set). Investigated each empirically -- hand-tracing this exact bug class already produced one wrong conclusion earlier in this phase (a detectEol design-doc arithmetic error), so these were verified with real CRLF fixtures rather than reasoned about on paper: - audit.cts (scanTodos): REAL bug, fixed. `bodyMatch.trim().split ('\n')[0]` leaked a trailing \r into a user-visible todo summary on CRLF input -- .trim() only strips the string's outer edges, not a \r sitting mid-string before the first bare \n. Now splitLines(...) [0]. - phase.cts (cmdPhaseInsert, bullet-style branch): REAL bug, fixed. [^\n]* in targetBulletPattern swallowed a line's trailing \r on CRLF input, shifting the computed insert position to land INSIDE the \r\n pair; combined with a hardcoded '\n' bullet separator, a CRLF ROADMAP.md ended up with a mixed CRLF/LF result after an insert. Fixed with two coupled changes (either alone still corrupts, verified both ways): [^\r\n]* in the pattern, and the new bullet's leading terminator now comes from detectEol(rawContent). - roadmap.cts (cmdRoadmapAnnotateDependencies phase-boundary scan): investigated, genuinely safe, left untouched. The .search(/\n#{2,4} .../) boundary-finder and the [^\n]*-based heading match were empirically verified on a 3-phase CRLF fixture -- the only stray \r ends up at the tail of an intermediate phaseSection string that is only ever used for .test()-based idempotency checks, never for an exact-match comparison or written back to disk. No corruption on round-trip. Every fix re-verified: npm run build:lib clean, npx eslint 'src/**/*.cts' --no-cache reports 0 problems (was 13), and each fixed function's existing LF-input tests were spot-checked unchanged. * fix(#3413): apply orthogonal review findings Two isolated review engines (correctness + security) ran against the full diff and found three majors, one real security issue, and several disclosure-worthy minors. All fixed or explicitly disclosed with evidence; nothing deferred. MAJOR — detectEol's tie-break contradicted its own documented contract. Code returned '\n' on a 1:1 crlf/bare-LF tie; every doc (design doc, CONTEXT.md, the function's own comment) says ties resolve to '\r\n'. The existing test masked this by reusing the same tie fixture the buggy code happened to satisfy, rather than a genuine LF-majority case. Root cause: an Edit attempted earlier in this phase to fix this exact arithmetic error was blocked by the tier guard, and a later dispatch was incorrectly told it had already landed. Fixed: condition is now crlfCount >= bareLfCount; the test fixture corrected to a genuine 2:1 majority, with a new explicit tie-case test. MAJOR — phase.cts's cmdPhaseInsert built an EOL-aware bulletEntry via detectEol(rawContent), justified by a comment claiming a hardcoded '\n' corrupts a CRLF ROADMAP.md. False: this write goes through platformWriteSync, whose normalizeContent/_normalizeMd unconditionally converts CRLF->LF for any .md target — the templating was inert dead code, erased before the file is ever written. Reverted to hardcoded '\n', comment corrected to state the true reasoning. The separate [^\n]* -> [^\r\n]* widening one function up (a real splice-position fix, independent of final EOL) was kept. MAJOR — roadmap-upgrade.cts's stated rationale for switching onto splitLines/joinLines was wrong (both functions always agreed on line representation, before and after — the claimed equality-check risk never existed), and the change it justified introduced a real regression: forcing every line onto one dominant terminator silently rewrites untouched lines' EOL on a mixed-CRLF/LF ROADMAP.md. This write path uses raw fs.writeFileSync, not platformWriteSync, so unlike the phase.cts case above the regression is genuinely live. Fixing this took two attempts. The first attempt (revert to split('\n')/join('\n') plus a suppression comment) was correctly blocked by an agent that discovered local/no-crlf-fragile-split is a PROTECTED_RULES entry in tests/portability-rule-disable-ban.test.cjs — a hard, out-of-band, ADR-1703-governed guardrail banning any eslint-disable of this rule anywhere in src/**/*.cts. That agent also detected and correctly disregarded an injected instruction that appeared in tool output during a git operation, per this session's untrusted-content policy. The actual fix: computeMigrationPlan reverted to roadmapContent.split('\n') (confirmed lint-clean — the rule's data-flow tracking only follows a variable's initializer, and this one is declared empty then reassigned in a try block). applyMigration's write-back now splices edits against the ORIGINAL content string via indexOf('\n', pos) boundary-walking instead of a full split/rejoin, so every untouched character — including every line's own terminator — is copied byte-for-byte. A capture-group split (/(\r\n|\n)/, preserving terminators inline) was tried first and empirically confirmed to still trip the rule before this approach was chosen instead. MINOR (security) — roadmap.cts's cmdRoadmapAnnotateDependencies used the STRING form of String#replace, so $&, $`, $', $1-$9 inside must_haves.truths content (author-controlled) were interpreted as replacement directives, splicing unrelated ROADMAP.md text into the result. Fixed with the function-replacement form, which is never pattern-interpreted. Verified before/after with the reviewer's exact repro. Also disclosed rather than silently left: test matrix row 31 (four planned CRLF-materialized regression tests) was never implemented as separate files — corrected to record the actual verification (a manual --check run plus incidental existing coverage via each script's normalizeLineEndings: normalizeEol alias). parseMustHavesBlock's LF behavior was claimed byte-for-byte unchanged but the old yaml.indexOf(blockMatch[0]) substring search could match an unrelated earlier occurrence of the header text (e.g. inside a quoted value) — the split-then-scan fix incidentally also closes this, a strict improvement now recorded in the design doc rather than left implicit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3413): checkpoint 2 red — missing eslint ignore entry, RuleTester config error Checkpoint 2 came back red with 5 failures on the reviewed sha, both gaps genuinely undetectable by any local gate. eslint.config.mjs was missing the 'gsd-core/bin/lib/text-lines.cjs' ignores-list entry (ADR-457: generated .cjs artifacts are excluded from direct type-aware linting). Phase 1's sibling entry (pattern.cjs) sits two lines above it and was the exact precedent read while researching the six-gate ripple for this module -- missed anyway. Caught by tests/repo-invariants.test.cjs's bin/lib coverage-tracking test, which only runs on the remote suite. tests/no-crlf-fragile-split.rule.test.cjs's row-32 case specified both `messageId` and `message` on the same RuleTester error assertion -- ESLint's RuleTester rejects that combination outright. This existed since the test was first authored and was never caught locally: `npx eslint` only lints the file's syntax, it does not execute RuleTester, and local `node --test` is hard-blocked in this repo -- the assertion had never actually RUN before this checkpoint. It was even present in checkpoint 1's failure list, listed there as one of the "expected RED" tests; I matched it against my expected-failures list by test NAME only and never inspected the actual failure detail closely enough to notice it was failing for the wrong reason (a RuleTester config error, not the intended message-text mismatch). Fixed by keeping `message` (the exact-text assertion the test exists to make) and dropping `messageId`. Verified the crlfFragileSplit message string in eslint-rules/no-crlf-fragile-split.cjs matches this assertion character-for-character, and swept every other invalid case in the file for the same double-specification bug (none found -- all pre-existing cases use messageId alone). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(#3413): add Fixed changeset for the #3360 CRLF parsing fix The sole user-visible effect of this phase. No breaking-change label or Changed fragment needed — ADR-3212's Backward Compatibility section names the Node floor (Phase 1, already shipped) as the epic's only breaking change; Phase 2 has none. * chore(#3413): backfill changeset pr number to 3420 --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Weibao Chen
|
430e66dac4 |
docs(registries): add gsd-reasonix EoS entry (#3403)
Adds one `type: "eos"` entry for a Reasonix host integration and regenerates docs/registries/eos-registry.md. Reasonix is a Go, single-static-binary, DeepSeek-native terminal coding agent. The integration renders the GSD workflow set as native Reasonix slash-command launchers; GSD workflows are not reimplemented. Every axis is sourced from Reasonix's own docs per the never-infer rule. `effortSurface` is omitted: Reasonix resolves effort from config and the /effort command with no argv route, so neither allowed value fits and the field is optional. Supersedes #3346, which proposed an in-tree capabilities/reasonix entry and was closed as filed against the wrong surface. Co-authored-by: onionviolet <apexofficial21@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com> |
||
|
Tom Boucher
|
dc3c81e93d |
chore(#3212): src/pattern.cts is the sole owner of runtime-value regex construction — Phase 1 (#3416)
* test(#3412): failing-first suite for the pattern-construction seam Phase 1 of epic #3212 (ADR-3212 §1/§2/§7). Tests only — src/pattern.cts and eslint-rules/no-adhoc-regex-escape.cjs do not exist yet, so both suites fail with MODULE_NOT_FOUND, which is the intended RED. Locks the measured behavior rather than the assumed behavior: RegExp.escape hex-escapes the leading character of nearly every string ("abc" -> "\x61bc"), so the suite asserts match-equivalence against an inlined historical oracle (the implementation being deleted) rather than byte-equivalence of pattern text — 200 seeded fast-check runs plus a fixed corpus, 0 mismatches. Also locks the latent character-class range bug this phase fixes as a side effect: a hyphen-bearing value interpolated into [...] currently forms a real range and matches an unintended character; post-migration it must not. * chore(#3412): src/pattern.cts owns runtime-value regex construction Phase 1 of epic #3212 (ADR-3212 §1/§2/§6/§7). Adds the pattern seam delegating to the built-in RegExp.escape, deletes every hand-rolled copy, and raises the Node floor to the Active LTS line. The census was low, three times over. ADR-3212 counted 10 copies; a graph query found 12; the new lint rule — once live — found 27 more. The difference is that the census counted named helper FUNCTIONS while the rule counts the escape SHAPE, so inline .replace(<class>, '\$&') copies were never in scope. ADR §1's actual requirement is that no module outside the seam escapes a value for regex use, so all of them are, and CLAUDE.md's no-defer rule makes them this change's work. Fourth consecutive epic here whose copy count was low — the argument for ADR-3180 Amendment 3's "state N found by the guard" rule. Also corrected mid-implementation: the survey reported phase-id.cts's escapeRegex had 0 external importers. It had 8 production importers, making its removal a public-surface change to an ADR-2121-owned module and requiring an update to that ADR's locked-surface test. Blast radius revised Medium-High -> High. RegExp.escape is match-equivalent but NOT text-equivalent: it hex-escapes the leading char of nearly every string ("abc" -> "\x61bc"). Equivalence is proven by a seeded fast-check property test against the deleted implementation as oracle. It also fixes a latent bug: a hyphen-bearing value interpolated into a character class previously formed a real range and matched an unintended character. Node floor 22 -> 24 (RegExp.escape is Node 24+), across engines, .nvmrc, package-lock, 9 CI matrix entries, and 5 docs. The aggregate `required-tests` context is unchanged and no job was added or removed, so branch protection cannot be orphaned by the dropped lanes. Enforced by eslint-rules/no-adhoc-regex-escape.cjs (shape-matched, with structural provenance for reviewed pattern-fragment constants rather than a name heuristic) plus a whole-tree companion guard covering the directories ESLint's globs miss. * fix(#3412): close the _SOURCE guard evasion, correct two false claims Three findings from the orthogonal review pass, all fixed. 1. The ESLint rule's `_SOURCE` provenance fallback was pure identifier- name matching with no binding check, so `new RegExp(userInput_SOURCE)` — a function parameter — sailed past the guard. That is the same rename-evasion class issue #3410 documents, reopened by the very fallback meant to complement the structural check. Now bound to the identifier's actual binding kind: import, require-derived const, or module-scope const; parameters, `let`/`var`, and unresolvable bindings fail closed. Four RuleTester cases cover the evasion and prove the legitimate cross-module case still passes. 2. src/pattern.cts's own header carried the stale pre-correction counts (12 copies / 17 call sites) while CONTEXT.md and the design doc carried the corrected ones (~39 / ~44) — a self-contradiction inside the PR whose entire purpose is deleting divergent copies. Rewritten, preserving the durable lesson: a named-function census cannot see inline copies; only a shape-matching guard can. 3. The claim that all deleted copies threw TypeError on non-string was false. phase-id.cts's copy — the one with 8 external importers — did String(value).replace(...) and never threw. The seam's locked signature does not coerce, so this is a real, now-disclosed behavior change rather than the pure preservation the tests asserted. Audited all 32 invocations across the 8 importers and 6 in-file callers: every one is safe by construction (upstream truthy guard or a string-producing derivation), verified by runtime probe against the compiled modules rather than by TS compilation, which cannot see a runtime undefined. Corrected the false claim in both the test comment and the design doc, and added it to Known limits. * docs(#3412): add Changed changeset for the Node 24 floor The only user-visible break in this phase. The escape-behavior change is internal and match-equivalent, so it carries no user-facing note. * fix(#3412): resolve the seam's require graph in script fixtures and packaging Checkpoint 2 came back red with 90 failures on the node24 lane. Three distinct defects, all introduced by routing scripts/ through the new pattern seam, none reproducible by any local gate: 1. ~82 failures — tests/adr-index-gate.test.cjs and tests/removed-but-needed-lint.test.cjs copy a scripts/*.cjs into an mkdtemp fixture and spawn it there (necessary: those scripts resolve their scan root from __dirname/.., so running the real script would scan the real repo). Each harness hand-listed the dependencies to copy alongside. Adding require('../gsd-core/bin/lib/pattern.cjs') to gen-adr-index.cjs made both lists silently incomplete -> MODULE_NOT_FOUND, plus 17 downstream 'did not emit parseable JSON' failures from the same crash. Fixed as a class, not an instance: new tests/helpers/copy-script- fixture.cjs walks a script's transitive static relative-require graph and copies it, so dependencies are derived and never re-declared. It throws (naming the unbuilt artifact) instead of letting the child die with a bare MODULE_NOT_FOUND. Verified for all four seam-consuming scripts: gen-adr-index, lint-removed-but-needed, gen-loop-host- contract, sync-runtime-launcher. 2. 2 failures — scripts/ ships wholesale but eslint-rules/ does not, so the new scripts/lint-no-adhoc-regex-escape.cjs would be MODULE_NOT_FOUND in a published install (#2858 guard). Excluded from the tarball, matching the existing precedent for gen-emitted- baseline.cjs, which is excluded for the identical reason, and locked with a test modeled on that one. Confirmed against a real npm pack: 890 files, 0 from eslint-rules/, and gsd-core/bin/lib/pattern.cjs present (so the other four scripts' requires are legitimate). 3. 6 failures — tests/phase-id.test.cjs asserted the literal escaped source text ('0*29', 'PROJ-42'). RegExp.escape is match-equivalent to the retired hand-rolled escaper but NOT text-equivalent: it hex- escapes the leading character and all hyphens ('0*\x329', '\x50ROJ\x2d42'). Verified NOT a behavior change — 576 match decisions across all three real interpolation prefixes, zero divergence. Those tests now compile each source into the same heading regex src/roadmap.cts's searchPhaseInContent builds and assert what matches and what does not, including the 'i'-flag canonicalization the hex escape has to preserve. Re-pinning the new literals would have rebuilt the same brittleness one layer down. Adds a test for the property the escape exists for: a dot in '1.2' must not act as a wildcard. Also shares one definition of 'a require' between the packaging guard and the fixture copier, so the two cannot disagree about what they scan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3412): refuse to copy a fixture dependency outside the fixture root copyScriptWithDeps resolved each relative require and joined the repo-relative result onto fixtureRoot. A require resolving OUTSIDE the repo yields a '../'-prefixed relative path, so path.join climbed out of the fixture and wrote into the surrounding temp dir (verified: repoRoot=/repo + depAbs=/etc/passwd wrote /tmp/etc/passwd). No script in the tree does this today, so this closes an available escape rather than an active one. Refuses via the existing unresolved- require path so the failure names the offending specifier. Covered by a negative proof that the guard fires and that nothing lands outside the fixture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3412): parse requires instead of pattern-matching them; restore the foreign-prefix contract Applies all findings from the second orthogonal review round, re-run because real code changed after round 1. HIGH (security) — extractRequires stripped BLOCK comments before LINE comments, so a '//' comment containing '/*' opened a phantom block comment, and a '//' inside a string literal truncated the line. Both hid real requires: 'const u="http://x"; require("./real.cjs")' returned [], and four real requires in gsd-core/bin/gsd-tools.cjs were invisible. Replaced with a real AST parse via espree. This is ADR-3212's own Decision 4 — tokenizer-first for stateful grammars — applied to the case it describes; comment/string/regex nesting is exactly such a grammar, which is why the regex version was wrong. The function was moved byte-identical out of the #2858 packaging guard, so the bug PRE-DATES this branch and has been a live blind spot there: a shipped script could have required an unshipped path undetected. Fixing it makes that guard strictly stronger than on next. espree is promoted from a transitive eslint dependency to an explicit devDependency rather than relying on hoisting. The script parse attempt sets ecmaFeatures.globalReturn because Node wraps CommonJS bodies in a function, making a top-level return legal — scripts/check-coverage-gate .cjs relies on it, and without the flag the guard throws on a file it is supposed to scan. Verified 0 unparseable across all 324 .cjs/.js under scripts/, bin/, and gsd-core/bin/, and 0 new violations against a real npm pack, so the exact extractor does not newly fail the guard. MEDIUM (security) — the repo-containment check guarded dependencies but not the entry path. One escapesContainment predicate now guards both. LOW (security) — containment was lexical while fs follows symlinks, and a directory symlink could mint a fresh dedupe key per level. realpath now resolves both repoRoot and each dependency before the decision, and the realpath-derived path is the dedupe key. Destination layout still uses the original repo-relative path, so copied trees are unchanged. MAJOR (standards) — the round-1 behavioral rewrite of phase-id tests lost the foreign-prefix contract: every assertion was satisfied by an impl returning [A-Z]+\x2d42, i.e. ANY project code — the exact #3599 bug class the exact-source prevents. The literal assertions it replaced were catching this. Now asserts the compiled regex REJECTS a different prefix with the same number. MAJOR (standards) — the test hand-duplicated production's heading regex with no parity guard (CLAUDE.md's 'Generative Fix Divergence'). Removed the parallel surface instead of policing it: src/roadmap.cts exports buildPhaseHeadingRegex, searchPhaseInContent calls it, the test imports it. Byte-identical .source and .flags verified for both escaped forms. MINOR — '..foo' no longer false-flagged as an escape; the inverted spurious-vs-missing doc claim corrected; the dead allow-test-rule header removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#3412): backfill changeset pr number to 3416 * fix(#3412): make the escape guard's own regex linear, reword an injection-scan collision Two CI failures on PR #3416, both in code this branch added. CodeQL js/redos (high) — REPLACE_CALL_RE's outer alternation let a bracket run be consumed EITHER by the character-class branch OR one character at a time by the trailing catch-all, so a failing match explored both parses of every pair. Measured on the real regex: n=26 -> 204ms, n=28 -> 791ms, n=30 -> 3475ms, a clean 2^n. This script scans repo source, so a file with a long bracket run after '.replace(/' would hang CI outright — a guard against undisciplined pattern construction was itself the worst pattern in the diff. Fixed the way ADR-3212 already prescribes: the catch-all branch now excludes '[' and ']' so a bracket can only be consumed by the class branch (this is what makes it linear), and every quantifier is bounded (the locked bounded-quantifiers decision) as a second line of defense. Now 0ms at n=2000. Disclosed coverage tradeoff, recorded at the constant: a regex literal with a BARE unescaped ']' outside a class is no longer matched by this backstop. No census shape has that form, and the AST rule remains the primary detector. Verified the guard did not go blind doing it: a real census-shape violation is still reported, and an allow-adhoc-regex-escape suppression comment is still honored. Regression test drives the exported findViolations on a 2000-repetition adversarial input and asserts the RESULT. It makes no wall-clock assertion — elapsed-time tests are forbidden — so a regression surfaces as a harness timeout, which is the correct signal. Prompt injection scan — 'must not act as a regex wildcard' in a test comment matched the scanner's jailbreak pattern act\s+as\s+(a|an|if| my). Reworded to 'behave as'. Deliberately NOT allowlisted: silencing a whole test file over one phrase would blunt the scanner permanently, and the comment has nothing to do with injection. Neither failure was reachable from the remote runner — CodeQL and the injection scan are not in that matrix, so the sha it passed was green and still wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tom Boucher
|
622c10b2c6 |
fix(#3025): refuse cross-runtime skill sync in sync-skills (#3404)
* fix(#3025): refuse cross-runtime skill sync in sync-skills Skill content and directory layout are runtime-specific — the installer applies per-runtime converters, adapter headers, brand swaps, and layout rules at install time, and grok/gemini resolve to ANOTHER runtime's skills root. A verbatim cross-runtime cp -r therefore produces content the installer would never have written for the destination, and can damage a runtime the user never named. #3024 (closed) un-masked this, making the corruption live. Fix (option b, user decision): add a functional Step 1 guard that refuses any --to != --from with an actionable installer pointer, before any resolution or copy. Identity sync (--from == --to) remains a no-op. The non-functional Step 5 comment is replaced; Arguments/Limitations updated. Regression: tests/sync-skills-cross-runtime-refuse.test.cjs (source-text- is-the-product) asserts the guard exits non-zero for cross-runtime, points at the installer, precedes the cp -r copy, and preserves identity. * docs(#3025): backfill changeset PR number (#3404) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
6dbc124018 | enhance(#3180): the sibling validators share one envelope and one owner — Phase 12 (#3407) | ||
|
sim
|
1fdc953978 | docs(#3309): backfill changeset pr numbers with #3405 | ||
|
sim
|
23884a6448 |
docs(#3309): add changeset fragments for the breaking changes
Three user-visible changes disclosed per CONTRIBUTING.md's changeset convention: --repair no longer auto-applies DESTRUCTIVE remedies (Changed), W021/W017 split into W026/W027 for their previously- conflated second subjects (Changed), and --backfill alone now actually works (Fixed, a latent-bug fix). pr:0 placeholder, backfilled once the PR number is known. |
||
|
Tom Boucher
|
3c4df10a50 | Merge pull request #3402 from open-gsd/refactor/3308-planning-snapshot-parsed-projection | ||
|
sim
|
1f5fc24610 | chore(#3308): backfill changeset PR number (#3402) | ||
|
sim
|
9ce44efe85 | docs(#3308): add changeset for planning-snapshot parsed projection | ||
|
Tom Boucher
|
2283168612 |
fix(#3170): anchor milestone one-liner extraction to a summary-shaped heading (#3401)
* test(#3170): extractOneLinerFromBody must anchor to a summary-shaped heading Regression for #3170: the function matched the first heading's first bold run, so an incidental first heading (rule list, deviation notes) contributed its bold text as the milestone accomplishment. Rows 1/2 fail RED on next; rows 3/4 guard Overview recognition and the #2660 Summary-heading form. * fix(#3170): anchor milestone one-liner extraction to a summary-shaped heading extractOneLinerFromBody matched the first heading's first bold run regardless of section, so an incidental first heading (rule list, deviation notes) contributed its bold text as the milestone accomplishment written into MILESTONES.md. Iterate headings and extract from the first Summary/Overview/ Accomplishments one with a bold run, falling back to null when none exists. The #2660 Summary-heading forms and the frontmatter one-liner precedence are preserved. * docs(#3170): add changeset * test(#3170): align extractOneLinerFromBody unit fixtures with summary-heading contract The core-utils unit fixtures used generic # Title headings encoding the old 'any first heading' contract; the #3170 fix anchors to a Summary/Overview/ Accomplishments heading (the function is summary-specific). Update the heading text to Summary-shaped; the extraction assertions (bold, frontmatter strip, colon-label, CRLF, unicode) are unchanged. * docs(#3170): backfill changeset PR number (3401) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
b8cb031ce2 |
fix(#3163): scope phase.add insertion to the current milestone (#3400)
* test(#3163): phase add must insert in the active milestone, not the trailing archive Regression for #3163: cmdPhaseAdd/cmdPhaseAddBatch pick the insertion point via rawContent.lastIndexOf('\n---'), the file's last horizontal rule — which on a roadmap with shipped/history material after the active phase list sits deep in archive. Rows 1/2/4 fail RED on next (entry lands after the archive heading); row 3 guards the no-milestone legacy fallback. * fix(#3163): scope phase.add insertion to the current milestone window cmdPhaseAdd and cmdPhaseAddBatch picked the insertion point via rawContent.lastIndexOf('\n---') — the file's last horizontal rule, which on a roadmap with shipped/history material after the active phase list sits deep in archive. Extract phaseEntryInsertOffset(rawContent, cwd): scope the search to currentMilestoneRawRanges' primary window so the entry lands at the end of the active phase list. Fall back to the legacy whole-file heuristic when no current milestone resolves, preserving simple no-milestone roadmaps. Applies to both cmdPhaseAdd and cmdPhaseAddBatch (identical expression); the decimal insert path was already header-anchored and is untouched. * docs(#3163): add changeset * docs(#3163): backfill changeset PR number (3400) --------- Co-authored-by: sim <sim@local> |
||
|
Rezolv
|
3ff9a7ffcd |
fix(#2570): parse leading date from last_activity so stale_activity fires with a description suffix (#2571)
* fix(#2570): parse leading date from last_activity so stale_activity fires with a description suffix templates/state.md prescribes `Last activity: [YYYY-MM-DD] — [What happened]`, and gsd-core's own STATE.md mirrors that suffix into frontmatter. Date.parse on the whole string returned NaN, and because staleActivity treats null as "not stale" (fails open), the only idle/staleness detector never fired on any project whose last_activity kept its description. parseActivityTimestamp now reads the leading ISO date/time token when a whole-string parse fails, validating the calendar date (ADR-227: reject an impossible date rather than let Date.parse roll it forward) and preferring the whole-string parse when it succeeds so a trailing zone name is not dropped. Composes with #3099 (LAST_ACTIVITY_UNPARSEABLE diagnostic), which merged to next after this branch: both key off parseActivityTimestamp === null, so a value whose leading date now parses takes the stale path and does NOT emit the diagnostic. A regression test in tests/smart-entry.unit.test.cjs asserts exactly that (stale true, emission count 0), guarding against two staleness signals on one field. Rebased onto next (flattened): resolved the add/add test conflict by keeping both the #2570 and #3099 describe blocks. Tests: unit + property, 80 pass. * fix(#2570): fail open when a named zone can't be reconstructed from the token (#2571 B1) The 2026-08-08 flatten dropped the zone handling earlier rounds built, so the fallback path -- reached only when a description suffix makes the whole-string parse fail, the #2570 case -- reconstructed `${date}${time}` WITHOUT any named zone. ISO_LEADING_RE's offset group captures only Z / +-HH:MM, so " GMT"/" EST" land in the un-captured suffix; Date.parse then read the reconstruction as LOCAL time, shifting the instant by the host's UTC offset -- a wrong, host-dependent value the diff's own comment warned against but guarded only on the other branch. Fix (the simpler of the two offered in review): when the remainder after the matched token begins with a letter (a named zone we cannot preserve), return null -- fail open to not-stale, matching the base's honest behaviour and ADR-227's "never propagate a wrong instant". The #2570 template suffix (" -- description") starts with a separator, so it still reconstructs and reads stale as intended. Tests (both fail-first, verified RED on the pre-fix head): - smart-entry.unit: a named-zone + description suffix (54 days old) enters the fallback and must read not-stale, not a still-old local instant. Host- independent by construction. - smart-entry.property (f): named-zone + suffix over 1-week..1-year ages and 8 zones stays total and fails open. Discloses the removal M2 flagged: TRAILING_ZONE_RE / UTC_ZONE_NAMES / timeCarriesOffset were dropped by the flatten; this restores the SAFETY (no wrong instant) via the simpler null contract rather than the allowlist. * fix(#2570): narrow the stale_activity fallback guard to a zone-designator shape The round-9 fail-open guard `/^\s*[A-Za-z]/` treated any letter-led remainder as an unpreservable named zone, so a leading real date followed by a bare space/tab/colon and an ordinary description (a hand-edited STATE.md that omits the template em dash) returned null and re-opened #2570 for exactly those shapes. Narrow the guard to ZONE_DESIGNATOR_RE -- a standalone short all-caps run -- and consult it ONLY when the leading token captured a time-of-day: a zone qualifies a clock time, so a bare date carries no zone hazard and always reconstructs to its UTC midnight. A plain description (including one that opens with a tech acronym like "CI green") reconstructs; a real named zone on a timed value (GMT/EST/...) still fails open (ADR-227: never propagate a wrong, host-dependent instant). Widen the property generator to the non-em-dash separators (space/tab/colon), the arm that structurally could not reach the fallback before, and add unit cases for whitespace/tab/colon-separated and bare-date+acronym descriptions. All fail-first on the prior guard; green across UTC/LA/Tokyo/Kiritimati. --------- Co-authored-by: Tom Boucher <trekkie@nomorestars.com> |
||
|
Behruz Nassre Esfahani
|
f0abdb1b89 |
fix(#2486): do not recommend or persist Claude-only worktree isolation on non-Claude runtimes (#2531)
* fix(#2486): runtime-branch the settings worktrees question + W020 health diagnostic On non-Claude runtimes /gsd:settings offered "Yes (Recommended)" for worktree isolation and persisted workflow.use_worktrees: true — the exact value the execution workflows fail closed on (#1521 guards). Branch the question on the same stamped config-get runtime read the guards use: Claude keeps the unchanged question; non-Claude offers only "No (Recommended)" / "Leave unchanged", never persists true, and warns when the config carries an inherited explicit true. /gsd:health gains W020, surfacing such a config with the guards' own predicate before execution-time failure. Docs state the runtime-conditional default. Fixes #2486 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(#2486): add changeset for PR #2531 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2486): reassign the health worktrees check W020 -> W024 (verify.cts namespace collision) The workflow-level check collided with the live W020 (git-worktree-list health) emitted by cmdValidateHealth in src/verify.cts — invisible from health.md's error_codes table, which stops at W019 and under-represents the real namespace (W010-W017, W020-W023 all live). W024 verified free. Adds a regression test pinning the chosen code against src/verify.cts so a future assignment cannot silently collide, a table note naming the namespace owner, and the changeset body reworded to house style. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2486): pre-select the recommended repair in the broken-inheritance case Review round 2: at settings.md:142 the pre-selection rule left "Leave unchanged" as the default when the config carried an explicit non-false use_worktrees — the exact broken state the adjacent notice warns about, so accepting the default kept a config that fails closed at execution time. "Leave unchanged" is now the default only when the key is absent (nothing to repair); explicit false AND explicit non-false both pre-select "No (Recommended)", aligning the default, the label, and the notice. Pinned by two source-contract assertions in the #2486 regression block. Goldens (settings.md hash x19) + size baseline regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2486): gate the worktrees question on dispatch.isolation, not the runtime name Review round 2: #2584 Phase 3 replaced the runtime-name test with a declared `dispatch.isolation` capability, invalidating this PR's premise. cursor declares harness-worktree and codex/opencode/kimi/ kimi-code declare orchestrator-worktree, so a `RUNTIME != claude` gate blocked a supported configuration on five runtimes and false-warned in health. - settings.md + health.md read `query dispatch-isolation` and branch on `ISOLATION = none`; the runtime-name read is gone from both, and the capability read needs no per-runtime stamping (it fail-closes unknown/ undocumented internally) - all "Claude Code-only primitive" prose rewritten, including the two gates the shell-syntax check missed (config-key list, JSON schema comment) - W024 reconciled across health.md + CONFIGURATION.md + planning-config.md (docs still said W020, which collides with a verify.cts code) - health.md error-codes table fixed: the namespace note no longer sits between rows orphaning I001 - the asymmetry note for the two workflows #2584 has not migrated (quick.md, diagnose-issues.md) is enforced by a set-equality test with a self-check table, so it cannot go stale in either direction Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(#2486): restore the Executor isolation section clobbered by #2661 `46ba02ac` (feat(#2630), the current next tip) reverted docs/CONFIGURATION.md to a pre-#2584 state: it restored the old "Non-Claude note" wording on the workflow.use_worktrees row and deleted the whole "Executor isolation per runtime" section. The change is unrelated to that PR's phase-estimation feature and looks like a stale-copy edit. This PR's use_worktrees row links to #executor-isolation-per-runtime, so the deletion leaves a dangling anchor. Restored byte-for-byte from |
||
|
JusticeWay
|
77374cfc32 |
fix(#2528): resolve digit-leading phase directories by bare number (#2559)
* fix(#2528): resolve digit-slug phase dirs by bare number — tokenizer rewind, shared bare-integer fallback, resolution-path parity gate
extractPhaseToken welded 2-digit slug words onto the phase token (phase 10
named "24/7 Autonomy" -> dir 10-24-7 -> token 10-24), making digit-prefixed
phase names unresolvable by bare number across every phase verb.
- phase-id: continuation segments must be the PURE 2-digit zero-padded form
the write side emits; a 1-digit terminator rewinds the absorbed run
(10-24-7 -> 10) while >=2-digit terminators keep the locked #2232
round-trip (14-06-2026-photos -> 14-06).
- phase-id: new matchPhaseDirs owner — primary exact-token match plus a
bare-integer leading-digit-run fallback for shapes the tokenizer cannot
rewind (05-80-20-cleanup); collisions stay #2237-loud.
- locator/find-phase/phase-plan-index all delegate selection to the owner;
plan-index gains the previously missing multi-match guard.
- tests: #2528 unit + fast-check metamorphic blocks; new 9-scenario
resolution-path parity gate across all three paths.
Fixes #2528
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(#2528): add changeset for PR #2559
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(#2528): align validation token grammar
* fix: address phase token review
* fix: restore phase grammar parity for numeric slugs
* docs: document digit-leading phase resolution
* docs: clarify ambiguous phase resolution behavior
* docs: register canonical phase directory selectors
* fix: align prefixed deep phase token parsing
* fix(#2528): route the fourth resolution site through matchPhaseDirs
Review BLOCKER. smart-entry.cts::detectVerifyFailed resolved the current
phase's directory with its own `.find(phaseTokenMatches)` and never
reached the shared selection, so the bare-integer-fallback family the
issue names — `05-80-20-cleanup`, `30-12-factor-refactor` — resolved
nowhere. The miss is silent by construction: an unresolved phase reports
"not failed", which is byte-identical to a healthy one, so a failed
verification simply never surfaced in /gsd or /gsd:progress.
`entries` is already sorted and matchPhaseDirs filters without
reordering, so matches[0] reproduces the previous selection exactly
wherever the old code resolved at all.
Wiring it into phase-resolution-parity.test.cjs as a fourth path then
exposed a second, older defect in the same function: phaseTokenFromDirName
shape-probed the UNSTRIPPED token, so a project-code-prefixed directory
(`MEM-05-…`, tokenizing to `MEM-05-80-20`) failed the leading-digit test
and was dropped before any resolution ran — every phase in a
project-coded plan was invisible to this check. The probe now runs on the
stripped token; the returned value is unchanged, so the comparePhaseNum
sort is untouched.
Path 4 has no JSON surface to compare, so the gate observes selection
indirectly: plant the failing artifact in exactly one directory and a
passing one everywhere else, then read the boolean. Reverting either fix
turns 5 of the 10 corpus scenarios red.
* refactor(#2528): collapse the duplicated extractCanonicalPlanId
Review MAJOR. The function existed as two independent, byte-identical
copies — src/core-utils.cts and src/phase.cts — and this PR had to patch
BOTH with the same single-digit-slug rewind rule. That is the generative
fix divergence CLAUDE.md names, and only the core-utils copy was under
test, so a future one-sided patch would have silently split plan-id
canonicalization between the plan listing and everything else.
Removed rather than parity-tested: core-utils was already the leaf owner
and already exported it, and phase.cts already imported that module, so
there is no second surface left for a parity test to police.
* test(#2528): pin matchPhaseDirs at the digit-width boundaries
Review MAJOR. The bare-integer fallback's correctness rests entirely on
capturing each directory's whole leading digit run before the zero-strip
compare; a regex that stopped short would turn every query into a prefix
match, and "1" would claim 10, 100, and 12 alike. The existing coverage
was example-based and never touched that boundary.
Adds the explicit 9/10 and 1/10/100 cases — including the forms where
only the wider directories exist, so an exact-width neighbour cannot
satisfy the assertion — plus a fast-check property over arbitrary
distinct leading runs. The property is stated as an invariant on the
result (every returned directory's leading run IS the query) rather than
an expected list, so it covers primary and fallback matches alike and
cannot be satisfied by reimplementing the selection in the test.
Both fail when the fallback regex is degraded to a prefix match.
* fix(#2528): route the remaining eight consumers through matchPhaseDirs
phaseTokenMatches had eight consumers left that each rebuilt the directory
selection around it by hand: phases-list, next-decimal, phase-remove, the
W021 milestone-consistency check, schema-drift, the init-manager overview,
milestone-complete's disk check, and roadmap analyze. Every one of them
reproduced the reported symptom in full after the tokenizer was fixed.
None of them derives a displayed phase number from the matched directory,
so none needs phaseNumberForMatch; the change at each site is the
selection and nothing else. matchPhaseDirs filters without reordering, so
matches[0] reproduces the prior .find() choice wherever the old code
resolved at all.
phaseTokenMatches now has no call sites outside phase-id.cts. It stays
exported as the primitive matchPhaseDirs is built from and as a pinned
canonical surface, but no consumer reaches past the owner to it.
* test(#2528): extend the parity gate to the migrated consumers
Each of the eight is observed through the surface a user sees, not
through the matcher, with a no-directory control so the assertions cannot
be satisfied by a consumer that resolves unconditionally. init-manager
and roadmap-analyze are additionally asserted to agree with each other.
* refactor(#2528): own the case-flexible phase grammar and the leading-digit-run fragment
validate.cts derived its case-flexible regex sources by running
`replaceAll('A-Z', 'A-Za-z')` over two constants exported by phase-id.cts.
That passes lint-phase-id-drift.cjs — there is no literal copy of the
grammar — but it depends on the owner rendering that exact substring. The
day phase-id.cts expresses the same class any other way the replaceAll
silently no-ops and validate.cts narrows to uppercase-only. The failure
mode is a NON-match, so nothing throws and no uppercase-only fixture
notices. Both variants are now derived once, beside the sources they
widen, and imported.
Also names the leading digit run the bare-integer fallback selects on.
It was spelled `/^(\d+)(?:-|$)/` where the fallback filters and `/^\d+/`
where phaseNumberForMatch reads the number back off the winner; selecting
on one run and displaying another would resolve a directory and then label
it with a number that never matched it.
* fix(#2528): refuse to remove a phase when two directories claim its number
cmdPhaseRemove was the only migrated site taking matches[0] with no
multi-match guard. Every sibling resolution path returns ambiguous_matches
and refuses to choose; this one is the DESTRUCTIVE path, so choosing
silently is strictly worse than anywhere else. With 05-80-20-a and
05-90-till-late on disk, `phase remove 5 --force` deleted one of them and
renumbered every phase after it — where the base resolved nothing, deleted
nothing, and the corpus in tests/phase-resolution-parity.test.cjs already
declared that exact input ambiguous.
The refusal is emitted before any file is touched and carries both
candidates. CONSUMER_SCENARIOS could not express the case — every row is
binary, resolving to one directory or to none — so the gate gains a
dedicated ambiguous test. It asserts on the filesystem, not only on the
reported directory_deleted: a null printed after an rmSync would satisfy
every other check.
* fix(#2528): pair digit-leading phase directories with their roadmap phase in validate health
W006/W007 are the ninth site of this bug class and the one a
`phaseTokenMatches` grep could never surface: they resolve roadmap↔disk by
intersecting TOKEN SETS, which is a dir→token labelling rather than the
query→dir selection matchPhaseDirs owns. On the canonical fixture the
label is wrong in both directions at once, so `validate health` reported
"Phase 5 in ROADMAP.md but no directory on disk" AND "Phase 05-80-20
exists on disk but not in ROADMAP.md" for the same directory.
collectDiskPhases now keeps the directory names behind each token, so
W006 can ask the canonical matcher whether a roadmap phase resolves to a
real directory, and W007 — which iterates directories and therefore has no
query to resolve — gets the inverse mapping it never had: a directory is
claimed when some roadmap phase resolves to it.
Both checks are additive: the token intersection still decides every shape
it already decided, and the resolution can only REMOVE a warning. The
regression test carries controls in the other direction — a roadmap phase
with no directory must still raise W006, an unclaimed directory must still
raise W007 — so it cannot be satisfied by a check that stopped reporting.
* docs(#2528): state and pin the directory-side scope of the bare-integer fallback
The matchPhaseDirs docblock claimed deep-decomposition lookups were
untouched. That is true of the QUERY side only — no non-bare query enters
the fallback — but the DIRECTORY side is what changed classification: a
bare `5` now reaches a lone `05-01-auth` and resolves it (phase_number
"05", phase_name "01-auth") where the base found nothing.
The widening is irreducible from directory names alone. `05-01-auth`
(sub-phase 5.1) and `30-12-factor-refactor` (phase 30 named "12-Factor
Refactor") are the same `NN-NN-<slug>` shape, and the discriminator that
would separate them — "is the second segment a valid decimal sub-phase" —
accepts `5.1` and `30.12` equally. Any rule strong enough to exclude the
first excludes the second, which is the defect #2528 exists to fix. So the
tie is broken in favour of resolving, the docblock now says so, and the
consequence is bounded where it matters: two such directories are two
matches, and every caller (including phase remove) refuses to choose.
Pins both directions, since nothing observed the directory side before.
* fix(#2528): count surviving phases by identity in phase remove's STATE resync
#2640 landed on `next` while this branch was open. Its STATE.md phase-count
resync re-derives "which directory was removed" from the query with
`phaseTokenMatches`, which is the tenth site of this issue's defect: the
bare-integer fallback resolves `05-80-20-cleanup` for query `5`, but the
token predicate does not, so the just-deleted directory is counted as still
present and the written `Total Phases` is one too high.
`targetDir` already IS the directory that was removed, and the block is gated
on it being non-null, so identity answers the question exactly — which is also
what the comment above the filter already claimed it did. This keeps
`phaseTokenMatches` out of `phase.cts` rather than re-importing it to satisfy
one call site: the module's public surface should not grow for a question that
does not need re-derivation.
Pinned in the parity gate with a control on a directory the tokenizer reads
correctly, so the assertion is about the digit-leading shape and not about the
counting rule changing for everything.
* fix(#2528): let the resolution layer own the digit-leading slug family alone
The tokenizer rewind this fix carried — pop the last absorbed continuation when
the segment that stopped the scan is a bare single digit — reads
"10-24-7-autonomy" (phase 10 named "24/7 Autonomy") correctly and silently
re-reads "10-24-7-zip" (sub-phase 10.24 named "7-Zip Integration") from "10-24"
to "10". The two names are string-identical in shape, so no local signal
separates them; the rule traded the reported ambiguity for the symmetric one a
level down, on a 15-caller chokepoint whose output also feeds query-less
derivations (STATE.md phase counts, W007, the #2562 key surface). A well-formed
sub-phase directory became unresolvable by its own id — the very symptom #2528
was filed about.
It also bought nothing. The bare-integer fallback in matchPhaseDirs already
resolves "10-24-7-autonomy" for query "10" whatever the token is: no primary
match, bare query, leading digit run "10". The reported case was covered twice,
by two rules, and the two disagreed about the case nobody reported.
So the rewind is removed rather than narrowed, in the tokenizer and in the five
surfaces kept in lockstep with it (BRACKET_PHASE_TOKEN_SOURCE,
PHASE_TOKEN_FROM_DIR_RE, canonicalPlanStem's pair grammar and its collision
branch, roadmap-parser's numericRe, extractCanonicalPlanId), together with the
SINGLE_DIGIT_RUN_SEGMENT_SOURCE owner constant they shared. Disambiguation now
lives only where a QUERY exists to disambiguate against, which is the same
bounded mechanism the "05-80-20-cleanup" shape already used.
Measured, not argued: over 29800 generated directory names, extractPhaseToken is
byte-identical to `next` on every input except the lowercase-continuation class
("01-20a", "05-80-20-25abc") — a rule about the segment itself, not a guess about
its neighbour.
Both readings now stay reachable by their own ids:
matchPhaseDirs(['10-24-7-autonomy'], '10') -> the dir (fallback)
matchPhaseDirs(['10-24-7-zip'], '10') -> the dir (fallback)
matchPhaseDirs(['10-24-7-zip'], '10-24') -> the dir (primary)
* test(#2528): pin the one-continuation boundary the rewind had no coverage for
The regressing shape was invisible to the suite by construction, not by luck:
the deep-rewind property built its cases from `continuationArb` with
`minLength: 2`, so it never exercised the single-continuation case — exactly one
genuine sub-phase level before a digit-leading slug — and every hand-written
fixture used the ambiguous shape only where "phase-plus-slug" was the intended
reading.
`continuationArb` is now `minLength: 1` and the property states the invariant
instead of the old rule: for any prefix, phase, 1-5 continuations and any
one-digit terminator, the token equals the FULL continuation run on both the
imperative and the regex surface, and `matchPhaseDirs([dir], token)` returns that
dir. That third assertion is the one that catches the class on its own — the old
behaviour made a well-formed directory unresolvable by its own id, which is a
property, not a fixture.
Around it: "10-24-7-zip" and "10-24-3d-printer" now sit beside
"10-24-7-autonomy" everywhere the family is pinned, so the two readings can never
diverge again; the 24/7 metamorphic property asserts the RESOLUTION result rather
than the token (the token is precisely the part no surface may decide); the
end-to-end parity corpus gains "a sub-phase with a digit-leading slug resolves by
its full id" across all four resolution paths; and the milestone-scoping residual
is pinned in three directions rather than left to prose.
Mutation: re-inserting the rewind and rebuilding turns 9 tests red, the
`minLength: 1` property first, and nothing else. Build success checked separately
(build:lib reports 0 `error TS`), so the mutation reached the artifact under test.
* test(#2528): pin the #2946 guard against digit-leading phase directories
The #2946 fix makes the milestone-complete unstarted-phase guard run
unconditionally, so whether it fires now rides entirely on the
directory-resolution owner this PR replaces. Two cases, both with STATE.md
carrying no `milestone:` field so the #2946 path is the one exercised:
- ROADMAP Phase 5, disk `05-80-20-cleanup` → guard must stay silent.
RED on next (fail-closed: the guard blocks a legitimate one-way-door
operation because phaseTokenMatches resolves neither 05 nor 80 for
that directory).
- ROADMAP Phase 80, same directory → guard must still fire. Green on
both sides; it pins the fail-open direction against a future widening
of the matcher.
* fix(#3175): stop the injection scanner reading RegExp.exec as code execution
The apostrophe fix in
|
||
|
0xdhx
|
c516c39b33 |
fix(#3203): stop npm-global installs validating bundled agents against themselves (#3229)
* fix(#3203): stop npm-global installs validating bundled agents against themselves getAgentsDir's claude branch derived the agents directory from __dirname, which is correct for repo runs and runtime-config-dir installs (where <root>/../agents IS the user's agents dir) but on an npm-global install resolves to the package's own bundled agents/, so checkAgentsInstalled validated the package against itself and agents_installed could never be false. The new-project and new-milestone halt/warn gates were silently dead for npm-global users. Keep the install-relative path for the shapes where it is correct, but when it lies inside a node_modules tree — the provably self-validating case — resolve getGlobalConfigDir('claude')/agents like every other runtime, honouring CLAUDE_CONFIG_DIR. GSD_AGENTS_DIR stays priority 1. Repair the doc comment that asserted the __dirname form was correct for both install shapes. Regression test mirrors the published npm-global layout (package under node_modules with a complete bundled agents/) and pins the resolved directory plus the issue's negative control (one agent missing from the config dir → agents_installed:false). Verified red against pre-fix code, green post-fix; the repo-layout W010 health test stays green. * chore(#3203): set changeset fragment pr to 3229 * docs(#3203): describe the node_modules guard as lexical, in CONTEXT.md and at the call site The Agent Install Check Module glossary entry asserted that Claude resolves the agents directory `__dirname`-relative unconditionally. That is the premise this PR falsified: on an npm-global install the install-relative path resolves to the package's own bundled `agents/`, so the check validated the package against itself and `agents_installed` could never be false. The inline doc comment above `getAgentsDir` was repaired with the fix; this external predicate was left behind and has been false since. CONTRIBUTING.md's `Fixed`-fragment docs exemption names this case explicitly — "Edit the docs anyway if a fix corrects something the docs got wrong." Both surfaces now describe the guard as what it is: an exact, case-sensitive path-segment test that TARGETS those layouts rather than detecting them, so neither claims more certainty than the predicate has. The call-site comment carried the same conflation the glossary did. A path merely carrying a directory of that name resolves the same way — the edge already disclosed on this PR — and a non-empty GSD_AGENTS_DIR overrides it. Comment-only in `src/`; no behaviour change. `CONFIG.LOCATION.SEAM.two-families` needs no change: `GSD_AGENTS_DIR -> getAgentsDir priority 1` is still accurate. --------- Co-authored-by: Tom Boucher <trekkie@nomorestars.com> |
||
|
Rezolv
|
3dff700aaa |
fix(#3102): render edge-probe coverage report so the resolution loop consumes it (#3391)
* fix(#3102): render edge-probe coverage report so the resolution loop consumes it Step 5.5 captured the edge-probe report into $COVERAGE, shape-checked it, and reduced it to coverage.applicable — the engine's per-requirement items[] never reached the model, so the resolution loop re-derived edge categories from prose (the data-flow twin of #2733's control-flow discard). The block's own comment claimed the opposite. Render $COVERAGE RAW into context after the well-formedness guard (schema-agnostic so an ADR-550 D7a-style re-cut cannot desync a bespoke renderer), and bind the rows in the resolution loop as a deterministic FLOOR the model unions with its own classification — floor, never ceiling, since the classifier has a measured recall gap (ADR-857 §98 / ADR-550 D7b). --auto consumes the same floor. Comment corrected to match. Regression test asserts a bare render of $COVERAGE, not a count-only cross. * chore(#3102): add changeset for the Step 5.5 edge-coverage render fix * chore(#3102): re-arm spec-phase.md emitted-drift ack for the Step 5.5 render growth The render + floor-binding prose grows spec-phase.md ~1818 bytes (32238 -> 34056, under the 40960 cap). Re-arms the existing spent spec-phase.md ack rather than adding a new fragment (a second key would collide with the base-relative duplicate check). |
||
|
Tom Boucher
|
86101ee612 |
fix(#3133): keep global Claude @-references on tilde, not $HOME (#3393)
* test(#3133): global Claude @-references must resolve on tilde, not $HOME Regression for #3133: on a global Claude install, _applyRuntimeRewrites rewrites @~/.claude/... -> @$HOME/.claude/..., but Claude Code does not expand $HOME in @-file references, so the include silently resolves to nothing and the skill loads an empty execution_context. Rows 1/2/6/9 fail RED on next; rows 3-5/7-8 guard #1284 (shell $HOME), local redirect, #3503 (no homedir leak), and non-Claude isolation. * fix(#3133): keep global Claude @-references on tilde, not $HOME On a global Claude install, _applyRuntimeRewrites rewrote every @~/.claude/ include to @$HOME/.claude/ because computePathPrefix returns the $HOME form for shell-context correctness (#1284: ~ does not expand inside double quotes). Claude Code does not expand $HOME in @-file references, so the rewritten include silently resolved to nothing and skills loaded an empty execution_context. Add a normalization pass in the claude case: when the prefix is the $HOME (global) form, restore @$HOME/.claude/ -> @~/.claude/ (the form Claude expands and the shipped tarball uses). Shell contexts keep $HOME; local installs (absolute prefix) are unaffected; computePathPrefix is untouched (all its tests stay green). * test(#3133): fix row 9 assertion — drop end-of-string anchor Row 9's regex used $ (end-of-string) but a realistic @-ref line ends with a newline, so the anchor could not match. The fix under test produces the correct @~/.claude/gsd-core/references/ui-brand.md tail; only the assertion was wrong. Drop the anchor and assert the tail + no @$HOME. * docs(#3133): add changeset * docs(#3133): backfill changeset PR number (3393) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
2b20b7e2cd |
fix(#3257): preserve full-line frontmatter comments through the parse→reconstruct pair + syncStateFrontmatter (#3387)
* test(#3257: full-line frontmatter comments survive the parse→reconstruct pair AND a mutating state verb parseYamlRegion dropped column-0 # comments and reconstructFrontmatter rebuilt from Object.entries alone, so full-line comments were silently destroyed on every mutating STATE verb. Add failing-first regressions: 3 unit tests for the public pair (comment between keys, leading+trailing, consecutive) and an e2e test running a state verb (state update) on a commented STATE.md — the e2e exercises syncStateFrontmatter's fresh-derivedFm rebuild path, which is the actual loss site the issue is filed against. RED — fails on next; fix follows. * fix(#3257: preserve full-line frontmatter comments through parse→reconstruct AND syncStateFrontmatter Carry column-0 # comments through the frontmatter pair via a Symbol-keyed channel (FULL_LINE_COMMENTS): parseYamlRegion captures ^# lines and attaches them to the next top-level key (leading) or a trailing slot; reconstructFrontmatter re-emits them in place. The Symbol is invisible to Object.entries/keys/JSON, so every existing reader is unchanged; the channel is created only when a comment is seen, so comment-less frontmatter is byte-identical. CRITICAL (isolated review): syncStateFrontmatter rebuilds its target via buildStateFrontmatter (fresh object) + an Object.keys carry-forward, both of which skip the Symbol — so the pair-preserving channel was lost on the very STATE verbs the issue names. Export propagateCommentChannel(source, target) from frontmatter.cts and call it in syncStateFrontmatter before reconstruct, copying the channel onto derivedFm (leading filtered to keys still present so a deleted key's annotation drops with it, trailing preserved). Decision A. * chore(#3257: add changeset fragment * chore(#3257: backfill changeset PR number (#3387) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
864f76b46f |
fix(#3255): updateTableCell scans for the table carrying the requested column (#3377)
* test(#3255): updateTableCell reaches a later table when the first lacks the column A ## Traceability section holding a summary table (no Status) above the requirement table (with Status) made updateTableCell bind to the first table and return 'unknown column: Status', so requirements.mark-complete left the row at Pending. Add a failing-first regression at the primitive. RED — fails on next; fix follows. * fix(#3255): updateTableCell scans for the table carrying the requested column updateTableCell bound to the FIRST GFM table in the scoped text and returned 'unknown column' if that table lacked the column — so a ## Traceability section holding a phase-summary table above the requirement table never reached the requirement table, and requirements.mark-complete left the row at Pending while reporting table_unmatched (the #2140 silent-divergence class one level deeper, flagged but not closed by the #2245 cross-section scoping fix). Scan candidate table headers and pick the first VALID table (delimiter + matching column count) whose columns include the requested column. Single-table behaviour is byte-identical (the one table carries the column). Error semantics preserved: no table -> 'no table found'; valid table without the column -> 'unknown column'; lone malformed table keeps its specific reason. Also fixes the derived hasRow/doneTable reasoning, which probes via the same call. * chore(#3255): add changeset fragment * chore(#3255): backfill changeset PR number (#3377) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
23e6d49929 |
fix(#3233): no-op state update-progress when the milestone scan finds zero plans (#3375)
* test(#3233): zero plans (0/0) is a no-op; plans-but-none-done still writes 0% cmdStateUpdateProgress mapped 0/0 through clampPercent to 0% and rewrote the shipped Progress record after milestone close. Replace the stale 'handles zero plans gracefully' test (which asserted the buggy percent:0) with a #3233 no-op regression (100% record preserved, updated:false), and add a negative-space guard: plans exist but none done must still write a legitimate 0%. RED — fails on next; fix follows. * fix(#3233): no-op state update-progress when the milestone scan finds zero plans cmdStateUpdateProgress mapped 0/0 through clampPercent to 0% and unconditionally rewrote the body Progress line, so after /gsd-complete-milestone archived the phases (.planning/phases/ empty, scope COMPLETE) a routine update-progress run destroyed the shipped record ([██████████] 100% → [░░░░░░░░░░] 0%). Add an early-return no-op when totalPlans === 0 — mirroring the established scope-withholding no-op (stderr WARNING + {updated:false, reason}) and computeProgressPercent's null-for-empty contract ('nothing to measure' ≠ '0% done'). The legitimate 0% case (plans exist, none summarized) is unaffected: totalPlans > 0 reaches clampPercent(0, N>0) = 0 and writes 0% as before. * test(#3233): unshadow 'Progress field missing' — clear the zero-plans guard The new totalPlans===0 no-op guard fires before the 'Progress field not found' branch, so the existing 'returns error when Progress field missing' test (no phase dirs → 0 plans) was passing for the wrong reason and that branch lost coverage. Give that test a phase dir + PLAN so totalPlans > 0 clears the guard and it reaches the branch it is named for. (Isolated review finding.) * chore(#3233): add changeset fragment * chore(#3233): backfill changeset PR number (#3375) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
ae7dc52972 |
fix(#3225): guard W006/W007 + consistency loops with isSentinelPhaseId (#3371)
* test(#3225): sentinel phase dirs no longer trigger W007 / consistency warnings / gaps The W006/W007 (validate health) and the parallel consistency disk↔roadmap and gap-numbering loops never got the isSentinelPhaseId guard that phase.cts has (#2786/#2949), so every sentinel phase dir (999.x/0.x — never-on-roadmap by convention) produced a spurious W007 and a spurious 'Gap in phase numbering: N → 999'. Add failing-first regressions for both surfaces + the gap check, each with a non-sentinel orphan negative-space guard. RED — fails on next; fix follows. * fix(#3225): guard W006/W007 + consistency + gap loops with isSentinelPhaseId cmdValidateHealth's W006/W007 loops, cmdValidateConsistency's parallel disk↔ roadmap loops, AND its gap-numbering check never got the isSentinelPhaseId guard that phase.cts has at 10+ sites (#2786/#2949). So any repo using the sentinel-id convention (999.x backlog/interim, 0.x drafts) got a permanent spurious W007 and a spurious 'Gap in phase numbering: N → 999', with advice to add-to-roadmap (violates the convention) or delete (destroys archived work). Add isSentinelPhaseId to the phaseIdMod destructure and skip sentinel ids in: W006 + W007 (cmdValidateHealth); the two plain-warning disk↔roadmap loops and the gap-numbering integerPhases filter (cmdValidateConsistency — same bug family, folded in inline per no-silent-defer). Additive only: non-sentinel orphans and real numbering gaps still warn. The gap-numbering guard was surfaced by the isolated review (a 999-interim dir would otherwise create a false 'N → 999' gap). Same family as #3167 (since fixed). * chore(#3225): add changeset fragment * chore(#3225): backfill changeset PR number (#3371) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
62f5f3b39e |
fix(#3224): register WINDOWS.md as a canonical .planning/ artifact (#3369)
* test(#3224): assert WINDOWS.md is a canonical .planning/ artifact The broken-windows ledger (.planning/WINDOWS.md, written by gsd-core's own windows command) was absent from CANONICAL_EXACT, so validate health flagged it W019 'Unrecognized' with advice to delete a file that can gate /gsd-ship. Add WINDOWS.md to the expected-canonical list + a dedicated predicate test. RED — fails on next; fix follows. * fix(#3224): register WINDOWS.md as a canonical .planning/ artifact CANONICAL_EXACT (src/artifacts.cts) was never updated when the broken-windows capability (#1950/#2441) started writing .planning/WINDOWS.md. validate health therefore flagged the ledger W019 'Unrecognized' with fix advice to archive or delete it — a file gsd-core itself produces (src/broken-windows.cts, LEDGER_FILE_NAME) and that can gate /gsd-ship under workflow.windows_enforce. Add WINDOWS.md to CANONICAL_EXACT, per the registry header's own maintenance mandate ('Add entries here whenever a new workflow produces a .planning/ root file'). isCanonicalPlanningFile now returns true for it, suppressing the false W019. Existing W019 behavior for genuinely-unrecognized files is unchanged. * chore(#3224): add changeset fragment * chore(#3224): backfill changeset PR number (#3369) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
bfd749cb9a |
fix(#3213): segment-boundary membership for letter-named phase dirs (#3368)
* test(#3213): add letter-named phase regression for getMilestonePhaseFilter The custom-ID branch's greedy capture excluded every letter-named phase directory (Phase A:..Phase L:) from the milestone, fabricating counts. Add two failing-first regression tests: a single letter phase + numeric control, and the full A..L + 00 tree from the issue's reproduction. RED — fails on next; fix follows in a separate fix: commit. * fix(#3213): segment-boundary membership for letter-named phase dirs The custom-ID branch of isDirInMilestone (getMilestonePhaseFilter) used a greedy capture ^([A-Za-z][A-Za-z0-9]*(?:-[A-Za-z0-9]+)*) that swallowed the whole hyphenated directory name (A-tool-output-contract was captured as 'A-tool-output-contract', not 'A'). The set lookup then failed and every letter-named phase directory (Phase A:..Phase L: — GSD's own convention, ADR-612 first-class non-numeric IDs) fell out of the milestone, silently fabricating progress/plan counts over whatever numeric dir survived. Replace the capture-then-lookup with a segment-boundary membership test: a directory belongs if its lowercased name EQUALS a declared phase ID or BEGINS with that ID followed by '-' (so 'A-tool-output-contract' matches ID 'a'; 'PROJ-42-description' matches ID 'proj-42'; 'AB-combined' does NOT match 'a'). IDs are sorted longest-first so a hyphenated id (proj-42) is tested before a prefix of it (proj). Additive only — numericRe still handles every leading-digit dir first, and this can only ADMIT a dir the greedy capture wrongly excluded, never exclude one already matched. * chore(#3213): add changeset fragment * chore(#3213): backfill changeset PR number (#3368) --------- Co-authored-by: sim <sim@local> |
||
|
Tom Boucher
|
e7993d77bf |
fix(#3207): create-and-switch on the first phase/milestone commit (#3363)
* test(#3207): invert fresh-create branching tests to expect create+switch The four fresh-create branching tests (#3079) locked in create-without- switch behavior that #3207 identifies as the regression: a fresh phase/ milestone branch is created but HEAD never moves onto it, so the first phase/milestone-scoped commit lands on the base branch. Invert those assertions to expect create+switch, and add two new tests: - fresh-create is non-silent (logs the create+switch) (#3207 AC3) - a second phase commit does not re-warn once HEAD is on the phase branch (#3207 AC5) The existing-branch path (#2539) is deliberately byte-for-byte unchanged. RED — fails on next; the fix follows in a separate fix: commit. * fix(#3207): create-and-switch on the first phase/milestone commit The #3079 fix (PR #3141) replaced git checkout -b with git branch (create-only) unconditionally, including the case where the strategy branch does not yet exist. That regressed #1278: the first phase- or milestone-scoped commit no longer landed on the strategy branch — it stayed on the base branch, and the strategy branch was left as an empty marker pointing at the pre-phase tip. Every subsequent commit then warned 'already exists; committing on the current branch instead of switching', wording that misleads because the tool itself created the branch moments before. Re-separate the two cases #3079 collapsed: - branch does NOT exist -> create AND switch (git checkout -b). The #3079 resurrection hazard cannot apply: the branch was just verified absent, so there is no merged-and-deleted ref to resurrect and no silent move onto an existing unrelated branch (#2539 AC2 is honored by the existing-branch arm, which is unchanged). The create is logged so the first phase-scoped commit is not silent (#3207 AC3). - branch ALREADY exists -> unchanged: no switch, commit on current branch, non-silent warning (#2539/#3079). Once the first commit switches HEAD onto the strategy branch, the currentBranch !== branchName guard skips the block on subsequent commits, so the misleading 'already exists' warning no longer recurs. * chore(#3207): add changeset fragment * chore(#3207): backfill changeset PR number (#3363) --------- Co-authored-by: sim <sim@local> |
||
|
0xdhx
|
0396d9cab1 |
enhance(#2483): stop the claude reviewer lane from inheriting CLAUDE.md + auto-memory (#2493)
* enhance(#2483): env-guard the claude reviewer leg against CLAUDE.md injection
The claude reviewer in workflows/review.md was a bare headless `claude -p`
spawn run from the project cwd, so it inherited the invoking user's global
CLAUDE.md, the project CLAUDE.md, and Claude Code auto-memory.
That made it the only reviewer leg seeing anything beyond the prompt file.
gather_context assembles PROJECT.md, the roadmap section, every PLAN file,
CONTEXT.md, RESEARCH.md and REQUIREMENTS.md into the prompt before any
reviewer runs; the gemini leg receives only that prompt and the codex leg
runs --ephemeral. Beyond the measured ~4k tokens/spawn, the asymmetry cuts
at the workflow's own premise: "independent review" meant something
different for the claude leg than for the other two.
Guard both dispatch lines with a per-invocation
`env CLAUDE_CODE_DISABLE_CLAUDE_MDS=1`. `env`, never `export` — the flag
must not leak into the orchestrating session (which may itself be Claude
Code on the SELF_CLI="auto" path) or into any later spawn.
review.md is the only claude -p call site in the installed tree, so this is
two lines on one surface. The self-skip logic is untouched.
* enhance(#2483): fix CRLF-fragile split and regenerate workflow baselines
Two CI failures from the first push, both mine:
1. lint-tests: the new regression test split readFileSync content on a
literal "\n". On a Windows git-autocrlf checkout that leaves a trailing
"\r" on every line (local/no-crlf-fragile-split). Use .split(/\r?\n/).
2. golden-install-parity / workflow-size-budget / workflow-compat: editing
gsd-core/workflows/review.md changes its content hash and byte size, and
both are pinned in committed baselines. Regenerated via the repo's own
generators (npm run size:baseline, npm run gen:golden).
The regenerated diffs are review.md-only: exactly one hash line per
golden-install-parity fixture and one size entry in workflow-size-baseline
— no unrelated drift swept in.
Full suite now green locally: 2113 pass, 0 fail, 3 skipped (run with HOME
and CLAUDE_CONFIG_DIR overridden to throwaway dirs; live profile verified
untouched afterward).
* enhance(#2483): adapt guard-test matcher to the effort-args dispatch reshape
The effortSurface wiring (#2481) reshaped the bare-model dispatch to
`claude $CLAUDE_EFFORT_ARGS -p -`; the invocation matcher's dash-first
form could no longer see it, and the count assertion failed exactly as
designed. The matcher now tolerates variable expansions between `claude`
and its first literal flag. Negative-controlled both ways: a stripped
guard and a deleted dispatch line each still fail.
* enhance(#2483): also guard the claude leg against auto-memory injection
CLAUDE_CODE_DISABLE_CLAUDE_MDS suppresses CLAUDE.md file loading;
auto-memory is an independently-toggled mechanism with its own flag.
Add CLAUDE_CODE_DISABLE_AUTO_MEMORY=1 to both dispatch lines, correct
the docs/COMMANDS.md and changeset claims that credited the first flag
with covering auto-memory, and extend the regression test to require
both flags on every claude invocation (negative-controlled: 2/4
assertions fail with the new flag removed).
* enhance(#2483): match the claude binary in command position, not argument position
The line-oriented invocation matcher counted any line where the token
`claude` was followed by a flag. #2589 (landed on next as
|
||
|
Dennis Kim
|
68a199cf5a |
fix(#2783): address wedged PRs in ship note protocol (#2818)
* fix(#2783): address wedged PRs in ship note protocol * chore: acknowledge ship.md growth * fix(#2783): repair ship workflow structure * fix(#2783): gate ship-note recovery on current PR state * fix(#2783): avoid scanner collision in poll loop * fix(#2783): address reviewer feedback on ship-note wedge handling * test: add timeout to spawnSync in ship-notes-wedged-pr.test.cjs to satisfy lint * test: update ghCalls bound expectation in ship-notes-wedged-pr.test.cjs * test: restore ghCalls expected count in ship-notes-wedged-pr.test.cjs |
||
|
Dennis Kim
|
5e951540af |
fix(#3162): resolve active state phase before drift scan (#3208)
* test(02-01): reproduce template state validation drift - derive command fixtures from the shipped STATE template - pair passed-verification drift with a clean opposite-result control * test(02-01): cover state phase resolution boundaries - exercise precedence conflicts fallbacks and fail-closed directory handling - prove canonical equality and reject outside-root verification evidence * docs: add changeset for PR #3208 * Address review feedback * fix(#3162): preserve phase validation after state refactor * test(#3162): align validation scope cases |
||
|
Behruz Nassre Esfahani
|
2076d450d7 |
fix(#2652): gate quick/diagnose dispatch on dispatch.isolation, not the runtime name (#2728)
* fix(#2652): gate quick/diagnose dispatch on dispatch.isolation, not runtime name quick.md and diagnose-issues.md kept the pre-#2584 `RUNTIME != "claude"` worktree gate, so every non-Claude runtime failed closed regardless of the capability it negotiated — including Codex, which declares orchestrator-worktree. Route both through the negotiated dispatch.isolation seam via a new shared reference, and migrate the two execute-phase reference fragments that carried the same runtime-name gate. - new gsd-core/references/dispatch-isolation-gate.md: canonical ISOLATION resolution, harness-flag resolution, single-agent degrade rule - quick.md / diagnose-issues.md read the gate; dispatch uses the {harnessFlag} placeholder rather than a hardcoded isolation="worktree" - execute-phase-wave-guard.md / execute-phase-between-wave-reset.md: migrate [ "$RUNTIME" = "claude" ] -> [ "$ISOLATION" = "harness-worktree" ] - every degrade site now clears BOTH USE_WORKTREES and ISOLATION; clearing one dispatched an isolated agent with no base guard and no manifest - parity guard in host-integration.test.cjs scans workflows AND references and matches six reintroduction shapes - migrate four tests that pinned the pre-#2584 runtime-name contract Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(#2652): use the /gsd:<cmd> namespace in the isolation degrade messages The degrade warnings cited /gsd-execute-phase, the retired hyphen form that slash-command-namespace.test.cjs rejects in Claude-facing source. Same length, so the quick.md size budget is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(#2652): add changeset for PR #2728 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(#2652): normalize dispatch-site paths to forward slashes for Windows path.relative() returns backslash-separated paths on Windows, so the #2652 dispatch-site parity test compared "gsd-core\workflows\quick.md" against the hardcoded forward-slash literal "gsd-core/workflows/quick.md" and failed on every windows-latest CI lane. Normalize with .replace(/\\/g, '/'), matching the existing convention used elsewhere in this suite (e.g. tests/branch-no-track-guard.test.cjs:37). * test(#2652): restore the size-growth acknowledgment The rebase dropped tests/emitted-drift-ack.json. #2757/#2758 fixed the ATTRIBUTION axis, but the SIZE-GROWTH axis is independent: diagnose-issues.md (+2086) and quick.md (+230) still need an ack naming them and saying why. Verified: 65/66 without it (both files named), 66/66 with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(#2652): convert execute-plan.md Pattern A onto the dispatch-isolation gate Pattern A hardcoded `isolation="worktree"` — Claude Code's own literal — gated only on `workflow.use_worktrees`, with no capability negotiation at all. It is the same defect #2652 fixes at the other four sites, just a different shape: the file contains no RUNTIME variable, so the new detector correctly does not flag it. Concrete break: a Codex user who follows this PR's own newly-documented pattern and sets `workflow.use_worktrees: true` to get isolated dispatch via /gsd:quick then runs a plan through /gsd-execute-plan Pattern A, and hits an unconverted path — either an Agent() call erroring on an unrecognized parameter or silent unisolated execution, depending on host tolerance. Pattern A is a single-agent dispatch site through the host's own subagent tool, so it takes the same treatment as quick.md and diagnose-issues.md: resolve ISOLATION/HARNESS_FLAG through the canonical reference, degrade to sequential on orchestrator-worktree hosts, and substitute the host's declared {harnessFlag} instead of Claude Code's literal. while the area was open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(#2652): add the INVENTORY row for dispatch-isolation-gate.md, refresh CONTEXT Two bookkeeping gaps flagged in review: INVENTORY.md had no row for the new gsd-core/references/dispatch-isolation-gate.md. INVENTORY-MANIFEST.json was regenerated correctly and its --check only diffs a live directory scan against the committed manifest, so CI passed regardless — but gen-inventory-manifest.cjs's own stderr guidance says to add the matching INVENTORY.md row. This is the repo's named "Inventory Drift" pattern. Placed with the dispatch/isolation cluster (worktree-branch-check, runtime-aware-dispatch) rather than alphabetically, matching how that table is grouped. CONTEXT.md's Host-Integration Interface entry still described dispatch.isolation as "declared and negotiated but not yet consumed by any scheduler — Phase 1 of #2584". That was already stale before this PR (execute-phase graduated in Phase 3) and more so now with three single-agent dispatch sites consuming it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(#2652): detect reversed-operand runtime gates; add a permutation property All five reintroduction regexes assumed $RUNTIME on the LEFT of the comparison, so `[ "claude" != "$RUNTIME" ]` — the same gate written backwards — evaded every one of them. Verified against the old patterns before fixing: all four reversed shapes (single bracket, double bracket, test builtin, JS template) scored EVADED. Each comparison shape is now generated in both operand orders from a single template, so a shape cannot be added in one order and forgotten in the other. The mutation table gains the four reversed cases. Also adds the fast-check property review suggested in place of the hand-rolled cases: it generates the cross product of the axes an author actually varies — bracket form, operator, operand order, quoting, spacing, runtime id — so a permutation the hand-written patterns miss surfaces here rather than in production. The 11 explicit cases stay as named regression anchors. execute-plan.md joins the scan's required-identities list now that it is a converted dispatch site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(#2652): acknowledge the execute-plan.md size growth The Pattern A conversion adds 811 bytes to an emitted workflow. Per #2719 the size axis needs its own acknowledgment, independent of attribution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(#2652): repin the execute-plan.md PROSE_ALLOWLIST line after the rebase The #2751 command-position gate pins its prose exemptions by line number. This branch inserts the dispatch-isolation resolution above the `validated downstream by gsd-tools uat classify-coverage` sentence, moving it from execute-plan.md:387 to :397 — which fired the gate twice for one displacement (an un-allowlisted mention at 397, a stale entry at 387). The prose itself is unchanged from next; only the pin moves. Fixes #2652 * fix(#2652): gate the #2649 base-check on ISOLATION in diagnose-issues.md The rebase onto next merged #2649's pre-dispatch base-check textually, but its degrade flipped USE_WORKTREES after ISOLATION was already resolved, so the degrade never reached the dispatch decision. Gate the block on ISOLATION = "harness-worktree" and degrade ISOLATION itself, the same pairing quick.md already uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2652): key quick.md post-dispatch bookkeeping on ISOLATION, not the Claude literal Review Blocker: the manifest append (l.822), worktree merge-back (l.825), and its skip clause (l.839) all conditioned on the literal isolation="worktree" — Claude Code's own rendering of {harnessFlag}. Cursor renders --worktree, so a newly-unblocked isolated Cursor run created a worktree whose committed work was never merged back and never cleaned up, silently. All three now key on ISOLATION = "harness-worktree" at dispatch. The existing parity detector cannot catch this class (its ISOLATION_TOKEN treats the literal as a legitimate marker), so this adds a dedicated literal-condition detector with a discrimination proof against both pre-fix sentences, a benign-mention control, and a positive pin on all three re-keyed conditions. Verified fail-first against the pre-fix quick.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2652): scope the use_worktrees=false install stamp to isolation=none runtimes `_stampNonClaudeRuntimeDefaults` rewrote every non-Claude runtime's `workflow.use_worktrees` read to `--default false`. That default resolved before `gsd_run query dispatch-isolation` was ever consulted, so the five runtimes that declare worktree support — cursor (harness-worktree) and codex/opencode/kimi/kimi-code (orchestrator-worktree) — got ISOLATION=none regardless of what they negotiated. The gate this PR migrates dispatch onto was therefore still deciding isolation by runtime name, one layer down. The stamp's #1521 premise was that worktree isolation *was* Claude Code's isolation="worktree" spawn parameter, which no other host honored. #2584 replaced that premise with the negotiated capability. The stamp is now scoped to runtimes whose negotiated isolation really is `none`, where the default it writes is the outcome the resolver reaches anyway. `_negotiatedDispatchIsolation` mirrors routeDispatchIsolation's resolution against the same registry — closed vocabulary, a harness-worktree host must declare its flag, an orchestrator-worktree host must carry a descriptor that resolves — and fails closed to `none` on anything else, so an undeclared or unknown runtime keeps today's behavior. Two #1515 tests pinned the superseded premise for codex and are re-pointed at the new contract rather than deleted: the safety property they protect is now held by the isolation gate's fail-closed resolution, not by a name-scoped install-time default. Verified fail-first — all five assertions red against the pre-fix source, green after. * test(#2652): acknowledge the emitted ripple and re-point the end-to-end stamp proof Scoping the use_worktrees stamp changes emitted output, and two gates caught it. `gsd-core/workflows/execute-phase.md` now differs at emit time for the five hosts that declare worktree support (cursor harness-worktree; codex, opencode, kimi, kimi-code orchestrator-worktree) — the source file is byte-identical, only the stamp is gone. Acknowledged in this PR's fragment. `tests/install.test.cjs`'s real-install assertion pinned the superseded premise end-to-end, asserting codex receives `--default false`. Re-pointed rather than deleted, matching the two unit tests: it now proves codex keeps the unstamped `true` read. A second arm installs windsurf — which declares isolation `none` — and asserts the false stamp is still applied there, so the change cannot silently degrade into "never stamp" without a test noticing. The ack entry collides with `2658-trae-instruction-file-path.json`, which is fully spent (merged via #2925, so all 25 of its entries are present at base and gate nothing) and is pruned for the same reason and by the same rule as the spent `2649-*` fragment this PR already removed. #2566 prunes the same file for the same collision on `new-project.md`; a delete/delete merges cleanly either way, and the base-side cleanup would make both unnecessary. * fix(#2652): re-record the sentinel when a dispatch site degrades isolation Review Blocker B1/B2/B3. Every isolation degrade in a dispatch site is decided in shell, where routeDispatchIsolation cannot see it. That resolver persists whatever it resolved to the run-scoped sentinel as an unconditional side effect (#3045), so a degrade that only reassigns $ISOLATION leaves the sentinel asserting harness-worktree while the dispatch correctly omits the harness flag. The shipped PreToolUse guard reads the sentinel at the instant of the Agent() call and denies that mismatch with exit 2 — the work does not run unisolated, it does not run at all. Latent on this branch and lands on rebase, since |
||
|
Rezolv
|
e87fb409ee |
enhance(#2573): stamp STATE.md with its commit and surface a freshness hint (#2622)
* enhance(#2573): stamp STATE.md with its commit and surface a commit-age freshness hint Adds a `state_head` stamp to STATE.md and derives a tri-state commit-age freshness proxy (state_commits_behind / state_commit_stale) through state.cjs's readStateHeadFreshness, surfaced on smart-entry signals and as health W024. The proxy is advisory: classify() deliberately does NOT consume it (ADR-1787 locks the classification/routing boundary — a signal, not a route). Composes with #3099 and #1882 (both merged to next after this branch): the commit-age proxy reads `state_head` while the LAST_ACTIVITY_UNPARSEABLE diagnostic reads `last_activity` — two different fields, not "two staleness signals on one field." A new regression test asserts a STATE.md carrying both an unparseable last_activity AND a valid state_head resolves each independently (diagnostic fires once; freshness reads state_head, commits_behind 0). Rebased onto next (flattened): resolved the add/add conflicts in src/smart-entry.cts (kept both the #2573 freshness import/derivation and the #3099 diagnostic import/call) and tests/smart-entry.unit.test.cjs (kept both describe blocks). Drift-ack for health.md's W024 row is unchanged (12348 B). Tests: smart-entry 62, state/state-transition/health/verify 639, all pass. * chore(#2573): allowlist health-validation test in the prompt-injection scan The scanner's `exec('` code-execution pattern matches the benign `re.exec('<phase-id>')` RegExp method calls in the phase-ID grammar tests (pre-existing: 16 such calls on next, this PR adds none). The file entered the diff-mode scan's changed-file set only because #2573's W024 state_head assertions touch it. Allowlist it alongside the other test files that carry pattern-matching content as data (same DEFECT.PROMPT-INJECTION-SCAN-COLLISION class). Scanner self-test 38/0; diff scan 14 files, 0 findings. |
||
|
Tom Boucher
|
33fca50d8a |
test(#3333): fold the runtime & install surface fix-* cluster — Wave 1 (#3341)
* test(#3333): fold the runtime & install surface fix-* cluster — Wave 1 Folds 11 legacy tests/fix-*.test.cjs regression files into their module's main test suite: 6 folded into existing suites (host-integration-descriptors, effort-surface-axis, trae-imperative-reference, hermes-skills-migration, gsd-agent-isolation-guard), 5 renamed to become the module's sole suite (cursor-hook-workspace-roots, cursor-subagent-isolation, lint-compiled-artifact-sync, hooks-commonjs-marker, shared-hooks-dir-resolution). All 195 test() blocks preserved with zero drops; lint-test-file-count.cjs and eslint remain clean. No production code changed. Wave 1 of 7 in #3315 (H3 of epic #3053). * test(#3333): replace try/finally with t.after() in isolation-guard tests CONTRIBUTING.md bans try/finally inside test bodies (masks failures, not an approved pattern). The fold in the prior commit carried 27 instances forward verbatim from the deleted fix-3045-dispatch-isolation-resolver.test.cjs into an otherwise-clean file. Converts each to the approved per-test t.after() cleanup pattern — same cleanup call, registered instead of finally-wrapped. No assertion, fixture, or test-name change; test( count unchanged at 50. Found by the Standards review pass on Wave 1 (#3333, H3 of epic #3053). * fix(#3333): restore raw NUL byte mangled by the fold in hermes-skills-migration.test.cjs The prior fold commit copied fix-2284-hermes-agent-delegate-task-projection's "collision-robust" test via a text-based Read/Write pipeline, which silently turned a raw NUL byte (0x00) embedded in two string literals into a regular space character. That corrupted the test's actual purpose (proving a NUL byte survives a string-rewrite operation untouched) and produced a genuine gsd-test failure: `24 !== 1` for `out.split(' ').length`, because splitting on a space finds every space in the sentence instead of the single NUL byte the test meant to isolate. Root-caused by diffing the raw bytes (via `git cat-file blob` + `cat -v`) between the pre-fold source and the folded target — confirmed exactly two bytes differ. Restored via a byte-precise patch (latin1 round-trip) touching only those two lines; test( count and every other byte unchanged. * fix(#3333): use \x00 escape sequence instead of a raw NUL byte in test fixture The prior commit restored a byte-exact raw NUL byte matching the original fix-2284 source, and the production function (applyClaudeCodeBrandSwap) was confirmed correct in a standalone repro. But the same raw byte still failed through gsd-test's remote pipeline. Root cause is upstream of gsd-core: some step in that transfer path does not carry a raw 0x00 byte through untouched. A raw embedded NUL byte was never necessary here — `\x00` as a 4-character escape sequence in the source text produces the identical runtime character (U+0000) without ever putting a raw byte in the tracked file, sidestepping any byte-oriented transfer step. Applied at both call sites (the fixture string and the split() delimiter). No behavior change; test( count unchanged at 76. * fix(#3333): harden copyWithPathReplacement against a source file vanishing mid-copy (TOCTOU) Surfaced by this PR's own gsd-test run: tests/install-minimal-hooks.test.cjs and tests/opencode-command-dir-plural.test.cjs intermittently crashed with ENOENT reading gsd-core/workflows/zzz-e5-drift-fixture.md. Root cause is unrelated to test-file consolidation — tests/planning-prompt-drift.test.cjs writes that fixture directly into the real, shared gsd-core/workflows/ tree (main() hardcodes its scan root to the real repo) and deletes it in t.after(); copyWithPathReplacement's readdirSync-then-read loop has no protection against the listed file vanishing before it gets there, so a concurrently-running install path can crash entirely on what is otherwise a completely benign race. Fixed by skipping (not crashing on) a listed entry that no longer exists by the time the loop reaches it. Added a regression test that deterministically reproduces the race (readdirSync snapshot still lists the file; it is deleted immediately after) and proves both outcomes: no throw, and the vanished entry's destination is never partially written. Per CLAUDE.md's no-defer rule, a defect surfaced while verifying this PR is fixed inline rather than deferred — this overrides one-concern-per-PR. * fix(#3333): fix third NUL-byte-mangled occurrence missed by prior fix passes The fold originally mangled three raw-NUL-byte occurrences to spaces, not two — the earlier byte-restore and escape-sequence commits both only targeted the fixture string and the split() delimiter, missing out.includes('[ ]') a few lines below (should read out.includes('[\x00]')). A remote gsd-test run kept failing on this exact assertion even after both prior fixes, which is what surfaced the miss. Verified via a standalone repro using the file's real (not retyped) fixture content: all six assertions in the collision-robust test now pass. Zero raw NUL bytes remain in the file; test( count unchanged at 76. * chore(#3333): add changeset for the copyWithPathReplacement TOCTOU fix Fixed-type fragment for the production defect fixed inline in this PR (bin/install.js's copyWithPathReplacement). Exempt from docs/ requirements per CONTRIBUTING.md (only Added/Changed/Deprecated/Removed require it). * chore(#3333): backfill changeset PR number (pr:0 -> pr:3341) --------- Co-authored-by: sim <sim@local> |