From 1b880fefd773d21111fd839e99b883977b124942 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 12 Jun 2026 21:24:09 -0400 Subject: [PATCH] feat(#1137): migrate review verification hooks to capabilities (#1147) * feat(#1137): migrate review verification hooks to capabilities * chore(#1147): add changeset --- .changeset/noble-orcas-tumble.md | 5 + capabilities/code-review/capability.json | 36 ++ capabilities/nyquist/capability.json | 30 ++ capabilities/security/capability.json | 67 ++++ docs/README.md | 1 + docs/how-to/develop-a-capability.md | 5 +- .../review-verification-capabilities.md | 68 ++++ gsd-core/bin/lib/capability-registry.cjs | 349 +++++++++++++++++- gsd-core/workflows/audit-milestone.md | 8 +- gsd-core/workflows/autonomous.md | 21 +- gsd-core/workflows/code-review-fix.md | 16 +- gsd-core/workflows/code-review.md | 14 +- gsd-core/workflows/execute-phase.md | 22 +- gsd-core/workflows/plan-phase.md | 10 +- gsd-core/workflows/quick.md | 9 +- gsd-core/workflows/secure-phase.md | 6 +- gsd-core/workflows/validate-phase.md | 6 +- gsd-core/workflows/verify-work.md | 31 +- tests/code-review.test.cjs | 31 +- tests/loop-hook-firing-spike.test.cjs | 2 +- tests/phase6-review-capabilities.test.cjs | 205 ++++++++++ ...policy-138-nyquist-config-default.test.cjs | 67 ++-- tests/secure-phase.test.cjs | 14 +- tests/verify-work-auto-transition.test.cjs | 17 +- tests/workflow-size-baseline.json | 20 +- 25 files changed, 951 insertions(+), 109 deletions(-) create mode 100644 .changeset/noble-orcas-tumble.md create mode 100644 capabilities/code-review/capability.json create mode 100644 capabilities/nyquist/capability.json create mode 100644 capabilities/security/capability.json create mode 100644 docs/reference/review-verification-capabilities.md create mode 100644 tests/phase6-review-capabilities.test.cjs diff --git a/.changeset/noble-orcas-tumble.md b/.changeset/noble-orcas-tumble.md new file mode 100644 index 000000000..6c968fa3d --- /dev/null +++ b/.changeset/noble-orcas-tumble.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 1147 +--- +Migrate code review, security, and Nyquist verification workflows to ADR-857 capability hooks. diff --git a/capabilities/code-review/capability.json b/capabilities/code-review/capability.json new file mode 100644 index 000000000..1404aae06 --- /dev/null +++ b/capabilities/code-review/capability.json @@ -0,0 +1,36 @@ +{ + "id": "code-review", + "role": "feature", + "title": "Code review", + "description": "Source-file code review and review-fix workflow support for completed execution work.", + "tier": "full", + "requires": [], + "skills": ["code-review"], + "agents": ["gsd-code-reviewer", "gsd-code-fixer"], + "hooks": [], + "config": { + "workflow.code_review": { + "type": "boolean", + "default": true, + "description": "Enable code-review participation in post-execution review flows." + }, + "workflow.code_review_depth": { + "type": "enum", + "values": ["quick", "standard", "deep"], + "default": "standard", + "description": "Default depth for code review when no --depth override is supplied." + } + }, + "steps": [ + { + "point": "execute:post", + "ref": { "skill": "code-review" }, + "produces": ["REVIEW.md"], + "consumes": ["SUMMARY.md"], + "when": "workflow.code_review", + "onError": "skip" + } + ], + "contributions": [], + "gates": [] +} diff --git a/capabilities/nyquist/capability.json b/capabilities/nyquist/capability.json new file mode 100644 index 000000000..aea60cf2d --- /dev/null +++ b/capabilities/nyquist/capability.json @@ -0,0 +1,30 @@ +{ + "id": "nyquist", + "role": "feature", + "title": "Nyquist validation", + "description": "Validation coverage audit that maps executed work back to tests and manual-only evidence.", + "tier": "full", + "requires": [], + "skills": ["validate-phase"], + "agents": ["gsd-nyquist-auditor"], + "hooks": [], + "config": { + "workflow.nyquist_validation": { + "type": "boolean", + "default": true, + "description": "Enable Nyquist validation coverage auditing." + } + }, + "steps": [ + { + "point": "verify:post", + "ref": { "skill": "validate-phase" }, + "produces": ["VALIDATION.md"], + "consumes": ["SUMMARY.md"], + "when": "workflow.nyquist_validation", + "onError": "halt" + } + ], + "contributions": [], + "gates": [] +} diff --git a/capabilities/security/capability.json b/capabilities/security/capability.json new file mode 100644 index 000000000..25cb2af6c --- /dev/null +++ b/capabilities/security/capability.json @@ -0,0 +1,67 @@ +{ + "id": "security", + "role": "feature", + "title": "Security enforcement", + "description": "Threat mitigation verification and ship-time security blocking for phases with security enforcement enabled.", + "tier": "full", + "requires": [], + "skills": ["secure-phase"], + "agents": ["gsd-security-auditor"], + "hooks": [], + "config": { + "workflow.security_enforcement": { + "type": "boolean", + "default": true, + "description": "Enable security threat-mitigation verification before phase advancement." + }, + "workflow.security_asvs_level": { + "type": "number", + "default": 1, + "description": "OWASP ASVS level used by security review guidance." + }, + "workflow.security_block_on": { + "type": "enum", + "values": ["critical", "high", "medium", "low", "none"], + "default": "high", + "description": "Minimum open threat severity that blocks advancement." + } + }, + "steps": [ + { + "point": "verify:post", + "ref": { "skill": "secure-phase" }, + "produces": ["SECURITY.md"], + "consumes": ["SUMMARY.md"], + "when": "workflow.security_enforcement", + "onError": "halt" + } + ], + "contributions": [ + { + "point": "plan:pre", + "into": "planner", + "fragment": { + "inline": "Each PLAN.md must include a block when security enforcement is active. Use the configured ASVS level and blocking threshold from workflow.security_asvs_level and workflow.security_block_on." + }, + "produces": [], + "consumes": ["CONTEXT.md"], + "when": "workflow.security_enforcement" + } + ], + "gates": [ + { + "point": "ship:pre", + "check": { + "predicate": { + "kind": "artifact-frontmatter-equals", + "artifact": "SECURITY.md", + "field": "threats_open", + "equals": 0 + } + }, + "when": "workflow.security_enforcement", + "blocking": true, + "onError": "halt" + } + ] +} diff --git a/docs/README.md b/docs/README.md index 29f0d27ae..cdc6106ea 100644 --- a/docs/README.md +++ b/docs/README.md @@ -52,6 +52,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) - [CONTEXT.md schema](reference/context-md.md) — field-by-field reference for `.planning/phases//CONTEXT.md` - [PLAN.md schema](reference/plan-md.md) — field-by-field reference for `.planning/phases//PLAN.md` - [Planning artifacts](reference/planning-artifacts.md) — all `.planning/` files and their roles +- [Review and verification capabilities](reference/review-verification-capabilities.md) — code review, security, and Nyquist capability ownership and hook contracts --- diff --git a/docs/how-to/develop-a-capability.md b/docs/how-to/develop-a-capability.md index 3dc349478..a9bc88f81 100644 --- a/docs/how-to/develop-a-capability.md +++ b/docs/how-to/develop-a-capability.md @@ -134,12 +134,15 @@ The rendered JSON should include the active hook, the declared `ref`, and the ma Host workflows should ask the resolver for active hooks and then dispatch from the resolved data. Do not add new direct `config-get workflow.` checks to a host workflow for a migrated feature. -The Phase 6 planning migration follows this pattern: +The Phase 6 migrations follow this pattern: - `research` registers a `plan:pre` step that invokes `gsd-phase-researcher`. - `ai-integration` owns the AI-SPEC planning activation and config key. - `pattern-mapper` registers a `plan:pre` step that invokes `gsd-pattern-mapper`. - `plan-phase.md` reads the resolved `PLAN_PRE_HOOKS_JSON` and dispatches `ref.agent` or `ref.skill` from that data. +- `code-review` registers an `execute:post` step and review workflows dispatch from the active hook's `ref.skill`. +- `security` registers a `plan:pre` contribution, a `verify:post` step, and a blocking `ship:pre` gate. +- `nyquist` registers a `verify:post` step for validation coverage auditing. This keeps "off means off" enforceable by construction: disabled Capabilities are absent from the active hook set, so the host workflow has nothing feature-specific to run. diff --git a/docs/reference/review-verification-capabilities.md b/docs/reference/review-verification-capabilities.md new file mode 100644 index 000000000..a9b2d5530 --- /dev/null +++ b/docs/reference/review-verification-capabilities.md @@ -0,0 +1,68 @@ +# Review and Verification Capabilities + +This reference describes the Capability Registry declarations used by GSD review and verification features in GSD 1.5 and later. It covers the first migrated review/verification capabilities from ADR-857 Phase 6: code review, security enforcement, and Nyquist validation. + +For the system design, see [ADR-857: Capability system](../adr/857-capability-system.md). + +## Terms + +**Capability**: A feature bundle declared in `capabilities//capability.json`. A capability owns skills, agents, config keys, and loop hooks. + +**Loop Extension Point**: A named point in the Discuss -> Plan -> Execute -> Verify -> Ship loop where capabilities can register hooks. + +**Step hook**: A hook that runs a capability skill as an ordered step. + +**Contribution hook**: A hook that injects guidance into a core workflow prompt. + +**Gate hook**: A hook that evaluates a predicate and can block when `blocking` is true. + +## Capability Ownership + +| Capability | Skills | Agents | Config keys | +|---|---|---|---| +| `code-review` | `code-review` | `gsd-code-reviewer`, `gsd-code-fixer` | `workflow.code_review`, `workflow.code_review_depth` | +| `security` | `secure-phase` | `gsd-security-auditor` | `workflow.security_enforcement`, `workflow.security_asvs_level`, `workflow.security_block_on` | +| `nyquist` | `validate-phase` | `gsd-nyquist-auditor` | `workflow.nyquist_validation` | + +The central config schema still accepts these keys during migration, but workflows must not branch directly on the boolean activation keys. Workflows resolve activation by calling `gsd-tools loop render-hooks `. + +## Hook Map + +| Point | Capability | Kind | Behavior | +|---|---|---|---| +| `plan:pre` | `security` | `contribution` | Adds threat-model guidance to planner context when security enforcement is active. | +| `execute:post` | `code-review` | `step` | Runs post-execution code review when code review is active. | +| `verify:post` | `security` | `step` | Runs security verification and produces `SECURITY.md` when security enforcement is active. | +| `verify:post` | `nyquist` | `step` | Runs validation coverage audit and produces `VALIDATION.md` when Nyquist validation is active. | +| `ship:pre` | `security` | `gate` | Blocks shipping unless `SECURITY.md` reports `threats_open: 0`. | + +## Activation Rules + +Workflows use the active hook list from the registry: + +```bash +EXECUTE_POST_HOOKS_JSON=$(gsd_run loop render-hooks execute:post --raw) +VERIFY_POST_HOOKS_JSON=$(gsd_run loop render-hooks verify:post --raw) +``` + +The resolver evaluates each hook's `when` key against the project config and the capability config default. If the key is absent and the capability declaration default is `true`, the hook is active. + +Direct command workflows self-gate the same way: + +- `/gsd:code-review` resolves the active `execute:post` hook whose `ref.skill == "code-review"`. +- `/gsd:secure-phase` resolves the active `verify:post` hook whose `ref.skill == "secure-phase"`. +- `/gsd:validate-phase` resolves the active `verify:post` hook whose `ref.skill == "validate-phase"`. + +## Authoring Notes + +When adding a review or verification capability: + +1. Declare owned skills, agents, config keys, and hooks in `capabilities//capability.json`. +2. Use unprefixed skill stems in `ref.skill`; workflows add the `gsd-` prefix when invoking skills. +3. Declare `produces` and `consumes` arrays for every step and contribution hook. +4. Use `onError: "halt"` for verification work that must stop advancement when it fails. +5. Use a blocking `gate` for ship-time predicates that must prevent release. +6. Regenerate `gsd-core/bin/lib/capability-registry.cjs` with `npm run gen:capability-registry`. +7. Add tests that prove disabled hooks are absent from `loop render-hooks` output and that workflows do not branch directly on the capability's activation config key. + +Do not add third-party code loading through a capability declaration. ADR-857 reserves that trust boundary for a separate design. diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index 76fb23681..396679ee4 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -298,6 +298,57 @@ const capabilities = { "extendedHookEvents": [] } }, + "code-review": { + "id": "code-review", + "role": "feature", + "title": "Code review", + "description": "Source-file code review and review-fix workflow support for completed execution work.", + "tier": "full", + "requires": [], + "skills": [ + "code-review" + ], + "agents": [ + "gsd-code-reviewer", + "gsd-code-fixer" + ], + "hooks": [], + "config": { + "workflow.code_review": { + "type": "boolean", + "default": true, + "description": "Enable code-review participation in post-execution review flows." + }, + "workflow.code_review_depth": { + "type": "enum", + "values": [ + "quick", + "standard", + "deep" + ], + "default": "standard", + "description": "Default depth for code review when no --depth override is supplied." + } + }, + "steps": [ + { + "point": "execute:post", + "ref": { + "skill": "code-review" + }, + "produces": [ + "REVIEW.md" + ], + "consumes": [ + "SUMMARY.md" + ], + "when": "workflow.code_review", + "onError": "skip" + } + ], + "contributions": [], + "gates": [] + }, "codebuddy": { "id": "codebuddy", "role": "runtime", @@ -809,6 +860,46 @@ const capabilities = { "extendedHookEvents": [] } }, + "nyquist": { + "id": "nyquist", + "role": "feature", + "title": "Nyquist validation", + "description": "Validation coverage audit that maps executed work back to tests and manual-only evidence.", + "tier": "full", + "requires": [], + "skills": [ + "validate-phase" + ], + "agents": [ + "gsd-nyquist-auditor" + ], + "hooks": [], + "config": { + "workflow.nyquist_validation": { + "type": "boolean", + "default": true, + "description": "Enable Nyquist validation coverage auditing." + } + }, + "steps": [ + { + "point": "verify:post", + "ref": { + "skill": "validate-phase" + }, + "produces": [ + "VALIDATION.md" + ], + "consumes": [ + "SUMMARY.md" + ], + "when": "workflow.nyquist_validation", + "onError": "halt" + } + ], + "contributions": [], + "gates": [] + }, "opencode": { "id": "opencode", "role": "runtime", @@ -1014,6 +1105,91 @@ const capabilities = { "contributions": [], "gates": [] }, + "security": { + "id": "security", + "role": "feature", + "title": "Security enforcement", + "description": "Threat mitigation verification and ship-time security blocking for phases with security enforcement enabled.", + "tier": "full", + "requires": [], + "skills": [ + "secure-phase" + ], + "agents": [ + "gsd-security-auditor" + ], + "hooks": [], + "config": { + "workflow.security_enforcement": { + "type": "boolean", + "default": true, + "description": "Enable security threat-mitigation verification before phase advancement." + }, + "workflow.security_asvs_level": { + "type": "number", + "default": 1, + "description": "OWASP ASVS level used by security review guidance." + }, + "workflow.security_block_on": { + "type": "enum", + "values": [ + "critical", + "high", + "medium", + "low", + "none" + ], + "default": "high", + "description": "Minimum open threat severity that blocks advancement." + } + }, + "steps": [ + { + "point": "verify:post", + "ref": { + "skill": "secure-phase" + }, + "produces": [ + "SECURITY.md" + ], + "consumes": [ + "SUMMARY.md" + ], + "when": "workflow.security_enforcement", + "onError": "halt" + } + ], + "contributions": [ + { + "point": "plan:pre", + "into": "planner", + "fragment": { + "inline": "Each PLAN.md must include a block when security enforcement is active. Use the configured ASVS level and blocking threshold from workflow.security_asvs_level and workflow.security_block_on." + }, + "produces": [], + "consumes": [ + "CONTEXT.md" + ], + "when": "workflow.security_enforcement" + } + ], + "gates": [ + { + "point": "ship:pre", + "check": { + "predicate": { + "kind": "artifact-frontmatter-equals", + "artifact": "SECURITY.md", + "field": "threats_open", + "equals": 0 + } + }, + "when": "workflow.security_enforcement", + "blocking": true, + "onError": "halt" + } + ] + }, "trae": { "id": "trae", "role": "runtime", @@ -1200,7 +1376,10 @@ const capabilities = { const bySkill = { "ai-integration-phase": "ai-integration", + "code-review": "code-review", "graphify": "graphify", + "validate-phase": "nyquist", + "secure-phase": "security", "ui-phase": "ui", "ui-review": "ui" }; @@ -1210,8 +1389,12 @@ const byAgent = { "gsd-ai-researcher": "ai-integration", "gsd-domain-researcher": "ai-integration", "gsd-eval-planner": "ai-integration", + "gsd-code-reviewer": "code-review", + "gsd-code-fixer": "code-review", + "gsd-nyquist-auditor": "nyquist", "gsd-pattern-mapper": "pattern-mapper", "gsd-phase-researcher": "research", + "gsd-security-auditor": "security", "gsd-ui-checker": "ui", "gsd-ui-auditor": "ui" }; @@ -1298,7 +1481,21 @@ const byLoopPoint = { "onError": "skip" } ], - "contributions": [], + "contributions": [ + { + "capId": "security", + "point": "plan:pre", + "into": "planner", + "fragment": { + "inline": "Each PLAN.md must include a block when security enforcement is active. Use the configured ASVS level and blocking threshold from workflow.security_asvs_level and workflow.security_block_on." + }, + "produces": [], + "consumes": [ + "CONTEXT.md" + ], + "when": "workflow.security_enforcement" + } + ], "gates": [ { "capId": "ui", @@ -1344,7 +1541,23 @@ const byLoopPoint = { ] }, "execute:post": { - "steps": [], + "steps": [ + { + "capId": "code-review", + "point": "execute:post", + "ref": { + "skill": "code-review" + }, + "produces": [ + "REVIEW.md" + ], + "consumes": [ + "SUMMARY.md" + ], + "when": "workflow.code_review", + "onError": "skip" + } + ], "contributions": [], "gates": [] }, @@ -1355,6 +1568,36 @@ const byLoopPoint = { }, "verify:post": { "steps": [ + { + "capId": "nyquist", + "point": "verify:post", + "ref": { + "skill": "validate-phase" + }, + "produces": [ + "VALIDATION.md" + ], + "consumes": [ + "SUMMARY.md" + ], + "when": "workflow.nyquist_validation", + "onError": "halt" + }, + { + "capId": "security", + "point": "verify:post", + "ref": { + "skill": "secure-phase" + }, + "produces": [ + "SECURITY.md" + ], + "consumes": [ + "SUMMARY.md" + ], + "when": "workflow.security_enforcement", + "onError": "halt" + }, { "capId": "ui", "point": "verify:post", @@ -1377,7 +1620,23 @@ const byLoopPoint = { "ship:pre": { "steps": [], "contributions": [], - "gates": [] + "gates": [ + { + "capId": "security", + "point": "ship:pre", + "check": { + "predicate": { + "kind": "artifact-frontmatter-equals", + "artifact": "SECURITY.md", + "field": "threats_open", + "equals": 0 + } + }, + "when": "workflow.security_enforcement", + "blocking": true, + "onError": "halt" + } + ] }, "ship:post": { "steps": [], @@ -1388,10 +1647,16 @@ const byLoopPoint = { const configKeys = { "workflow.ai_integration_phase": "ai-integration", + "workflow.code_review": "code-review", + "workflow.code_review_depth": "code-review", "graphify.enabled": "graphify", "intel.enabled": "intel", + "workflow.nyquist_validation": "nyquist", "workflow.pattern_mapper": "pattern-mapper", "workflow.research": "research", + "workflow.security_enforcement": "security", + "workflow.security_asvs_level": "security", + "workflow.security_block_on": "security", "workflow.ui_phase": "ui", "workflow.ui_review": "ui", "workflow.ui_safety_gate": "ui" @@ -1404,6 +1669,23 @@ const configSchema = { "default": true, "description": "Prompt for an AI-SPEC design contract before planning phases that involve AI systems." }, + "workflow.code_review": { + "owner": "code-review", + "type": "boolean", + "default": true, + "description": "Enable code-review participation in post-execution review flows." + }, + "workflow.code_review_depth": { + "owner": "code-review", + "type": "enum", + "default": "standard", + "description": "Default depth for code review when no --depth override is supplied.", + "values": [ + "quick", + "standard", + "deep" + ] + }, "graphify.enabled": { "owner": "graphify", "type": "boolean", @@ -1416,6 +1698,12 @@ const configSchema = { "default": false, "description": "Enable the intel code-intelligence command." }, + "workflow.nyquist_validation": { + "owner": "nyquist", + "type": "boolean", + "default": true, + "description": "Enable Nyquist validation coverage auditing." + }, "workflow.pattern_mapper": { "owner": "pattern-mapper", "type": "boolean", @@ -1428,6 +1716,31 @@ const configSchema = { "default": true, "description": "Run phase research before planning when research artifacts are missing or explicitly refreshed." }, + "workflow.security_enforcement": { + "owner": "security", + "type": "boolean", + "default": true, + "description": "Enable security threat-mitigation verification before phase advancement." + }, + "workflow.security_asvs_level": { + "owner": "security", + "type": "number", + "default": 1, + "description": "OWASP ASVS level used by security review guidance." + }, + "workflow.security_block_on": { + "owner": "security", + "type": "enum", + "default": "high", + "description": "Minimum open threat severity that blocks advancement.", + "values": [ + "critical", + "high", + "medium", + "low", + "none" + ] + }, "workflow.ui_phase": { "owner": "ui", "type": "boolean", @@ -2368,9 +2681,18 @@ const capabilityClusters = { "ai-integration": [ "ai-integration-phase" ], + "code-review": [ + "code-review" + ], "graphify": [ "graphify" ], + "nyquist": [ + "validate-phase" + ], + "security": [ + "secure-phase" + ], "ui": [ "ui-phase", "ui-review" @@ -2384,12 +2706,30 @@ const profileMembership = { "full" ] }, + "code-review": { + "tier": "full", + "profiles": [ + "full" + ] + }, "graphify": { "tier": "full", "profiles": [ "full" ] }, + "nyquist": { + "tier": "full", + "profiles": [ + "full" + ] + }, + "security": { + "tier": "full", + "profiles": [ + "full" + ] + }, "ui": { "tier": "full", "profiles": [ @@ -2405,6 +2745,7 @@ const _requiresGraph = { "augment": [], "claude": [], "cline": [], + "code-review": [], "codebuddy": [], "codex": [], "copilot": [], @@ -2415,12 +2756,14 @@ const _requiresGraph = { "intel": [], "kilo": [], "kimi": [], + "nyquist": [], "opencode": [], "pattern-mapper": [ "research" ], "qwen": [], "research": [], + "security": [], "trae": [], "ui": [], "windsurf": [] diff --git a/gsd-core/workflows/audit-milestone.md b/gsd-core/workflows/audit-milestone.md index f36405b94..b22e5ceb5 100644 --- a/gsd-core/workflows/audit-milestone.md +++ b/gsd-core/workflows/audit-milestone.md @@ -143,13 +143,15 @@ For each REQ-ID, determine status using all three sources: ## 5.5. Nyquist Compliance Discovery -Skip if `workflow.nyquist_validation` is explicitly `false` (absent = enabled). +Skip if the Nyquist capability is inactive. ```bash -NYQUIST_CONFIG=$(gsd_run query config-get workflow.nyquist_validation --raw --default true 2>/dev/null) +VERIFY_POST_HOOKS_JSON=$(gsd_run loop render-hooks verify:post --raw) ``` -If `false`: skip entirely. +Resolve active step hooks from `VERIFY_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "validate-phase"`. + +If no active validate-phase step hook exists: skip entirely. For each phase directory, check `*-VALIDATION.md`. If exists, parse frontmatter (`nyquist_compliant`, `wave_0_complete`). diff --git a/gsd-core/workflows/autonomous.md b/gsd-core/workflows/autonomous.md index 5d303426f..56cc9079e 100644 --- a/gsd-core/workflows/autonomous.md +++ b/gsd-core/workflows/autonomous.md @@ -453,17 +453,22 @@ Skill(skill="gsd-execute-phase", args="${PHASE_NUM} --no-transition") Auto-invoke code review and fix chain. Autonomous mode chains both review and fix (unlike execute-phase/quick which only suggest fix). -**Config gate:** +**Capability dispatch:** ```bash -CODE_REVIEW_ENABLED=$(gsd_run query config-get workflow.code_review 2>/dev/null || echo "true") -``` -If `"false"`: display "Code review skipped (workflow.code_review=false)" and proceed to 3d. - -``` -Skill(skill="gsd-code-review", args="${PHASE_NUM}") +EXECUTE_POST_HOOKS_JSON=$(gsd_run loop render-hooks execute:post --raw) ``` -Parse status from REVIEW.md frontmatter. If "clean" or "skipped": proceed to 3d. If findings found: auto-invoke: +Resolve active step hooks from `EXECUTE_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "code-review"`. + +If no active code-review step hook exists: display "Code review skipped (code-review capability inactive)" and proceed to 3d. This covers `workflow.code_review=false` through the Capability Registry; do not query the code-review toggle directly here. + +For each active code-review step hook, dispatch the skill using the registry-provided stem: + +``` +Skill(skill="gsd-${ref.skill}", args="${PHASE_NUM}") +``` + +Parse status from REVIEW.md frontmatter. If "clean" or "skipped": proceed to 3d. If findings found after the capability-dispatched review, auto-invoke the consolidated fix entry point: ``` Skill(skill="gsd-code-review", args="${PHASE_NUM} --fix --auto") ``` diff --git a/gsd-core/workflows/code-review-fix.md b/gsd-core/workflows/code-review-fix.md index 925f3cc6a..4adcabaf9 100644 --- a/gsd-core/workflows/code-review-fix.md +++ b/gsd-core/workflows/code-review-fix.md @@ -74,21 +74,23 @@ FIX_REPORT_PATH="${PHASE_DIR}/${PADDED_PHASE}-REVIEW-FIX.md" -Check if code review is enabled via config: +Check if code review is active via the capability registry: ```bash -CODE_REVIEW_ENABLED=$(gsd_run query config-get workflow.code_review 2>/dev/null || echo "true") +EXECUTE_POST_HOOKS_JSON=$(gsd_run loop render-hooks execute:post --raw) ``` -If CODE_REVIEW_ENABLED is "false": +Resolve active step hooks from `EXECUTE_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "code-review"`. + +If no active code-review step hook exists: ``` -Code review fix skipped (workflow.code_review=false in config) +Code review fix skipped (code-review capability inactive) ``` Exit workflow. -Default is true — only skip on explicit false. This check runs AFTER phase validation so invalid phase errors are shown first. +Default is active through the Capability Registry schema — only skip when the registry resolves no active code-review step hook. This check runs AFTER phase validation so invalid phase errors are shown first. -Note: This reuses the `workflow.code_review` config key rather than introducing a separate `workflow.code_review_fix` key. Rationale: fixes are meaningless without review, so a single toggle makes sense. If independent control is needed later, a separate key can be added in v2. +Note: This reuses the code-review capability activation rather than introducing a separate code-review-fix capability. Rationale: fixes are meaningless without review, so a single activation boundary makes sense. If independent control is needed later, a separate key can be added in v2. @@ -491,7 +493,7 @@ echo "════════════════════════ - [ ] Phase validated before config gate check -- [ ] Config gate checked (workflow.code_review) +- [ ] Capability gate checked (execute:post code-review hook) - [ ] REVIEW.md existence verified (error if missing) - [ ] REVIEW.md status checked (skip if clean/skipped) - [ ] Agent spawned with correct config (review_path, fix_scope, fix_report_path) diff --git a/gsd-core/workflows/code-review.md b/gsd-core/workflows/code-review.md index 3e083aaf2..1f241e281 100644 --- a/gsd-core/workflows/code-review.md +++ b/gsd-core/workflows/code-review.md @@ -73,19 +73,21 @@ fi -Check if code review is enabled via config: +Check if code review is active via the capability registry: ```bash -CODE_REVIEW_ENABLED=$(gsd_run query config-get workflow.code_review 2>/dev/null || echo "true") +EXECUTE_POST_HOOKS_JSON=$(gsd_run loop render-hooks execute:post --raw) ``` -If CODE_REVIEW_ENABLED is "false": +Resolve active step hooks from `EXECUTE_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "code-review"`. + +If no active code-review step hook exists: ``` -Code review skipped (workflow.code_review=false in config) +Code review skipped (code-review capability inactive) ``` Exit workflow. -Default is true — only skip on explicit false. This check runs AFTER phase validation so invalid phase errors are shown first. +Default is active through the Capability Registry schema — only skip when the registry resolves no active code-review step hook. This check runs AFTER phase validation so invalid phase errors are shown first. @@ -678,7 +680,7 @@ If `--files` validation fails unexpectedly on macOS, install coreutils or use ab - [ ] Phase validated before config gate check -- [ ] Config gate checked (workflow.code_review) +- [ ] Capability gate checked (execute:post code-review hook) - [ ] --fix/--all/--auto flags parsed via code-review-flags.cjs typed IR (not ad-hoc bash) - [ ] Depth resolved with validation (quick|standard|deep) - [ ] File scope computed with 3 tiers: --files > SUMMARY.md > git diff diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index 470466b0e..ad5233f46 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -1061,20 +1061,22 @@ After all waves: **Security gate check:** ```bash -SECURITY_CFG=$(gsd_run query config-get workflow.security_enforcement --raw 2>/dev/null || echo "true") +VERIFY_POST_HOOKS_JSON=$(gsd_run loop render-hooks verify:post --raw) SECURITY_FILE=$(ls "${PHASE_DIR}"/*-SECURITY.md 2>/dev/null | head -1) ``` -If `SECURITY_CFG` is `false`: skip. +Resolve active step hooks from `VERIFY_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "secure-phase"`. -If `SECURITY_CFG` is `true` AND `SECURITY_FILE` is empty (no SECURITY.md yet): +If no active secure-phase step hook exists: skip. + +If an active secure-phase step hook exists AND `SECURITY_FILE` is empty (no SECURITY.md yet): Include in the next-steps routing output: ``` ⚠ Security enforcement enabled — run before advancing: /gsd:secure-phase {PHASE} ${GSD_WS} ``` -If `SECURITY_CFG` is `true` AND SECURITY.md exists: check frontmatter `threats_open`. If > 0: +If an active secure-phase step hook exists AND SECURITY.md exists: check frontmatter `threats_open`. If > 0: ``` ⚠ Security gate: {threats_open} threats open /gsd:secure-phase {PHASE} — resolve before advancing @@ -1166,18 +1168,20 @@ Selected wave finished successfully. This phase still has incomplete plans, so p -**This step is REQUIRED and must not be skipped.** Auto-invoke code review on the phase's source changes. Advisory only — never blocks execution flow. +**This step is REQUIRED to evaluate the capability hook.** When the code-review capability is active, auto-invoke code review on the phase's source changes. Advisory only — never blocks execution flow. -**Config gate:** +**Capability gate:** ```bash -CODE_REVIEW_ENABLED=$(gsd_run query config-get workflow.code_review 2>/dev/null || echo "true") +EXECUTE_POST_HOOKS_JSON=$(gsd_run loop render-hooks execute:post --raw) ``` -If `CODE_REVIEW_ENABLED` is `"false"`: display "Code review skipped (workflow.code_review=false)" and proceed to next step. +Resolve active step hooks from `EXECUTE_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "code-review"`. + +If no active code-review step hook exists: display "Code review skipped (code-review capability inactive)" and proceed to next step. **Invoke review:** ``` -Skill(skill="gsd-code-review", args="${PHASE_NUMBER}") +Skill(skill="gsd-${ref.skill}", args="${PHASE_NUMBER}") ``` **Check results using deterministic path (not glob):** diff --git a/gsd-core/workflows/plan-phase.md b/gsd-core/workflows/plan-phase.md index a8bdf0b5e..3b679f808 100644 --- a/gsd-core/workflows/plan-phase.md +++ b/gsd-core/workflows/plan-phase.md @@ -577,17 +577,19 @@ test -f "${PHASE_DIR}/${PADDED_PHASE}-VALIDATION.md" && echo "VALIDATION_CREATED ## 5.55. Security Threat Model Gate -> Skip if `workflow.security_enforcement` is explicitly `false`. Absent = enabled. +> Capability-driven dispatch. Resolves active `plan:pre` hooks via the capability registry; the security hook's `when` condition is evaluated by the registry. ```bash -SECURITY_CFG=$(gsd_run query config-get workflow.security_enforcement --raw 2>/dev/null || echo "true") +PLAN_PRE_HOOKS_JSON=$(gsd_run loop render-hooks plan:pre --raw) SECURITY_ASVS=$(gsd_run query config-get workflow.security_asvs_level --raw 2>/dev/null || echo "1") SECURITY_BLOCK=$(gsd_run query config-get workflow.security_block_on --raw 2>/dev/null || echo "high") ``` -**If `SECURITY_CFG` is `false`:** Skip to step 5.6. +Resolve active contribution hooks from `PLAN_PRE_HOOKS_JSON` where `kind == "contribution"` and `capId == "security"`. -**If `SECURITY_CFG` is `true`:** Display banner: +**If no active security contribution hook exists:** Skip to step 5.6. + +**If an active security contribution hook exists:** Display banner: ``` ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ diff --git a/gsd-core/workflows/quick.md b/gsd-core/workflows/quick.md index eef7d2cfa..b11ec9423 100644 --- a/gsd-core/workflows/quick.md +++ b/gsd-core/workflows/quick.md @@ -776,11 +776,14 @@ Note: For quick tasks producing multiple plans (rare), spawn executors in parall Skip this step entirely if `$FULL_MODE` is false. -**Config gate:** +**Capability gate:** ```bash -CODE_REVIEW_ENABLED=$(gsd_run query config-get workflow.code_review 2>/dev/null || echo "true") +EXECUTE_POST_HOOKS_JSON=$(gsd_run loop render-hooks execute:post --raw) ``` -If `"false"`, skip with message "Code review skipped (workflow.code_review=false)". + +Resolve active step hooks from `EXECUTE_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "code-review"`. + +If no active code-review step hook exists, skip with message "Code review skipped (code-review capability inactive)". **Scope files from executor's commits:** ```bash diff --git a/gsd-core/workflows/secure-phase.md b/gsd-core/workflows/secure-phase.md index eb5f90643..90a6d20cf 100644 --- a/gsd-core/workflows/secure-phase.md +++ b/gsd-core/workflows/secure-phase.md @@ -26,10 +26,12 @@ Parse: `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`. ```bash AUDITOR_MODEL=$(gsd_run query resolve-model gsd-security-auditor --raw) -SECURITY_CFG=$(gsd_run query config-get workflow.security_enforcement --raw 2>/dev/null || echo "true") +VERIFY_POST_HOOKS_JSON=$(gsd_run loop render-hooks verify:post --raw) ``` -If `SECURITY_CFG` is `false`: exit with "Security enforcement disabled. Enable via /gsd:settings." +Resolve active step hooks from `VERIFY_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "secure-phase"`. + +If no active secure-phase step hook exists: exit with "Security enforcement disabled. Enable via /gsd:settings." Display banner: `GSD > SECURE PHASE {N}: {name}` diff --git a/gsd-core/workflows/validate-phase.md b/gsd-core/workflows/validate-phase.md index dfe00dd03..1504a4f70 100644 --- a/gsd-core/workflows/validate-phase.md +++ b/gsd-core/workflows/validate-phase.md @@ -26,10 +26,12 @@ Parse: `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`. ```bash AUDITOR_MODEL=$(gsd_run query resolve-model gsd-nyquist-auditor --raw) -NYQUIST_CFG=$(gsd_run query config-get workflow.nyquist_validation --raw --default true) +VERIFY_POST_HOOKS_JSON=$(gsd_run loop render-hooks verify:post --raw) ``` -If `NYQUIST_CFG` is `false`: exit with "Nyquist validation is disabled. Enable via /gsd:settings." +Resolve active step hooks from `VERIFY_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "validate-phase"`. + +If no active validate-phase step hook exists: exit with "Nyquist validation is disabled. Enable via /gsd:settings." Display banner: `GSD > VALIDATE PHASE {N}: {name}` diff --git a/gsd-core/workflows/verify-work.md b/gsd-core/workflows/verify-work.md index dc59a19e6..307093842 100644 --- a/gsd-core/workflows/verify-work.md +++ b/gsd-core/workflows/verify-work.md @@ -457,16 +457,31 @@ Present summary: **If issues == 0:** ```bash -SECURITY_CFG=$(gsd_run query config-get workflow.security_enforcement --raw 2>/dev/null || echo "true") +VERIFY_POST_HOOKS_JSON=$(gsd_run loop render-hooks verify:post --raw) SECURITY_FILE=$(ls "${PHASE_DIR}"/*-SECURITY.md 2>/dev/null | head -1) ``` -If `SECURITY_CFG` is `true` AND `SECURITY_FILE` is empty: -``` -⚠ Security enforcement enabled — /gsd:secure-phase {phase} has not run. -Run before advancing to the next phase. +Resolve active step hooks from `VERIFY_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "secure-phase"`. -All tests passed. Ready to continue. +If an active secure-phase step hook exists AND `SECURITY_FILE` is empty, dispatch the registry-provided skill stem: + +``` +Skill(skill="gsd-${ref.skill}", args="{phase}") +``` + +After the skill returns, refresh `SECURITY_FILE`: + +```bash +SECURITY_FILE=$(ls "${PHASE_DIR}"/*-SECURITY.md 2>/dev/null | head -1) +``` + +If `SECURITY_FILE` is still empty, stop before phase advancement and present: + +``` +⚠ Security enforcement enabled — /gsd:secure-phase {phase} did not produce SECURITY.md. +Resolve the security review failure before advancing to the next phase. + +All tests passed, but phase advancement is blocked until security review produces SECURITY.md. - `/gsd:secure-phase {phase}` — security review (required before advancing) - `/gsd:plan-phase {next}` — Plan next phase @@ -474,13 +489,13 @@ All tests passed. Ready to continue. - `/gsd:ui-review {phase}` — visual quality audit (if frontend files were modified) ``` -If `SECURITY_CFG` is `true` AND `SECURITY_FILE` exists: check frontmatter `threats_open`. If > 0: +If an active secure-phase step hook exists AND `SECURITY_FILE` exists: check frontmatter `threats_open`. If > 0: ``` ⚠ Security gate: {threats_open} threats open /gsd:secure-phase {phase} — resolve before advancing ``` -If `SECURITY_CFG` is `false` OR (`SECURITY_FILE` exists AND `threats_open` is `0`): +If no active secure-phase step hook exists OR (`SECURITY_FILE` exists AND `threats_open` is `0`): **Auto-transition: mark phase complete in ROADMAP.md and STATE.md** diff --git a/tests/code-review.test.cjs b/tests/code-review.test.cjs index 2dfe7780a..4cfa05637 100644 --- a/tests/code-review.test.cjs +++ b/tests/code-review.test.cjs @@ -462,11 +462,18 @@ describe('CR-INTEGRATION: workflow integration points', () => { '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'); + test('execute-phase.md resolves code-review capability hook', () => { + const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'execute-phase.md'), 'utf-8'); + const gateMatch = content.match(/]*>([\s\S]*?)<\/step>/); + assert.ok(gateMatch, 'execute-phase.md missing code_review_gate step'); + const gateContent = gateMatch[1]; - assert.match(content, /config-get\s+workflow\.code_review/, - 'execute-phase.md missing config-get workflow.code_review call'); + assert.ok(gateContent.includes('loop render-hooks execute:post'), + 'execute-phase.md code_review_gate must resolve execute:post capability hooks'); + assert.ok(gateContent.includes('ref.skill == "code-review"'), + 'execute-phase.md code_review_gate must identify the code-review capability hook'); + assert.ok(!gateContent.match(/config-get\s+workflow\.code_review/), + 'execute-phase.md code_review_gate must not read workflow.code_review directly'); }); test('execute-phase.md does NOT contain ls.*REVIEW.md.*head pattern', { skip: !PLUGIN_AVAILABLE ? 'Plugin dir not installed' : false }, () => { @@ -488,11 +495,19 @@ describe('CR-INTEGRATION: workflow integration points', () => { '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'); + test('quick.md resolves code-review capability hook', () => { + const content = fs.readFileSync(path.join(WORKFLOWS_DIR, 'quick.md'), 'utf-8'); + const start = content.indexOf('**Step 6.25: Code review (auto)**'); + const end = content.indexOf('**Step 6.5: Verification', start); + assert.ok(start !== -1 && end !== -1, 'quick.md missing Step 6.25 code review section'); + const reviewContent = content.slice(start, end); - assert.match(content, /config-get\s+workflow\.code_review/, - 'quick.md missing config-get workflow.code_review call'); + assert.ok(reviewContent.includes('loop render-hooks execute:post'), + 'quick.md code review step must resolve execute:post capability hooks'); + assert.ok(reviewContent.includes('ref.skill == "code-review"'), + 'quick.md code review step must identify the code-review capability hook'); + assert.ok(!reviewContent.match(/config-get\s+workflow\.code_review/), + 'quick.md code review step must not read workflow.code_review directly'); }); // autonomous.md tests read from the repo's canonical workflow source (WORKFLOWS_DIR), diff --git a/tests/loop-hook-firing-spike.test.cjs b/tests/loop-hook-firing-spike.test.cjs index 59a40a900..723e1f482 100644 --- a/tests/loop-hook-firing-spike.test.cjs +++ b/tests/loop-hook-firing-spike.test.cjs @@ -252,7 +252,7 @@ describe('spike #1018 — off means off (structural proof)', () => { "onError must be 'skip' on the ui-review step at verify:post"); // Also verify the rendered output contains the structured heading for this point const rendered = renderLoopHooks(resolved); - assert.match(rendered, /### Step 1: skill:ui-review \(ui\)/, + assert.match(rendered, /### Step \d+: skill:ui-review \(ui\)/, 'rendered must include the properly-structured Step block heading for ui-review at verify:post'); }); diff --git a/tests/phase6-review-capabilities.test.cjs b/tests/phase6-review-capabilities.test.cjs new file mode 100644 index 000000000..8021b63fa --- /dev/null +++ b/tests/phase6-review-capabilities.test.cjs @@ -0,0 +1,205 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.resolve(__dirname, '..'); +const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); + +function workflow(name) { + return fs.readFileSync(path.join(ROOT, 'gsd-core', 'workflows', name), 'utf8'); +} + +function doc(name) { + return fs.readFileSync(path.join(ROOT, 'docs', name), 'utf8'); +} + +function sectionBetween(content, startNeedle, endNeedle) { + const start = content.indexOf(startNeedle); + assert.ok(start !== -1, `${startNeedle} section must exist`); + const end = endNeedle ? content.indexOf(endNeedle, start) : -1; + return content.slice(start, end === -1 ? undefined : end); +} + +function hookAt(point, kind, capId) { + return registry.byLoopPoint[point][kind].find((hook) => hook.capId === capId); +} + +describe('ADR-857 phase 6 verification and review capability migration', () => { + test('registry declares code-review, security, and nyquist feature capabilities', () => { + for (const capId of ['code-review', 'security', 'nyquist']) { + assert.ok(registry.capabilities[capId], `${capId} capability must exist`); + assert.equal(registry.capabilities[capId].role, 'feature'); + } + }); + + test('code-review capability owns review skills, agents, config, and execute hook', () => { + assert.equal(registry.bySkill['code-review'], 'code-review'); + assert.equal(registry.byAgent['gsd-code-reviewer'], 'code-review'); + assert.equal(registry.byAgent['gsd-code-fixer'], 'code-review'); + assert.equal(registry.configKeys['workflow.code_review'], 'code-review'); + assert.equal(registry.configKeys['workflow.code_review_depth'], 'code-review'); + + const hook = hookAt('execute:post', 'steps', 'code-review'); + assert.ok(hook, 'code-review must register an execute:post step'); + assert.deepEqual(hook.ref, { skill: 'code-review' }); + assert.equal(hook.when, 'workflow.code_review'); + assert.deepEqual(hook.produces, ['REVIEW.md']); + assert.ok(hook.consumes.includes('SUMMARY.md')); + }); + + test('security capability owns secure-phase wiring and blocking ship gate', () => { + assert.equal(registry.bySkill['secure-phase'], 'security'); + assert.equal(registry.byAgent['gsd-security-auditor'], 'security'); + assert.equal(registry.configKeys['workflow.security_enforcement'], 'security'); + assert.equal(registry.configKeys['workflow.security_asvs_level'], 'security'); + assert.equal(registry.configKeys['workflow.security_block_on'], 'security'); + + const step = hookAt('verify:post', 'steps', 'security'); + assert.ok(step, 'security must register a verify:post step'); + assert.deepEqual(step.ref, { skill: 'secure-phase' }); + assert.equal(step.when, 'workflow.security_enforcement'); + assert.equal(step.onError, 'halt'); + + const gate = hookAt('ship:pre', 'gates', 'security'); + assert.ok(gate, 'security must register a blocking ship:pre gate'); + assert.equal(gate.blocking, true); + assert.equal(gate.onError, 'halt'); + + const planContribution = hookAt('plan:pre', 'contributions', 'security'); + assert.ok(planContribution, 'security must register a plan:pre contribution for threat-model guidance'); + assert.equal(planContribution.into, 'planner'); + assert.equal(planContribution.when, 'workflow.security_enforcement'); + }); + + test('nyquist capability owns validate-phase wiring and config', () => { + assert.equal(registry.bySkill['validate-phase'], 'nyquist'); + assert.equal(registry.byAgent['gsd-nyquist-auditor'], 'nyquist'); + assert.equal(registry.configKeys['workflow.nyquist_validation'], 'nyquist'); + + const hook = hookAt('verify:post', 'steps', 'nyquist'); + assert.ok(hook, 'nyquist must register a verify:post step'); + assert.deepEqual(hook.ref, { skill: 'validate-phase' }); + assert.equal(hook.when, 'workflow.nyquist_validation'); + assert.deepEqual(hook.produces, ['VALIDATION.md']); + assert.ok(hook.consumes.includes('SUMMARY.md')); + }); + + test('autonomous code review resolves execute:post hooks instead of inlining code_review config', () => { + const content = workflow('autonomous.md'); + const section = sectionBetween( + content, + '**3c.5. Code Review and Fix**', + '**3d. Post-Execution Routing**', + ); + + assert.ok(section.includes('loop render-hooks execute:post')); + assert.ok(section.includes('gsd-${ref.skill}')); + assert.ok(!section.includes('config-get workflow.code_review')); + }); + + test('execute-phase code-review gate resolves execute:post hooks instead of inlining code_review config', () => { + const content = workflow('execute-phase.md'); + const section = sectionBetween(content, ''); + + assert.ok(section.includes('loop render-hooks execute:post')); + assert.ok(section.includes('gsd-${ref.skill}')); + assert.ok(!section.includes('config-get workflow.code_review')); + }); + + test('quick full-mode code review resolves execute:post hooks instead of inlining code_review config', () => { + const content = workflow('quick.md'); + const section = sectionBetween(content, '**Step 6.25: Code review (auto)**', '**Step 6.5: Verification'); + + assert.ok(section.includes('loop render-hooks execute:post')); + assert.ok(section.includes('gsd-code-reviewer')); + assert.ok(!section.includes('config-get workflow.code_review')); + }); + + test('code-review command self-gates through execute:post hooks', () => { + const content = workflow('code-review.md'); + const section = sectionBetween(content, '', ''); + + assert.ok(section.includes('loop render-hooks execute:post')); + assert.ok(section.includes('ref.skill == "code-review"')); + assert.ok(!section.includes('config-get workflow.code_review')); + }); + + test('code-review-fix command self-gates through execute:post hooks', () => { + const content = workflow('code-review-fix.md'); + const section = sectionBetween(content, '', ''); + + assert.ok(section.includes('loop render-hooks execute:post')); + assert.ok(section.includes('ref.skill == "code-review"')); + assert.ok(!section.includes('config-get workflow.code_review')); + }); + + test('verify-work resolves verify:post security hooks instead of inlining security_enforcement config', () => { + const content = workflow('verify-work.md'); + const transitionStart = content.indexOf('If issues == 0:'); + assert.ok(transitionStart !== -1, 'verify-work transition block must exist'); + const section = content.slice(transitionStart, content.indexOf('', transitionStart)); + + assert.ok(section.includes('loop render-hooks verify:post')); + assert.ok(section.includes('gsd-${ref.skill}')); + assert.ok(!section.includes('config-get workflow.security_enforcement')); + }); + + test('plan-phase threat model gate resolves plan:pre security capability hooks', () => { + const content = workflow('plan-phase.md'); + const section = sectionBetween(content, '## 5.55. Security Threat Model Gate', '## 5.6. UI Design Contract Gate'); + + assert.ok(section.includes('loop render-hooks plan:pre')); + assert.ok(section.includes('capId == "security"')); + assert.ok(!section.includes('config-get workflow.security_enforcement')); + }); + + test('secure-phase command self-gates through verify:post hooks', () => { + const content = workflow('secure-phase.md'); + const initFence = ['```bash', 'AUDITOR_MODEL='].join('\n'); + const section = sectionBetween(content, initFence, 'Display banner:'); + + assert.ok(section.includes('loop render-hooks verify:post')); + assert.ok(section.includes('ref.skill == "secure-phase"')); + assert.ok(!section.includes('config-get workflow.security_enforcement')); + }); + + test('validate-phase command self-gates through verify:post hooks', () => { + const content = workflow('validate-phase.md'); + const initFence = ['```bash', 'AUDITOR_MODEL='].join('\n'); + const section = sectionBetween(content, initFence, 'Display banner:'); + + assert.ok(section.includes('loop render-hooks verify:post')); + assert.ok(section.includes('ref.skill == "validate-phase"')); + assert.ok(!section.includes('config-get workflow.nyquist_validation')); + }); + + test('documentation covers migrated review and verification capability hooks', () => { + const content = doc('reference/review-verification-capabilities.md'); + const manual = doc('how-to/develop-a-capability.md'); + const index = doc('README.md'); + + for (const term of [ + '`code-review`', + '`security`', + '`nyquist`', + '`execute:post`', + '`verify:post`', + '`ship:pre`', + 'gsd-tools loop render-hooks ', + ]) { + assert.ok(content.includes(term), `capability reference must document ${term}`); + } + + assert.ok( + index.includes('reference/review-verification-capabilities.md'), + 'docs index must link the review and verification capabilities reference', + ); + assert.ok( + manual.includes('`security` registers a `plan:pre` contribution, a `verify:post` step, and a blocking `ship:pre` gate.'), + 'capability developer manual must mention review/verification hook examples', + ); + }); +}); diff --git a/tests/policy-138-nyquist-config-default.test.cjs b/tests/policy-138-nyquist-config-default.test.cjs index 74976aa73..827cd8f7e 100644 --- a/tests/policy-138-nyquist-config-default.test.cjs +++ b/tests/policy-138-nyquist-config-default.test.cjs @@ -1,21 +1,38 @@ 'use strict'; // Policy regression test for issue #138: -// config-get workflow.nyquist_validation calls in validate-phase.md and -// audit-milestone.md MUST include --default so they don't error / emit -// stderr noise when the key is absent. -// -// Form-agnostic: matches on `config-get workflow.nyquist_validation` + `--default` -// regardless of the runtime prefix ($GSD_SDK vs gsd_run), so this test -// survives PR #379's prefix rename. +// Nyquist activation must be absent-safe. ADR-857 phase 6 moved that defaulting +// into `loop render-hooks verify:post`, so workflows must consume the capability +// hook instead of calling config-get directly. const { test } = 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 WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); +function readWorkflow(name) { + return fs.readFileSync(path.join(WORKFLOWS_DIR, name), 'utf8'); +} + +function assertNyquistCapabilityGate(name) { + const content = readWorkflow(name); + assert.ok( + content.includes('loop render-hooks verify:post'), + `${name} must resolve Nyquist activation through verify:post capability hooks` + ); + assert.ok( + content.includes('ref.skill == "validate-phase"'), + `${name} must identify the validate-phase capability hook` + ); + assert.ok( + !content.includes('config-get workflow.nyquist_validation'), + `${name} must not read workflow.nyquist_validation directly after capability cutover` + ); +} + function findNyquistConfigLine(filePath) { const content = fs.readFileSync(filePath, 'utf8'); const lines = content.split('\n'); @@ -27,22 +44,26 @@ function findNyquistConfigLine(filePath) { return null; } -test('validate-phase.md: config-get workflow.nyquist_validation must include --default', () => { - const filePath = path.join(WORKFLOWS_DIR, 'validate-phase.md'); - const result = findNyquistConfigLine(filePath); - assert.ok(result, 'Should find a line containing config-get workflow.nyquist_validation in validate-phase.md'); - assert.ok( - result.line.includes('--default'), - `Line ${result.lineNumber} of validate-phase.md is missing --default flag.\nFound: ${result.line.trim()}` - ); +test('validate-phase.md: Nyquist activation uses verify:post capability hook', () => { + assertNyquistCapabilityGate('validate-phase.md'); }); -test('audit-milestone.md: config-get workflow.nyquist_validation must include --default', () => { - const filePath = path.join(WORKFLOWS_DIR, 'audit-milestone.md'); - const result = findNyquistConfigLine(filePath); - assert.ok(result, 'Should find a line containing config-get workflow.nyquist_validation in audit-milestone.md'); - assert.ok( - result.line.includes('--default'), - `Line ${result.lineNumber} of audit-milestone.md is missing --default flag.\nFound: ${result.line.trim()}` - ); +test('audit-milestone.md: Nyquist activation uses verify:post capability hook', () => { + assertNyquistCapabilityGate('audit-milestone.md'); +}); + +test('legacy Nyquist config helper still detects unsafe direct reads', () => { + const tmp = path.join(os.tmpdir(), `policy-138-synthetic-${process.pid}.md`); + try { + fs.writeFileSync(tmp, 'NYQUIST_CFG=$(gsd_run query config-get workflow.nyquist_validation --raw)\n'); + const result = findNyquistConfigLine(tmp); + assert.ok(result, 'synthetic direct config read should be detected'); + assert.ok(!result.line.includes('--default'), 'synthetic unsafe read intentionally lacks --default'); + } finally { + try { + fs.unlinkSync(tmp); + } catch { + // Best-effort cleanup for synthetic file. + } + } }); diff --git a/tests/secure-phase.test.cjs b/tests/secure-phase.test.cjs index ccb4f171e..2038f092b 100644 --- a/tests/secure-phase.test.cjs +++ b/tests/secure-phase.test.cjs @@ -178,11 +178,19 @@ describe('SECURE: secure-phase workflow file', () => { ); }); - test('contains security_enforcement config check', () => { + test('contains security capability hook check', () => { const content = fs.readFileSync(wfPath, 'utf-8'); assert.ok( - content.includes('security_enforcement'), - 'must check security_enforcement config setting' + content.includes('loop render-hooks verify:post'), + 'must resolve security activation through verify:post capability hooks' + ); + assert.ok( + content.includes('ref.skill == "secure-phase"'), + 'must identify the secure-phase capability hook' + ); + assert.ok( + !content.includes('config-get workflow.security_enforcement'), + 'must not read workflow.security_enforcement directly after capability cutover' ); }); diff --git a/tests/verify-work-auto-transition.test.cjs b/tests/verify-work-auto-transition.test.cjs index e42728b35..9083b5026 100644 --- a/tests/verify-work-auto-transition.test.cjs +++ b/tests/verify-work-auto-transition.test.cjs @@ -48,27 +48,28 @@ describe('verify-work.md — auto-transition after UAT passes with 0 issues', () test('security gate check gates the transition (no auto-transition when security pending)', () => { const content = fs.readFileSync(VERIFY_WORK, 'utf-8'); - // The security check must appear before the transition reference - const securityCfgIdx = content.indexOf('SECURITY_CFG'); + // The capability-resolved security check must appear before the transition reference. + const securityHookIdx = content.indexOf('loop render-hooks verify:post'); const transitionIdx = content.indexOf('transition.md'); assert.ok( - securityCfgIdx !== -1, - 'verify-work.md must check SECURITY_CFG before transitioning' + securityHookIdx !== -1, + 'verify-work.md must resolve verify:post capability hooks before transitioning' ); assert.ok( - securityCfgIdx < transitionIdx, - 'SECURITY_CFG check must appear before transition.md reference' + securityHookIdx < transitionIdx, + 'verify:post capability hook check must appear before transition.md reference' ); }); test('transition is only invoked when security gate is cleared or disabled', () => { const content = fs.readFileSync(VERIFY_WORK, 'utf-8'); // Transition must be guarded by security check: - // Either SECURITY_CFG is false, or security file exists with 0 open threats + // Either no active secure-phase hook exists, or security file exists with 0 open threats. const hasGuardedTransition = content.includes('transition.md') && ( - content.includes("SECURITY_CFG") && + content.includes('loop render-hooks verify:post') && + content.includes('ref.skill == "secure-phase"') && (content.includes('threats_open') || content.includes('SECURITY_FILE')) ); assert.ok( diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index 594aa9310..7f7eb2d11 100644 --- a/tests/workflow-size-baseline.json +++ b/tests/workflow-size-baseline.json @@ -6,13 +6,13 @@ "ai-integration-phase.md": 14761, "analyze-dependencies.md": 3887, "audit-fix.md": 10988, - "audit-milestone.md": 17556, + "audit-milestone.md": 17637, "audit-uat.md": 7425, - "autonomous.md": 41838, + "autonomous.md": 42263, "check-todos.md": 9431, "cleanup.md": 9897, - "code-review-fix.md": 23676, - "code-review.md": 31397, + "code-review-fix.md": 23890, + "code-review.md": 31602, "complete-milestone.md": 30228, "debug.md": 13505, "diagnose-issues.md": 12425, @@ -24,7 +24,7 @@ "docs-update.md": 54770, "edit-phase.md": 12883, "eval-review.md": 9923, - "execute-phase.md": 92885, + "execute-phase.md": 93166, "execute-plan.md": 29980, "explore.md": 10497, "extract-learnings.md": 12849, @@ -51,20 +51,20 @@ "note.md": 6563, "pause-work.md": 13654, "plan-milestone-gaps.md": 11765, - "plan-phase.md": 94326, + "plan-phase.md": 94519, "plan-review-convergence.md": 22949, "plant-seed.md": 11741, "pr-branch.md": 4994, "profile-user.md": 20650, "progress.md": 29387, - "quick.md": 46282, + "quick.md": 46412, "reapply-patches.md": 20393, "remove-phase.md": 8469, "remove-workspace.md": 7507, "resume-project.md": 15288, "review.md": 38031, "scan.md": 7688, - "secure-phase.md": 12187, + "secure-phase.md": 12282, "session-report.md": 4044, "settings-advanced.md": 39621, "settings-integrations.md": 15801, @@ -84,7 +84,7 @@ "ultraplan-phase.md": 10468, "undo.md": 10431, "update.md": 21053, - "validate-phase.md": 10630, + "validate-phase.md": 10745, "verify-phase.md": 28430, - "verify-work.md": 30614 + "verify-work.md": 31160 }