Files
msd-core/tests/graphify-visualization.test.cjs
Tom Boucher 15af0f5536 enhance(#3951): B6+B7 — widen two unreachable lint rules and make the guard ledger true (#3965)
* fix(#3951): two lint rules that could not reach the code they govern

B6 names two widenings. Measuring them first turned up a defect the criterion did
not know about, and refuted the reason it gave for one of them.

1. no-adhoc-markdown-parsing self-gates on its own filename.

   Lines 107-110 short-circuit create() to {} unless the path matches
   /(?:^|\/)src\/[^/]+\.cts$/. B6 says to widen the files: glob in
   eslint.config.mjs - but doing only that ships an INERT rule, because the gate
   still returns {} for every new path. Both halves have to change, and the gate
   is the load-bearing one.

   That same regex hides a live hole: [^/]+ is FLAT-ONLY, so it requires the file
   to sit directly in src/. The registered glob is src/**/*.cts, which includes
   subdirectories. 28 .cts files - health-diagnostic-rules/ (10),
   installer-migrations/ (11), observability/ (3), host-integration-adapters/ (2),
   vendor/ (2) - are inside the registered glob and silently skipped.

   Measured with the gate neutralized: 0 violations there today. The hole is
   hiding nothing right now, and is fixed anyway, because "no violations today" is
   not a property that keeps holding.

   The fix is not invented: require-subprocess-timeout.cjs:196 already carries the
   correct form of this guard, /(?:^|\/)src\/.*\.cts$/ with .*, one directory over.
   Checked the other 21 rules for the same bug - no-adhoc-regex-escape and
   no-private-binary-resolution short-circuit only to exempt their own seam file,
   which is the right shape, and no-crlf-fragile-split has no filename gate at
   all. This bug is unique to the one rule.

2. no-adhoc-regex-escape could not see the shape that actually occurs.

   Line 396 gated the whole UNSAFE-NEW-REGEXP arm on arg.type === 'Identifier'.
   Every check below it - the _SOURCE provenance check, the
   isSoleReturnOfOwnParameter shape - lives inside that branch, so
   new RegExp(obj['key']) and new RegExp(cfg.pattern) were never examined at all.
   Runtime data arrives as a property access far more often than as a bare
   identifier, which is exactly why this rule never fired on the #3477 ReDoS.

   Widened to MemberExpression, measured by AST walk across all five registered
   blocks rather than by grep. 27 sites, zero TSAsExpression:

     18  safe new RegExp(X.source, flags)  -> exempted, keyed strictly on the
         PROPERTY being `source`, never on the object. Keying on the object would
         wave through X.anything and buy nothing. B6 estimated ~10; that was an
         undercount.
      3  _SOURCE-suffixed constants reached through a required module namespace
         (phaseId.BRACKET_PHASE_TOKEN_SOURCE) -> the same provenance-exempt class
         the rule already recognizes for bare identifiers, extended to reach them.
         Without this the widening produces 3 false flags.
      6  real findings -> marked, each a test extracting a pattern from a shipped
         file at test time, where the runtime contract IS the product.

   Deliberately the NARROW MemberExpression form. The rule's own
   isSoleReturnOfOwnParameter doc comment records that an earlier broad
   "any non-literal identifier" heuristic produced ~25 false positives and was
   rejected; a re-run of the census after this change flags exactly the 6 above
   and nothing else.

Verified by execution, not by reading: the gate now accepts src/<subdir>/x.cts,
still accepts flat src/x.cts, and still exempts paths outside src/ - each pinned
by a test proven to fail against the old regex. build:lib, lint and lint:ci all
exit 0.

Refs #3951

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3951): give no-adhoc-markdown-parsing its reach, and fix the 80 parses it finds

The rule self-gates on filename AND is registered on one glob, so widening either
half alone is inert. Both move here: the gate now accepts tests/**/*.cjs and
scripts/**/*.cjs alongside src/**/*.cts, and eslint.config.mjs registers it on the
same two.

A test pins that the gate and the registration AGREE, in both directions. The
original defect was a gate narrower than its registration; the failure mode of
this fix is a gate wider than its registration. Both are silent, so the test
asserts the pair rather than either half.

80 violations across 43 files, all in tests/, zero in scripts/. 70 are routed
through the existing seams - scanFencedBlocks, collectSection, stripFencedCode,
tokenizeHeadings from markdown-sectionizer; splitTableRow, parseMarkdownTable,
findTableWithColumns from markdown-table. Headerless STATE.md tables use
splitTableRow per line, because parseMarkdownTable needs a real delimiter row.

10 are suppressed, 12.5%, well under the third that would have meant the rule is
mis-scoped for tests/ rather than the tests carrying debt. Each names its reason:
three regression guards (#3873 / bug-#21) are deliberately independent of the
generator's own fence handling, and routing them through the seam would have them
test the generator against itself; one is a negative-text probe that extracts
nothing; six are a shell-pipe-to-jq detector whose regex coincidentally matches the
table fingerprint and is not markdown parsing at all.

All ten sit in tests whose subject is .md content, which is normally a reason to
prefer the seam. The marker used is allow-adhoc-markdown, distinct from
no-source-grep's allow-test-rule, and lint:ci's lint-allow-test-rule-refs reports
the same 280/280 unverified count as before - checked rather than assumed, because
those two markers are easy to conflate.

The widening earned its keep immediately: it found a test that passed for the
wrong reason.

  tests/config-field-docs.test.cjs asserted notEqual(<cell>, '600') against the
  TYPE column instead of the DEFAULT column. notEqual('number', '600') is true
  forever, so the guard against workflow.subagent_timeout regressing to the old
  seconds default could never fire. docs/CONFIGURATION.md:434 is
  `| workflow.subagent_timeout | number | 300000 | ... |`, so the default is cell
  index 2; the assertion is now row-scoped through splitTableRow and reads 300000.

That is the argument for the widening in one case: the violation was invisible to
lint, the suite was green, and the assertion was vacuous. A rule that cannot reach
a file cannot tell you the file is lying.

Not fixed here, and recorded rather than assumed: #3426/#3239 are NOT reachable by
this widening. tests/package-legitimacy-gate.test.cjs yields zero violations even
with the gate bypassed - its hand-rolled scans are real, but built from line
filters and split('|') rather than the regex-literal fingerprints this rule
detects. They need new detectors. The epic assumed a wider glob would catch them.

build:lib, lint and lint:ci all exit 0; the post-fix census across tests/** and
scripts/** is 0 violations.

Refs #3951

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3951): B7 — and #3356's defects were still live in the code

B7 asks that each closed child be driven fail-first with a behavioral identity
test at the CONSUMER's output. Four of eleven children had no test citing their
issue number. Auditing them by BEHAVIOR rather than by number-grep changed the
answer for three of the four.

#3364 and #2540 — traceability only. Both were implemented by #3941 and their
consumer-output tests exist and were shown failing-first; neither cited its
originating issue, so an audit that greps for the number reports them uncovered.
Tagged the specific asserting test in each file, following the citation form those
files already use.

#3372 — covered, but only at helper level, and the triage narrowed it. Of the four
commands the issue names, only estimate-cli's collectCalibrationSamples actually
enumerates phase dirs from disk; smart-entry, audit and roadmap-upgrade derive from
ROADMAP/body text and never reach the sentinel path, so they are benign by
construction and were left alone rather than "fixed" into churn. The existing #3882
rows asserted the helper's return value. Added a consumer-output test driving
`query estimate-calibrate` and asserting sample_count and the persisted document.
RED proof: reverted collectCalibrationSamples to a raw readdirSync and ran the real
CLI - sample_count 3, sentinel leaked; restored - sample_count 2.

#3356 — NOT covered, and BOTH halves of the defect were still live in source. The
issue is closed; the bug was not fixed. Fixed here rather than writing tests that
document a bug as correct.

  Defect 1, the contradicted row. quick.md:627 claimed
  `quick-tasks-append` performs "the equivalent write" to the Step 7c row. It did
  not: the `#` cell was a positional ordinal and `Directory` read `—`, because the
  route had no way to receive a quick id or task directory. Added OPTIONAL
  `--quick-id` / `--slug` / `--directory`. A caller with neither - fast.md, the
  original #2133 caller - omits them and gets the byte-identical prior row, so
  nothing existing changes. A caller that HAS a real id and directory now gets the
  canonical row quick.md:632 renders. The false-equivalence sentence itself is
  corrected rather than left to mislead the next reader.

  Defect 2, the forced re-derive. The route called readModifyWriteStateMd with no
  options, so a body-only append to the Quick Tasks table triggered a full
  re-derive of the disk-derived progress.* frontmatter. Every other body-only
  writer passes { resync: false } - src/state.cts's own docstring prescribes it -
  and this route was the lone outlier. RED proof: reverted the option, seeded a
  project with 2 real phase dirs and a curated total_phases of 25, ran
  quick-tasks-append; total_phases collapsed to 2. Restored; it stayed 25.

That second one is the shape this epic exists to close: a silent write that
replaces curated state with a re-derivation nobody asked for, exit 0 throughout.

build:lib, lint and lint:ci all exit 0.

Refs #3951

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(#3951): amend B6's ledger to what was measured, and document the new flags

The ADR gains a ledger amendment in its own correction style - the sixth wrong
premise it records, found the same way as the other five, by measuring before
building.

B6 says the net guard count must fall. It rose: 62 -> 69, +7, measured from the
epic's filing commit to origin/next. The attribution is the point, though. Five of
the seven came from PRs unrelated to this epic, one was added by a phase of it, and
the epic did retire something sub-file - #3884 removed a detector with an explicit
"net: -1 detector, 0 added" ledger. Every named casualty is load-bearing, two
already carry retractions in this same document, and a sweep of all 22 rules plus
every scripts/lint-* found no provably dead guard. There is no honest way to make
the count fall; forcing it would trade coverage for a number, which is the Goodhart
outcome Decision 6 exists to prevent.

The amendment also records that B6's own prescribed fix for one widening was inert.
no-adhoc-markdown-parsing self-gates on its filename, so widening only the files:
glob - which is what the criterion says to do - ships a rule that still returns {}
for every new path. And #3426/#3239 are not reachable by that widening at all;
their scans use line filters and split('|'), not the regex fingerprints the rule
detects. The roster row tracked them against the wrong mechanism.

Three roster rows updated from aspiration to fact: the two widenings are DONE with
their measured counts, and lint-phase-enumeration-drift is marked RETAINED rather
than "expected casualty - verify before retiring", because Phase 5 verified it and
kept it.

The rule Decision 6 should carry forward is stated plainly: a guard ledger is a
claim about COVERAGE, not about COUNT. "Net count must fall" is measurable and
wrong. "Every guard is reachable, and each retirement names what makes its defect
unrepresentable" is the property that was actually wanted.

CLI-TOOLS.md documents the optional --quick-id/--slug/--directory flags and says
plainly that omitting them keeps the pre-#3356 row byte-identical, plus that the
append no longer re-derives progress frontmatter.

New features fragment (id 3951); FEATURES.md regenerated rather than hand-edited.
Changeset is Changed, pr:0 pending backfill.

Refs #3951

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3951): correct four rows that pinned the lint rule's old narrow reach

The remote suite came back RED with 5 failures, all in tests/eslint-rules.test.cjs.
They are stale tests, not a regression: four rows assert that
no-adhoc-markdown-parsing is inert outside src/*.cts, which is exactly the
contract this deliverable changes.

Confirmed by reading rather than inferred from the names - the row at :1981 used
filename: 'tests/some.test.cjs' and filename: 'scripts/helper.cjs', the two roots
the rule now covers on purpose.

Worth recording WHY local gates missed this. npm run lint and lint:ci were green,
and the touched test files passed standalone. Lint only reports violations in real
files; these rows assert the rule's REACH using synthetic RuleTester filenames, so
nothing but the full suite could see them. Local green on a rule change says
nothing about the rule's own tests.

Each row is rewritten with BOTH halves rather than flipped from valid to invalid:

  - the same fingerprint under tests/ or scripts/ is now flagged, with the right
    messageId
  - the negative space is preserved - the same fingerprint under a path outside
    all three roots (gsd-core/bin/lib/foo.cjs) is still NOT flagged

The second half is the one that matters. Without it the rule has no boundary and
nothing would catch an over-wide gate later, which is the mirror image of the bug
this deliverable just fixed.

Each row is renamed to state the current contract; the old names said
"non-src/*.cts ... is not flagged" and would have been actively misleading once
the bodies changed.

Proven to test the widening rather than restate it: every flagged half was run
against HEAD~2's pre-widening rule and does NOT fire there, then against the
current rule and does. 12/12 on that probe; the full file is 178/178.

Swept for the same staleness elsewhere and found none.
require-subprocess-timeout's own "inert outside src/*.cts" row is untouched -
that rule's gate was not widened here - and no-adhoc-regex-escape's test file
already carries correctly-targeted rows.

Refs #3951

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3951): acknowledge the quick.md growth the attribution guard reported

The full suite came back RED with one failure, and it is mine:

  1 file(s) grew without an acknowledgment:
    quick.md grew 364 bytes

gsd-core/workflows/quick.md is runtime-loaded emitted content, so correcting
its false 'performs the equivalent write' claim trips emitted-attribution by
construction. This is the acknowledgment, not a workaround - there is nothing
to regenerate.

The fragment names ONE path, which is the only one the guard reported. The four
spent acknowledgments it also listed (audit-uat, plan-phase, progress, review)
belong to other fragments whose ripple the base already absorbs; they are inert,
not failures, and are deliberately NOT copied here - naming paths I did not
change would make this record false in the other direction.

Byte figure corrected before committing: the guard reported 37220 -> 37584
(+364), but origin/next has since moved and quick.md is 37232 there now, so the
measured delta is +352. The reason text says so and names the base as a moving
figure rather than pinning a number that is already stale.

Refs #3951

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3951): move the quick.md growth ack to a trailer, delete the obsolete fragment

The acknowledgment mechanism changed under this branch. Merging next brought in
the redesign - it also deleted .github/workflows/ack-fragment-sweep.yml, which
was in the merge status and which I did not register at the time - and the guard
now says so directly:

  Add a trailer to a commit in this PR (never a new file).
    Emitted-Drift-Ack-Growth: quick.md - <why this growth is deliberate>

So tests/emitted-drift-acks/3951-quick-append-equivalence.json is obsolete on
arrival. A fragment file is no longer read by anything, and leaving it would be a
dead record that looks like an active one. It is deleted here rather than kept
"just in case".

The byte figure moved again with the merge: 37232 -> 37596, +364. The earlier
fragment said +352, measured before the merge auto-merged quick.md itself. The
trailer carries no number, which is the better design - the figure was stale
twice in two attempts.

Refs #3951

Emitted-Drift-Ack-Growth: quick.md — #3356/#3951 replaces a false claim with an accurate one. Line 627 said the `quick-tasks-append` shortcut "performs the equivalent write" to the Step 7c row rendered above it; it did not, and that was the documented half of #3356 — with no quick id or task directory the route emitted a positional ordinal in `#` and an em-dash in `Directory`, a visibly different row. The corrected sentence has to carry three facts the original elided: what the shortcut actually writes when it has neither input, that this is honest behavior for its real caller (`fast.md`, which has neither), and how a caller with both now gets the byte-identical canonical row via the new optional `--quick-id`/`--slug`/`--directory` flags. Prose is the product here — an executing agent reads this line to decide whether the shortcut is safe for its case, and a shorter correction would either drop the flags (leaving the reader unable to act on the fix) or drop the limitation (recreating the false claim in gentler words).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#3951): backfill changeset pr number

Refs #3951

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 23:10:49 -04:00

833 lines
32 KiB
JavaScript

'use strict';
// Tests for graphify.cjs — staleness, mvp-viz, and regressions describe blocks.
// Split from the consolidated 2336-LOC file. Refs #3761.
const { describe, test, beforeEach, afterEach, before, after } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const os = require('node:os');
const { runNode } = require('./helpers/process-seam.cjs');
const { throwIfFailed } = require('./helpers/git-fixture.cjs');
const { BUILD_TIMEOUT_MS, INSTALL_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const { createTempProject, createTempGitProject, cleanup } = require('./helpers.cjs');
const {
graphifyStatus,
} = require('../gsd-core/bin/lib/graphify.cjs');
const { scanFencedBlocks } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs');
const {
enableGraphify,
writeGraphJson,
gitHead,
commitEmpty,
SAMPLE_NODES_MINIMAL,
} = require('./helpers/graphify.cjs');
// ─── staleness describe ──────────────────────────────────────────────────────
describe('staleness', () => {
// Regression for #3170: graphifyStatus surfaces built_at_commit staleness.
// graphify v0.7+ embeds `built_at_commit` into graph.json at write time.
// Tri-state on commit_stale: null means "we don't know" (pre-v0.7 graph or
// no git), which is semantically distinct from false ("known fresh").
describe('git-aware', () => {
let tmpDir;
let planningDir;
beforeEach(() => {
tmpDir = createTempGitProject();
planningDir = path.join(tmpDir, '.planning');
enableGraphify(planningDir);
});
afterEach(() => cleanup(tmpDir));
test('graph rebuilt at HEAD: commits_behind=0, commit_stale=false', () => {
const head = gitHead(tmpDir);
writeGraphJson(planningDir, { nodes: SAMPLE_NODES_MINIMAL, edges: [], built_at_commit: head });
const result = graphifyStatus(tmpDir);
assert.equal(result.built_at_commit, head.slice(0, 7),
'short hash from graph.built_at_commit');
assert.equal(result.current_commit, head.slice(0, 7),
'short hash of git HEAD');
assert.equal(result.commits_behind, 0,
'zero commits between HEAD and itself');
assert.equal(result.commit_stale, false,
'commit_stale is explicitly false when commits_behind === 0');
});
test('graph 5 commits behind HEAD: commits_behind=5, commit_stale=true', () => {
const built = gitHead(tmpDir);
for (let i = 0; i < 5; i += 1) commitEmpty(tmpDir, `c${i}`);
writeGraphJson(planningDir, { nodes: SAMPLE_NODES_MINIMAL, edges: [], built_at_commit: built });
const result = graphifyStatus(tmpDir);
assert.equal(result.commits_behind, 5);
assert.equal(result.commit_stale, true);
assert.equal(result.built_at_commit, built.slice(0, 7));
assert.notEqual(result.current_commit, built.slice(0, 7),
'current_commit reflects HEAD, not graph build commit');
});
test('built_at_commit absent (pre-v0.7 graph): all four new fields null', () => {
// No built_at_commit on the graph -- GSD must not fabricate one.
writeGraphJson(planningDir, { nodes: SAMPLE_NODES_MINIMAL, edges: [] });
const result = graphifyStatus(tmpDir);
assert.equal(result.built_at_commit, null);
assert.equal(result.commits_behind, null);
assert.equal(result.commit_stale, null,
'tri-state: null means "we do not know", not "fresh"');
// current_commit may still be non-null since we are in a git repo,
// but without a baseline it cannot drive staleness.
assert.notEqual(result.current_commit, undefined,
'current_commit field is always present even when null');
});
test('rebased-away built_at_commit: commits_behind=null, commit_stale=null', () => {
// built_at_commit references a commit that never existed in this repo.
const ghostHash = '0000000000000000000000000000000000000001';
writeGraphJson(planningDir, { nodes: SAMPLE_NODES_MINIMAL, edges: [], built_at_commit: ghostHash });
const result = graphifyStatus(tmpDir);
assert.equal(result.built_at_commit, ghostHash.slice(0, 7),
'echoes the field even if unreachable -- caller can decide what to do');
assert.equal(result.commits_behind, null,
'cannot count commits to an unreachable commit');
assert.equal(result.commit_stale, null,
'unknown distance means unknown staleness');
});
test('malformed built_at_commit (dashed argv): rejected before git invocation', () => {
// Argument-injection fence: a graph.json with a hostile built_at_commit
// must never reach `git` as an argv element. The implementation should
// validate /^[0-9a-f]{4,40}$/i and treat anything else as absent.
const malicious = '--upload-pack=evil';
writeGraphJson(planningDir, { nodes: SAMPLE_NODES_MINIMAL, edges: [], built_at_commit: malicious });
const result = graphifyStatus(tmpDir);
assert.equal(result.built_at_commit, null,
'malformed value is rejected, not echoed');
assert.equal(result.commits_behind, null);
assert.equal(result.commit_stale, null);
});
});
describe('non-git cwd', () => {
let tmpDir;
let planningDir;
beforeEach(() => {
tmpDir = createTempProject();
planningDir = path.join(tmpDir, '.planning');
enableGraphify(planningDir);
});
afterEach(() => cleanup(tmpDir));
test('cwd has no .git: current_commit=null, derived fields=null', () => {
const built = 'abcdef1234567890abcdef1234567890abcdef12';
writeGraphJson(planningDir, { nodes: SAMPLE_NODES_MINIMAL, edges: [], built_at_commit: built });
const result = graphifyStatus(tmpDir);
assert.equal(result.built_at_commit, built.slice(0, 7),
'graph field is echoed even without a local repo');
assert.equal(result.current_commit, null,
'no HEAD without git');
assert.equal(result.commits_behind, null);
assert.equal(result.commit_stale, null);
});
});
describe('back-compat', () => {
let tmpDir;
let planningDir;
beforeEach(() => {
tmpDir = createTempGitProject();
planningDir = path.join(tmpDir, '.planning');
enableGraphify(planningDir);
writeGraphJson(planningDir, {
nodes: SAMPLE_NODES_MINIMAL,
edges: [{ source: 'n1', target: 'n2', label: 'x', confidence: 'EXTRACTED' }],
hyperedges: [],
built_at_commit: gitHead(tmpDir),
});
});
afterEach(() => cleanup(tmpDir));
test('existing fields are unchanged when commit-staleness fields are added', () => {
const result = graphifyStatus(tmpDir);
// Existing contract — must not regress.
assert.equal(result.exists, true);
assert.equal(result.node_count, 2);
assert.equal(result.edge_count, 1);
assert.equal(result.hyperedge_count, 0);
assert.equal(typeof result.last_build, 'string');
assert.equal(typeof result.stale, 'boolean',
'mtime-based stale flag stays as-is for back-compat');
assert.equal(typeof result.age_hours, 'number');
});
test('disabled response is unchanged (commit-staleness fields not added)', () => {
const tmp2 = createTempProject();
try {
const result = graphifyStatus(tmp2);
assert.equal(result.disabled, true,
'disabled path returns the existing shape, no commit fields');
assert.equal(result.built_at_commit, undefined,
'commit-staleness fields are only added on the success path');
} finally {
cleanup(tmp2);
}
});
});
});
// ─── mvp-viz describe ─────────────────────────────────────────────────────────
describe('mvp-viz', () => {
// Contract: commands/gsd/graphify.md documents MVP visual differentiation.
// Per PRD Q5: distinct node color + 'MVP' label suffix.
// Tests parse the markdown skill into structured IR (YAML frontmatter +
// fenced code blocks) and assert on the parsed structures, not raw text.
const CMD = path.join(__dirname, '..', 'commands', 'gsd', 'graphify.md');
/**
* Parse the narrow YAML subset used in this skill's frontmatter:
* key: scalar
* key:
* - item
* - item
*/
function parseSkillFrontmatter(text) {
const lines = text.split(/\r?\n/);
const out = {};
let _activeKey = null;
let activeList = null;
for (const raw of lines) {
const listItem = raw.match(/^\s+-\s+(.+?)\s*$/);
if (listItem && activeList) {
activeList.push(listItem[1]);
continue;
}
const kv = raw.match(/^([A-Za-z][A-Za-z0-9_-]*):\s*(.*)$/);
if (!kv) continue;
const [, key, rawValue] = kv;
const value = rawValue.trim();
if (value === '') {
_activeKey = key;
activeList = [];
out[key] = activeList;
} else {
_activeKey = null;
activeList = null;
out[key] = value;
}
}
return out;
}
/**
* Walk markdown body line-by-line and return every fenced code block as
* { lang, content } records. Tracks fence state explicitly.
*/
function extractFencedBlocks(body) {
const lines = body.split(/\r?\n/);
const blocks = [];
let active = null;
for (const line of lines) {
const open = line.match(/^```(\S*)\s*$/);
if (active === null) {
if (open) active = { lang: open[1] || '', lines: [] };
continue;
}
if (line.trim() === '```') {
blocks.push({ lang: active.lang, content: active.lines.join('\n') });
active = null;
continue;
}
active.lines.push(line);
}
return blocks;
}
function loadSkill() {
// Local rename (`markdown` not `content`) so the no-source-grep lint
// doesn't conflate this readFileSync-bound variable with the
// `b.content.includes(...)` calls below — those operate on parsed
// fenced-block records, not raw file text.
const markdown = fs.readFileSync(CMD, 'utf8');
const lines = markdown.split(/\r?\n/);
const delims = [];
for (let i = 0; i < lines.length; i += 1) {
if (lines[i].trim() === '---') delims.push(i);
if (delims.length === 2) break;
}
assert.equal(delims.length, 2, 'graphify.md must have a closed frontmatter block');
const frontmatterText = lines.slice(delims[0] + 1, delims[1]).join('\n');
const body = lines.slice(delims[1] + 1).join('\n');
return {
frontmatter: parseSkillFrontmatter(frontmatterText),
body,
fencedBlocks: extractFencedBlocks(body),
};
}
// Parse MVP section from graphify.md body as structured IR (not raw grep).
// Extracts: mentionsMvp, colorRuleLine, labelRuleLine, fallbackLine.
function parseMvpVizContract(body) {
const lines = body.split(/\r?\n/);
const lowerLines = lines.map(line => line.toLowerCase());
const mvpLines = lines.filter(line => line.toLowerCase().includes('mvp'));
return {
mentionsMvp: mvpLines.length > 0,
colorRuleLine: mvpLines.find(line => {
const lower = line.toLowerCase();
return lower.includes('color') || lower.includes('fill') || line.includes('#');
}) || '',
labelRuleLine: mvpLines.find(line => {
const lower = line.toLowerCase();
return lower.includes('label') || lower.includes('suffix');
}) || '',
fallbackLine: lowerLines.find(line =>
(line.includes('mode') && (line.includes('null') || line.includes('absent') || line.includes('not mvp'))) ||
(line.includes('standard') && (line.includes('render') || line.includes('fallback')))
) || '',
};
}
test('graphify.md documents distinct color for MVP-mode phases', () => {
const { body } = loadSkill();
const contract = parseMvpVizContract(body);
assert.ok(contract.mentionsMvp, 'must mention MVP in color rule');
assert.ok(contract.colorRuleLine.length > 0, 'must reference a color/fill rule for MVP nodes');
});
test('graphify.md documents MVP label suffix on node text', () => {
const { body } = loadSkill();
const contract = parseMvpVizContract(body);
assert.ok(contract.labelRuleLine.length > 0, 'must add an MVP label/suffix to node text');
});
test('graphify.md specifies fallback when phase mode is null/absent', () => {
const { body } = loadSkill();
const contract = parseMvpVizContract(body);
assert.ok(contract.fallbackLine.length > 0, 'must specify fallback when mode is not mvp');
});
// Counter-test: a non-mvp phase must NOT carry mode:'mvp' in the contract.
// The fallbackLine ensures standard rendering is documented for the non-mvp case.
test('non-mvp phase render path is documented (counter-test)', () => {
const { body } = loadSkill();
const contract = parseMvpVizContract(body);
// The fallback line is required precisely because non-mvp phases exist;
// its presence is the counter-assertion that mvp rendering is NOT applied globally.
assert.ok(
contract.fallbackLine.length > 0,
'fallback documentation confirms mvp rendering is not applied to non-mvp phases',
);
// Additionally: the MVP label should only be a suffix, not a full replacement;
// so the standard label path (no MVP suffix) must be documented.
assert.ok(
contract.mentionsMvp,
'mvp mention is present, meaning mvp is treated as a special case, not the default',
);
});
});
// ─── regressions describe ─────────────────────────────────────────────────────
describe('regressions', () => {
// ── Regression for #3166 ────────────────────────────────────────────────────
// /gsd-graphify build lost artifacts because the skill spawned a Task
// sub-agent that backgrounded `graphify update .`. Sub-agent isolation
// SIGTERM'd the post-extraction phase before graph.json / graph.html /
// GRAPH_REPORT.md were written.
// Fix: skill runs the build inline in a single foreground Bash call.
// Structural fence: skill is parsed into (a) a YAML frontmatter map and
// (b) a list of fenced code blocks. Assertions run against parsed structures,
// never against raw markdown text.
const SKILL_PATH = path.join(__dirname, '..', 'commands', 'gsd', 'graphify.md');
function parseBug3166SkillFrontmatter(text) {
const lines = text.split(/\r?\n/);
const out = {};
let _activeKey = null;
let activeList = null;
for (const raw of lines) {
const listItem = raw.match(/^\s+-\s+(.+?)\s*$/);
if (listItem && activeList) {
activeList.push(listItem[1]);
continue;
}
const kv = raw.match(/^([A-Za-z][A-Za-z0-9_-]*):\s*(.*)$/);
if (!kv) continue;
const [, key, rawValue] = kv;
const value = rawValue.trim();
if (value === '') {
_activeKey = key;
activeList = [];
out[key] = activeList;
} else {
_activeKey = null;
activeList = null;
out[key] = value;
}
}
return out;
}
function extractBug3166FencedBlocks(body) {
const lines = body.split(/\r?\n/);
const blocks = [];
let active = null;
for (const line of lines) {
const open = line.match(/^```(\S*)\s*$/);
if (active === null) {
if (open) active = { lang: open[1] || '', lines: [] };
continue;
}
if (line.trim() === '```') {
blocks.push({ lang: active.lang, content: active.lines.join('\n') });
active = null;
continue;
}
active.lines.push(line);
}
return blocks;
}
function loadBug3166Skill() {
const markdown = fs.readFileSync(SKILL_PATH, 'utf8');
const lines = markdown.split(/\r?\n/);
const delims = [];
for (let i = 0; i < lines.length; i += 1) {
if (lines[i].trim() === '---') delims.push(i);
if (delims.length === 2) break;
}
assert.equal(delims.length, 2, 'graphify.md must have a closed frontmatter block');
const frontmatterText = lines.slice(delims[0] + 1, delims[1]).join('\n');
const body = lines.slice(delims[1] + 1).join('\n');
return {
frontmatter: parseBug3166SkillFrontmatter(frontmatterText),
body,
fencedBlocks: extractBug3166FencedBlocks(body),
};
}
// Regression for #3166
test('graphify.md allowed-tools does not include Task (inline build fence)', () => {
const { frontmatter } = loadBug3166Skill();
assert.ok(Array.isArray(frontmatter['allowed-tools']),
'allowed-tools must be a YAML block list');
assert.ok(frontmatter['allowed-tools'].length > 0,
'allowed-tools must declare at least one tool');
assert.ok(!frontmatter['allowed-tools'].includes('Task'),
'Task must NOT be in allowed-tools — sub-agent isolation truncates ' +
'graphify v0.7+ post-extraction phase (#3166). Build runs inline.');
});
// Regression for #3166
test('graphify.md frontmatter retains Read and Bash (inline build prerequisites)', () => {
const { frontmatter } = loadBug3166Skill();
const tools = frontmatter['allowed-tools'];
assert.ok(tools.includes('Read'), 'Read required for config gate');
assert.ok(tools.includes('Bash'), 'Bash required for inline build chain');
});
// Regression for #3166
test('no fenced code block in graphify.md invokes Task() agent spawn syntax', () => {
const { fencedBlocks } = loadBug3166Skill();
const offending = fencedBlocks.filter(b => b.content.includes('Task('));
assert.deepEqual(offending, [],
'no fenced code block in graphify.md may contain `Task(` invocation ' +
'syntax — sub-agent spawning truncates graphify v0.7+ post-extraction ' +
'phase (#3166). Prose mentioning the word "Task" is fine; only the ' +
'call expression inside a code block is forbidden.');
});
// Regression for #3166
test('a bash code block invokes the inline graphify update . pipeline', () => {
const { fencedBlocks } = loadBug3166Skill();
const bashBlocks = fencedBlocks.filter(b => b.lang === 'bash');
assert.ok(bashBlocks.length > 0, 'skill must contain at least one bash block');
assert.ok(
bashBlocks.some(b => b.content.includes('graphify update .')),
'a bash code block must invoke `graphify update .`'
);
assert.ok(
bashBlocks.some(b => /gsd_run\s+graphify build snapshot/.test(b.content)),
'a bash code block must invoke `gsd_run graphify build snapshot`'
);
});
// ── Regression for #3579 ────────────────────────────────────────────────────
// graphify auto-update hook was dead-on-arrival in 1.50.0-canary.x because:
// Gap 1: scripts/build-hooks.js HOOKS_TO_COPY did not include
// gsd-graphify-update.sh
// Gap 2: hooks/lib/gsd-graphify-rebuild.sh not copied by installer
// Test strategy: run the actual build and assert filesystem outcomes.
const REPO_ROOT_3579 = path.resolve(__dirname, '..');
const HOOKS_DIR_3579 = path.join(REPO_ROOT_3579, 'hooks');
const DIST_DIR_3579 = path.join(HOOKS_DIR_3579, 'dist');
const BUILD_SCRIPT_3579 = path.join(REPO_ROOT_3579, 'scripts', 'build-hooks.js');
const INSTALL_SCRIPT_3579 = path.join(REPO_ROOT_3579, 'bin', 'install.js');
// Regression for #3579: Gap 1 — build-hooks.js packages every top-level hooks/*.sh
describe('#3579 Gap 1: build-hooks.js packages every top-level hooks/*.sh into dist', () => {
before(() => {
const r = runNode([BUILD_SCRIPT_3579], { timeoutMs: BUILD_TIMEOUT_MS });
throwIfFailed(r, `node ${BUILD_SCRIPT_3579}`);
});
test('every top-level hooks/*.sh is emitted to hooks/dist/ by the build', () => {
const topLevelSh = fs
.readdirSync(HOOKS_DIR_3579, { withFileTypes: true })
.filter((e) => e.isFile() && e.name.endsWith('.sh'))
.map((e) => e.name);
assert.ok(topLevelSh.length > 0, 'expected at least one top-level hooks/*.sh in source');
const missing = topLevelSh.filter(
(sh) => !fs.existsSync(path.join(DIST_DIR_3579, sh))
);
assert.deepStrictEqual(
missing,
[],
`every top-level hooks/*.sh must be emitted to hooks/dist/ by scripts/build-hooks.js; missing from dist: ${JSON.stringify(missing)}`
);
});
test('hooks/dist/gsd-graphify-update.sh exists after build', () => {
assert.ok(
fs.existsSync(path.join(DIST_DIR_3579, 'gsd-graphify-update.sh')),
'expected hooks/dist/gsd-graphify-update.sh to exist after build (Gap 1)'
);
});
test('hooks/dist/lib/gsd-graphify-rebuild.sh exists after build', () => {
assert.ok(
fs.existsSync(path.join(DIST_DIR_3579, 'lib', 'gsd-graphify-rebuild.sh')),
'expected hooks/dist/lib/gsd-graphify-rebuild.sh to exist after build (Gap 2)'
);
});
});
// Regression for #3579: installer deploys graphify hook + lib helper to target
describe('#3579: installer deploys graphify hook + lib helper to target', () => {
let tmpDir;
let installStdout;
before(() => {
const r1 = runNode([BUILD_SCRIPT_3579], { timeoutMs: BUILD_TIMEOUT_MS });
throwIfFailed(r1, `node ${BUILD_SCRIPT_3579}`);
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3579-install-'));
const r2 = runNode(
[INSTALL_SCRIPT_3579, '--claude', '--global', '--yes', '--no-sdk'],
{
env: { ...process.env, CLAUDE_CONFIG_DIR: tmpDir },
timeoutMs: INSTALL_TIMEOUT_MS,
}
);
throwIfFailed(r2, `node ${INSTALL_SCRIPT_3579}`);
installStdout = r2.stdout;
});
after(() => {
cleanup(tmpDir);
});
test('hooks/gsd-graphify-update.sh present at install target', () => {
const dest = path.join(tmpDir, 'hooks', 'gsd-graphify-update.sh');
assert.ok(fs.existsSync(dest), `expected ${dest} to exist after install`);
});
test('hooks/lib/gsd-graphify-rebuild.sh present at install target', () => {
const dest = path.join(tmpDir, 'hooks', 'lib', 'gsd-graphify-rebuild.sh');
assert.ok(fs.existsSync(dest), `expected ${dest} to exist after install`);
});
test('installer does not warn about missing gsd-graphify-update.sh', () => {
assert.ok(
!installStdout.includes('Missing expected hook: gsd-graphify-update.sh'),
`installer output must not warn about missing graphify hook; got:\n${installStdout}`
);
assert.ok(
!installStdout.includes(
'Skipped graphify auto-update hook — gsd-graphify-update.sh not found'
),
`installer must not skip graphify hook configuration; got:\n${installStdout}`
);
});
});
});
// ────────────────────────────────────────────────────────────────────────
// Folded from tests/bug-622-graphify-optional-graph-html.test.cjs — consolidation epic #1969 (B6 #1975)
// ────────────────────────────────────────────────────────────────────────
{
const { describe: __foldDescribe } = require('node:test');
__foldDescribe("folded:bug-622-graphify-optional-graph-html (consolidation epic #1969 B6 #1975)", () => {
// allow-test-rule: source-text-is-the-product (see #622)
// This test extracts the deployed Step 3 shell block from commands/gsd/graphify.md
// and executes it to prove that a skipped graph.html (due to the graphify HTML viz
// node limit) does not abort the chain (#622). The deployed markdown text IS the
// product surface — the block the runtime executes — so asserting on its execution
// behavior requires reading the source text.
'use strict';
/**
* Regression test for bug #622.
*
* The `/gsd-graphify build` Step 3 shell chain in commands/gsd/graphify.md
* aborted when `graph.html` was intentionally skipped (graph exceeds the HTML
* viz node limit, default 5000). The unconditional `cp graphify-out/graph.html`
* failed with "cannot stat", and the `&&` chain aborted before the
* GRAPH_REPORT.md copy, snapshot, and status steps ran.
*
* Fix: guard the graph.html copy with
* `{ [ -f graphify-out/graph.html ] && cp … || true; }`
* so the chain continues when the file is absent.
*/
const { describe, test, before, after } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const { runHook } = require('./helpers/process-seam.cjs');
const { toLegacyResult } = require('./helpers/git-fixture.cjs');
const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const { createTempDir, cleanup, readFileNormalized } = require('./helpers.cjs');
// Path to the command doc (relative to repo root)
const GRAPHIFY_MD = path.join(__dirname, '..', 'commands', 'gsd', 'graphify.md');
/**
* Extract the Step 3 fenced bash block from graphify.md.
* The block starts with the line `graphify update .` and ends at the next
* closing ``` fence.
*
* Returns the bash source text (without the fence lines themselves).
*
* readFileNormalized() strips \r\n -> \n before the match below runs — the
* extracted block is later spawned via runHook('-c', ..., {interpreter:
* 'bash'}) in runBlock(), so an un-normalized read on a Windows checkout
* would break bash mid-script
* (DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE, #2650).
*/
function extractStep3Block() {
const content = readFileNormalized(GRAPHIFY_MD);
// Find the ```bash fence that CONTAINS `graphify update .` (including any
// leading preamble line), without crossing into other fences.
const lines = content.split('\n');
for (const block of scanFencedBlocks(lines)) {
if (block.closeLineIdx === -1) continue;
if ((block.infoString || '').trim() !== 'bash') continue;
const body = lines.slice(block.openLineIdx + 1, block.closeLineIdx).join('\n');
if (body.includes('graphify update .')) return body.trim();
}
return null;
}
// ─── shared sandbox dirs ──────────────────────────────────────────────────────
let sandbox;
let fakeBin;
let fakeHome;
before(() => {
sandbox = createTempDir('gsd-622-sandbox-');
fakeBin = createTempDir('gsd-622-fakebin-');
fakeHome = createTempDir('gsd-622-fakehome-');
});
after(() => {
cleanup(sandbox);
cleanup(fakeBin);
cleanup(fakeHome);
});
// ─── helpers ─────────────────────────────────────────────────────────────────
/**
* Write a minimal fake `graphify` executable into fakeBin.
* It just exits 0 so the `graphify update .` step succeeds.
*/
function writeFakeGraphify() {
const exe = path.join(fakeBin, 'graphify');
fs.writeFileSync(exe, ['#!/bin/sh', 'exit 0'].join('\n'), { mode: 0o755 });
}
/**
* Write a minimal gsd-tools.cjs stub into fakeHome that exits 0 for any
* invocation (covers the `graphify build snapshot` and `graphify status` steps).
*/
function writeFakeGsdTools() {
const binDir = path.join(fakeHome, '.claude', 'gsd-core', 'bin');
fs.mkdirSync(binDir, { recursive: true });
fs.writeFileSync(
path.join(binDir, 'gsd-tools.cjs'),
['#!/usr/bin/env node', 'process.exit(0);'].join('\n'),
{ mode: 0o755 },
);
}
/**
* Populate the sandbox with the minimal directory structure and output files
* that a real `graphify update .` would produce. `includeHtml` controls
* whether graphify-out/graph.html is created (simulating the node-limit skip
* when false).
*/
function populateSandbox(includeHtml) {
// graphify-out/ — simulates graphify CLI output directory
const outDir = path.join(sandbox, 'graphify-out');
fs.mkdirSync(outDir, { recursive: true });
fs.writeFileSync(path.join(outDir, 'graph.json'), '{}');
fs.writeFileSync(path.join(outDir, 'GRAPH_REPORT.md'), '# report');
if (includeHtml) {
fs.writeFileSync(path.join(outDir, 'graph.html'), '<html/>');
}
// .planning/graphs/ — destination directory
const graphsDir = path.join(sandbox, '.planning', 'graphs');
fs.mkdirSync(graphsDir, { recursive: true });
}
/**
* Execute the extracted Step 3 block in the sandbox.
*/
function runBlock(block) {
// The extracted block is a shell chain (&&, [ -f ] guards, ||) — it stays
// a `bash -c` invocation rather than being decomposed into argv. Bash
// FAN-OUT: the block spawns `graphify update .` (and further commands
// chained via && / ||) under one `bash` interpreter, not a single CLI
// probe — the wrong class for `PROBE_TIMEOUT_MS`. Same class as the
// observed CI failures in tests/quick-branching.test.cjs (PR #3787 run
// 32668773524) and tests/worktree-safety.test.cjs (`next` run
// 32608945654). See HOOK_FANOUT_TIMEOUT_MS in ./helpers/timeouts.cjs for
// the class rationale.
const r = runHook('-c', [block], {
interpreter: 'bash',
cwd: sandbox,
env: {
...process.env,
PATH: fakeBin + ':' + process.env.PATH,
HOME: fakeHome,
},
timeoutMs: HOOK_FANOUT_TIMEOUT_MS,
});
return toLegacyResult(r);
}
// ─── tests ───────────────────────────────────────────────────────────────────
describe('bug #622: graph.html absence must not abort the Step 3 shell chain', () => {
let block;
before(() => {
block = extractStep3Block();
});
test('Step 3 bash block is present in graphify.md (sanity gate)', () => {
assert.ok(block !== null, 'Step 3 bash block starting with "graphify update ." was not found in commands/gsd/graphify.md');
assert.ok(block.length > 0, 'Extracted bash block must not be empty');
});
test('graph.html absent: chain exits 0 and all other artifacts are copied (#622 regression)', (t) => {
// Use t.after for per-test cleanup so sandbox is fresh for each test
t.after(() => {
// Remove and recreate sandbox so the next test starts with an empty dir
cleanup(sandbox);
fs.mkdirSync(sandbox, { recursive: true });
});
writeFakeGraphify();
writeFakeGsdTools();
populateSandbox(false); // no graph.html — simulates node-limit skip
const result = runBlock(block);
// Chain must not abort
assert.equal(result.status, 0, [
'Expected exit 0 but got ' + result.status,
'stderr: ' + result.stderr,
'stdout: ' + result.stdout,
].join('\n'));
// graph.json was copied (step before the guarded line)
assert.ok(
fs.existsSync(path.join(sandbox, '.planning', 'graphs', 'graph.json')),
'.planning/graphs/graph.json must be copied even when graph.html is absent',
);
// GRAPH_REPORT.md was copied (step AFTER the guarded line — key regression assertion)
assert.ok(
fs.existsSync(path.join(sandbox, '.planning', 'graphs', 'GRAPH_REPORT.md')),
'.planning/graphs/GRAPH_REPORT.md must be copied (the chain must not abort at graph.html)',
);
// graph.html must NOT exist in the destination (correctly skipped)
assert.ok(
!fs.existsSync(path.join(sandbox, '.planning', 'graphs', 'graph.html')),
'.planning/graphs/graph.html must NOT be created when source is absent',
);
});
test('graph.html present: chain exits 0 and graph.html is copied (happy path)', (t) => {
t.after(() => {
cleanup(sandbox);
fs.mkdirSync(sandbox, { recursive: true });
});
writeFakeGraphify();
writeFakeGsdTools();
populateSandbox(true); // include graph.html
const result = runBlock(block);
assert.equal(result.status, 0, [
'Expected exit 0 but got ' + result.status,
'stderr: ' + result.stderr,
'stdout: ' + result.stdout,
].join('\n'));
// graph.html must exist in the destination (normal copy)
assert.ok(
fs.existsSync(path.join(sandbox, '.planning', 'graphs', 'graph.html')),
'.planning/graphs/graph.html must be copied when the source file is present',
);
// Other artifacts also copied
assert.ok(
fs.existsSync(path.join(sandbox, '.planning', 'graphs', 'graph.json')),
'.planning/graphs/graph.json must be copied',
);
assert.ok(
fs.existsSync(path.join(sandbox, '.planning', 'graphs', 'GRAPH_REPORT.md')),
'.planning/graphs/GRAPH_REPORT.md must be copied',
);
});
});
});
}