golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

feat(#1023): first phase-6 cutover — ui-review (verify:post) inline → loop.render-hooks dispatch (#1024)

Browse Source 
Replace the inlined ui-review invocation in autonomous.md §3d.5 with a
loop.render-hooks verify:post dispatch — the first workflow to consume
render-hooks and fire a skill from it (closes the #1018 live-execution residual
as real wiring). Capability-driven, equivalence-preserving for the current
registry (only ui-review at verify:post, default on): fires gsd-ui-review under
the same precondition (UI-SPEC exists via consumes-gate + workflow.ui_review).

Gate findings (real pattern issues, fixed so every future cutover inherits them):
- bug-2643 static "Skill() references a real skill" check vs templated
  Skill(skill="gsd-${ref.skill}") dispatch → skip ${...}-templated names.
- Coverage moved, not lost: gen-capability-registry now validates
  steps[].ref.skill in skills + ref.agent in agents + rejects gsd- double-prefix.
- Tightened §3d.5 tests; markdown clarity (consumes rule, LLM-native JSON read,
  UI-REVIEW.md score hint).

gsd-ui-review skill + §3a.5/ui-phase untouched. §5.6/ui-phase cutover deferred
(#1022 step-can-halt-vs-gate model question).

Closes #1023

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-10 22:40:58 -04:00
committed by GitHub
parent 7eea991884
commit 2ac6592096
5 changed files with 300 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

79
tests/autonomous-ui-steps.test.cjs
View File

@@ -83,33 +83,86 @@ describe('autonomous workflow ui-phase and ui-review integration (#1375)', () =>
);
});
test('UI review step checks for UI-SPEC existence before running', () => {
// The UI review should only run if a UI-SPEC was created/exists
test('UI review step dispatches loop render-hooks verify:post', () => {
// Phase 6 cutover: §3d.5 now dispatches render-hooks verify:post instead of
// inlining a direct skill="gsd-ui-review" call.
const reviewSection = content.slice(content.indexOf('3d.5'));
assert.ok(
reviewSection.includes('loop render-hooks verify:post'),
'UI review step should dispatch loop render-hooks verify:post to resolve active hooks'
);
});
test('UI review step constructs skill via gsd- prefix dispatch and is non-blocking', () => {
// Phase 6 cutover: §3d.5 dispatches loop render-hooks verify:post, invokes skills via
// gsd-${ref.skill} prefix, gates on UI_SPEC_FILE for consumes:UI-SPEC.md hooks,
// and is explicitly advisory/non-blocking.
// All four properties must be present within the §3d.5 section itself.
const sectionStart = content.indexOf('3d.5');
// Bound to the closing </step> tag of the execute_phase step
const sectionEnd = content.indexOf('</step>', sectionStart);
assert.ok(sectionStart !== -1, '§3d.5 heading must be present in autonomous.md');
assert.ok(sectionEnd !== -1, '</step> must follow §3d.5');
const reviewSection = content.slice(sectionStart, sectionEnd);
assert.ok(
reviewSection.includes('loop render-hooks verify:post'),
'§3d.5 must dispatch `loop render-hooks verify:post` to resolve active capability hooks'
);
assert.ok(
reviewSection.includes('gsd-${ref.skill}'),
'§3d.5 must construct skill name via `gsd-${ref.skill}` prefix (capability-driven dispatch)'
);
assert.ok(
reviewSection.includes('UI_SPEC_FILE'),
'§3d.5 must gate on UI_SPEC_FILE for hooks that consume UI-SPEC.md'
);
assert.ok(
reviewSection.includes('advisory') || reviewSection.includes('non-blocking') || reviewSection.includes('regardless of result'),
'§3d.5 must be explicitly advisory/non-blocking'
);
});
test('UI review step gates on UI-SPEC file via consumes check', () => {
// The consumes:[UI-SPEC.md] gate is still enforced; UI_SPEC_FILE is still defined
// and used as the precondition for hooks that consume UI-SPEC.md.
const reviewSection = content.slice(content.indexOf('3d.5'));
assert.ok(
reviewSection.includes('UI_SPEC_FILE'),
'UI review step should check for UI-SPEC file existence'
'UI review step should still gate on UI_SPEC_FILE for hooks that consume UI-SPEC.md'
);
assert.ok(
reviewSection.includes('consumes'),
'UI review step should reference hook consumes array for the UI-SPEC gate'
);
});
test('UI review step respects workflow.ui_review config toggle', () => {
assert.ok(
content.includes('workflow.ui_review'),
'should respect workflow.ui_review config toggle'
);
});
test('UI review step respects workflow.ui_review config toggle (resolved via render-hooks)', () => {
// Phase 6 cutover: workflow.ui_review is no longer inlined as `config-get workflow.ui_review`.
// Instead, §3d.5 calls `loop render-hooks verify:post` which internally honours the
// `when: workflow.ui_review` field declared in the capability registry.
// The §3d.5 section must use render-hooks (not a literal `config-get workflow.ui_review`)
// so the toggle is resolved by the capability system, not duplicated inline.
const sectionStart = content.indexOf('3d.5');
const sectionEnd = content.indexOf('</step>', sectionStart);
assert.ok(sectionStart !== -1, '§3d.5 heading must be present in autonomous.md');
assert.ok(sectionEnd !== -1, '</step> must follow §3d.5');
const reviewSection = content.slice(sectionStart, sectionEnd);
test('UI review step invokes gsd:ui-review skill', () => {
assert.ok(
content.includes('skill="gsd-ui-review"'),
'should invoke gsd-ui-review via Skill()'
reviewSection.includes('render-hooks'),
'§3d.5 must resolve the workflow.ui_review toggle via render-hooks (not inline config-get)'
);
assert.ok(
!reviewSection.includes('config-get workflow.ui_review'),
'§3d.5 must NOT inline `config-get workflow.ui_review` — the toggle is owned by the capability registry'
);
});
test('UI review is advisory (non-blocking)', () => {
const reviewSection = content.slice(content.indexOf('3d.5'));
assert.ok(
reviewSection.includes('advisory') || reviewSection.includes('non-blocking') || reviewSection.includes('regardless of score'),
reviewSection.includes('advisory') || reviewSection.includes('non-blocking') || reviewSection.includes('regardless of result'),
'UI review should be advisory and not block phase progression'
);
});

24
tests/bug-2643-skill-frontmatter-name.test.cjs
View File

@@ -155,13 +155,27 @@ describe('skill frontmatter name parity (#2643 / #2808)', () => {
test('every workflow Skill(skill="gsd-<cmd>") resolves to an emitted skill name', () => {
const workflowFiles = collectFiles(WORKFLOWS_DIR);
const referenced = new Set();
const templatedSkipped = [];
for (const f of workflowFiles) {
const src = fs.readFileSync(f, 'utf-8');
for (const n of extractSkillNamesHyphen(src)) referenced.add(n);
for (const n of extractSkillNamesHyphen(src)) {
// Skip template expressions (e.g. `gsd-${ref.skill}`): these are
// capability-dispatched — the skill stem is resolved at runtime from
// the `loop render-hooks` registry output (ADR-857 phase 6), so there
// is no single literal skill file to validate against here.
// The capability registry's own validateStep gate (gen-capability-registry.cjs)
// is responsible for ensuring each `steps[].ref.skill` corresponds to a
// real skill declared in the capability's `skills` array.
if (n.includes('${')) {
templatedSkipped.push(path.basename(f) + ': ' + n);
} else {
referenced.add(n);
}
}
}
assert.ok(
referenced.size > 0,
`expected at least one Skill(skill="gsd-<cmd>") reference in workflows under ${WORKFLOWS_DIR}`
`expected at least one literal Skill(skill="gsd-<cmd>") reference in workflows under ${WORKFLOWS_DIR}`
);
const emitted = new Set();
@@ -182,5 +196,11 @@ describe('skill frontmatter name parity (#2643 / #2808)', () => {
[],
'workflow refs not emitted as skill names: ' + missing.join(', '),
);
// Informational: report how many templated dispatches were intentionally skipped.
// (Templated names are validated by the capability registry, not statically here.)
if (templatedSkipped.length > 0) {
// Not a failure — just a note for test output transparency.
// Use a diagnostic comment: node:test does not have a skip-within-test API.
}
});
});

163
tests/capability-registry.test.cjs
View File

@@ -727,6 +727,169 @@ describe('Fix #4: step.ref must be exclusive skill XOR agent', () => {
});
});
describe('double-prefix guard: step.ref.skill must not start with "gsd-"', () => {
// ref.skill is an unprefixed stem (e.g. "ui-review"). Workflow dispatch prepends
// "gsd-" at runtime. A stem already starting with "gsd-" would produce "gsd-gsd-..."
// at dispatch time, silently invoking a non-existent skill.
test('ref.skill starting with "gsd-" is rejected', () => {
const cap = {
...UI_CAP,
steps: [
{
point: 'verify:post',
ref: { skill: 'gsd-ui-review' }, // wrong: stem must NOT have gsd- prefix
produces: ['UI-REVIEW.md'],
consumes: ['UI-SPEC.md'],
when: 'workflow.ui_review',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
assert.ok(errors.length > 0, 'Expected an error for gsd-prefixed ref.skill');
assert.ok(
errors.some((e) => e.includes('gsd-') && (e.includes('double') || e.includes('unprefixed') || e.includes('must not start'))),
'Error should mention the double-prefix problem, got: ' + JSON.stringify(errors),
);
});
test('ref.skill without "gsd-" prefix is accepted (stem only)', () => {
const cap = {
...UI_CAP,
steps: [
{
point: 'verify:post',
ref: { skill: 'ui-review' }, // correct: unprefixed stem
produces: ['UI-REVIEW.md'],
consumes: ['UI-SPEC.md'],
when: 'workflow.ui_review',
onError: 'skip',
},
],
};
const prefixErrors = validateCapability(cap, 'ui').filter((e) => e.includes('gsd-') && e.includes('stem'));
assert.deepEqual(prefixErrors, [], 'No prefix errors expected for unprefixed stem, got: ' + JSON.stringify(prefixErrors));
});
test('real UI capability.json uses unprefixed ref.skill values', () => {
// Verify the live capability uses unprefixed stems and therefore passes the new guard.
const errors = validateCapability(UI_CAP, 'ui');
const prefixErrors = errors.filter((e) => e.includes('must not start with'));
assert.deepEqual(prefixErrors, [], 'Live UI capability.json should not trigger the double-prefix guard: ' + JSON.stringify(prefixErrors));
});
});
// ─── Fix: ref.skill/ref.agent membership in declared skills/agents ────────────
describe('ref membership check: step.ref.skill must be in cap.skills', () => {
// A capability declares skills: ["ui-phase", "ui-review"].
// A step with ref.skill "typo-skill" (not in skills) must be rejected.
test('step.ref.skill NOT in cap.skills is rejected', () => {
const cap = {
...UI_CAP,
steps: [
{
point: 'plan:pre',
ref: { skill: 'typo-skill' }, // not in skills: ["ui-phase", "ui-review"]
produces: ['UI-SPEC.md'],
consumes: ['CONTEXT.md'],
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
assert.ok(errors.length > 0, 'Expected errors for undeclared ref.skill');
assert.ok(
errors.some((e) => e.includes('typo-skill') && e.includes('not declared')),
'Error should mention "typo-skill" and "not declared", got: ' + JSON.stringify(errors),
);
});
test('step.ref.skill IN cap.skills is accepted', () => {
const cap = {
...UI_CAP,
steps: [
{
point: 'plan:pre',
ref: { skill: 'ui-phase' }, // declared in skills: ["ui-phase", "ui-review"]
produces: ['UI-SPEC.md'],
consumes: ['CONTEXT.md'],
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
const membershipErrors = errors.filter((e) => e.includes('not declared') && e.includes('ui-phase'));
assert.deepEqual(
membershipErrors, [],
'No membership errors expected for declared ref.skill, got: ' + JSON.stringify(membershipErrors),
);
});
test('real UI capability passes: ui-phase and ui-review are both in skills', () => {
// Regression guard: the real UI capability must not trigger the new membership check.
const errors = validateCapability(UI_CAP, 'ui');
const membershipErrors = errors.filter((e) => e.includes('not declared'));
assert.deepEqual(
membershipErrors, [],
'Real UI capability should pass membership check for all ref.skill values, got: ' + JSON.stringify(membershipErrors),
);
});
});
describe('ref membership check: step.ref.agent must be in cap.agents', () => {
// A capability declares agents: ["gsd-ui-checker", "gsd-ui-auditor"].
// A step with ref.agent "gsd-unknown-agent" (not in agents) must be rejected.
test('step.ref.agent NOT in cap.agents is rejected', () => {
const cap = {
...UI_CAP,
steps: [
{
point: 'plan:pre',
ref: { agent: 'gsd-unknown-agent' }, // not in agents: ["gsd-ui-checker", "gsd-ui-auditor"]
produces: ['UI-SPEC.md'],
consumes: ['CONTEXT.md'],
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
assert.ok(errors.length > 0, 'Expected errors for undeclared ref.agent');
assert.ok(
errors.some((e) => e.includes('gsd-unknown-agent') && e.includes('not declared')),
'Error should mention "gsd-unknown-agent" and "not declared", got: ' + JSON.stringify(errors),
);
});
test('step.ref.agent IN cap.agents is accepted', () => {
const cap = {
...UI_CAP,
steps: [
{
point: 'plan:pre',
ref: { agent: 'gsd-ui-checker' }, // declared in agents
produces: ['UI-SPEC.md'],
consumes: ['CONTEXT.md'],
when: 'workflow.ui_phase',
onError: 'skip',
},
],
};
const errors = validateCapability(cap, 'ui');
const membershipErrors = errors.filter((e) => e.includes('not declared') && e.includes('gsd-ui-checker'));
assert.deepEqual(
membershipErrors, [],
'No membership errors expected for declared ref.agent, got: ' + JSON.stringify(membershipErrors),
);
});
});
describe('Fix: 3-node requires cycle (A→B→C→A) is detected', () => {
test('three-node requires cycle is reported as an error', () => {
const capA = {