Files
msd-core/tests/code-review.test.cjs
Tom Boucher 463cffd894 chore(#604): rename get-shit-done/ runtime directory to gsd-core/ (#615)
* chore(#604): rename get-shit-done/ runtime directory to gsd-core/

Renames the installed runtime directory `get-shit-done/` to `gsd-core/` so the
on-disk name matches the package (`@opengsd/gsd-core`), repo, and binary
(`gsd-tools`). The npm package name and binary are unchanged; npx/npm consumers
are unaffected.

Mechanical (bulk, ~90% of the diff):
- `git mv get-shit-done gsd-core`
- Swept path/identifier references across the repo via
  `perl -pe 's/get-shit-done(?!-\w)/gsd-core/g'`. The negative lookahead
  preserves the five legitimate slug variants that are NOT the directory:
  get-shit-done-{OLD,cc,classic,cli,redux} (old package/repo names).
- Build/manifest wiring: package.json (bin, files, coverage globs),
  tsconfig.build.json (outDir), ~86 .gitignore build-output entries,
  stryker.config.mjs, scan-ignore files, install.js path strings.
- Frozen (not rewritten): CHANGELOG.md history; translated docs
  (README.<locale>.md and docs/{ja-JP,ko-KR,pt-BR,zh-CN}/).

New logic (review here):
- src/installer-migrations/003-rename-get-shit-done-to-gsd-core.cts: a proper
  ADR-0008 installer migration. On upgrade it walks the legacy
  `~/.claude/get-shit-done/` tree, classifies each file via the prior install
  manifest, and emits remove-managed / backup-and-remove for managed files
  while PRESERVING unknown user-added files. Symlink-safe (skips a symlinked
  root and symlinked entries; bounds-checks every path under configDir). The
  framework rolls back on install failure. Emptied dirs may remain (framework
  has no recursive dir-removal primitive) — documented.
- scripts/lint-legacy-dir-name.cjs: CI regression guard forbidding the bare
  `get-shit-done` directory token (split token to avoid self-match; case-
  insensitive; `(?!-\w)` lookahead allows the slug variants; allowlists
  CHANGELOG, translated docs, and `gsd-allow-legacy-name` marker lines).
  Wired into the lint-tests CI job.
- Restored scripts/lint-package-identity-drift.cjs detection regexes (the
  mechanical sweep had wrongly rewritten the old-name patterns it exists to
  detect) and marked them as intentional legacy references.
- TDD tests for the migration and the guard; do.md slash-command guard regex
  tightened so a `/gsd-core/bin` path segment is not mistaken for a command;
  changeset + docs/installer-migrations.md row added.

Breaking: the installed runtime path moves `~/.claude/get-shit-done/` ->
`~/.claude/gsd-core/`. Migration 003 removes the stale legacy dir's managed
files (preserving user files) on upgrade. Users with custom hooks/configs
hardcoding the old path must update them.

Closes #604

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

* fix(#604): unsweep pending changesets + allowlist injection-example docs

CI fixes for the rename PR:
- Do not sweep pending .changeset/*.md (ephemeral release-note fragments,
  like CHANGELOG); reverted those body edits so 5 pre-existing malformed
  fragments (missing type/pr) no longer enter the PR diff and trip docs-lint.
  Allowlisted .changeset/ in the legacy-name guard accordingly.
- Allowlisted TEST-EXAMPLES.md and docs/explanation/security-model.md in
  prompt-injection-scan.sh: they contain intentional injection examples /
  security-model prose; the path-reference rewrites are kept.

CodeQL alerts on this PR are pre-existing (alert lines unchanged by this PR;
none in the new migration/guard) and are out of scope for the rename.

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

* fix(#604): resolve CodeQL alerts surfaced on this PR

The rename diff touched files carrying pre-existing CodeQL findings; per the
no-pre-existing-dismissal rule, fixing every surfaced alert rather than waving
them off. All behavior-preserving:

- scripts/ci-test-scope.cjs: build the config-path match from string
  .includes() instead of a RegExp over an arg-derived value (js/regex-injection).
- src/profile-output.cts: escape backslashes before pipe-escaping desc/safeName
  so the table-cell escape is complete (js/incomplete-sanitization).
- tests/{bug-2643,bug-2808,docs-parity-live-registry}: two-pass HTML-comment
  strip so a bare/unclosed `<!--` cannot survive (js/incomplete-multi-character-sanitization).
- tests/inline-plan-threshold: drop the no-op `\s`->`\s` identity replace,
  keep the meaningful POSIX-class conversion (js/identity-replacement).

Verified: build:lib green; the touched test files + ci-test-scope + profile-output
suites pass; lint:legacy-name clean.

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

* fix(#604): correctly resolve remaining CodeQL alerts (regex-injection + sanitization)

The prior commit's fixes for two alerts were ineffective:
- ci-test-scope.cjs js/regex-injection: the alert is the CLI-arg-derived `file`
  reaching static regex `.test(file)` calls (not the config rule). Removed ALL
  regex over file/t — startsWith/includes/=== string checks + an isWindowsHint
  helper — so there is no regex sink for the tainted value.
- js/incomplete-multi-character-sanitization (3 test files): a single
  `.replace(/<!--...-->/g,'')` can let `<!--` re-form. Replaced with a fixpoint
  loop (replace until stable) plus a final bare-opener strip.

Verified: no regex over file/t remains; ci-test-scope + the 3 test suites pass;
lint:legacy-name clean.

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

* fix(#604): make ci-test-scope + comment-strippers regex-free to clear CodeQL

CodeQL flags the regex PATTERNS syntactically (regex-injection on the
--files arg split; incomplete-multi-character-sanitization on the <!--...-->
replace), so loop fixes do not satisfy it. Made these paths regex-free:
- ci-test-scope.cjs splitFiles: char-by-char separator tokenizer (no /[,\\s]+/).
- 3 test files: indexOf/slice HTML-comment stripper (no .replace(/<!--/)).
Behavior preserved; ci-test-scope + the 3 suites pass; guard clean.

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

* fix(#604): unblock security base64 scan on the large rename diff

The security job hit its 10m timeout: base64-scan.sh choked on the binary
test fixture tests/feat-3594-parser-property-style.test.cjs (embedded NUL/
non-UTF8 bytes -> thousands of bogus blobs + "ignored null byte" warnings),
and the ~800-file rename diff is slow to scan regardless.

- scripts/base64-scan.sh: skip binary-by-content files (grep -Iq .) — they
  can't carry base64-obfuscated *text* and feeding NUL bytes through the
  per-line scanner is pathologically slow. collect_files already filtered
  binary *extensions*; this catches binary *content* in text extensions.
- .github/workflows/security-scan.yml: raise the security job timeout 10m->30m
  to accommodate very large diffs (the scan itself is unchanged).

Verified locally: scan skips the fixture, 0 "ignored null byte" warnings,
0 findings, exit 0.

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

* fix(#604): sweep get-shit-done refs introduced by merging next

The branch was updated with next (#614/#384/#618 etc.), which reference the
get-shit-done/ dir (still named that on next). Swept the stale references in
the merged files to gsd-core so the rename stays consistent and lint:legacy-name
passes:
- commands/gsd/discuss-phase.md (runtime-launcher shim paths)
- src/core.cts (getAgentsDir layout comments)
- tests/bug-384-agents-runtime-aware.test.cjs (require path to runtime lib)

Verified: guard 0 violations; build green.

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

* fix(#604): exclude gsd-core/ path segments from bug-3683 command cross-ref invariant

The #614 runtime-launcher shim added to discuss-phase.md references
`${_GSD_RUNTIME_ROOT}/gsd-core/bin/...`. bug-3683's REF_PATTERN excluded path-y
refs only via lookbehind, but `}` precedes `/gsd-core/` in the shim, so it
mis-read the directory path as a dangling `/gsd-core` command ref (same class as
the #604 bug-2954 fix). Added a trailing `(?![\w-]*\/)` so `/gsd-<x>/...` path
segments are not treated as slash-command references.

Verified locally on BOTH platforms before pushing:
- mac (node 26) full suite: 0 failures
- gsd-test-runner (linux, node22 image) full suite: 0 failures
- bug-3683 + bug-2954 pass.

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

* fix(#604): lazily resolve findProjectRoot in gsd-tools (harden flaky CI)

CI intermittently failed state.test's gsd-tools subprocess with
"findProjectRoot is not a function" (flip-flopping across legs; not reproducible
on mac full suite, gsd-test linux full suite, test:unit, or state.test x8).
findProjectRoot is a re-export from core.cjs (sourced from project-root.cjs);
binding it via destructure at module-load can be undefined under a load-ordering
edge. Resolve it lazily at call time via a small wrapper so the lookup happens
after core.cjs is fully initialized.

Verified green on BOTH platforms before pushing:
- mac (node 26) full suite: 0 failures
- gsd-test-runner (linux, node22) full suite: 0 failures
- state.test.cjs: 106/106; gsd-tools loads cleanly.

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

* fix(#604): allowlist verification-patterns.md placeholder examples in secret scan

The rename git-mv'd references/verification-patterns.md into gsd-core/, pulling
it into the secret-scan diff. It documents stub/placeholder RED-FLAG env-var
examples (illustrative Stripe test-key / database-URL / API-key placeholders) —
not real credentials. Added it to .secretscanignore with the strict annotation,
mirroring the existing gsd-core/workflows/plan-phase.md exception.

Verified locally: secret-scan-lint --strict OK; secret-scan --diff origin/next
exits 0 with 0 findings.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 18:35:29 -04:00

560 lines
25 KiB
JavaScript

// allow-test-rule: source-text-is-the-product
// Reads .md/.json/.yml product files whose deployed text IS what the
// runtime loads — testing text content tests the deployed contract.
/**
* GSD Code Review Tests
*
* Validates all code review artifacts from Phases 1-4:
* - Agent frontmatter (gsd-code-reviewer, gsd-code-fixer)
* - Command structure (code-review.md, code-review-fix.md)
* - Workflow structure (code-review.md, code-review-fix.md)
* - Config key registration (workflow.code_review, workflow.code_review_depth)
* - Workflow integration points (execute-phase, quick, autonomous)
*
* Test structure:
* - CR-AGENT: Hermetic agent tests (repo files only)
* - CR-CMD: Hermetic command tests (repo files only)
* - CR-WORKFLOW: Hermetic workflow tests (repo files only)
* - CR-CONFIG: Hermetic config tests (repo files only)
* - CR-INTEGRATION: Conditional integration tests (skip if plugin dir absent)
*/
const { test, describe } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const os = require('os');
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
// --- Test Environment Setup ---
const AGENTS_DIR = path.join(__dirname, '..', 'agents');
const COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd');
const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows');
/**
* Parse top-level (non-nested, non-escaped) Skill() invocations from a workflow .md file.
*
* Returns an array of structured objects: [{ skill, args }]
* - `skill` is the value of the `skill="..."` keyword argument
* - `args` is the value of the `args="..."` keyword argument (or null if absent)
*
* Skips occurrences inside escaped string contexts like
* prompt="... Skill(skill=\"x\", args=\"y\") ..."
* by walking the file character-by-character and tracking whether we are inside
* a double-quoted string. Escaped quotes (\") are treated as literal content.
*
* This avoids regex/.includes() text-matching: callers receive a structured list
* and assert against fields and tokenized args.
*/
function parseWorkflowSkillInvocations(content) {
const invocations = [];
let i = 0;
let inString = false;
while (i < content.length) {
const ch = content[i];
if (inString) {
if (ch === '\\' && i + 1 < content.length) {
// Skip escape sequence (e.g. \" or \\)
i += 2;
continue;
}
if (ch === '"') {
inString = false;
}
i += 1;
continue;
}
if (ch === '"') {
inString = true;
i += 1;
continue;
}
// Look for top-level "Skill(" at this position
if (content.startsWith('Skill(', i)) {
const callStart = i + 'Skill('.length;
// Find the matching close paren, respecting strings/escapes inside the call
let j = callStart;
let depth = 1;
let innerInString = false;
while (j < content.length && depth > 0) {
const c = content[j];
if (innerInString) {
if (c === '\\' && j + 1 < content.length) {
j += 2;
continue;
}
if (c === '"') innerInString = false;
j += 1;
continue;
}
if (c === '"') {
innerInString = true;
} else if (c === '(') {
depth += 1;
} else if (c === ')') {
depth -= 1;
if (depth === 0) break;
}
j += 1;
}
const callBody = content.slice(callStart, j);
const parsed = parseSkillCallBody(callBody);
if (parsed) invocations.push(parsed);
i = j + 1;
continue;
}
i += 1;
}
return invocations;
}
/**
* Parse the body of a Skill(...) call into { skill, args }.
* Body looks like: skill="name", args="value" (args optional).
* Returns null if no skill keyword is found.
*/
function parseSkillCallBody(body) {
const kwargs = {};
const isIdentChar = (c) => /[A-Za-z0-9_]/.test(c);
const isWs = (c) => /\s/.test(c);
let i = 0;
while (i < body.length) {
// Skip whitespace and commas
while (i < body.length && (isWs(body[i]) || body[i] === ',')) i += 1;
if (i >= body.length) break;
// Read identifier key
const keyStart = i;
while (i < body.length && isIdentChar(body[i])) i += 1;
const key = body.slice(keyStart, i);
if (!key) break;
// Expect '='
while (i < body.length && isWs(body[i])) i += 1;
if (body[i] !== '=') break;
i += 1;
while (i < body.length && isWs(body[i])) i += 1;
// Expect quoted value
if (body[i] !== '"') break;
i += 1;
let value = '';
while (i < body.length) {
const c = body[i];
if (c === '\\' && i + 1 < body.length) {
value += body[i + 1];
i += 2;
continue;
}
if (c === '"') {
i += 1;
break;
}
value += c;
i += 1;
}
kwargs[key] = value;
}
if (!('skill' in kwargs)) return null;
return { skill: kwargs.skill, args: 'args' in kwargs ? kwargs.args : null };
}
// Plugin directory resolution (cross-platform safe)
const PLUGIN_WORKFLOWS_DIR = process.env.GSD_PLUGIN_ROOT || path.join(os.homedir(), '.claude', 'gsd-core', 'workflows');
const PLUGIN_AVAILABLE = fs.existsSync(PLUGIN_WORKFLOWS_DIR);
// --- CR-AGENT: code review agent frontmatter ---
describe('CR-AGENT: code review agent frontmatter', () => {
test('gsd-code-reviewer.md has required frontmatter fields', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-reviewer.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('name:'), 'gsd-code-reviewer missing name:');
assert.ok(frontmatter.includes('description:'), 'gsd-code-reviewer missing description:');
assert.ok(frontmatter.includes('tools:'), 'gsd-code-reviewer missing tools:');
assert.ok(frontmatter.includes('color:'), 'gsd-code-reviewer missing color:');
});
test('gsd-code-fixer.md has required frontmatter fields', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('name:'), 'gsd-code-fixer missing name:');
assert.ok(frontmatter.includes('description:'), 'gsd-code-fixer missing description:');
assert.ok(frontmatter.includes('tools:'), 'gsd-code-fixer missing tools:');
assert.ok(frontmatter.includes('color:'), 'gsd-code-fixer missing color:');
});
test('gsd-code-reviewer.md has Read, Bash, Glob, Grep, Write tools', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-reviewer.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('Read'), 'gsd-code-reviewer missing Read tool');
assert.ok(frontmatter.includes('Bash'), 'gsd-code-reviewer missing Bash tool');
assert.ok(frontmatter.includes('Glob'), 'gsd-code-reviewer missing Glob tool');
assert.ok(frontmatter.includes('Grep'), 'gsd-code-reviewer missing Grep tool');
assert.ok(frontmatter.includes('Write'), 'gsd-code-reviewer missing Write tool');
});
test('gsd-code-fixer.md has Read, Edit, Write, Bash, Grep, Glob tools', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('Read'), 'gsd-code-fixer missing Read tool');
assert.ok(frontmatter.includes('Edit'), 'gsd-code-fixer missing Edit tool');
assert.ok(frontmatter.includes('Write'), 'gsd-code-fixer missing Write tool');
assert.ok(frontmatter.includes('Bash'), 'gsd-code-fixer missing Bash tool');
});
test('gsd-code-reviewer.md does not have skills: in frontmatter', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-reviewer.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(!frontmatter.includes('skills:'),
'gsd-code-reviewer has skills: in frontmatter — breaks Gemini CLI');
});
test('gsd-code-fixer.md does not have skills: in frontmatter', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(!frontmatter.includes('skills:'),
'gsd-code-fixer has skills: in frontmatter — breaks Gemini CLI');
});
test('gsd-code-fixer.md rollback uses git checkout (not Write tool)', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8');
assert.ok(content.includes('git checkout --'),
'gsd-code-fixer rollback should use git checkout -- {file} for atomic rollback');
assert.ok(!content.includes('PRE_FIX_CONTENT'),
'gsd-code-fixer should not use PRE_FIX_CONTENT in-memory capture (use git checkout instead)');
});
test('gsd-code-fixer.md success_criteria consistent with rollback strategy (git checkout)', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8');
const successCriteria = content.match(/<success_criteria>([\s\S]*?)<\/success_criteria>/)?.[1] || '';
assert.ok(successCriteria.includes('git checkout'),
'gsd-code-fixer success_criteria must reference git checkout rollback');
assert.ok(!successCriteria.includes('Write tool with captured'),
'gsd-code-fixer success_criteria must not say Write tool for rollback');
});
test('gsd-code-fixer.md flags logic-bug fixes for human review', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8');
assert.ok(content.includes('requires human verification'),
'gsd-code-fixer should flag logic-bug fixes as requiring human verification');
});
test('gsd-code-reviewer.md REVIEW.md spec includes files_reviewed_list field', () => {
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-reviewer.md'), 'utf-8');
assert.ok(content.includes('files_reviewed_list'),
'gsd-code-reviewer REVIEW.md frontmatter spec must include files_reviewed_list for --auto scope persistence');
});
});
// --- CR-CMD: code review command structure ---
describe('CR-CMD: code review command structure', () => {
test('code-review.md has correct frontmatter name: gsd:code-review', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('name: gsd:code-review'),
'code-review.md missing correct name in frontmatter');
});
// #2790: code-review-fix.md was consolidated into code-review.md as the --fix flag.
test('code-review.md has --fix flag absorbing code-review-fix (#2790)', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
assert.ok(content.includes('--fix'),
'code-review.md must document --fix flag (absorbed code-review-fix)');
});
test('code-review.md references workflow: code-review.md', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
assert.ok(content.includes('code-review.md'),
'code-review.md does not reference its workflow');
});
test('code-review.md references code-review-fix workflow via --fix (#2790)', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
assert.ok(content.includes('code-review-fix') || content.includes('--fix'),
'code-review.md must reference code-review-fix workflow or --fix flag');
});
test('code-review.md has argument-hint in frontmatter', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('argument-hint:'),
'code-review.md missing argument-hint');
});
test('code-review.md argument-hint includes --fix flag (#2790: absorbed code-review-fix)', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('argument-hint:') && content.includes('--fix'),
'code-review.md must have argument-hint with --fix');
});
test('code-review.md has allowed-tools in frontmatter', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('allowed-tools:'),
'code-review.md missing allowed-tools');
});
test('code-review.md has allowed-tools in frontmatter (covers fix too, #2790)', () => {
const content = fs.readFileSync(path.join(COMMANDS_DIR, 'code-review.md'), 'utf-8');
const frontmatter = content.split('---')[1] || '';
assert.ok(frontmatter.includes('allowed-tools:'),
'code-review.md missing allowed-tools');
});
});
// --- CR-WORKFLOW: code review workflow structure ---
describe('CR-WORKFLOW: code review workflow structure', () => {
test('code-review.md workflow has <step name="initialize">', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review.md'), 'utf-8');
assert.ok(content.includes('<step name="initialize">'),
'code-review.md workflow missing initialize step');
});
test('code-review.md workflow has <step name="check_config_gate">', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review.md'), 'utf-8');
assert.ok(content.includes('<step name="check_config_gate">'),
'code-review.md workflow missing check_config_gate step');
});
test('code-review.md workflow references gsd-code-reviewer agent', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review.md'), 'utf-8');
assert.ok(content.includes('gsd-code-reviewer'),
'code-review.md workflow does not reference gsd-code-reviewer agent');
});
test('code-review-fix.md workflow has <step name="initialize">', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review-fix.md'), 'utf-8');
assert.ok(content.includes('<step name="initialize">'),
'code-review-fix.md workflow missing initialize step');
});
test('code-review-fix.md workflow references gsd-code-fixer agent', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review-fix.md'), 'utf-8');
assert.ok(content.includes('gsd-code-fixer'),
'code-review-fix.md workflow does not reference gsd-code-fixer agent');
});
test('code-review-fix.md workflow has iteration cap', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review-fix.md'), 'utf-8');
// Check for iteration logic with cap
assert.ok(content.includes('MAX_ITERATIONS') || (content.includes('3') && content.includes('iteration')),
'code-review-fix.md workflow missing iteration cap logic');
});
test('code-review.md --files path traversal guard rejects paths outside repo', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review.md'), 'utf-8');
// Guard must resolve and compare against REPO_ROOT
assert.ok(content.includes('REPO_ROOT') && content.includes('realpath'),
'code-review.md missing path traversal guard (realpath + REPO_ROOT check)');
assert.ok(content.includes('File path outside repository'),
'code-review.md missing rejection message for paths outside repo');
});
test('code-review.md uses portable while-read loop for array dedup (not mapfile)', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review.md'), 'utf-8');
// mapfile is bash 4+ only; macOS ships bash 3.2. Dedup must use portable while-read.
// Note: 'mapfile' may appear in platform_notes documentation — check bash code blocks only
const codeBlocks = content.match(/```bash[\s\S]*?```/g) || [];
const hasMapfileInCode = codeBlocks.some(block => block.includes('mapfile -t'));
assert.ok(!hasMapfileInCode,
'code-review.md bash code blocks use mapfile which is bash 4+ only — breaks macOS default bash 3.2');
assert.ok(content.includes('while IFS= read -r'),
'code-review.md should use portable while-read loop instead of mapfile');
});
test('code-review-fix.md uses portable while-read loop for array construction (not mapfile)', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'code-review-fix.md'), 'utf-8');
const codeBlocks = content.match(/```bash[\s\S]*?```/g) || [];
const hasMapfileInCode = codeBlocks.some(block => block.includes('mapfile -t'));
assert.ok(!hasMapfileInCode,
'code-review-fix.md bash code blocks use mapfile which is bash 4+ only — breaks macOS default bash 3.2');
assert.ok(content.includes('while IFS= read -r'),
'code-review-fix.md should use portable while-read loop instead of mapfile');
});
});
// --- CR-CONFIG: config key registration ---
describe('CR-CONFIG: config key registration', () => {
test('config-set accepts workflow.code_review', () => {
const tmpDir = createTempProject();
try {
const result = runGsdTools('config-set workflow.code_review true', tmpDir);
assert.ok(result.success, `config-set should accept workflow.code_review: ${result.error}`);
} finally {
cleanup(tmpDir);
}
});
test('config-set accepts workflow.code_review_depth', () => {
const tmpDir = createTempProject();
try {
const result = runGsdTools('config-set workflow.code_review_depth standard', tmpDir);
assert.ok(result.success, `config-set should accept workflow.code_review_depth: ${result.error}`);
} finally {
cleanup(tmpDir);
}
});
test('config-get workflow.code_review returns value set via config-set', (t) => {
const tmpDir = createTempProject();
t.after(() => cleanup(tmpDir));
const setResult = runGsdTools(['config-set', 'workflow.code_review', 'true'], tmpDir);
assert.ok(setResult.success, `config-set workflow.code_review failed: ${setResult.error}`);
const getResult = runGsdTools(['config-get', 'workflow.code_review'], tmpDir);
assert.ok(getResult.success, `config-get workflow.code_review failed: ${getResult.error}`);
assert.strictEqual(getResult.output, 'true',
`workflow.code_review should return "true", got ${getResult.output}`);
});
test('config-get workflow.code_review_depth returns value set via config-set', (t) => {
const tmpDir = createTempProject();
t.after(() => cleanup(tmpDir));
const setResult = runGsdTools(['config-set', 'workflow.code_review_depth', 'standard'], tmpDir);
assert.ok(setResult.success, `config-set workflow.code_review_depth failed: ${setResult.error}`);
const getResult = runGsdTools(['config-get', 'workflow.code_review_depth'], tmpDir);
assert.ok(getResult.success, `config-get workflow.code_review_depth failed: ${getResult.error}`);
assert.strictEqual(getResult.output, '"standard"',
`workflow.code_review_depth should return '"standard"', got ${getResult.output}`);
});
});
// --- CR-INTEGRATION: workflow integration points ---
describe('CR-INTEGRATION: workflow integration points', () => {
test('execute-phase.md contains code_review_gate step', { skip: !PLUGIN_AVAILABLE ? 'Plugin dir not installed' : false }, () => {
const content = fs.readFileSync(path.join(PLUGIN_WORKFLOWS_DIR, 'execute-phase.md'), 'utf-8');
assert.ok(content.includes('code_review_gate'),
'execute-phase.md missing code_review_gate step name');
});
test('execute-phase.md contains config-get workflow.code_review', { skip: !PLUGIN_AVAILABLE ? 'Plugin dir not installed' : false }, () => {
const content = fs.readFileSync(path.join(PLUGIN_WORKFLOWS_DIR, 'execute-phase.md'), 'utf-8');
assert.match(content, /config-get\s+workflow\.code_review/,
'execute-phase.md missing config-get workflow.code_review call');
});
test('execute-phase.md does NOT contain ls.*REVIEW.md.*head pattern', { skip: !PLUGIN_AVAILABLE ? 'Plugin dir not installed' : false }, () => {
const content = fs.readFileSync(path.join(PLUGIN_WORKFLOWS_DIR, 'execute-phase.md'), 'utf-8');
// Extract code_review_gate section to check
const gateMatch = content.match(/<step name="code_review_gate">([\s\S]*?)<\/step>/);
if (gateMatch) {
const gateContent = gateMatch[1];
assert.ok(!gateContent.match(/ls.*REVIEW\.md.*head/),
'execute-phase.md code_review_gate uses non-deterministic glob pattern (ls | head)');
}
});
test('quick.md contains code-review invocation', { skip: !PLUGIN_AVAILABLE ? 'Plugin dir not installed' : false }, () => {
const content = fs.readFileSync(path.join(PLUGIN_WORKFLOWS_DIR, 'quick.md'), 'utf-8');
assert.ok(content.includes('code-review') || content.includes('code_review'),
'quick.md missing code-review invocation');
});
test('quick.md contains config-get workflow.code_review', { skip: !PLUGIN_AVAILABLE ? 'Plugin dir not installed' : false }, () => {
const content = fs.readFileSync(path.join(PLUGIN_WORKFLOWS_DIR, 'quick.md'), 'utf-8');
assert.match(content, /config-get\s+workflow\.code_review/,
'quick.md missing config-get workflow.code_review call');
});
// autonomous.md tests read from the repo's canonical workflow source (WORKFLOWS_DIR),
// not the user-installed plugin dir. The plugin dir can lag behind the repo until the
// user re-installs, so asserting against it produces false negatives. The repo file
// is the source of truth and is always present in CI checkouts.
test('autonomous.md contains gsd-code-review skill invocation', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'autonomous.md'), 'utf-8');
// Parse Skill(...) invocations into structured objects and assert canonical
// hyphen form is referenced. Canonical command form is hyphen
// (gsd-code-review); colon form (gsd:code-review) is the legacy
// frontmatter-name form removed in PR #2819.
const invocations = parseWorkflowSkillInvocations(content);
const skillNames = invocations.map(inv => inv.skill);
assert.ok(skillNames.includes('gsd-code-review'),
`autonomous.md must invoke Skill(skill="gsd-code-review", ...); found skills: ${JSON.stringify(skillNames)}`);
assert.ok(!skillNames.includes('gsd:code-review'),
'autonomous.md must not use legacy colon form gsd:code-review (canonical is hyphen form)');
});
test('autonomous.md auto-fix uses consolidated gsd-code-review --fix invocation (#2790)', () => {
// After #2790, gsd-code-review-fix was absorbed into gsd-code-review as
// the --fix flag. The autonomous workflow must invoke the consolidated
// form, not the deleted gsd-code-review-fix skill.
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'autonomous.md'), 'utf-8');
const invocations = parseWorkflowSkillInvocations(content);
const skillNames = invocations.map(inv => inv.skill);
assert.ok(!skillNames.includes('gsd-code-review-fix'),
`autonomous.md must not invoke deleted gsd-code-review-fix skill (consolidated into --fix); found: ${JSON.stringify(skillNames)}`);
assert.ok(!skillNames.includes('gsd:code-review-fix'),
'autonomous.md must not use legacy colon form gsd:code-review-fix');
// Find a gsd-code-review invocation that carries the --fix flag (the
// consolidated auto-fix entry point).
const fixInvocation = invocations.find(inv => {
if (inv.skill !== 'gsd-code-review') return false;
const tokens = new Set((inv.args ?? '').split(/\s+/).filter(Boolean));
return tokens.has('--fix');
});
assert.ok(fixInvocation,
`autonomous.md must invoke Skill(skill="gsd-code-review", args="... --fix ...") for auto-fix; found: ${JSON.stringify(invocations)}`);
});
test('autonomous.md contains --auto flag on consolidated --fix invocation (#2790)', () => {
const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'autonomous.md'), 'utf-8');
// Find the gsd-code-review invocation that carries --fix (the consolidated
// auto-fix entry point), then assert --auto is one of its arg tokens.
// Tokenize via whitespace-split to avoid substring matches that could
// conflate --auto with --auto-foo.
const invocations = parseWorkflowSkillInvocations(content);
const fixInvocation = invocations.find(inv => {
if (inv.skill !== 'gsd-code-review') return false;
const tokens = new Set((inv.args ?? '').split(/\s+/).filter(Boolean));
return tokens.has('--fix');
});
assert.ok(fixInvocation, 'autonomous.md missing Skill(skill="gsd-code-review", args="... --fix ...") invocation');
const argTokens = new Set((fixInvocation.args ?? '').split(/\s+/).filter(Boolean));
assert.ok(argTokens.has('--auto'),
`autonomous.md gsd-code-review-fix args missing --auto flag; got args="${fixInvocation.args}"`);
});
});