feat(#1031): autonomous §3a.5 plan:pre ui-phase cutover — last inlined ui-phase site (step-only) (#1033)
Cut over autonomous.md §3a.5 (autonomous plan:pre ui-phase step) to the loop.render-hooks plan:pre dispatch — completing the ui-phase migration begun in #1026 (plan-phase.md §5.6). Step-only, non-blocking: autonomous is always pipeline, so it fires active kind==step hooks and never runs the manual-only plan:pre blocking gate. Skip condition keys on "no active step hooks" (not empty activeHooks), so the gate-only {ui_phase:false, ui_safety_gate:true} case skips silently with no spurious warning — matching OLD §3a.5. Fires gsd-ui-phase under the identical precondition (frontend + no UI-SPEC + workflow.ui_phase active), bare ${PHASE_NUM} args. Replaces the inline ui-safety-gate.cjs probe + config-get with render-hooks + the ui.plan-gate check verb. Codex caught the gate-only spurious-warning divergence on the first pass; fixed + re-confirmed equivalence-preserving. gsd-ui-phase skill, §5.6, §3d.5 untouched. Closes #1031 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:
@@ -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**
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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'),
|
||||
|
||||
Reference in New Issue
Block a user