diff --git a/.github/workflows/hydra-gates-package.yml b/.github/workflows/hydra-gates-package.yml index b229905..d01f493 100644 --- a/.github/workflows/hydra-gates-package.yml +++ b/.github/workflows/hydra-gates-package.yml @@ -51,6 +51,35 @@ jobs: - name: Run the invariant suite run: bash hydra-gates/tests/test-hydra-gates-bin.sh + # FIRST, with no node_modules at all — this is the only state in which the + # DEGRADED contract can be exercised. Gate 22's whole point is that a + # validator which could not resolve Ajv must exit 3 and say so, never 0; + # once ajv is installed below, `NODE_PATH=/nonexistent` no longer hides it + # (node still resolves from ./node_modules) and that assertion self-skips. + - name: "Manifest validator: the no-Ajv degraded contract" + run: bash hydra-gates/scripts/lib/test_check_manifest.sh + + # Ajv is what gates 22 and 53 validate manifests WITH. Without it the + # vendored validator can only run its structural lint, and the suite below + # would skip exactly the paths worth testing. It is a devDependency of + # every fleet app already; installing it here means CI exercises the same + # code path a real `npm ci` repo does. + - name: Install ajv for the manifest-validation helpers + run: npm install --no-save --no-audit --no-fund ajv ajv-formats + + # These three cover the gate-22 verdict contract and the gate-53 ADR-020 + # diff scoping. They lived in scripts/lib/ and were run by NOTHING — + # test_check_manifest.sh in particular had been green for its whole life + # while pointing at a fixture directory that did not exist (a missing + # manifest path makes the validator print "Tier 0, skipping" and exit 0, + # which is what two of its three assertions expected). + - name: Manifest gate helper suites + run: | + set -eu + bash hydra-gates/scripts/lib/test_check_manifest.sh + node hydra-gates/scripts/lib/test_manifest_scope_filter.js + python3 hydra-gates/scripts/lib/test_manifest_diff_scope.py + install-from-published-location: name: "Install from published location (php:8.3-cli)" runs-on: ubuntu-latest diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index b133f38..cea4654 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -2698,9 +2698,14 @@ jobs: # Installed INTO the package dir so `require('ajv')` from # scripts/lib/check_manifest.js resolves by walking up to # gates/hydra-gates/node_modules. A global install would need NODE_PATH - # and would silently not be found — which the manifest gates survive via - # a vendored fallback, meaning the miss would not be visible here. The - # coverage block at the end of the run is what makes any such gap loud. + # and would silently not be found. + # + # Since 2026-08-03 a miss here is loud rather than survivable: gate-22 + # used to fall back to a structural lint that checks only the AppHost + # blocks and report the result as a PASS — a manifest certified without + # ever being schema-validated. It now FAILS with a named reason (the + # validator exits 3 DEGRADED), exactly as gate-53 already refused to run + # fail-open. If this step is ever removed, both gates say so. run: npm --prefix gates/hydra-gates install --no-save --no-audit --no-fund ajv - name: Resolve the diff base diff --git a/hydra-gates/README.md b/hydra-gates/README.md index 0c8a7e0..e000e10 100644 --- a/hydra-gates/README.md +++ b/hydra-gates/README.md @@ -73,14 +73,45 @@ Then `composer update conduction/hydra-gates`. `vendor/conduction/hydra-gates` lands at about 1.2 MB. The org profile, the website and the docs tree are `export-ignore`d and do not follow. +### Upgrading to `v1.1.0` from `v1.0.x` + +`^1.0` picks this up on the next `composer update`, and **verdicts move**. Three +things to do before you upgrade: + +1. **Make `ajv` resolvable before the gates run** — `npm ci`, or + `npm --prefix install --no-save ajv` (Option A's shared + workflow already does this). Gate 22 previously fell back to a structural + lint that checks only the AppHost blocks and reported the result as a PASS; + it now **fails** with a named reason rather than certifying a manifest it + never schema-validated. Gate 53 has always refused to run without it. +2. **Expect gate-22 verdicts to move in both directions.** Its verdict used to + come from the app's own `npm run check:manifest`; it now comes from the + vendored canonical validator, and the app script is surfaced as an advisory. + Apps whose local checker was weaker will surface real findings; apps failed + by a stale app-local page-type enum will go green. +3. **Treat `: SKIPPED` as "did not run", not as a pass**, if you parse + `^\[gate-N\]` lines. New verdict; see *Reading a green* below. + +The upside: **gate 53 becomes usable under `--scope-to-diff`**. It was +previously unenablable on any repo with manifest debt, because a one-line change +reproduced the full-repo finding count exactly. + --- ## What it needs at runtime `bash`, `git`, `python3` (about twenty gates are Python helpers) and `node` -(gates 22 and 53 only, and they additionally want `ajv` resolvable). PHP is -required only because composer is one of the two delivery mechanisms; **no gate -executes PHP**. +(gates 22 and 53 only). PHP is required only because composer is one of the two +delivery mechanisms; **no gate executes PHP**. + +**Gates 22 and 53 need `ajv` resolvable, and both now say so instead of +degrading quietly.** `ajv` is already a transitive devDependency in every fleet +app's `package-lock.json`, so a `npm ci` resolves it; a bare checkout without +`node_modules` does not. Gate 53 has always refused to run without it. Gate 22 +used to fall back to a structural lint that checks only the AppHost blocks and +report the result as a PASS — a manifest certified without ever being +schema-validated. It now fails with a named reason instead. Set `NODE_PATH` or +run `npm ci` before the gates if you see it. **No gate needs a Nextcloud runtime.** Nothing under `scripts/` loads `../../lib/base.php` — that constraint belongs to `phpunit`, not to the gates, @@ -147,25 +178,95 @@ clean one. So: construction. - A genuinely empty diff is **stated as empty** rather than reported as a pass. +### Scope granularity, and where file granularity is not enough + +Most gates scope by **file**: a finding in a file the PR did not touch does not +block. Gates 51 (schema-property-titles) and 55 (detail-page-discipline) go +finer and scope by the **changed lines** inside a touched file, so legacy debt +elsewhere in a file you edited does not block either. + +Gate 53 (effective-manifest-crossref) used to scope by file, and for that gate +file granularity was indistinguishable from no scoping at all: an app's entire +navigation surface lives in `src/manifest.json` + `src/manifest.d/*`, so +touching any of it re-judged all of it. Measured 2026-08-03 on a one-line +`title` change: + +| repo | full-repo | diff-scoped (before) | diff-scoped (after) | +| --- | --- | --- | --- | +| pipelinq | 24 | 24 | 0 blocking, 24 reported PRE-EXISTING | +| shillinq | 246 | 246 | 0 blocking, 397 reported PRE-EXISTING | + +Gate 53 now separates two things that are not the same: + +- **Answering** a cross-reference needs the whole assembled manifest. You cannot + resolve `menu[].route` → page id, or check the ADR-044 no-orphan-removal + invariant, from a diff. That part of the gate is legitimately whole-repo and + stays so. +- **Blocking** on the answer does not. Every finding carries a JSON pointer that + resolves to a page id, a menu id or a top-level block, and it blocks only when + the PR touched that entry. + +Findings on untouched entries are **printed as `PRE-EXISTING`, never dropped**, +and their count is reported on stdout, so a scoped green cannot be read as "the +manifest is clean". Findings that address the manifest as a whole (no entry to +attribute them to), and any PR whose scope cannot be determined — a brand-new +fragment untracked at base, a changed register JSON, a parse failure — block +regardless. Unverifiable scope is never treated as narrow scope. + +The only part of the suite that is whole-repo **by nature** — as opposed to by +oversight — is that residual set of gate-53 invariants. Everything else measured +on 2026-08-03 (gates 34, 51, 55) narrows correctly, and the two other +whole-manifest checks in the same family, gates 22 and 52, are triggered only +by a change to the artefact they judge. + --- ## Reading a green A green from this package says how much it covers, because a green that overstates its coverage is the same defect as `|| echo '...skipping'` one layer -up. The runner's own closing line reads `ALL 61 GATES GREEN` regardless of how -many gates ran; measured on openbuild, 59 of 61 report and gates 24 and 33 skip -silently when their prerequisites are absent. So every run ends with: +up. + +Every gate in the runner is wrapped in a prerequisite test (`if [ -d src ]`, +`if [ -f tests/axe/report.json ]`, …). Until 2026-08-03 a gate whose +prerequisite was absent emitted **nothing at all** — no line, no count, no +trace — and the runner still closed with `ALL 63 GATES GREEN`. Measured across +13 fleet repos, **gate 33 (axe-core) had never run in any of them**: the +`tests/axe/report.json` it consumes is produced by a `scripts/run-browser-tests.sh` +that exists in no app, while `axe-core` sits in every app's `devDependencies` +so the prerequisite looks wired. Every green the fleet had ever produced +excluded accessibility runtime checking, and nothing said so. Gate 24 +(integration-parity) was absent in most repos for the same structural reason. + +Both layers now account for it. A gate that cannot run says so on its own line: + +``` +[gate-24] integration-parity: SKIPPED — no scripts/check-integration-parity.sh … +[gate-33] axe-core: SKIPPED — no tests/axe/report.json in this repo — axe-core + never ran against a rendered DOM, so contrast / landmark / + ARIA-validity / live-region accessibility is UNVERIFIED. … +``` + +and every run — `bin/hydra-gates` **and** a direct `run-hydra-gates.sh` +invocation — ends with the accounting: ``` -[hydra-gates] COVERAGE: 59 of 61 declared gates reported a result. -[hydra-gates] GATES THAT DID NOT RUN: 24 33 -[hydra-gates] RESULT: ALL GATES PASSED — EXCEPT GATES 24 33, WHICH DID NOT RUN. -[hydra-gates] This green covers 59 gates. It says NOTHING about gates 24 33. +[hydra-gates] COVERAGE: 60 of 63 declared gates reported a result. +[hydra-gates] GATES THAT DID NOT RUN — they inspected NOTHING, and their subject +[hydra-gates] matter is UNVERIFIED by this run: +[hydra-gates] gate-4 composer-audit +[hydra-gates] gate-24 integration-parity +[hydra-gates] gate-33 axe-core +[hydra-gates] 60 GATE(S) GREEN — but 3 of 63 DID NOT RUN (named above). +[hydra-gates] This is NOT 'all 63 gates green'. … ``` +A `SKIPPED` line is **not** counted as coverage — a gate reporting that it did +nothing did nothing. `--require-full-coverage` turns an incomplete run into +exit 98. + The inventory is read out of the runner itself rather than hardcoded, so adding -gate 62 does not silently leave the coverage check measuring against a stale 61. +gate 64 does not silently leave the coverage check measuring against a stale 63. ### Waivers @@ -188,14 +289,29 @@ A green earned by passing is then distinguishable from one earned by waiving. ``` bash hydra-gates/tests/test-hydra-gates-bin.sh # or: composer test:package + +# manifest gate helpers (gate 22's verdict contract, gate 53's diff scoping). +# Install ajv first or the schema paths state a SKIP instead of passing: +npm install --no-save ajv ajv-formats +bash hydra-gates/scripts/lib/test_check_manifest.sh +node hydra-gates/scripts/lib/test_manifest_scope_filter.js +python3 hydra-gates/scripts/lib/test_manifest_diff_scope.py ``` -Asserts the four invariants — exit-code-is-count, loud unresolvable base, stated -empty diff, self-describing coverage — against a synthesized fixture repo. The -positive control runs in **both** directions: the injected violation must be -*named* by the gate that catches it, and the same fixture must go green once it -is removed. A one-directional control cannot distinguish "the check caught it" -from "the check never ran". +Asserts the invariants — exit-code-is-count, loud unresolvable base, stated +empty diff, self-describing coverage, named unrun gates, one-line-per-verdict — +against a synthesized fixture repo. The positive control runs in **both** +directions: the injected violation must be *named* by the gate that catches it, +and the same fixture must go green once it is removed. A one-directional control +cannot distinguish "the check caught it" from "the check never ran". + +`test_check_manifest.sh` was itself an example of that failure until +2026-08-03: it pointed at a `scripts/test-fixtures/manifest-validation/` +directory that had never existed, and `check_manifest.js ` prints +"Tier 0, skipping" and exits 0 — which is exactly what two of its three +assertions expected. It had been green its whole life while validating nothing. +The fixtures now ship, and the suite refuses to run at all if they go missing +again. CI for this package lives in [`.github/workflows/hydra-gates-package.yml`](../.github/workflows/hydra-gates-package.yml): diff --git a/hydra-gates/bin/hydra-gates b/hydra-gates/bin/hydra-gates index b444e0e..57af4f9 100755 --- a/hydra-gates/bin/hydra-gates +++ b/hydra-gates/bin/hydra-gates @@ -28,13 +28,21 @@ # shallow clone it may not exist at all. # # 3. A COVERAGE ASSERTION ON THE GREEN. -# The runner ends with "ALL 61 GATES GREEN" whether or not 61 gates ran. -# Measured on openbuild, only 59 of the 61 emit a line — gates 24 and 33 -# skip silently when their prerequisites are absent. A green that counts -# gates it never executed is the same defect as `|| echo 'skipping'`, one -# layer up. We diff the gate numbers that actually reported against the -# inventory declared by the runner itself, and say plainly which gates did -# not run and what the green therefore does not cover. +# The runner used to end with "ALL 63 GATES GREEN" whether or not 63 gates +# ran. Measured on openbuild, only 59 of 61 emitted a line — gates 24 and +# 33 skipped silently when their prerequisites were absent; measured +# 2026-08-03 across 13 repos, gate-33 (axe-core) has never run in ANY of +# them. A green that counts gates it never executed is the same defect as +# `|| echo 'skipping'`, one layer up. We diff the gate numbers that +# actually reported against the inventory declared by the runner itself, +# and say plainly which gates did not run and what the green therefore +# does not cover. +# +# As of 2026-08-03 the runner performs the same accounting in its own +# summary, so a direct `run-hydra-gates.sh` invocation (which most of the +# fleet uses) is no longer blind to it. Gates that state their own absence +# now print `[gate-N] : SKIPPED — `; that line is NOT counted +# as "reported" below — a gate that says it did nothing did nothing. # # 4. A LOUD, STATED SKIP FOR MISSING PREREQUISITES. # Never `|| echo '...skipping'`. A prerequisite that is genuinely absent is @@ -267,8 +275,13 @@ RC="${PIPESTATUS[0]}" # adds gate 62, and a coverage check that silently measures against a stale # inventory is the very defect this step exists to catch. # --------------------------------------------------------------------------- -DECLARED="$(grep -oE '_(pass|fail) [0-9]+' "${RUNNER}" | awk '{print $2}' | sort -un)" -EMITTED="$(grep -oE '^\[gate-[0-9]+\]' "${_out}" | grep -oE '[0-9]+' | sort -un)" +DECLARED="$(grep -oE '_(pass|fail|skip) [0-9]+' "${RUNNER}" | awk '{print $2}' | sort -un)" +# A `[gate-N] name: SKIPPED — reason` line is the gate REPORTING THAT IT DID NOT +# RUN. Counting it as coverage would turn the fix into the bug: the whole point +# of making the skip visible is that it stays outside the "reported a result" +# tally. Excluded here by verdict, not by gate number. +EMITTED="$(grep -E '^\[gate-[0-9]+\]' "${_out}" | grep -v ': SKIPPED' \ + | grep -oE '^\[gate-[0-9]+\]' | grep -oE '[0-9]+' | sort -un)" DECLARED_N="$(printf '%s\n' "${DECLARED}" | grep -c . || true)" EMITTED_N="$(printf '%s\n' "${EMITTED}" | grep -c . || true)" diff --git a/hydra-gates/scripts/lib/check_manifest.js b/hydra-gates/scripts/lib/check_manifest.js index 6887d5b..69e11b6 100644 --- a/hydra-gates/scripts/lib/check_manifest.js +++ b/hydra-gates/scripts/lib/check_manifest.js @@ -44,6 +44,13 @@ // 0 — manifest validates with zero errors (or no manifest → caller skips) // 1 — manifest fails validation (errors printed one per line: "at : …") // 2 — vendored canonical schema could not be loaded (gate misconfiguration) +// 3 — DEGRADED: Ajv was not resolvable (or the merged schema would not +// compile), so only the AppHost structural lint ran and it found nothing. +// This is NOT a pass: the schema was never applied. Callers must surface +// it as a distinct verdict — a silent downgrade to a weaker check is the +// failure mode this whole package exists to remove. Exit 1 still wins +// when the structural lint DID find something, so a real finding is +// never masked by the degradation. 'use strict' @@ -54,10 +61,23 @@ const path = require('path') // It is the ADR-040 SUPERSET (published v2 base + observability + deepLinks). const CANONICAL_SCHEMA_PATH = path.resolve(__dirname, '..', 'schemas', 'app-manifest-v2.schema.json') -const MANIFEST_PATH = process.argv[2] - ? path.resolve(process.argv[2]) +// `--scope-ids FILE` (ADR-020): findings on manifest entries the PR did not +// touch are reported as PRE-EXISTING instead of blocking. Absent → full-repo. +const _argv = process.argv.slice(2) +let SCOPE_IDS_FILE = null +const _positional = [] +for (let i = 0; i < _argv.length; i++) { + if (_argv[i] === '--scope-ids') { SCOPE_IDS_FILE = _argv[++i]; continue } + if (_argv[i].startsWith('--scope-ids=')) { SCOPE_IDS_FILE = _argv[i].slice('--scope-ids='.length); continue } + _positional.push(_argv[i]) +} + +const MANIFEST_PATH = _positional[0] + ? path.resolve(_positional[0]) : path.resolve(process.cwd(), 'src', 'manifest.json') +const scopeFilter = require('./manifest_scope_filter.js') + // Base-schema candidates, in priority order. The CANONICAL hydra-vendored // schema wins (2026-07-06 manifest audit, item 4): pinned-first meant "pass" // certified against whatever schema generation the app happened to have @@ -144,12 +164,27 @@ function semanticChecks(manifest) { // line — both on stdout (every stdout line is valid JSON). Human-readable // `at : ` diagnostics go to stderr, which also keeps // run-hydra-gates.sh's `grep -cE '^at /'` failure count working. -function report(errors) { +function report(allErrors, degradedReason, manifest) { + // ADR-020: answer over the whole manifest, block only on entries the PR + // touched. `scope` is null on a full-repo run and everything blocks. + const scope = scopeFilter.loadScope(SCOPE_IDS_FILE) + const parts = scopeFilter.partition(allErrors, manifest || {}, scope) + scopeFilter.reportScope('check_manifest', parts) + const errors = parts.blocking const failed = errors.length > 0 ? 1 : 0 if (failed === 1) { for (const e of errors) console.error(`at ${e.path || '/'}: ${String(e.message).split('\n')[0]}`) console.log(JSON.stringify({ file: path.relative(process.cwd(), MANIFEST_PATH), schemaVersion: 'v2', errors })) } + // A zero-finding run that never applied the schema is NOT a pass. Say so on + // both channels and exit 3 so the caller cannot mistake it for one. + if (failed === 0 && degradedReason) { + console.error(`[check_manifest] DEGRADED — SCHEMA VALIDATION DID NOT HAPPEN: ${degradedReason}`) + console.error('[check_manifest] The AppHost structural lint found nothing, but it checks only the observability/deepLinks blocks.') + console.error('[check_manifest] Reporting this as a pass would certify a manifest that was never validated against the canonical schema.') + console.log(JSON.stringify({ status: 'degraded', checked: 1, failed: 0, reason: degradedReason })) + process.exit(3) + } console.log(JSON.stringify({ status: failed === 1 ? 'failed' : 'passed', checked: 1, failed })) process.exit(failed) } @@ -340,7 +375,7 @@ function main() { validate = ajv.compile(schema) } catch (e) { console.error(`[check_manifest] Ajv could not compile the merged schema (${e.message}); falling back to AppHost structural lint`) - return finishStructural(manifest) + return finishStructural(manifest, `Ajv could not compile the merged canonical schema (${e.message})`) } const errors = [] if (validate(manifest)) { @@ -351,20 +386,20 @@ function main() { } } errors.push(...semanticChecks(manifest)) - return report(errors) + return report(errors, null, manifest) } console.error('[check_manifest] Ajv not installed; using AppHost structural lint (observability/deepLinks still validated for-real)') - return finishStructural(manifest) + return finishStructural(manifest, 'Ajv is not resolvable from this process (no node_modules, no NODE_PATH)') } -function finishStructural(manifest) { +function finishStructural(manifest, degradedReason) { const errors = structuralLintAppHost(manifest) if (errors.length === 0) { console.error('[check_manifest] AppHost structural lint against canonical ADR-040 enums: PASS') } errors.push(...semanticChecks(manifest)) - return report(errors) + return report(errors, degradedReason, manifest) } main() diff --git a/hydra-gates/scripts/lib/check_manifest_crossref.js b/hydra-gates/scripts/lib/check_manifest_crossref.js index 08f70fc..1b65bfd 100644 --- a/hydra-gates/scripts/lib/check_manifest_crossref.js +++ b/hydra-gates/scripts/lib/check_manifest_crossref.js @@ -72,16 +72,23 @@ try { let APP_DIR = process.cwd() let MANIFEST_FILE = null +// `--scope-ids FILE` (ADR-020) — see manifest_scope_filter.js. The joins are +// still answered against the WHOLE assembled manifest; the flag only decides +// which of the answers block this PR. +let SCOPE_IDS_FILE = null { const argv = process.argv.slice(2) for (let i = 0; i < argv.length; i++) { if (argv[i] === '--app-dir' && argv[i + 1]) { APP_DIR = path.resolve(argv[++i]); continue } if (argv[i] === '--manifest' && argv[i + 1]) { MANIFEST_FILE = path.resolve(argv[++i]); continue } + if (argv[i] === '--scope-ids' && argv[i + 1]) { SCOPE_IDS_FILE = argv[++i]; continue } console.error(`[check_manifest_crossref] unknown argument: ${argv[i]}`) process.exit(2) } } +const scopeFilter = require('./manifest_scope_filter.js') + // --- findings accumulator ---------------------------------------------------- const findings = [] @@ -397,23 +404,38 @@ function main() { } } - report(manifestLabel) + report(manifestLabel, manifest) } // Emit the gate-22 report shape: per-file findings line (when any findings, // error OR warn), then always the summary line — both valid JSON on stdout. // Human diagnostics on stderr; WARNs never set the failure exit code. -function report(manifestLabel) { - const errors = findings.filter((f) => f.severity === 'error') +function report(manifestLabel, manifest) { + // ADR-020 diff scoping. WARNs are advisory already and are never scoped out + // — they cost nothing and vanishing them would hide debt twice over. Only + // error-severity findings are partitioned into blocking vs pre-existing. + const scope = scopeFilter.loadScope(SCOPE_IDS_FILE) + const errorFindings = findings.filter((f) => f.severity === 'error') + const parts = scopeFilter.partition(errorFindings, manifest || {}, scope) + const preexisting = new Set(parts.preexisting) + const errors = parts.blocking const failed = errors.length > 0 ? 1 : 0 for (const f of findings) { const first = String(f.message).split('\n')[0] if (f.severity === 'warn') { console.error(`at ${f.path || '/'}: WARN ${first}`) + } else if (preexisting.has(f)) { + console.error(`at ${f.path || '/'}: PRE-EXISTING ${first}`) } else { console.error(`at ${f.path || '/'}: ${first}`) } } + if (parts.preexisting.length > 0) { + console.error(`[check_manifest_crossref] diff-scope (ADR-020): ${parts.preexisting.length} cross-reference finding(s) sit on manifest entries this PR did not touch — reported above as PRE-EXISTING, not blocking.`) + } + if (parts.unscopable.length > 0) { + console.error(`[check_manifest_crossref] ${parts.unscopable.length} finding(s) address the manifest as a WHOLE and block regardless of scope.`) + } if (findings.length > 0) { console.log(JSON.stringify({ file: path.relative(process.cwd(), manifestLabel.replace(' (effective)', '')), diff --git a/hydra-gates/scripts/lib/manifest_diff_scope.py b/hydra-gates/scripts/lib/manifest_diff_scope.py new file mode 100644 index 0000000..ead1757 --- /dev/null +++ b/hydra-gates/scripts/lib/manifest_diff_scope.py @@ -0,0 +1,219 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: 2026 Conduction +# SPDX-License-Identifier: EUPL-1.2 +"""Gate-53 helper — compute the ADR-020 diff scope of a manifest change. + +WHY THIS EXISTS (measured 2026-08-03) +------------------------------------- +gate-53 (effective-manifest-crossref) was diff-scoped at FILE granularity: if a +PR touched ``src/manifest.json``, any ``src/manifest.d/*.json`` fragment, or +``src/menu-layout.json``, it re-judged the ENTIRE assembled manifest. Because an +app's whole navigation surface lives in that one input set, file granularity is +indistinguishable from no scoping at all. Measured: + + pipelinq one-line ``title`` change on one index page + → full-repo 24 findings, diff-scoped 24 findings + shillinq one-line ``title`` change in ONE of 80 fragments + → full-repo 246 findings, diff-scoped 246 findings + +Every one of those findings sat on a page the PR had never touched. Enabling the +gate would have blocked every manifest-touching PR in those repos on inherited +debt, however small the change — which is how a gate gets switched off. + +The cross-reference joins themselves genuinely need the WHOLE assembled manifest +to be ANSWERED (you cannot resolve ``menu[].route`` → page id from a diff). That +is not the same as needing the whole manifest to be BLOCKING. This helper draws +the second line: the answer is computed repo-wide, and the finding blocks only +when the PR touched the entry the finding is ABOUT. It is the same model gate-55 +(detail-page-discipline) already uses, and gate-55 measurably scopes correctly. + +WHAT IT EMITS +------------- +One token per line on stdout, for consumption by +``manifest_scope_filter.js``:: + + page: a pages[] entry whose JSON object span intersects the diff + menu: a menu[] entry (or nested child) whose span intersects + key: a non-pages/non-menu top-level key whose span intersects + (e.g. ``key:observability``, ``key:deepLinks``) + ALLMENU menu-layout.json changed — relocations/removals restructure + the merged menu wholesale, so every /menu finding is in scope + ALL scope could not be determined; EVERYTHING blocks + +``ALL`` is emitted whenever the answer is unknowable rather than empty — a file +untracked at base (brand-new manifest/fragment), a JSON parse failure, a git +invocation that fails, or a changed register JSON (a register edit can orphan a +schema reference anywhere in the manifest). Fail TOWARD enforcement: an +unverifiable scope must never be reported as a narrow one. That is the same +rule the runner applies to an unresolvable base ref. + +Usage:: + + HYDRA_GATE_BASE_REF=origin/development \\ + manifest_diff_scope.py [ ...] + +Only paths that are manifest INPUTS are interpreted; anything else is ignored. +With no base ref set the helper prints ``ALL`` (a full-repo run scopes nothing). +""" + +import importlib.util +import os +import sys + +_HERE = os.path.dirname(os.path.abspath(__file__)) + + +def _load_line_parser(): + """Borrow the line-tracking JSON parser from check_detail_page_discipline. + + That module already carries a tokenizer that records the start/end line of + every object and array — the machinery gate-55 uses for its page-span + scoping. Importing it keeps ONE parser in the package: a second copy would + be a second thing to keep in sync, and a scope computation that disagrees + with gate-55's about where a page begins is worse than no scoping at all. + """ + path = os.path.join(_HERE, "check_detail_page_discipline.py") + spec = importlib.util.spec_from_file_location("_hydra_dpd", path) + if spec is None or spec.loader is None: + return None + mod = importlib.util.module_from_spec(spec) + try: + spec.loader.exec_module(mod) + except Exception: # noqa: BLE001 — any import failure means "cannot scope" + return None + return mod + + +_DPD = _load_line_parser() + + +def _is_manifest_page_input(rel): + return rel == "src/manifest.json" or rel.startswith("src/manifest.d/") + + +def _spans_of(node): + """(start_line, end_line) for a parsed node, or None when untracked.""" + start = getattr(node, "start_line", 0) + end = getattr(node, "end_line", 0) + if not start or not end: + return None + return (start, end) + + +def _intersects(span, changed): + if span is None: + return True # unknown span → assume touched (fail toward enforcement) + lo, hi = span + for ln in changed: + if lo <= ln <= hi: + return True + return False + + +def _collect_menu_ids(entry, changed, out): + """Emit menu ids for `entry` and any nested children the diff touches. + + A parent whose own span is touched puts the parent in scope; a touched CHILD + also puts the child in scope on its own, so editing one leaf of a 30-item + menu tree does not drag the whole tree in. + """ + if not isinstance(entry, dict): + return + span = _spans_of(entry) + ident = entry.get("id") or entry.get("route") + children = entry.get("children") + child_hit = False + if isinstance(children, list): + for child in children: + before = len(out) + _collect_menu_ids(child, changed, out) + if len(out) > before: + child_hit = True + if isinstance(ident, str) and ident and (child_hit or _intersects(span, changed)): + out.add("menu:" + ident) + + +def _scope_one_file(path, base_ref, tokens): + """Add tokens for one changed manifest input. Returns False on 'cannot scope'.""" + if _DPD is None: + return False + changed = _DPD._changed_lines(path, base_ref) + if changed is None: + # Untracked at base (new file) / git unavailable / bad base — unknowable. + return False + if not changed: + return True # tracked and byte-identical: contributes nothing + try: + with open(path, "r", encoding="utf-8") as fh: + doc = _DPD._Parser(fh.read()).parse() + except (OSError, ValueError): + return False + if not isinstance(doc, dict): + return False + + for key, value in doc.items(): + if key == "pages" and isinstance(value, list): + for page in value: + if not isinstance(page, dict): + continue + pid = page.get("id") + if isinstance(pid, str) and pid and _intersects(_spans_of(page), changed): + tokens.add("page:" + pid) + elif key == "menu" and isinstance(value, list): + for entry in value: + _collect_menu_ids(entry, changed, tokens) + else: + span = None + if hasattr(value, "start_line"): + span = _spans_of(value) + elif key in doc.key_lines: + line = doc.key_lines[key] + span = (line, line) + if _intersects(span, changed): + tokens.add("key:" + key) + return True + + +def main(argv): + base_ref = os.environ.get("HYDRA_GATE_BASE_REF", "").strip() + if not base_ref: + print("ALL") + return 0 + + tokens = set() + scoped_anything = False + for raw in argv[1:]: + rel = raw.lstrip("./") + if rel == "src/menu-layout.json": + # Relocations and removals rewrite the merged menu wholesale — a + # single removal can orphan a route declared in a different file. + # Do not pretend a per-entry answer exists. + tokens.add("ALLMENU") + scoped_anything = True + continue + if "register" in rel and rel.endswith(".json") and rel.startswith("lib/"): + # A register edit can invalidate a register/schema slug referenced + # from ANY page. There is no per-page answer to be had. + print("ALL") + return 0 + if not _is_manifest_page_input(rel): + continue + if not _scope_one_file(rel, base_ref, tokens): + print("ALL") + return 0 + scoped_anything = True + + if not scoped_anything: + # No manifest input in the changed set at all. The caller only invokes + # us when one WAS touched, so reaching here means our idea of "manifest + # input" disagrees with the caller's — do not narrow on a disagreement. + print("ALL") + return 0 + + for token in sorted(tokens): + print(token) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/hydra-gates/scripts/lib/manifest_scope_filter.js b/hydra-gates/scripts/lib/manifest_scope_filter.js new file mode 100644 index 0000000..605ba85 --- /dev/null +++ b/hydra-gates/scripts/lib/manifest_scope_filter.js @@ -0,0 +1,180 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: EUPL-1.2 +// +// manifest_scope_filter.js — ADR-020 diff scoping for manifest findings. +// +// Shared by check_manifest.js and check_manifest_crossref.js (gate-53). Given +// the scope token file produced by manifest_diff_scope.py and the ASSEMBLED +// manifest, decide whether a finding BLOCKS this PR or is pre-existing debt on +// an entry the PR never touched. +// +// The split this module encodes: +// +// ANSWERING a cross-reference needs the whole assembled manifest — you cannot +// resolve `menu[].route` → page id from a diff. That is why gate-53 is, and +// must remain, a whole-manifest CHECK. +// +// BLOCKING on the answer does not. Every finding carries a JSON pointer into +// the assembled manifest, and that pointer resolves to a page id, a menu id +// or a top-level block. If the PR did not touch that entry, the finding is +// inherited debt and must not block — ADR-020. +// +// Findings that resolve to NOTHING addressable (pointer `/`, an assembly +// failure, a whole-document invariant) stay blocking under every scope. Those +// are the legitimately-whole-repo part of this gate, and they are reported +// under their own heading so the distinction is visible rather than assumed. +// +// Out-of-scope findings are NOT discarded: they are printed as +// `at : PRE-EXISTING ` so a reader can still see the debt, and +// counted separately. Silently dropping them would replace one invisible +// failure with another. + +'use strict' + +const fs = require('fs') + +/** + * Read a scope token file written by manifest_diff_scope.py. + * + * @param {string} file Path to the token file (one token per line). + * @return {object|null} `{ all, allMenu, pages:Set, menu:Set, keys:Set }`, or + * null when no scoping applies (no file given, or unreadable → full-repo). + */ +function loadScope(file) { + if (!file) return null + let text + try { + text = fs.readFileSync(file, 'utf8') + } catch (_) { + // An unreadable scope file is an unknown scope, not an empty one. + return { all: true, allMenu: true, pages: new Set(), menu: new Set(), keys: new Set() } + } + const scope = { all: false, allMenu: false, pages: new Set(), menu: new Set(), keys: new Set() } + for (const raw of text.split('\n')) { + const line = raw.trim() + if (!line || line.startsWith('#')) continue + if (line === 'ALL') { scope.all = true; continue } + if (line === 'ALLMENU') { scope.allMenu = true; continue } + const idx = line.indexOf(':') + if (idx <= 0) continue + const kind = line.slice(0, idx) + const value = line.slice(idx + 1) + if (kind === 'page') scope.pages.add(value) + else if (kind === 'menu') scope.menu.add(value) + else if (kind === 'key') scope.keys.add(value) + } + return scope +} + +function decodeSegment(seg) { + return seg.replace(/~1/g, '/').replace(/~0/g, '~') +} + +/** + * Resolve a JSON pointer into the assembled manifest to the scope tokens the + * finding is attributable to. + * + * @param {string} ptr JSON pointer, e.g. `/pages/12/config/widgets/0`. + * @param {object} manifest The ASSEMBLED manifest. + * @return {Array|null} Tokens, or null when the finding addresses + * nothing scopable (a whole-document invariant → always blocking). + */ +function tokensForPointer(ptr, manifest) { + if (typeof ptr !== 'string' || ptr === '' || ptr === '/') return null + const segs = ptr.replace(/^\//, '').split('/').map(decodeSegment) + if (segs.length === 0) return null + const head = segs[0] + + if (head === 'pages') { + const idx = Number(segs[1]) + const pages = Array.isArray(manifest && manifest.pages) ? manifest.pages : [] + const page = Number.isInteger(idx) ? pages[idx] : undefined + const id = page && typeof page.id === 'string' ? page.id : null + // A page whose id cannot be read cannot be attributed — block it. + return id ? ['page:' + id] : null + } + + if (head === 'menu') { + // Walk /menu/[/children/...] collecting every id on the path, so a + // finding on a leaf is in scope when either the leaf OR an ancestor was + // edited (editing a parent's `order` legitimately re-homes its children). + const tokens = [] + let node = Array.isArray(manifest && manifest.menu) ? manifest.menu : [] + let i = 1 + let cursor = node[Number(segs[i])] + while (cursor && typeof cursor === 'object') { + const id = typeof cursor.id === 'string' ? cursor.id : (typeof cursor.route === 'string' ? cursor.route : null) + if (id) tokens.push('menu:' + id) + i += 1 + if (segs[i] !== 'children' || !Array.isArray(cursor.children)) break + i += 1 + cursor = cursor.children[Number(segs[i])] + } + return tokens.length ? tokens : null + } + + // Anything else is a top-level block: /observability/..., /deepLinks/0/..., + // /menu-layout/removals/2, /version, ... + return ['key:' + head] +} + +/** + * Partition findings into blocking vs pre-existing under a diff scope. + * + * @param {Array} findings Each `{ path, message, ... }`. + * @param {object} manifest The assembled manifest. + * @param {object|null} scope Result of loadScope(); null → no scoping. + * @return {object} `{ blocking, preexisting, unscopable }`. + */ +function partition(findings, manifest, scope) { + if (!scope || scope.all) { + return { blocking: findings.slice(), preexisting: [], unscopable: [] } + } + const blocking = [] + const preexisting = [] + const unscopable = [] + for (const f of findings) { + const tokens = tokensForPointer(f.path, manifest) + if (tokens === null) { + unscopable.push(f) + blocking.push(f) + continue + } + const hit = tokens.some((t) => { + if (t.startsWith('page:')) return scope.pages.has(t.slice(5)) + if (t.startsWith('menu:')) return scope.allMenu || scope.menu.has(t.slice(5)) + if (t.startsWith('key:')) { + const key = t.slice(4) + if (key === 'menu' && scope.allMenu) return true + if (key === 'menu-layout' && scope.allMenu) return true + return scope.keys.has(key) + } + return true + }) + if (hit) blocking.push(f) + else preexisting.push(f) + } + return { blocking, preexisting, unscopable } +} + +/** + * Emit the standard accounting lines for a scoped run. + * + * @param {string} tag Helper name for the log prefix. + * @param {object} parts Result of partition(). + * @return {void} + */ +function reportScope(tag, parts) { + if (parts.preexisting.length === 0 && parts.unscopable.length === 0) return + for (const f of parts.preexisting) { + console.error(`at ${f.path || '/'}: PRE-EXISTING ${String(f.message).split('\n')[0]}`) + } + if (parts.preexisting.length > 0) { + console.error(`[${tag}] diff-scope (ADR-020): ${parts.preexisting.length} finding(s) sit on manifest entries this PR did not touch — reported above as PRE-EXISTING, not blocking.`) + } + if (parts.unscopable.length > 0) { + console.error(`[${tag}] ${parts.unscopable.length} finding(s) address the manifest as a WHOLE (no page/menu entry to attribute them to) and block regardless of scope.`) + } +} + +module.exports = { loadScope, tokensForPointer, partition, reportScope } diff --git a/hydra-gates/scripts/lib/test_check_manifest.sh b/hydra-gates/scripts/lib/test_check_manifest.sh index c1c6790..13b8649 100755 --- a/hydra-gates/scripts/lib/test_check_manifest.sh +++ b/hydra-gates/scripts/lib/test_check_manifest.sh @@ -3,11 +3,25 @@ # # test_check_manifest.sh — gate-22 (manifest-validation) verification. # -# Proves the 3-way contract of scripts/lib/check_manifest.js: -# 1. valid-apphost → PASS (observability + deepLinks validated for-real) +# Proves the contract of scripts/lib/check_manifest.js: +# 1. valid-apphost → PASS (observability + deepLinks validated for-real) # 2. malformed-apphost → FAIL (unknown check type / metric kind / missing # deepLink required key → really validating, not fail-open) -# 3. non-apphost → PASS (no observability/deepLinks → unaffected) +# 3. non-apphost → PASS (no observability/deepLinks → unaffected) +# 4. no Ajv → exit 3 DEGRADED, never 0. A run that never applied +# the schema is not a pass, and it must say so on both channels. +# 5. --scope-ids → ADR-020: a finding on an entry the PR did not touch +# is reported PRE-EXISTING and does not set the exit code; the SAME +# finding does block once that entry is in scope. +# +# THE FIXTURES ARE PART OF THIS TEST. Before 2026-08-03 this file referenced +# ../test-fixtures/manifest-validation/*.json — a directory that has never +# existed in this repository. `node check_manifest.js ` prints +# "no src/manifest.json — Tier 0, skipping" and exits 0, which is exactly what +# assertions 1 and 3 expected. Two of the three therefore passed by inspecting +# NOTHING; only the malformed case (which wanted rc=1) ever reported the truth. +# A suite that is green because its inputs are absent is the same defect this +# package exists to catch, one level down. # # Run twice: once with Ajv available (full schema path) and once with Ajv # forced unavailable (structural-lint fallback), so both code paths are covered. @@ -18,41 +32,113 @@ VALIDATOR="${SCRIPT_DIR}/check_manifest.js" FIX="${SCRIPT_DIR}/../test-fixtures/manifest-validation" _fails=0 -_assert() { #