diff --git a/.changeset/steady-foxes-rest.md b/.changeset/steady-foxes-rest.md new file mode 100644 index 000000000..75b107720 --- /dev/null +++ b/.changeset/steady-foxes-rest.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3869 +--- +**Capabilities can now do work before UAT, not only block it** — the `verify:pre` extension point dispatched gate hooks only, so a capability declaring a step or contribution there was rejected at registry-build time and the whole verify lane was closed to anything that wanted to contribute to what UAT covers. It now dispatches contribution, step, and gate hooks, and `extract_tests` additively consumes the artefacts those steps declare via `produces`. (#3866) diff --git a/docs/how-to/develop-a-capability.md b/docs/how-to/develop-a-capability.md index 20ab4a3e7..9f863b580 100644 --- a/docs/how-to/develop-a-capability.md +++ b/docs/how-to/develop-a-capability.md @@ -119,6 +119,33 @@ Choose the hook kind that matches the behaviour: Declare file artefact flow with `produces` and `consumes`. The registry uses those arrays to order hooks and to reject unsatisfied dependencies. +### Check the point dispatches your kind + +A point having a call site does not mean it dispatches every kind. Each host workflow decides, per +point, which kinds its dispatch text handles — the wire-on-demand model — and the registry build +enforces the result: `gen-capability-registry.cjs` scans the host workflows with `getWiredKinds()` +and rejects a manifest that declares a kind the point does not dispatch. You find out at +`npm run gen:capability-registry`, not at runtime: + +``` +capability "demo" steps[0].point "ship:pre" registers a step hook, but the host call site's +dispatch text never covers `kind == "step"` (it covers: gate). A hand-rolled single-kind consumer +silently never dispatches the other kinds — dispatch every registered kind per +gsd-core/references/loop-hook-dispatch.md. +``` + +That error is the answer, not a bug to route around. Either pick a point that dispatches your kind, +express the behaviour as a kind the point does dispatch, or open an issue to wire the arm — that +last route is what [#3866](https://github.com/open-gsd/gsd-core/issues/3866) did for `verify:pre`, +which dispatched only `gate` and so let a capability refuse to let UAT start but never contribute +to what UAT covers. + +For which kinds a given point dispatches, and the `verify:pre` step / `produces` semantics, see +[Valid `point` values](../reference/capability-manifest.md#valid-point-values) — that table is the +single source of truth. One operational note it does not carry: a `verify:pre` `produces` name +whose artefact is absent at UAT time is reported and skipped, so a step that was inactive, skipped, +or failed never drops a deliverable and never blocks UAT. + ## Ship skills — and know what you are shipping A skill your Capability declares is not an inert asset. Its `SKILL.md` body is copied **verbatim** into the user's runtime skills directory at install, where it becomes an agent-invocable instruction file. GSD does **not** scan it: there is no content inspection of any kind, at install or at any later point. What you write is what reaches the agent. diff --git a/docs/reference/capability-manifest.md b/docs/reference/capability-manifest.md index c37ee0561..bc145e9e8 100644 --- a/docs/reference/capability-manifest.md +++ b/docs/reference/capability-manifest.md @@ -123,6 +123,10 @@ Gates check a condition at a loop extension point and optionally block progressi The 12 loop extension points are a **closed, additive-only vocabulary**. Every `steps`, `contributions`, and `gates` entry must use one of these identifiers exactly. +A valid point is necessary but not sufficient: each host workflow decides, per point, which hook **kinds** its dispatch text handles, and a point may dispatch a subset. The registry build derives the real answer from the host workflows (`getWiredKinds()` in `scripts/gen-loop-host-contract.cjs`) and rejects a manifest declaring a kind the point does not dispatch — so an unsupported combination is a build-time error naming the point, the kind, and the kinds that point does cover. It is never a hook that renders and is then silently dropped. + +`verify:pre` dispatches all three kinds. A step there is **advisory**: it runs before UAT begins and never blocks it — a precondition that must halt verification is a `gate`. Its `produces` artefact names are consumed **additively** by the verify workflow's `extract_tests` step, which can deepen what UAT covers but cannot suppress a checkpoint. See [Develop a capability](../how-to/develop-a-capability.md#check-the-point-dispatches-your-kind) for the authoring workflow. + | Point | Phase | Position | |---|---|---| | `discuss:pre` | Discuss | Before the discuss step executes | diff --git a/gsd-core/workflows/verify-work.md b/gsd-core/workflows/verify-work.md index 2327deb90..c51e04a33 100644 --- a/gsd-core/workflows/verify-work.md +++ b/gsd-core/workflows/verify-work.md @@ -61,15 +61,26 @@ MVP_MODE=$(gsd_run query phase.mvp-mode "${phase_number}" ${GSD_WS} --pick activ -**Verify:pre gate dispatch.** Before verification begins, dispatch every active -gate hook registered at the `verify:pre` loop extension point. Each gate is -data-driven — resolved from the capability registry, not hardcoded here. +**Verify:pre capability dispatch.** Before verification begins, dispatch every +active hook registered at the `verify:pre` loop extension point — of **every** +kind, not gates alone. Each hook is data-driven — resolved from the capability +registry, not hardcoded here. ```bash VERIFY_PRE_HOOKS_JSON=$(gsd_run loop render-hooks verify:pre --raw) PHASE_DIR=$(printf '%s' "$INIT" | jq -r '.phase_dir // empty') ``` +Read the `activeHooks` array from `VERIFY_PRE_HOOKS_JSON` in-context (do NOT pipe through a shell parser). + +**If `activeHooks` is empty or absent:** skip silently to `check_active_session`. + +**Contribution dispatch:** inject every `kind == "contribution"` fragment per @gsd-core/references/loop-hook-dispatch.md (skip when none), before the steps and gates below. + +**Step dispatch:** dispatch every `kind == "step"` hook per @gsd-core/references/loop-hook-dispatch.md (skip when none) — not one shape of one. A step here is advisory: it never blocks the start of UAT, and a step that errors is routed by its own `onError` without failing verification. ⚠ **Validate `ref.command` in-context before any shell use** (third-party manifest input) — loop-hook-dispatch.md § `step`. + +Record the union of `produces` artefact names declared by the active step entries as `VERIFY_PRE_PRODUCED` — `extract_tests` consumes it below. An entry declaring `produces: []` contributes nothing, which is the normal case. + ⚠ **Validate `check` before shell use** (third-party manifest input) — `loop-hook-dispatch.md` § `gate`. Resolve active gate hooks from `VERIFY_PRE_HOOKS_JSON` where `kind == "gate"`. @@ -187,6 +198,35 @@ Read the JSON result (`mode`, `total`, `all_auto_covered`, `auto_passed[]`, `pre The cold-start smoke test injection below still applies in `coverage` mode. +**Verify:pre produced-artefact seam (#3866).** If `VERIFY_PRE_PRODUCED` (recorded in +`verify_pre_hooks`) is empty or absent, skip this paragraph entirely — derivation is unchanged. +Otherwise, for each artefact name in it, locate the artefact the producing step wrote under +`$PHASE_DIR` and merge its checkpoints into the test list **additively**. + +**The artefact contract.** A consumable artefact is a Markdown file holding a list of checkpoint +entries in the **same shape `extract_tests` already emits and `create_uat_file` already consumes** — +each entry a `name` (brief test name) and an `expected` (specific, user-observable outcome). +Nothing else is read: extra fields are ignored, not an error. There is no new schema and no new +parser — a producing step writes what a checkpoint already looks like. An artefact that yields zero +parseable entries is treated exactly like an absent one (see below). + +Merge rules: + +- ⚠ **Validate the artefact name in-context before resolving it** (third-party manifest input). + An artefact name is a registry-declared name, **not** a path: check the value you read from + `produces` against `^[A-Za-z0-9][A-Za-z0-9._-]*$` yourself — **never** by pasting it into a + shell command to be tested there. A name carrying `/`, `..`, a leading `-`, a leading path + separator, or any shell metacharacter is a malformed manifest: record a warning, skip that + name, continue. Only a validated name is resolved, and only against what the step wrote inside + `$PHASE_DIR` — never above it, and never through a symlink that leaves it. +- A name with no artefact on disk means that step was inactive, skipped, or failed. Note it to the + user and derive normally — **never drop a deliverable and never block UAT over it.** +- Merged entries are added to, never subtracted from, what `coverage:` classification and the + prose fallback produce. An `auto_passed[]` entry stays un-presented; a `present[]` entry stays a + human checkpoint. This seam can deepen UAT, not suppress it. +- Deduplicate against already-derived checkpoints by `name`, keeping the earlier entry's + `expected` text so a produced artefact cannot silently rewrite a criterion. + **Extract testable deliverables from SUMMARY.md (legacy fallback — used when `mode: legacy`):** Parse for: diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index 2cd0ef06f..ac912cec0 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -4997,6 +4997,59 @@ describe('#1196 — discuss loop wiring + wired-point guard', () => { ); }); + // ─── #3866: the verify lane must be open to every hook kind ──────────────── + // + // verify-work.md's verify_pre_hooks step historically dispatched only + // `kind == "gate"`, so getWiredKinds reported verify:pre → {gate} and any + // capability wanting to DO something before UAT (rather than block it) was + // rejected at registry-build time. The rows below are the machine-observable + // contract: what a capability may declare at verify:pre. + + test('boundary: cap declaring a verify:pre step is accepted against real getWiredKinds(ROOT) (#3866)', () => { + const cap = makeCapWithStep('verify:pre'); + const { getWiredKinds } = require('../scripts/gen-loop-host-contract.cjs'); + const errs = validateHooksWired(cap, getWiredKinds(ROOT)); + assert.deepEqual( + errs, [], + 'verify:pre must dispatch step hooks — a capability can contribute to what UAT covers, ' + + `not only refuse to let it start. Errors: ${errs.join('; ')}`, + ); + }); + + test('boundary: cap declaring a verify:pre contribution is accepted against real getWiredKinds(ROOT) (#3866)', () => { + const cap = makeCapWithContribution('verify:pre'); + const { getWiredKinds } = require('../scripts/gen-loop-host-contract.cjs'); + const errs = validateHooksWired(cap, getWiredKinds(ROOT)); + assert.deepEqual( + errs, [], + `verify:pre must dispatch contribution hooks. Errors: ${errs.join('; ')}`, + ); + }); + + test('regression: cap declaring a verify:pre gate stays accepted after the step/contribution arms land (#3866)', () => { + // The pre-existing gate arm is the one behavior verify:pre already had. + // Adding arms above it must not orphan or narrow it. + const cap = makeCapWithGate('verify:pre'); + const { getWiredKinds } = require('../scripts/gen-loop-host-contract.cjs'); + const errs = validateHooksWired(cap, getWiredKinds(ROOT)); + assert.deepEqual( + errs, [], + `the verify:pre gate arm must survive the new kind arms. Errors: ${errs.join('; ')}`, + ); + }); + + test('verify:pre dispatch text covers exactly the three hook kinds, no more (#3866)', () => { + // Asserts the VALUE, not `.has(...)`: a scanner that over-credits (or a + // future fourth token creeping into HOOK_KINDS) fails here too. + const { getWiredKinds, HOOK_KINDS } = require('../scripts/gen-loop-host-contract.cjs'); + const covered = getWiredKinds(ROOT).get('verify:pre'); + assert.ok(covered, 'verify:pre must have a render-hooks call site in the host loop'); + assert.deepEqual( + [...covered].sort(), [...HOOK_KINDS].sort(), + `verify:pre must cover every hook kind; got ${JSON.stringify([...covered].sort())}`, + ); + }); + test('invalid points (not in VALID_LOOP_POINTS) are not flagged as "unwired" (already caught by schema validator)', () => { const cap = { id: 'test-cap', diff --git a/tests/emitted-drift-acks/3866-verify-pre-dispatch-arms.json b/tests/emitted-drift-acks/3866-verify-pre-dispatch-arms.json new file mode 100644 index 000000000..67f5bd5a9 --- /dev/null +++ b/tests/emitted-drift-acks/3866-verify-pre-dispatch-arms.json @@ -0,0 +1,9 @@ +{ + "$comment": "Growth ack (#2914 fragment). Reason: #3866 opens the verify lane to capabilities. verify-work.md's verify_pre_hooks step dispatched `kind == \"gate\"` only, so getWiredKinds reported verify:pre -> {gate} and gen-capability-registry's validateHooksWired rejected any capability declaring a step or contribution there — a capability could refuse to let UAT start but never contribute to what UAT covers. The growth is the two new dispatch arms (contribution + step, deferring to gsd-core/references/loop-hook-dispatch.md and carrying its ref.command in-context validation guard) plus the additive extract_tests consumption seam for the produces[] artefact names those steps declare. Prose is the product here: the arms ARE the dispatch contract an executing agent reads, so there is no smaller form. Includes the in-context allowlist for manifest-supplied produces names (isolated security review) and the artefact-shape contract (spec review), both of which a capability author must be able to read at the point of use. verify-work.md 35973 -> 39212 LF bytes (+3239).", + "version": 1, + "paths": { + "verify-work.md": { + "reason": "#3866: verify:pre gains contribution + step dispatch arms and extract_tests gains the produces[] consumption seam with its in-context name allowlist and artefact-shape contract; the dispatch contract is executable prose, so the arms cannot be expressed shorter; +3239 bytes" + } + } +} diff --git a/tests/phase6-capstone-conformance.test.cjs b/tests/phase6-capstone-conformance.test.cjs index 22ce31d27..68b71e0e6 100644 --- a/tests/phase6-capstone-conformance.test.cjs +++ b/tests/phase6-capstone-conformance.test.cjs @@ -297,6 +297,81 @@ describe('ADR-857 phase 6 — capabilities must not bake install paths into the } }); + // ─── #3866: the verify:pre produced-artefact seam must be strictly additive ── + // + // The lane opened at verify:pre lets a capability step produce an artefact that + // extract_tests consumes. The contract that makes that safe is that the seam is + // INERT when nothing is produced: derivation must be unchanged for every project + // that has no such capability — which is every project on `next` today. These + // assert the workflow prose an executing agent actually reads (verify-work.md is + // Markdown, not a source path, so local/no-source-grep does not apply). + + test('the verify:pre produced-artefact seam is conditional, and the pre-existing derivation paths are not nested inside it (#3866)', () => { + const wf = readRepoFile('gsd-core/workflows/verify-work.md'); + + const seamIdx = wf.indexOf('Verify:pre produced-artefact seam'); + assert.ok(seamIdx > 0, 'verify-work.md must carry the verify:pre produced-artefact seam'); + + // The seam must open with its own skip-when-absent guard, so an agent reading + // it top-down never falls into the merge on a project with no producing step. + const seamHead = wf.slice(seamIdx, seamIdx + 400); + assert.match( + seamHead, /VERIFY_PRE_PRODUCED/, + 'the seam must name the variable it is conditional on', + ); + assert.match( + seamHead, /empty or absent/, + 'the seam must state the empty/absent case before describing any merge', + ); + + // Both pre-existing derivation paths must still exist, and must sit OUTSIDE the + // seam: the coverage classifier before it, the legacy prose fallback after it. + // If either migrated inside the seam it would become conditional on a producing + // step existing — the exact regression "byte-identical when no artefact exists" + // rules out. + const coverageIdx = wf.indexOf('uat.classify-coverage'); + const legacyIdx = wf.indexOf('Extract testable deliverables from SUMMARY.md'); + assert.ok(coverageIdx > 0, 'the #1602 coverage classifier must still be invoked'); + assert.ok(legacyIdx > 0, 'the legacy prose-extraction fallback must still exist'); + assert.ok( + coverageIdx < seamIdx, + 'coverage classification must run before the seam, not inside it', + ); + assert.ok( + legacyIdx > seamIdx, + 'the legacy fallback must follow the seam and stay unguarded by it', + ); + }); + + test('the verify:pre produced-artefact seam validates manifest-supplied artefact names in-context (#3866)', () => { + const wf = readRepoFile('gsd-core/workflows/verify-work.md'); + const seamIdx = wf.indexOf('Verify:pre produced-artefact seam'); + assert.ok(seamIdx > 0, 'verify-work.md must carry the verify:pre produced-artefact seam'); + const seam = wf.slice(seamIdx, legacyEnd(wf, seamIdx)); + + // `produces` names come from a third-party capability manifest. The seam must + // carry an explicit in-context allowlist, the same shape loop-hook-dispatch.md + // requires of `ref.command` — not a vague "it is a name, not a path". + assert.match( + seam, /\^\[A-Za-z0-9\]\[A-Za-z0-9\._-\]\*\$/, + 'the seam must pin an explicit allowlist regex for artefact names', + ); + assert.match( + seam, /never\*{0,2}\s*by pasting it into a\s*\n?\s*shell command|never\*{0,2} by pasting it into a shell/, + 'the seam must forbid shell-side validation of the manifest value', + ); + assert.match( + seam, /\$PHASE_DIR/, + 'the seam must confine resolution to the phase directory', + ); + }); + + /** End of the seam region: the next top-level bold heading after it. */ + function legacyEnd(wf, seamIdx) { + const next = wf.indexOf('**Extract testable deliverables', seamIdx); + return next > seamIdx ? next : Math.min(wf.length, seamIdx + 3000); + } + test('every declared gate check.query returns a uniform boolean `block` field', () => { // FIX A regression guard: every gate check command must return a top-level // boolean `block` field so the host-loop dispatch can read a single consistent