feat(#1026): §5.6/ui-phase cutover — first gate dispatch (plan:pre step + blocking gate) (#1028)

Replace plan-phase.md §5.6 (a 6-branch inline UI gate) with a capability-driven
loop.render-hooks plan:pre dispatch — the FIRST gate dispatch in any workflow.
A step (ui-phase, when:workflow.ui_phase) + a new blocking gate
(when:workflow.ui_safety_gate). New ui.plan-gate check verb returns
{frontend, hasUiSpec, block}; the dispatch runs it unconditionally then fires
the active step (pipeline) or halts on the active blocking gate (manual). The
gate-handling (run check.query; halt if blocking+block) is the reusable
phase-6 template for blocking-gate cutovers.

Config semantics fixed per #1022 + maintainer call: ui_phase gates plan-time
UI-SPEC generation, ui_safety_gate gates the planning block. Common case + all
ui_phase=false cases are equivalence-preserving; the one intended change is
{ui_phase:true, ui_safety_gate:false} now auto-generating in pipelines.

Review found it broken twice (non-generic dispatch, phase-lookup divergence,
then the step-only check nested in a gate loop) — fixed; final Codex pass
verified all 8 (ui_phase,ui_safety_gate)x{pipeline,manual} cases correct.
gsd-ui-phase skill + autonomous §3a.5 untouched (§3a.5 deferred).
getRoadmapPhaseWithFallback mirrors cmdRoadmapGetPhase for lookup parity.

Closes #1026

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-11 00:58:21 -04:00
committed by GitHub
parent 5f48a42514
commit 9ed8c7d574
10 changed files with 972 additions and 125 deletions

View File

@@ -66,49 +66,71 @@ describe('Workflow .md structural guard (#3718)', () => {
// The UI safety gate invocation is embedded in workflow prose-as-code.
// These structural tests guard against regression where someone re-introduces
// the shell-based locale-prefix invocation that silently breaks on Windows PowerShell.
for (const [label, filePath] of [
['plan-phase.md', PLAN_PHASE_PATH],
['autonomous.md', AUTONOMOUS_PATH],
]) {
test(`${label} must invoke ui-safety-gate.cjs via stdin, anchored to the GSD install dir`, () => {
const content = fs.readFileSync(filePath, 'utf-8');
assert.ok(
content.includes('ui-safety-gate.cjs'),
`${label}: must reference ui-safety-gate.cjs for cross-shell portability (#3718)`
);
// Scope structural assertions to the gate invocation region so we test the
// gate's OWN resolution, not §1's unrelated RUNTIME_DIR usage for gsd-tools.
const gi = content.indexOf('ui-safety-gate.cjs');
const region = content.slice(Math.max(0, gi - 800), gi + 200);
// plan-phase.md (post-#1026): §5.6 now delegates UI gate evaluation to
// `gsd_run check ui-plan-gate` (a CLI command that internally calls checkUiPresence
// from ui-safety-gate.cjs). The direct shell invocation of ui-safety-gate.cjs was
// intentionally removed from the workflow — cross-shell portability is now provided
// by the CLI command layer, not inline shell code.
test('plan-phase.md must invoke check ui-plan-gate (capability-driven UI gate — #1026)', () => {
const content = fs.readFileSync(PLAN_PHASE_PATH, 'utf-8');
assert.ok(
content.includes('check ui-plan-gate'),
'plan-phase.md: §5.6 must delegate to `check ui-plan-gate` (capability-driven, #1026)'
);
assert.ok(
!content.includes('LC_ALL=C grep'),
'plan-phase.md: must NOT contain LC_ALL=C grep — that silently fails on Windows PowerShell (#3718)'
);
// Must NOT reintroduce the old shell-based path-search loop for ui-safety-gate.cjs
assert.ok(
!content.includes('UI_GATE_JS=$(for _c in'),
'plan-phase.md: must NOT contain the old shell-based ui-safety-gate.cjs path-search (old §5.6 pattern)'
);
});
// #448: the helper ships inside the GSD package, so it must be resolved
// against the GSD install dir (RUNTIME_DIR), NOT the consuming project's
// git root — otherwise the node call fails and the gate silently no-ops.
assert.ok(
region.includes('RUNTIME_DIR'),
`${label}: UI gate must resolve ui-safety-gate.cjs against RUNTIME_DIR (the GSD install dir), not the consuming project's git root (#448)`
);
assert.ok(
!region.includes('GSD_REPO_ROOT'),
`${label}: UI gate must NOT anchor the helper to GSD_REPO_ROOT (the consuming project's git root) — that silently no-ops in installed repos (#448)`
);
// Retain a git rev-parse --show-toplevel fallback when RUNTIME_DIR is unset (#3718).
assert.ok(
region.includes('git rev-parse --show-toplevel'),
`${label}: must retain a git rev-parse --show-toplevel fallback for the install-dir resolution`
);
// Confirm stdin pipe usage (printf or echo piped to node)
assert.ok(
content.includes('printf') || content.includes('echo'),
`${label}: must pipe phase section via stdin (printf/echo | node) to avoid ARG_MAX`
);
assert.ok(
!content.includes('LC_ALL=C grep'),
`${label}: must NOT contain LC_ALL=C grep — that silently fails on Windows PowerShell (#3718)`
);
});
}
// autonomous.md: still directly invokes ui-safety-gate.cjs via node stdin.
// This test is unchanged — autonomous.md §3a.5 has not been cut over to the
// capability-driven pattern yet (deferred per #1026 implementation notes).
test('autonomous.md must invoke ui-safety-gate.cjs via stdin, anchored to the GSD install dir', () => {
const label = 'autonomous.md';
const content = fs.readFileSync(AUTONOMOUS_PATH, 'utf-8');
assert.ok(
content.includes('ui-safety-gate.cjs'),
`${label}: must reference ui-safety-gate.cjs for cross-shell portability (#3718)`
);
// Scope structural assertions to the gate invocation region so we test the
// gate's OWN resolution, not §1's unrelated RUNTIME_DIR usage for gsd-tools.
const gi = content.indexOf('ui-safety-gate.cjs');
const region = content.slice(Math.max(0, gi - 800), gi + 200);
// #448: the helper ships inside the GSD package, so it must be resolved
// against the GSD install dir (RUNTIME_DIR), NOT the consuming project's
// git root — otherwise the node call fails and the gate silently no-ops.
assert.ok(
region.includes('RUNTIME_DIR'),
`${label}: UI gate must resolve ui-safety-gate.cjs against RUNTIME_DIR (the GSD install dir), not the consuming project's git root (#448)`
);
assert.ok(
!region.includes('GSD_REPO_ROOT'),
`${label}: UI gate must NOT anchor the helper to GSD_REPO_ROOT (the consuming project's git root) — that silently no-ops in installed repos (#448)`
);
// Retain a git rev-parse --show-toplevel fallback when RUNTIME_DIR is unset (#3718).
assert.ok(
region.includes('git rev-parse --show-toplevel'),
`${label}: must retain a git rev-parse --show-toplevel fallback for the install-dir resolution`
);
// Confirm stdin pipe usage (printf or echo piped to node)
assert.ok(
content.includes('printf') || content.includes('echo'),
`${label}: must pipe phase section via stdin (printf/echo | node) to avoid ARG_MAX`
);
assert.ok(
!content.includes('LC_ALL=C grep'),
`${label}: must NOT contain LC_ALL=C grep — that silently fails on Windows PowerShell (#3718)`
);
});
});
// ── Cross-shell spawn test (#3718) ────────────────────────────────────────────

View File

@@ -100,6 +100,18 @@ describe('UI pilot capability', () => {
assert.ok(uiPhaseStep, 'plan:pre.steps should contain the ui-phase step');
assert.strictEqual(uiPhaseStep.capId, 'ui');
// byLoopPoint['plan:pre'].gates contains the new ui.plan-gate (#1026)
const planPreGates = registry.byLoopPoint['plan:pre'].gates;
assert.ok(Array.isArray(planPreGates), 'plan:pre.gates should be an array');
const uiPlanGate = planPreGates.find(
(g) => g.check && g.check.query === 'ui.plan-gate',
);
assert.ok(uiPlanGate, 'plan:pre.gates should contain the ui.plan-gate (#1026)');
assert.strictEqual(uiPlanGate.capId, 'ui');
assert.strictEqual(uiPlanGate.blocking, true);
assert.strictEqual(uiPlanGate.when, 'workflow.ui_safety_gate');
assert.strictEqual(uiPlanGate.onError, 'halt');
// byLoopPoint['execute:wave:post'].gates contains the UI safety gate
const execWavePostGates = registry.byLoopPoint['execute:wave:post'].gates;
assert.ok(Array.isArray(execWavePostGates), 'execute:wave:post.gates should be an array');

View File

@@ -0,0 +1,318 @@
'use strict';
/**
* Behavioral tests for the `check ui-plan-gate` subcommand (#1026).
*
* Tests the `computeUiPlanGate` pure function exported from check-command-router.cjs.
* Uses in-memory tmpdir fixtures — no real CLI subprocess needed.
*
* Return shape: { frontend: bool, hasUiSpec: bool, block: bool, uiSpecPath: string|null }
* Invariant: block = frontend && !hasUiSpec
*
* Per RULESET.TESTS.boundary-coverage: exercises all three branches:
* (a) frontend+no-spec → block:true
* (b) frontend+spec → block:false
* (c) non-frontend → block:false
*
* Per RULESET.TESTS.coderabbit-fix-prefer: calls the exported function and asserts typed fields.
*/
const { describe, test, before, after } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { cleanup } = require('./helpers.cjs');
const { computeUiPlanGate } = require('../gsd-core/bin/lib/check-command-router.cjs');
// ─── Helpers ──────────────────────────────────────────────────────────────────
/**
* Create a minimal project dir with:
* .planning/ROADMAP.md — one phase section with `phaseSection` body
* .planning/phases/01-test-phase/ — phase directory
* (optionally) a *-UI-SPEC.md inside the phase dir
*/
function makeProject({ phaseSection = '', hasUiSpec = false } = {}) {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ui-plan-gate-test-'));
const planningDir = path.join(tmpDir, '.planning');
const phasesDir = path.join(planningDir, 'phases');
const phaseDir = path.join(phasesDir, '01-test-phase');
fs.mkdirSync(phaseDir, { recursive: true });
fs.writeFileSync(path.join(planningDir, 'config.json'), JSON.stringify({}), 'utf8');
// Minimal ROADMAP.md with one phase section
const roadmapContent = [
'# Project Roadmap',
'',
'## Phase 1: Test Phase',
'',
phaseSection,
'',
].join('\n');
fs.writeFileSync(path.join(planningDir, 'ROADMAP.md'), roadmapContent, 'utf8');
if (hasUiSpec) {
fs.writeFileSync(path.join(phaseDir, '01-UI-SPEC.md'), '# UI Design Contract\n', 'utf8');
}
return { tmpDir, phaseDir };
}
// ─── Tests ─────────────────────────────────────────────────────────────────────
describe('computeUiPlanGate — ui.plan-gate check logic (#1026)', () => {
let frontendNoSpec, frontendWithSpec, nonFrontend;
before(() => {
// Branch (a): frontend + no UI-SPEC → block:true
frontendNoSpec = makeProject({
phaseSection: 'Build the user interface and dashboard components for the frontend.',
hasUiSpec: false,
});
// Branch (b): frontend + UI-SPEC exists → block:false
frontendWithSpec = makeProject({
phaseSection: 'Build the frontend dashboard with React components and UI forms.',
hasUiSpec: true,
});
// Branch (c): no frontend indicators → block:false
nonFrontend = makeProject({
phaseSection: 'Add a REST API endpoint and database migration for the user table.',
hasUiSpec: false,
});
});
after(() => {
for (const { tmpDir } of [frontendNoSpec, frontendWithSpec, nonFrontend]) {
try { cleanup(tmpDir); } catch { /* ignore */ }
}
});
describe('return shape', () => {
test('result has required keys: frontend, hasUiSpec, block, uiSpecPath', () => {
const result = computeUiPlanGate(nonFrontend.tmpDir, '1');
assert.ok(typeof result === 'object' && result !== null, 'result must be an object');
assert.ok(typeof result.frontend === 'boolean', 'frontend must be boolean');
assert.ok(typeof result.hasUiSpec === 'boolean', 'hasUiSpec must be boolean');
assert.ok(typeof result.block === 'boolean', 'block must be boolean');
assert.ok('uiSpecPath' in result, 'uiSpecPath key must be present');
});
test('block invariant: block === frontend && !hasUiSpec for all scenarios', () => {
for (const [label, { tmpDir }] of [
['frontendNoSpec', frontendNoSpec],
['frontendWithSpec', frontendWithSpec],
['nonFrontend', nonFrontend],
]) {
const r = computeUiPlanGate(tmpDir, '1');
assert.strictEqual(
r.block,
r.frontend && !r.hasUiSpec,
`${label}: block invariant violated — frontend=${r.frontend} hasUiSpec=${r.hasUiSpec} block=${r.block}`,
);
}
});
});
describe('branch (a) — frontend + no UI-SPEC → block:true', () => {
test('detects frontend indicators in phase section', () => {
const r = computeUiPlanGate(frontendNoSpec.tmpDir, '1');
assert.strictEqual(r.frontend, true, 'should detect frontend indicators');
});
test('hasUiSpec is false when no *-UI-SPEC.md exists', () => {
const r = computeUiPlanGate(frontendNoSpec.tmpDir, '1');
assert.strictEqual(r.hasUiSpec, false, 'hasUiSpec must be false');
});
test('block is true when frontend + no UI-SPEC', () => {
const r = computeUiPlanGate(frontendNoSpec.tmpDir, '1');
assert.strictEqual(r.block, true, 'block must be true');
});
test('uiSpecPath is null when no UI-SPEC', () => {
const r = computeUiPlanGate(frontendNoSpec.tmpDir, '1');
assert.strictEqual(r.uiSpecPath, null, 'uiSpecPath must be null');
});
});
describe('branch (b) — frontend + UI-SPEC exists → block:false', () => {
test('detects frontend indicators in phase section', () => {
const r = computeUiPlanGate(frontendWithSpec.tmpDir, '1');
assert.strictEqual(r.frontend, true, 'should detect frontend indicators');
});
test('hasUiSpec is true when *-UI-SPEC.md exists', () => {
const r = computeUiPlanGate(frontendWithSpec.tmpDir, '1');
assert.strictEqual(r.hasUiSpec, true, 'hasUiSpec must be true');
});
test('block is false when UI-SPEC exists', () => {
const r = computeUiPlanGate(frontendWithSpec.tmpDir, '1');
assert.strictEqual(r.block, false, 'block must be false when spec exists');
});
test('uiSpecPath is a non-empty string ending in -UI-SPEC.md', () => {
const r = computeUiPlanGate(frontendWithSpec.tmpDir, '1');
assert.ok(typeof r.uiSpecPath === 'string' && r.uiSpecPath.length > 0,
'uiSpecPath must be a non-empty string');
assert.ok(r.uiSpecPath.endsWith('-UI-SPEC.md'), 'uiSpecPath must end with -UI-SPEC.md');
});
});
describe('branch (c) — non-frontend phase → block:false', () => {
test('frontend is false for non-UI phase section', () => {
const r = computeUiPlanGate(nonFrontend.tmpDir, '1');
assert.strictEqual(r.frontend, false, 'should NOT detect frontend indicators');
});
test('block is false for non-frontend phases', () => {
const r = computeUiPlanGate(nonFrontend.tmpDir, '1');
assert.strictEqual(r.block, false, 'block must be false');
});
});
describe('uses checkUiPresence word-boundary rules — no detection reimplementation', () => {
test('"microfrontend" (compound word) does NOT trigger frontend:true', () => {
const proj = makeProject({
phaseSection: 'Refactor the microfrontend architecture for better code reuse.',
hasUiSpec: false,
});
try {
const r = computeUiPlanGate(proj.tmpDir, '1');
assert.strictEqual(r.frontend, false,
'"microfrontend" compound word must NOT trigger frontend (word-boundary rule from checkUiPresence)');
} finally {
try { cleanup(proj.tmpDir); } catch { /* ignore */ }
}
});
test('"micro-frontend" (hyphenated) triggers frontend:true', () => {
const proj = makeProject({
phaseSection: 'Refactor the micro-frontend architecture for better code reuse.',
hasUiSpec: false,
});
try {
const r = computeUiPlanGate(proj.tmpDir, '1');
assert.strictEqual(r.frontend, true,
'"micro-frontend" must trigger frontend detection (word-boundary rule from checkUiPresence)');
} finally {
try { cleanup(proj.tmpDir); } catch { /* ignore */ }
}
});
});
describe('graceful degradation', () => {
test('non-existent project dir returns frontend:false, block:false (no crash)', () => {
const r = computeUiPlanGate('/tmp/nonexistent-gsd-test-dir-xyz', '1');
assert.strictEqual(typeof r.frontend, 'boolean', 'frontend must be boolean');
assert.strictEqual(r.frontend, false, 'missing roadmap → no frontend indicators');
assert.strictEqual(r.block, false, 'missing roadmap → block false');
});
test('missing ROADMAP.md returns frontend:false gracefully (no phaseLookupFailed)', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ui-gate-nomap-'));
try {
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-phase'), { recursive: true });
const r = computeUiPlanGate(tmpDir, '1');
assert.strictEqual(r.frontend, false, 'no ROADMAP → no frontend indicators');
assert.strictEqual(r.block, false, 'no ROADMAP → no block');
// phaseLookupFailed must NOT be set when ROADMAP.md is absent (no-roadmap project)
assert.ok(
!r.phaseLookupFailed,
'phaseLookupFailed must NOT be set when ROADMAP.md is absent (no-roadmap project is not a lookup failure)'
);
} finally {
try { cleanup(tmpDir); } catch { /* ignore */ }
}
});
test('ROADMAP.md present but phase not found → phaseLookupFailed:true (not silent false)', () => {
// This verifies FIX 2: when ROADMAP.md exists but the phase header is absent,
// we surface phaseLookupFailed rather than silently degrading to frontend:false,
// so an onError:halt gate cannot be silently bypassed by a typo in the phase number.
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ui-gate-noPhase-'));
try {
const planningDir = path.join(tmpDir, '.planning');
const phasesDir = path.join(planningDir, 'phases');
fs.mkdirSync(path.join(phasesDir, '01-test-phase'), { recursive: true });
// ROADMAP.md exists but has no Phase 99 header
fs.writeFileSync(path.join(planningDir, 'ROADMAP.md'), [
'# Project Roadmap',
'',
'## Phase 1: Test Phase',
'',
'Build the frontend dashboard with React components.',
'',
].join('\n'), 'utf8');
// Phase 99 is not in the roadmap
const r = computeUiPlanGate(tmpDir, '99');
assert.strictEqual(r.phaseLookupFailed, true,
'phaseLookupFailed must be true when ROADMAP.md exists but phase is not found');
// frontend should be false because section is empty
assert.strictEqual(r.frontend, false, 'empty section → no frontend indicators');
} finally {
try { cleanup(tmpDir); } catch { /* ignore */ }
}
});
});
describe('full-roadmap fallback (FIX 2 — mirrors roadmap.get-phase two-pass lookup)', () => {
test('phase in non-current milestone section is found via full-roadmap fallback', () => {
// Simulates a project where STATE.md declares milestone v1.0, but Phase 1 is
// in the v0.9 section (an older milestone, NOT in a <details> block).
// extractCurrentMilestone(content, cwd) returns only the v1.0 section → misses Phase 1.
// stripShippedMilestones(content) returns the FULL roadmap (strips only <details>) → finds Phase 1.
// computeUiPlanGate must find it via the stripShippedMilestones fallback, matching
// what `gsd_run query roadmap.get-phase` does.
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ui-gate-milestone-'));
try {
const planningDir = path.join(tmpDir, '.planning');
const phasesDir = path.join(planningDir, 'phases');
fs.mkdirSync(path.join(phasesDir, '01-test-phase'), { recursive: true });
// STATE.md declares current milestone = v1.0
fs.writeFileSync(path.join(planningDir, 'STATE.md'), [
'---',
'milestone: v1.0',
'---',
'',
'State content.',
].join('\n'), 'utf8');
// ROADMAP.md: Phase 1 is in v0.9 (NOT <details>), Phase 2 is in v1.0.
// With STATE.md pointing to v1.0, extractCurrentMilestone returns the v1.0 section only.
const roadmap = [
'# Project Roadmap',
'',
'## v0.9 — Previous Milestone',
'',
'### Phase 1: Frontend Dashboard',
'',
'Build the user interface and dashboard components for the frontend.',
'',
'## v1.0 — Current Milestone',
'',
'### Phase 2: API Layer',
'',
'Add REST API endpoints.',
'',
].join('\n');
fs.writeFileSync(path.join(planningDir, 'ROADMAP.md'), roadmap, 'utf8');
// Phase 1 is a frontend phase in a non-current milestone — must still be detected
const r = computeUiPlanGate(tmpDir, '1');
assert.strictEqual(r.frontend, true,
'frontend:true must be detected for phase in non-current milestone (full-roadmap fallback)');
assert.ok(
!r.phaseLookupFailed,
'phaseLookupFailed must be false when phase is found via full-roadmap fallback'
);
} finally {
try { cleanup(tmpDir); } catch { /* ignore */ }
}
});
});
});

View File

@@ -110,40 +110,81 @@ describe('spike #1018 — off means off (structural proof)', () => {
"onError must be 'skip' as declared in the registry");
});
// ── Case 2: UI off ─────────────────────────────────────────────────────────
// ── Case 2: STEP-only-off (ui_phase=false, ui_safety_gate=true) ──────────────
//
// config { workflow: { ui_phase: false } } → step filtered out.
// hostConsume must report activeCount=0, skillsToInvoke=[], and
// rendered must equal the base/no-active form — the OFF output IS the base.
// (#1026) plan:pre now has TWO hooks — a step (when: workflow.ui_phase) and a
// gate (when: workflow.ui_safety_gate). They are INDEPENDENT toggles by design.
// Turning off ui_phase suppresses the step but the gate (ui_safety_gate=true)
// still fires → rendered is NOT the zero-hooks base (it contains the gate block).
//
// This case proves the step surface is a pure function of step-kind hooks:
// activeCount=0, skillsToInvoke=[] — "step off means step off".
// It also asserts the gate IS present in activeHooks and rendered differs from
// the empty base, documenting that the two toggles are genuinely independent.
test('UI off: config {workflow:{ui_phase:false}} → activeCount=0, skillsToInvoke=[], rendered equals zero-hooks base', () => {
const offConfig = { workflow: { ui_phase: false } };
test('STEP-only-off: config {workflow:{ui_phase:false, ui_safety_gate:true}} → step absent, gate present, rendered ≠ base', () => {
const stepOffConfig = { workflow: { ui_phase: false, ui_safety_gate: true } };
const resolved = resolveLoopHooks({
point: 'plan:pre',
registry: realRegistry,
config: offConfig,
config: stepOffConfig,
});
const envelope = { activeHooks: resolved.activeHooks, rendered: renderLoopHooks(resolved) };
const consumed = hostConsume(envelope);
const base = makeBaseEnvelope('plan:pre');
const baseConsumed = hostConsume(base);
// Step-level aggregate: step is off
assert.strictEqual(consumed.activeCount, 0,
'Expected 0 active steps when ui_phase=false');
assert.deepEqual(consumed.skillsToInvoke, [],
'skillsToInvoke must be [] when ui_phase=false');
// STRUCTURAL ASSERTION (the spike's point):
// The off-state host output is identical to the host's zero-hooks base.
// The aggregate is a pure function of activeHooks — nothing in host source
// was mutated. This proves "off means off" by construction.
assert.strictEqual(consumed.rendered, baseConsumed.rendered,
'OFF rendered output must be byte-identical to the zero-hooks base rendered output (pure function of activeHooks)');
assert.strictEqual(consumed.activeCount, baseConsumed.activeCount,
'OFF activeCount must equal base activeCount');
assert.deepEqual(consumed.skillsToInvoke, baseConsumed.skillsToInvoke,
'OFF skillsToInvoke must equal base skillsToInvoke');
// Gate is still active: activeHooks is NOT empty (contains the gate hook)
const gateHooks = resolved.activeHooks.filter(h => h.kind === 'gate');
assert.ok(gateHooks.length > 0,
'Gate hook must still be present in activeHooks when ui_safety_gate=true');
// Rendered is NOT the zero-hooks base because the gate block is present
const base = makeBaseEnvelope('plan:pre');
assert.notStrictEqual(consumed.rendered, base.rendered,
'rendered must NOT equal the zero-hooks base when the gate is still active (ui_safety_gate=true)');
// Rendered must NOT include a step block for ui-phase
assert.ok(
!consumed.rendered.includes('### Step') || !consumed.rendered.includes('ui-phase'),
'OFF rendered must not include an active step block for ui-phase',
);
});
// ── Case 2b: ALL-OFF (ui_phase=false, ui_safety_gate=false) ─────────────────
//
// Both toggles off → activeHooks is genuinely empty → rendered is byte-identical
// to the zero-hooks base produced by the empty-registry helper.
//
// THIS is the clean structural "off means off → base output" proof.
// It must exist as a concrete, computable assertion — not be elided because a
// partial-off case happens to have a gate. The empty base is computed, not
// hand-coded, so if renderLoopHooks ever changes its empty format this still holds.
test('ALL-OFF: config {workflow:{ui_phase:false, ui_safety_gate:false}} → activeHooks empty AND rendered === base', () => {
const allOffConfig = { workflow: { ui_phase: false, ui_safety_gate: false } };
const resolved = resolveLoopHooks({
point: 'plan:pre',
registry: realRegistry,
config: allOffConfig,
});
const envelope = { activeHooks: resolved.activeHooks, rendered: renderLoopHooks(resolved) };
// STRUCTURAL ASSERTION (the spike's core proof — restored):
// When every hook at this point is toggled off, activeHooks must be empty
// and rendered must be byte-identical to the computed zero-hooks base.
// This proves the host aggregate is a pure function of activeHooks — no
// capability leaks through when all its controlling config keys are false.
assert.strictEqual(resolved.activeHooks.length, 0,
'ALL-OFF: activeHooks must be empty when both ui_phase and ui_safety_gate are false');
const base = makeBaseEnvelope('plan:pre');
assert.strictEqual(envelope.rendered, base.rendered,
'ALL-OFF: rendered must be byte-identical to the zero-hooks base when activeHooks is empty');
});
// ── Case 3: Synthetic multi-hook ──────────────────────────────────────────

View File

@@ -10,7 +10,7 @@ const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
describe('plan-phase UI-SPEC missing behavior', () => {
describe('plan-phase §5.6 UI Design Contract Gate', () => {
const workflowPath = path.join(
__dirname,
'..',
@@ -19,38 +19,289 @@ describe('plan-phase UI-SPEC missing behavior', () => {
'plan-phase.md'
);
// Load once
test('workflow file exists', () => {
assert.ok(fs.existsSync(workflowPath), `Expected workflow file at ${workflowPath}`);
});
test('does NOT contain hard-blocking exit redirect to /gsd-ui-phase', () => {
const text = fs.readFileSync(workflowPath, 'utf8');
// The hard redirect pattern: AskUserQuestion option exits with "Run /gsd-ui-phase... Exit workflow."
// This is the pattern from line ~503 in the original file
// ── Capability-driven dispatch ─────────────────────────────────────────────
test('§5.6 dispatches loop render-hooks plan:pre (capability-driven)', () => {
const section = extractSection56(workflowPath);
assert.ok(
section.includes('loop render-hooks plan:pre'),
'§5.6 must dispatch `loop render-hooks plan:pre` to resolve active capability hooks'
);
});
test('§5.6 invokes gsd_run check ui-plan-gate UNCONDITIONALLY (not inside a gate loop)', () => {
const section = extractSection56(workflowPath);
// The fix for {ui_phase:true, ui_safety_gate:false}: the check must be run
// UNCONDITIONALLY — established before any per-hook branching — so the step's
// frontend/UI-SPEC precondition is available even when no gate hook is active.
// The literal `check ui-plan-gate` must appear in the section.
assert.ok(
section.includes('check ui-plan-gate'),
'§5.6 must run `gsd_run check ui-plan-gate` unconditionally (not gated inside a kind==gate loop)'
);
// The "including the step-only case" wording proves the unconditional intent.
assert.ok(
section.includes('step-only case'),
'§5.6 must document "step-only case" to prove the check runs independently of gate presence'
);
});
test('§5.6 establishes GATE= before any branch-5 step dispatch (check precedes step)', () => {
const section = extractSection56(workflowPath);
// GATE= must be assigned before Branch 5 fires step hooks.
// Both must be present; GATE= must appear earlier in the section text.
const gateIdx = section.indexOf('GATE=$(gsd_run check ui-plan-gate');
const branch5Idx = section.indexOf('Branch 5');
assert.ok(gateIdx !== -1, '§5.6 must contain `GATE=$(gsd_run check ui-plan-gate` assignment');
assert.ok(branch5Idx !== -1, '§5.6 must contain Branch 5');
assert.ok(
gateIdx < branch5Idx,
`§5.6 GATE= check must appear BEFORE Branch 5 step dispatch (gateIdx=${gateIdx} branch5Idx=${branch5Idx})`
);
});
test('§5.6 invokes gsd_run check with gate check.query (generic dispatch — ui.plan-gate or ui-plan-gate)', () => {
const section = extractSection56(workflowPath);
// The dispatch MUST use the `check` verb and reference the gate query.
// OLD: hardcoded `check ui-plan-gate` inside gate loop
// NEW: unconditional `check ui-plan-gate` — literal command; check.query = "ui.plan-gate"
// Both forms must be accepted: the section references either the literal query or the
// generic pattern. The check router normalizes dots to hyphens so both are equivalent.
assert.ok(
section.includes('check ui-plan-gate') ||
section.includes('check ui.plan-gate') ||
(section.includes('check') && section.includes('check.query')),
'§5.6 must invoke `gsd_run check` with the gate\'s check.query (generic dispatch — not hardcoded for a specific check)'
);
});
test('§5.6 reads frontend, hasUiSpec, block from gate result', () => {
const section = extractSection56(workflowPath);
assert.ok(section.includes('frontend'), '§5.6 must read `frontend` from gate result');
assert.ok(section.includes('hasUiSpec'), '§5.6 must read `hasUiSpec` from gate result');
assert.ok(section.includes('block'), '§5.6 must read `block` from gate result');
});
test('§5.6 does NOT inline config-get workflow.ui_phase (toggle owned by registry)', () => {
const section = extractSection56(workflowPath);
assert.ok(
!section.includes('config-get workflow.ui_phase'),
'§5.6 must NOT inline `config-get workflow.ui_phase` — toggle is resolved by render-hooks'
);
});
test('§5.6 does NOT inline config-get workflow.ui_safety_gate (toggle owned by registry)', () => {
const section = extractSection56(workflowPath);
assert.ok(
!section.includes('config-get workflow.ui_safety_gate'),
'§5.6 must NOT inline `config-get workflow.ui_safety_gate` — toggle is resolved by render-hooks'
);
});
// ── 6-branch equivalence ───────────────────────────────────────────────────
test('Branch 1: activeHooks empty → skip to step 6 (NOT §5.7)', () => {
const section = extractSection56(workflowPath);
// Branch 1 (both toggles off) MUST skip to step 6 — NOT §5.7.
// OLD §5.6 branch-1 target was "step 6". §5.7 (Schema Push) is NOT skipped
// when both UI toggles are off — schema detection is independent.
assert.ok(
section.includes('activeHooks'),
'§5.6 Branch 1 must reference activeHooks'
);
// The skip target must be step 6
assert.ok(
section.match(/activeHooks[^.]*?step 6/s) ||
section.match(/empty[^.]*?step 6/s) ||
section.match(/absent[^.]*?step 6/s) ||
/Branch 1[^.]*?step 6/s.test(section),
'§5.6 Branch 1 (both-off / empty activeHooks) must skip to step 6, NOT §5.7'
);
});
test('Branch 2: no frontend indicators → skip silently to §5.7', () => {
const section = extractSection56(workflowPath);
assert.ok(
section.includes('frontend') && section.includes('5.7'),
'§5.6 Branch 2 must route non-frontend phases to §5.7'
);
});
test('Branch 3: hasUiSpec true → sets UI_SPEC_PATH, displays using-contract message', () => {
const section = extractSection56(workflowPath);
assert.ok(
section.includes('hasUiSpec') && section.includes('UI_SPEC_PATH') && section.includes('Using UI design contract'),
'§5.6 Branch 3 must set UI_SPEC_PATH and display the using-contract message'
);
});
test('Branch 4: --skip-ui in $ARGUMENTS → skip to step 6', () => {
const content = fs.readFileSync(workflowPath, 'utf8');
assert.ok(
content.includes('--skip-ui'),
'§5.6 must include --skip-ui bypass option (Branch 4)'
);
});
test('Branch 5: AUTO_CHAIN=true → step hooks dispatched via gsd-${ref.skill}', () => {
const section = extractSection56(workflowPath);
assert.ok(
section.includes('AUTO_CHAIN') && section.includes('gsd-${ref.skill}'),
'§5.6 Branch 5 must dispatch step hooks via gsd-${ref.skill} in pipeline mode'
);
});
test('Branch 5: pipeline dispatch uses kind=="step" and ref.skill filter', () => {
const section = extractSection56(workflowPath);
assert.ok(
section.includes('kind') && section.includes('ref.skill'),
'§5.6 Branch 5 must filter activeHooks by kind=="step" and ref.skill'
);
});
test('Branch 5: step hooks fire independently of whether a gate is active ({ui_phase:T,ui_safety_gate:F} regression guard)', () => {
const section = extractSection56(workflowPath);
// KEY REGRESSION GUARD: the step must fire even when ui_safety_gate=false (no gate hook).
// The new wording "independently of whether a gate is active" makes this explicit.
assert.ok(
section.includes('independently of whether a gate is active'),
'§5.6 Branch 5 must state step hooks fire "independently of whether a gate is active" — guards {ui_phase:T,ui_safety_gate:F} regression'
);
});
test('Branch 6: AUTO_CHAIN=false → generic gate handling (kind==gate, blocking, block→exit; no active gate→continue)', () => {
const section = extractSection56(workflowPath);
assert.ok(
section.includes('AUTO_CHAIN'),
'§5.6 Branch 6 must read AUTO_CHAIN flag'
);
// Branch 6 is the generic gate-handling branch:
// - iterates activeHooks where kind=="gate" and blocking:true
// - only EXITs when block:true is set on that gate
// - if no active blocking gate is present (e.g. ui_safety_gate off), continues to step 6
assert.ok(
section.includes('kind == "gate"') || section.includes('kind=="gate"'),
'§5.6 Branch 6 must filter by kind=="gate" (generic gate-handling)'
);
assert.ok(
/blocking.*true/i.test(section),
'§5.6 Branch 6 must check `blocking` is true for the gate'
);
// Must halt on block:true — both "Exit the plan-phase workflow" AND "Do not continue" are required
assert.ok(
section.includes('Exit the plan-phase workflow'),
'§5.6 Branch 6 must say "Exit the plan-phase workflow"'
);
assert.ok(
section.includes('Do not continue'),
'§5.6 Branch 6 must say "Do not continue"'
);
// If no active blocking gate → continue (no block when ui_safety_gate is off)
assert.ok(
section.includes('no active blocking gate') || section.includes('no block'),
'§5.6 Branch 6 must document: no active blocking gate → continue to step 6 (no block)'
);
});
test('Branch 6: recommendation block contains EXACT warning heading + /gsd:ui-phase + --skip-ui', () => {
const section = extractSection56(workflowPath);
// Assert the EXACT halt recommendation block text, not substring-OR
assert.ok(
section.includes('⚠ UI-SPEC.md missing for Phase'),
'§5.6 must include the "⚠ UI-SPEC.md missing for Phase" warning heading'
);
assert.ok(
section.includes('Recommended next step'),
'§5.6 must include the "Recommended next step" label in the recommendation block'
);
assert.ok(
section.includes('/gsd:ui-phase'),
'§5.6 must include /gsd:ui-phase recommendation in the block'
);
assert.ok(
section.includes('--skip-ui'),
'§5.6 must include --skip-ui as bypass option in the recommendation block'
);
// The "Also available" line must be present (shows the skip-ui option)
assert.ok(
section.includes('Also available'),
'§5.6 recommendation block must include the "Also available" section'
);
});
// ── Generic gate-dispatch contract ────────────────────────────────────────
test('§5.6 documents gate check.query binding (check.query="ui.plan-gate" → check ui-plan-gate)', () => {
const section = extractSection56(workflowPath);
// The section must document WHERE "ui-plan-gate" comes from — it is the gate hook's
// check.query value "ui.plan-gate" normalized to a hyphen form by the check router.
// The literal `check ui-plan-gate` command is acceptable (and now required by bug-3706),
// but the section must also document the check.query binding so the contract is auditable.
assert.ok(
section.includes('check.query') || section.includes('check ui-plan-gate'),
'§5.6 must reference check.query or the literal check ui-plan-gate command (gate binding contract)'
);
});
test('§5.6 documents that check router normalizes dots to hyphens (ui.plan-gate → ui-plan-gate)', () => {
const section = extractSection56(workflowPath);
// The dot-normalization rule must be documented so the declared check.query is runnable
assert.ok(
section.includes('ui.plan-gate') || section.includes('normalizes dots') || section.includes('dots to hyphens'),
'§5.6 must document that the check router normalizes dots to hyphens for the declared check.query'
);
});
test('§5.6 partial-off case: ui_phase=true, ui_safety_gate=false → step fires, gate does NOT block', () => {
const section = extractSection56(workflowPath);
// The intended behavior change: ui_phase gates the step, ui_safety_gate gates the block.
// When only ui_safety_gate is false, the step fires but the gate doesn't halt.
// Document this in the section.
assert.ok(
section.includes('workflow.ui_phase') || section.includes('ui_phase'),
'§5.6 must reference workflow.ui_phase (step hook config key)'
);
assert.ok(
section.includes('workflow.ui_safety_gate') || section.includes('ui_safety_gate'),
'§5.6 must reference workflow.ui_safety_gate (gate hook config key)'
);
});
// ── Legacy pattern that must NOT appear ────────────────────────────────────
test('does NOT contain hard-blocking exit redirect to /gsd-ui-phase (old pattern)', () => {
const content = fs.readFileSync(workflowPath, 'utf8');
const hardExitPattern = /Generate UI-SPEC first.*Exit workflow/s;
assert.ok(
!hardExitPattern.test(text),
'plan-phase.md must NOT contain a hard "Generate UI-SPEC first → Exit workflow" redirect. ' +
'It should offer a primary recommendation with --skip-ui bypass option instead.'
!hardExitPattern.test(content),
'plan-phase.md must NOT contain a hard "Generate UI-SPEC first → Exit workflow" redirect'
);
});
test('contains --skip-ui bypass option when UI-SPEC.md is missing', () => {
const text = fs.readFileSync(workflowPath, 'utf8');
test('does NOT inline the shell-based ui-safety-gate.cjs path-search block (old §5.6)', () => {
const section = extractSection56(workflowPath);
// The old §5.6 resolved the gate helper via a for-loop over path candidates
assert.ok(
text.includes('--skip-ui'),
'plan-phase.md must include --skip-ui as a bypass option when UI-SPEC.md is missing'
);
});
test('contains a primary recommendation block for missing UI-SPEC', () => {
const text = fs.readFileSync(workflowPath, 'utf8');
const hasRecommendationPattern =
text.includes('Recommended next step') &&
text.includes('gsd-ui-phase');
assert.ok(
hasRecommendationPattern,
'plan-phase.md must include a "Recommended next step" recommendation for /gsd-ui-phase when UI-SPEC.md is missing'
!section.includes('UI_GATE_JS=$(for _c in'),
'§5.6 must NOT contain the old shell-based ui-safety-gate.cjs path-search (now delegated to check ui-plan-gate)'
);
});
});
/**
* Extract the text of §5.6 through (but not including) §5.7.
* Scoped to avoid false positives from other parts of the file.
*/
function extractSection56(workflowPath) {
const content = fs.readFileSync(workflowPath, 'utf8');
const start = content.indexOf('## 5.6.');
assert.ok(start !== -1, '§5.6 heading must be present in plan-phase.md');
const end = content.indexOf('## 5.7.', start);
assert.ok(end !== -1, '§5.7 heading must follow §5.6 in plan-phase.md');
return content.slice(start, end);
}