diff --git a/gsd-core/workflows/autonomous.md b/gsd-core/workflows/autonomous.md index 991e4c89c..1135630af 100644 --- a/gsd-core/workflows/autonomous.md +++ b/gsd-core/workflows/autonomous.md @@ -318,47 +318,42 @@ Check `has_context`. If false → go to handle_blocker: "Discuss for phase ${PHA **3a.5. UI Design Contract (Frontend Phases)** -Check if this phase has frontend indicators and whether a UI-SPEC already exists: +Resolve active `plan:pre` hooks: ```bash -PHASE_SECTION=$(gsd_run query roadmap.get-phase ${PHASE_NUM} 2>/dev/null) -# Shell-free word-boundary gate (#3718): Node.js helper — no locale env-var dependency. -# Reads via stdin to avoid OS ARG_MAX limits on large phase text. -# Resolve the helper against the GSD install dir via RUNTIME_DIR (#448) — NOT the consuming -# project's git root — falling back to git toplevel / $HOME/.claude. Exit codes mirror grep (0=UI,1=none). -_GSD_RT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}" -UI_GATE_JS=$(for _c in "$_GSD_RT/gsd-core/bin/lib/ui-safety-gate.cjs" "$_GSD_RT/bin/lib/ui-safety-gate.cjs" "$_GSD_RT/.claude/bin/lib/ui-safety-gate.cjs" "$HOME/.claude/gsd-core/bin/lib/ui-safety-gate.cjs" "$HOME/.claude/bin/lib/ui-safety-gate.cjs"; do [ -f "$_c" ] && { echo "$_c"; break; }; done) -if [ -n "$UI_GATE_JS" ]; then printf '%s' "$PHASE_SECTION" | node "$UI_GATE_JS" >/dev/null 2>&1; HAS_UI=$?; else echo "WARN: ui-safety-gate.cjs not found via RUNTIME_DIR/\$HOME (#448) — assuming UI present" >&2; HAS_UI=0; fi UI_SPEC_FILE=$(ls "${PHASE_DIR}"/*-UI-SPEC.md 2>/dev/null | head -1) +HOOKS_JSON=$(gsd_run loop render-hooks plan:pre --raw) ``` -Check if UI phase workflow is enabled: +Read the `activeHooks` array directly from `HOOKS_JSON` (in-context — do NOT invoke a shell pipeline). **Compute the active UI step hooks** = entries from `activeHooks` where `kind == "step"` and `ref.skill` is set. **If there are NO active step hooks → skip silently to 3b.** (This covers `workflow.ui_phase=false` — including configurations where only a gate-only entry is present, e.g. `ui_phase=false` + `ui_safety_gate=true` produces `activeHooks=[{kind:"gate"}]`. Autonomous never runs the plan:pre gate — it is always pipeline mode — so a gate-only active set is equivalent to no active step and is silently skipped here. This matches OLD §3a.5 behaviour.) + +(At least one active step hook ⇒ `workflow.ui_phase` is on.) Run the UI-SPEC gate: ```bash -UI_PHASE_CFG=$(gsd_run query config-get workflow.ui_phase 2>/dev/null || echo "true") +GATE=$(gsd_run check ui-plan-gate "${PHASE_NUM}" --raw) ``` -**If `HAS_UI` is 0 (frontend indicators found) AND `UI_SPEC_FILE` is empty (no UI-SPEC exists) AND `UI_PHASE_CFG` is not `false`:** +Read `frontend` and `hasUiSpec` from `GATE` (in-context). -Display: +**If `frontend` is false:** Skip silently to 3b. + +**If `hasUiSpec` is true (UI-SPEC already exists):** Skip silently to 3b. + +**Otherwise (frontend phase + no UI-SPEC):** For each active step hook (the `kind == "step"` set from above, in array order): ``` -Phase ${PHASE_NUM}: Frontend phase detected — generating UI design contract... +Skill(skill="gsd-${ref.skill}", args="${PHASE_NUM}") ``` -``` -Skill(skill="gsd-ui-phase", args="${PHASE_NUM}") -``` +(Prepend `gsd-` to `ref.skill` — so `ui-phase` → `gsd-ui-phase`. Bare `${PHASE_NUM}` args — autonomous style, same pattern as the verify:post dispatch.) Entries where `kind == "gate"` are silently ignored — autonomous is always pipeline mode, there is no blocking gate here. -Verify UI-SPEC was created: +After all step hooks return, re-read: ```bash UI_SPEC_FILE=$(ls "${PHASE_DIR}"/*-UI-SPEC.md 2>/dev/null | head -1) ``` -**If `UI_SPEC_FILE` is still empty after ui-phase:** Display warning `Phase ${PHASE_NUM}: UI-SPEC generation did not produce output — continuing without design contract.` and proceed to 3b. - -**If `HAS_UI` is 1 (no frontend indicators) OR `UI_SPEC_FILE` is not empty (UI-SPEC already exists) OR `UI_PHASE_CFG` is `false`:** Skip silently to 3b. +**If `UI_SPEC_FILE` is still empty:** Display warning `Phase ${PHASE_NUM}: UI-SPEC generation did not produce output — continuing without design contract.` and proceed to 3b. NON-BLOCKING. **3b. Plan** diff --git a/tests/autonomous-ui-steps.test.cjs b/tests/autonomous-ui-steps.test.cjs index b8ab59fb0..bb722e7e4 100644 --- a/tests/autonomous-ui-steps.test.cjs +++ b/tests/autonomous-ui-steps.test.cjs @@ -22,6 +22,15 @@ describe('autonomous workflow ui-phase and ui-review integration (#1375)', () => }); describe('step 3a.5 — UI design contract before planning', () => { + // Helper: extract the §3a.5 section text (from heading to the next "**3b." heading) + function getSection3a5(c) { + const start = c.indexOf('**3a.5.'); + const end = c.indexOf('**3b.', start); + assert.ok(start !== -1, '§3a.5 heading must be present in autonomous.md'); + assert.ok(end !== -1, '**3b. must follow §3a.5 in autonomous.md'); + return c.slice(start, end); + } + test('autonomous.md contains a UI design contract step between discuss and plan', () => { assert.ok( content.includes('3a.5'), @@ -29,43 +38,144 @@ describe('autonomous workflow ui-phase and ui-review integration (#1375)', () => ); }); - test('UI design contract step detects frontend indicators via shell-free Node gate (#3718)', () => { - // After #3718: the gate is implemented in bin/lib/ui-safety-gate.cjs (Node.js) - // piped from stdin, avoiding silent failure on Windows PowerShell and ARG_MAX. - // After #448: the helper is resolved against the GSD install dir (RUNTIME_DIR), - // not the consuming project's git root, so it is actually found at runtime. + test('§3a.5 dispatches loop render-hooks plan:pre to resolve active capability hooks', () => { + // Phase 5.6/ui-phase cutover: §3a.5 now dispatches render-hooks plan:pre instead of + // inlining ui-safety-gate.cjs RUNTIME_DIR probe + config-get workflow.ui_phase. + const section = getSection3a5(content); assert.ok( - content.includes('ui-safety-gate.cjs'), - 'should invoke shell-free Node gate for cross-platform portability (#3718)' - ); - assert.ok( - content.includes('RUNTIME_DIR'), - 'should resolve the gate helper against the GSD install dir (RUNTIME_DIR), not the consuming project root (#448)' + section.includes('loop render-hooks plan:pre'), + '§3a.5 must dispatch `loop render-hooks plan:pre` to resolve active capability hooks' ); }); - test('UI design contract step checks for existing UI-SPEC.md', () => { + test('§3a.5 uses check ui-plan-gate to determine frontend and hasUiSpec', () => { + const section = getSection3a5(content); assert.ok( - content.includes('UI-SPEC.md'), - 'should check for existing UI-SPEC.md' + section.includes('check ui-plan-gate'), + '§3a.5 must call `check ui-plan-gate` to gate on frontend indicators and existing UI-SPEC' + ); + assert.ok( + section.includes('frontend'), + '§3a.5 must read the `frontend` field from the gate result' + ); + assert.ok( + section.includes('hasUiSpec'), + '§3a.5 must read the `hasUiSpec` field from the gate result' ); }); - test('UI design contract step respects workflow.ui_phase config toggle', () => { + test('§3a.5 constructs skill via gsd-${ref.skill} prefix (capability-driven dispatch)', () => { + const section = getSection3a5(content); assert.ok( - content.includes('workflow.ui_phase'), - 'should respect workflow.ui_phase config toggle' + section.includes('gsd-${ref.skill}'), + '§3a.5 must construct skill name via `gsd-${ref.skill}` prefix (matches §3d.5 style)' ); }); - test('UI design contract step invokes gsd:ui-phase skill', () => { + test('§3a.5 checks for existing UI-SPEC.md', () => { + const section = getSection3a5(content); assert.ok( - content.includes('skill="gsd-ui-phase"'), - 'should invoke gsd-ui-phase via Skill()' + section.includes('UI-SPEC.md'), + '§3a.5 must check for existing UI-SPEC.md' ); }); - test('UI design contract step appears before plan step (3b)', () => { + test('§3a.5 does NOT inline ui-safety-gate.cjs or RUNTIME_DIR probe (replaced by registry)', () => { + const section = getSection3a5(content); + assert.ok( + !section.includes('ui-safety-gate.cjs'), + '§3a.5 must NOT inline ui-safety-gate.cjs (replaced by check ui-plan-gate)' + ); + assert.ok( + !section.includes('RUNTIME_DIR'), + '§3a.5 must NOT probe RUNTIME_DIR (replaced by check ui-plan-gate via capability registry)' + ); + }); + + test('§3a.5 does NOT inline config-get workflow.ui_phase (resolved by registry via render-hooks)', () => { + const section = getSection3a5(content); + assert.ok( + !section.includes('config-get workflow.ui_phase'), + '§3a.5 must NOT inline `config-get workflow.ui_phase` — toggle is owned by the capability registry' + ); + }); + + test('§3a.5 is step-only and non-blocking — no gate-halt or exit', () => { + const section = getSection3a5(content); + // Must not introduce blocking gate language + assert.ok( + !section.includes('EXIT') && !section.includes('exit the') && !section.includes('halt'), + '§3a.5 must NOT introduce any gate-halt or exit — it is step-only and non-blocking' + ); + // Must be explicitly non-blocking + assert.ok( + section.includes('NON-BLOCKING') || section.includes('non-blocking') || section.includes('continue') || section.includes('proceed'), + '§3a.5 must be explicitly non-blocking (warning + continue)' + ); + }); + + test('§3a.5 skip condition is "no active step hooks" — not "empty activeHooks"', () => { + // Equivalence-preservation fix (#1031): the skip must key on KIND=="step" hooks, + // not on activeHooks being empty. A gate-only result (ui_phase=false + + // ui_safety_gate=true → activeHooks=[{kind:"gate"}]) must also skip silently. + const section = getSection3a5(content); + + // Must compute active step hooks (kind=="step") before deciding to skip + assert.ok( + section.includes('kind == "step"') || section.includes("kind == 'step'") || section.includes('kind=="step"'), + '§3a.5 must gate the skip decision on kind=="step" entries' + ); + + // The skip condition must explicitly mention "no active step" (or equivalent), + // NOT "empty or absent" (which was the old incorrect condition) + assert.ok( + section.includes('NO active step') || section.includes('no active step') || section.includes('no active UI step'), + '§3a.5 skip condition must reference "no active step hook(s)" — not just empty activeHooks' + ); + + // The prose must NOT use the old "empty or absent" language as the skip guard + assert.ok( + !section.includes('empty or absent'), + '§3a.5 must NOT use "empty or absent" as the skip condition — that misses gate-only sets' + ); + }); + + test('§3a.5 gate-only case: {ui_phase:false, ui_safety_gate:true} → silent skip, no warning', () => { + // Scenario: workflow.ui_phase=false AND workflow.ui_safety_gate=true. + // render-hooks plan:pre returns activeHooks=[{kind:"gate"}] (gate is controlled + // separately by ui_safety_gate, default true). There are NO step hooks. + // §3a.5 must skip silently to 3b — no warning, no gate run, no skill dispatch. + // The warning must ONLY be reachable after an active step hook actually fired. + const section = getSection3a5(content); + + // The gate-only/ui_phase=false case must be documented as a silent skip + assert.ok( + section.includes('ui_phase') || section.includes('ui_safety_gate') || section.includes('gate-only'), + '§3a.5 must document the gate-only / ui_phase=false silent-skip case' + ); + + // Must NOT invoke check ui-plan-gate before filtering for step hooks + // i.e. the step-hook filter must appear before the GATE command in prose order + const stepCheckPos = section.indexOf('kind == "step"'); + const gateRunPos = section.indexOf('check ui-plan-gate'); + assert.ok( + stepCheckPos !== -1 && gateRunPos !== -1, + '§3a.5 must contain both kind=="step" check and check ui-plan-gate' + ); + assert.ok( + stepCheckPos < gateRunPos, + '§3a.5 must check for active step hooks BEFORE running check ui-plan-gate — ' + + 'gate-only case (no step hooks) must skip before the gate command is reached' + ); + + // Confirm gate entries are explicitly excluded from dispatch + assert.ok( + section.includes('gate') && (section.includes('ignored') || section.includes('silently')), + '§3a.5 must explicitly note kind=="gate" entries are silently ignored' + ); + }); + + test('§3a.5 appears before plan step (3b)', () => { const uiPhasePos = content.indexOf('3a.5'); const planPos = content.indexOf('**3b. Plan**'); assert.ok( diff --git a/tests/bug-3706-ui-safety-gate-false-positives.test.cjs b/tests/bug-3706-ui-safety-gate-false-positives.test.cjs index 627b8c54e..dcf0f2d8b 100644 --- a/tests/bug-3706-ui-safety-gate-false-positives.test.cjs +++ b/tests/bug-3706-ui-safety-gate-false-positives.test.cjs @@ -89,42 +89,32 @@ describe('Workflow .md structural guard (#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', () => { + // autonomous.md §3a.5 (post-#1031 cutover): §3a.5 now delegates to + // `loop render-hooks plan:pre` + `check ui-plan-gate` (capability-driven pattern, + // matching plan-phase.md §5.6). The direct shell invocation of ui-safety-gate.cjs + // was intentionally removed — cross-shell portability is provided by the CLI layer. + test('autonomous.md must invoke check ui-plan-gate (capability-driven UI gate — #1031)', () => { 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. + // §3a.5 must delegate to the capability-driven gate, not inline shell code 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)` + content.includes('check ui-plan-gate'), + `${label}: §3a.5 must delegate to \`check ui-plan-gate\` (capability-driven, #1031)` + ); + // §3a.5 must dispatch render-hooks plan:pre + assert.ok( + content.includes('loop render-hooks plan:pre'), + `${label}: §3a.5 must dispatch \`loop render-hooks plan:pre\` (capability hook resolution)` + ); + // Must NOT reintroduce the old shell-based path-search loop for ui-safety-gate.cjs + assert.ok( + !content.includes('ui-safety-gate.cjs'), + `${label}: must NOT inline ui-safety-gate.cjs invocation — replaced by check ui-plan-gate (#1031)` ); 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` + !content.includes('UI_GATE_JS=$(for _c in'), + `${label}: must NOT contain the old shell-based ui-safety-gate.cjs path-search (old §3a.5 pattern)` ); assert.ok( !content.includes('LC_ALL=C grep'),