* test(#2847): add failing-first regression tests for gap-closure frontmatter schema gap
--gaps did not load a machine-checked requirement for gap_closure: true.
The planner's only validation gate (frontmatter.validate --schema plan)
never required it, and plan-phase.md's downstream_consumer contract never
mentioned it either, so gap-closure plans could pass validation while
missing the field that /gsd:execute-phase --gaps-only filters on.
These tests are RED against current production code: no plan-gap-closure
schema exists yet, and neither agents/gsd-planner.md's validate_plan step
nor plan-phase.md's downstream_consumer block references gap_closure
conditionally.
* fix(#2847): enforce gap_closure via plan-gap-closure schema
--gaps did not load a machine-checked requirement for gap_closure: true.
The planner's only validation gate (frontmatter.validate --schema plan)
never required it, so a gap-closure plan could pass validation while
missing the field /gsd:execute-phase --gaps-only filters on, silently
spawning zero executors.
Add a plan-gap-closure schema (every plan-required field plus
gap_closure) and make the planner's validate_plan step select it when
gap_closure mode is active, plan otherwise. Standard/reviews-mode plans
are unaffected: plan's required fields are unchanged.
plan-phase.md's downstream_consumer block was investigated for a
symmetric mention but deliberately left untouched: it sits 36 bytes
under the frozen ADR-857 PRE_PHASE6 ceiling and the validate_plan step
in gsd-planner.md is the actual call site, needing no help from
plan-phase.md's prose.
* fix(#2847): compact validate_plan edit under gsd-planner.md size caps
Merging origin/next (7 commits, including #2775's gsd-planner.md
STRIDE-row edit) left only 22 chars of headroom under four separate
hard-coded 49152-char caps on gsd-planner.md (planner-decomposition,
precondition-element, reversibility-tagging, security.test.cjs). The
verbose validate_plan prose from the previous commit overran all four.
Compact the edit to a single line (net +17 chars vs origin/next) while
keeping the functional content: schema name, mode condition, and the
unchanged base required-fields list.
Also:
- Fix a real bug in the fix-2847 negative-assertion test: plan-phase.md
mentions the literal string "<downstream_consumer>" twice in
backtick-quoted prose before the actual opening tag, so a plain
indexOf() grabbed the wrong start position and swallowed ~10KB of
unrelated content (including a "gap_closure" hit in a Mode: enum
line), producing a false failure. Anchor on the tag starting its own
line instead.
- Merge the emitted-drift-ack fragment for gsd-planner.md with the
#2775 fragment brought in by the merge (both named the same path;
two ack sources may never name the same path) and correct its byte
delta to the actual final number.
* fix(#2847): drop stale merge-inherited emitted-drift-ack fragments
Merging origin/next brought in three new emitted-drift-ack fragments
(1700, 2658, 2775) relative to this branch's fork point. #2775
collided with my own gsd-planner.md key and was already consolidated.
#1700 and #2658 don't collide, but none of their entries name a path
this branch's actual diff touches (git diff --name-only
origin/next...HEAD) — the ripples they explain are already baked into
the current next baseline, so they explain nothing here and the
emitted-attribution gate correctly reports them as stale (verified
live: spike-wrap-up.md from #1700).
Delete both fragment files. Neither is referenced by any test beyond
a stray comment pointing at an unrelated diagnosis artifact path, not
the ack fragment itself.
* fix(#2847): restore merge-inherited ack fragments deleted in error
1700-spike-manifest-idea-scoping.json and 2658-trae-instruction-file-path.json
exist on origin/next (landed via other, already-merged PRs) and arrived
on this branch unchanged via the origin/next merge. The previous commit
deleted them to satisfy a stale-acknowledgment finding, but the finding
was about the acks being MODIFIED in this diff, not about needing to
stop existing — deleting them would have silently reverted two other
PRs' already-merged, already-justified byte growth.
Restored byte-identical to origin/next (git diff origin/next -- <path>
empty for both). 2775-planner-package-legitimacy-gate.json stays
consolidated into 2847-gap-closure-validate-plan-step.json: that one
was a genuine hard key-collision (two fragments naming the same
gsd-planner.md path, which lint-emitted-drift-ack hard-blocks), not a
pass-through case.
* fix(#2847): bind --schema to gap_closure mode, not hardcode it
Prior revision left the validate_plan bash invocation unconditional
(--schema plan)) while only the prose sentence above it described the
gap_closure-mode branch. An agent executing the shown line literally
always validated with the plan schema, so a gap-closure plan missing
gap_closure: true still reported valid:true — #2847 reproducing
unchanged. Existing tests didn't catch it: they checked for substring
presence anywhere in the step, which the prose alone satisfied.
Change the bash line to --schema "$SCHEMA" — a real shell-variable
reference in the same placeholder convention this file already uses
for "$PLAN_PATH" (never literally assigned; the agent resolves it from
context, same as PLAN_PATH). A genuine if/then bash conditional
already exists elsewhere in this file (load_project_state's
INIT @file: check), confirming executed conditionals, not merely
descriptive prose, are the established pattern here.
Rewrite the regression test to assert on the bash block's literal
--schema argument: reject a hardcoded plan) or plan-gap-closure)
literal, require a variable reference, and require the step's prose to
bind that same variable name. Verified RED against the prior revision
and GREEN against this one before committing either state.
* fix(#2847): CRLF-safe tests, drop unexplained ack, require gap_closure=true
Four items from independent review, all landing together per request:
1. The #2847 regression test file had two CRLF-fragile regexes
(local/no-crlf-fragile-split): a bare \n on readFileSync content
means a real \r\n checkout returns invocationLine === null and all
four executable-content assertions stop asserting anything while
still reporting green. Both now use \r?\n. Prior lint report of
exit 0 was a false green from a stale eslint cache.
2. The 2847 drift-ack fragment explained nothing: a direct edit to
agents/gsd-planner.md is self-explaining, drift-acks exist for
emitted-artifact ripple that cannot be traced to a changed source
path. Deleted. Restored the 2775 fragment byte-identical to next
(git diff --name-status next...HEAD -- tests/emitted-drift-acks/
now prints nothing) — it only conflicted with the now-deleted 2847
fragment, never needed touching itself.
3. plan-gap-closure validated gap_closure by PRESENCE only
(unchanged since the original #2847 fix), so gap_closure: false
satisfied it — --gaps-only filters strictly on gap_closure === true,
so a false-valued plan still validates green and still spawns zero
executors: #2847's exact reported symptom, one value away. Added an
optional requiredValues map to FRONTMATTER_SCHEMAS; plan-gap-closure
now requires gap_closure to equal the string "true" (extractFrontmatter
parses every scalar as a string) in addition to being present. Every
other schema/field keeps the original presence-only contract. The
row that had documented the hole instead of closing it now asserts
the fix; a matching unit test locks requiredValues on
FRONTMATTER_SCHEMAS.
4. The "names the plain plan schema" assertion matched the bare
substring "plan" anywhere in the step, which verify.plan-structure
satisfies incidentally a few lines below — the assertion could not
fail even if the plain-plan branch were deleted from the prose.
Changed to match the standalone backtick-quoted plan token.
* fix(#2847): remove contradictory leftover assertion in Row 6 test
The gap_closure:false test asserted !present.includes('gap_closure')
(correct — matches the implementation's fold-wrong-value-into-missing
semantics) immediately followed by a stale, unedited leftover from an
earlier draft of the same test asserting the opposite:
present.includes('gap_closure'). The second could never pass once the
first did; both were in the same diff.
Verified before committing: searched every consumer of frontmatter.validate
output (agents/gsd-planner.md, docs/CLI-TOOLS.md, all other test files)
for any read of the present field — none exist. Nothing depends on
"present" meaning "physically exists regardless of value correctness",
so the implementation's fold (present/missing stay a full partition of
required) is the right call; the test needed to agree with it, not the
other way around. Manually replayed all six rows in the plan-gap-closure
describe block against the built CLI to confirm each now passes.
* fix(#2847): prototype-key guard, wrong-value diagnostic, doc fixes, vacuous tests
Six items from an independent SHIP_VERDICT:no review, landing together
per request:
1. Prototype-key crash (src/frontmatter.cts): FRONTMATTER_SCHEMAS[schemaName]
was an unguarded lookup, so --schema __proto__ (also constructor,
toString, hasOwnProperty, valueOf) resolved to an Object.prototype
member instead of undefined, the `!schema` check never fired, and the
command crashed with an uncaught TypeError and a stack trace instead of
"Unknown schema". Now reachable from prompt state (--schema is an
agent-bound $SCHEMA), not just an unreachable literal. Guarded with
Object.prototype.hasOwnProperty.call before the lookup, checked and
rejected before assignment so `schema`'s type stays non-optional. Added
a test for all five prototype keys.
2. Wrong-value diagnostic (src/frontmatter.cts, agents/gsd-planner.md):
the strict gap_closure === "true" check from the previous fix was
correct (fail-closed) but silent about WHY — a plan with
gap_closure: True got "missing", indistinguishable from genuinely
absent, even though the field is plainly in the file. Added an
`invalidValue` field to the validate JSON (present but wrong-valued,
disjoint from missing/present) and updated validate_plan's prose to
state the exact required literal and explain invalidValue, within the
remaining byte budget (49130/49152).
3. docs/reference/plan-md.md: fixed three inaccuracies in the gap_closure
row — "this field plus every field above" implied `requirements`
(documented Required: Yes) is schema-enforced, it is not; "Type:
boolean" implied YAML True/TRUE/yes/1 are accepted, they are rejected
(exact string match on literal lowercase true); "must never carry it"
stated an unenforced rule as fact. Also switched /gsd:plan-phase and
/gsd:execute-phase to the house-style hyphen form for docs/.
4. Vacuous negative assertions (tests/fix-2847-gap-closure-frontmatter.test.cjs):
RegExp#test coerces a null invocationLine to the string "null", so both
hardcoded-literal checks passed vacuously even if the step or its bash
block were deleted entirely. Added a truthy precondition check first.
5. Deleted vacuous/pass-always tests: four in tests/frontmatter.unit.test.cjs
strictly subsumed by (or, for the "superset" test, tautologically
guaranteed by the same spread as) the deepEqual exact-list test; two
describe blocks in the #2847 regression file that were already GREEN at
the RED commit (5e5897cd2f17ebf2fc55757bae651bbbeb236289) and pinned
untouched files rather than covering anything this change altered — one
of them additionally forbade any future legitimate gap_closure mention
in plan-phase.md, a trap for whoever frees up that file's byte budget
later.
6. .changeset/clever-newts-wake.md: switched /gsd:plan-phase and
/gsd:execute-phase to /gsd-plan-phase and /gsd-execute-phase — changesets
render verbatim into CHANGELOG.md with no converter in the path, so the
colon form would have reached readers naming a command no runtime
registers.
* chore(#2847): backfill changeset pr number (#3018)
---------
Co-authored-by: sim <sim@local>
9640968f8e