fix(gate-46): resolve a spec wherever archiving left it, and accept #T<n> (#228) - #246
Merged
Merged
Conversation
…T<n> (#228) Gate-46 was a treadmill: the repo's own BLOCKING phpcs SpecTagSniff instructs `@spec openspec/changes/{name}/tasks.md#task-N`, and a change directory is by definition temporary. 260 of portaliq's 385 tags did as told and dangled the moment their change was archived. The sniff is being repointed separately; this is the gate half — and without it, following the CORRECTED advice dangles too. WHY POINTING AT openspec/specs/ WAS NOT ALREADY SAFE A capability's spec.md occupies three homes over its life: 1. in flight openspec/changes/<change>/specs/<cap>/spec.md 2. archived openspec/changes/archive/<date>-<change>/specs/<cap>/spec.md 3. canonical openspec/specs/<cap>/spec.md build_archive_index already carried a tag from (1) to (2), keyed on the CHANGE name. That is not enough, twice over: * archiving does NOT reliably promote the delta spec into openspec/specs/. procest carries 18 tags at openspec/specs/process-mining-bottlenecks/spec.md — written exactly as the corrected sniff instructs — whose spec only ever existed at (2). Correct advice, dangling reference. * the change name need not equal the capability name. procest's realtime-updates-ui capability lives under the change adopt-live-updates-ui, so no CHANGE-keyed index can reach it. 10 more findings. Keying on the CAPABILITY spans all three homes and is stable across both the archive move and the promotion that may follow, so a tag written once keeps resolving whatever stage the change has reached. This widens WHERE the gate looks, never WHAT counts as resolved. The index is consulted only after every literal spelling has failed, so a tag whose own path exists is still judged against THAT file — a stale archived copy cannot vouch for a requirement the canonical spec has dropped. The fragment must still name a heading that exists. #T<n>, THE SHORTHAND NOBODY ACCEPTED portaliq writes #T3 against `### Task 3: Fail-closed trust re-checks…`, procest writes #T02 against `## 2. Controller + routes`. The task is right there; only the reference spelling is unusual. 117 findings, latent from the day they were written. Deliberately NOT wired into the positional-checkbox rule: #task-N also means "the Nth checkbox", and giving T<n> that reading would let #T99 resolve against any file with 99 checkboxes. MEASURED, full-tree, repos at origin/development portaliq 100 -> 24 doriath 79 -> 79 procest 82 -> 15 larpingapp 1 -> 1 openconnector 0 -> 0 total 262 -> 119 doriath is the control and it did not move. Its 77 findings are tags naming requirements — "Listing and download", "Masked presentation", "Lease management API" — that appear NOWHERE in the repo, in any of the three homes. Those are true positives and this change rescues none of them. Every survivor was checked by hand: procest's #T05 against a file whose sections stop at 4, its #task-2-5 against a list that runs 2.1-2.4b, portaliq's 24 deleted change dirs, larpingapp's manifest-v2-vue-scaffold which exists in no home at all. A FINDING COUNT IS NOT A DEFECT COUNT portaliq's 100 findings were 29 distinct targets — one dangling target annotated on 15 methods emits 15 lines. The summary now prints both, so the job reads as "repoint 29 targets" rather than "grind 100 tags". 13 new tests (37 -> 50), each mutation-checked: disabling the shorthand fails 8, never consulting the capability index fails 4, making the index apply always instead of last-resort fails the redirect control, and a has_anchor that returns True unconditionally fails every true-positive control in the file.
rubenvdlinde
pushed a commit
that referenced
this pull request
Aug 8, 2026
This was referenced Aug 8, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Aug 9, 2026
…ver a dead interpreter, and three could not see the defect they exist for (#280) * fix(gates 45-55): eleven gates passed over an unopened scope, eight over a dead interpreter, and three could not see the defect they exist for Every gate in this band was given ONE textbook true positive of exactly what it exists to catch, planted in a real fleet repo, then removed again. Where a gate could not fail, it was repaired; where it could, the plant is now a regression test. Measured at package sha 34370f6. ## 1. All eleven reported PASS over a scope they never opened (#242/#240/#258/#268) On a README-only diff against larpingapp, gates 45-55 printed eleven PASS lines and the summary read "53 of 53 applicable gates ran". Not one of them had opened a file. Gates 4/6/7/19/25/28/62/63 have answered the identical situation with NOT APPLICABLE since #268; this band never adopted it. Gates 47 and 48 are the sharper case: they can only answer a question about a CHANGE SET, so on every builder full-repo run in the fleet — no base ref at all — they printed a co-change verdict they had not formed. ## 2. Eight reported PASS over a crashed interpreter (#147/#249/#262) A planted defect only fires when the gate runs, so no plant can see this. With a `python3` on PATH that exits 1 on every call, on a tree carrying real findings: gate-46 PASS — over the 277 unresolved @SPEC findings, across 104 distinct targets, it had reported one run earlier on the same files gate-47 PASS — on the same diff where it had just reported FAIL gate-45/49/50 PASS (`2>/dev/null` discarded status and traceback) gate-51/54/55 PASS (`|| true` discarded the status) gate-52 FAIL — "1 custom-widget finding(s)", a fabricated finding: the helper returned its COUNT as its exit status, the same channel Python uses for a traceback (#209). The count was also clamped to 99 to fit in a byte. It now prints `findings=N` on stdout and exits boolean; no `findings=` line means the helper died. gate-54 was the quietest: its advisory WARN half reads the same log, so a dead helper silenced both halves at once. ## 3. gate-45 was the residue of #272's fix (.github#274) #272 migrated gates 35/40/42/44 off `[ -d src ]` onto `_a11y_has_markup_dir` and left the twelfth member of the family behind. On a templates-only app gate-45 reported NOT APPLICABLE — "this repo ships no frontend" — over a `<style>` block with `transition:` and no reduced-motion fallback, in the same file gate-43 FAILED on in the same run. `na` is the one verdict that removes a gate from coverage accounting. The regression test was already written and gate-45 was excluded from it by name, with a comment explaining why. Removing the name from ARM 4's skip list in test_gate_a11y_markup_scope.sh IS the test; it fails against 34370f6. ## 4. gate-47: prose satisfied it, and a qualified attribute did not `_ANNOTATION_RE` was an unanchored alternation of string literals, and it was wrong in both directions from that one regex — the pairing #269 found in gate-48 and never carried to its sibling. FALSE POSITIVE rewording ONE docblock sentence that merely NAMES the annotation ("becomes `@NoAdminRequired` again, paired with a real ownership check") made the gate demand a test co-change. A gate satisfiable by prose manufactures the appearance of a security review (#191). FALSE NEGATIVE `#[\OCP\AppFramework\Http\Attribute\NoAdminRequired]` was invisible. A commit adding exactly that to a controller — opening an admin-only endpoint to every authenticated user — with no test in the diff reported PASS. Now position-anchored, by the same rule check_csrf_removal.py already used. ## 5. gate-50: a false positive and a false negative in the same regex FALSE NEGATIVE the app-id argument had to be a QUOTED STRING, so every read written the fleet-standard way — `getValueString( Application::APP_ID, 'listing_register', '')` — was invisible. Identical code with `'larpingapp'` FAILED. Same family as #184. 7 security-relevant reads across 5 repos sit behind a constant today. FALSE POSITIVE the empty-compare guard required a closing paren immediately after the empty string, so the correct compound guard `if ($reg === '' || $sch === '')` was reported as unguarded — twice, on code the gate was asking for. A guard that is a boolean `return` rather than an `if` was rejected too. Both directions are now asserted, including the opencatalogi#86 shape that mixes them: one read guarded, the next unguarded two lines later. ## 6. gate-53 did not block the PR that creates larpingapp#286 Reintroducing #286 exactly — the check-in tab deleted from src/manifest.json, `EventRoster` left registered in src/registry.js — reported PASS. Direction 1 of the registry cross-reference stays advisory for LEGACY orphans, correctly: the gate cannot tell "wire it" from "delete it". But when the DIFF ITSELF removed the last reference it can, and that finding now blocks. Pre-existing orphans are untouched (larpingapp carries one today), so this is prevention, not a burn-down list nobody can close. ## Verified working, repaired nothing gate-46 (dangling file, dangling fragment, valid anchor), gate-48 (short and fully-qualified attribute removal; a comment reword correctly stays green), gate-49, gate-51 (title, description and nested items.properties independently), gate-52's ratchet (growth fails, shrink passes), gate-54 (flat, nested and $ref-carrying), gate-55. ## Deliberately NOT enforced `title == key` on a schema property is a real gate-51 defect — the renderer uses `prop.title || key`, so the user sees the raw technical key. Measured across 10 repos: 148 occurrences, ALL of them in softwarecatalog, where they are VNG-standardised element names (`identifier`, `type`, `name`) that must not be renamed. Enforcing it would produce 148 findings with no legitimate end state in the one repo that has them. Reported rather than gated (#252). ## Divergence to reconcile gate-45 now answers an empty in-scope set with `na`; gate-40 answers it with PASS, by a deliberate choice in #272 that cited the invariant test this PR reworks. The invariant now discriminates on the REASON — the applicability table's own phrasing must not appear once its prerequisite holds — so both behaviours are expressible. The family should pick one. ## Testing New: hydra-gates/scripts/lib/test_gate_45_to_55_acceptance.sh — 31 arms across six families, discovered by run-helper-suites.sh. Against the package as merged on main it fails 20 of 31; the 11 that pass are exactly the anti-widening and no-regression controls. Every mutation asserts its anchor is present before it plants. Repos used, chosen for different shapes: larpingapp (register-owning, manifest-driven, ships registry.js), nldesign (PHP templates, no .vue, no register), doriath (ships no phpcs SpecTagSniff — the #246 control, held at 81 findings across 46 targets before and after the plant), openconnector (41 register files). Full package suite: 52 discovered suites pass, 2 quarantined as documented; 60/60 entry-point invariants. * fix(gate-50): the fail-mode window started where the call began, not where it ended The constant-app-id fix in the parent commit made procest's config reads visible for the first time and immediately produced 3 findings on lib/Service/AiService.php — all three false positives, and both causes are ordinary code the window could never have seen: multi-line call PHPCS formats each read across five lines. Two of them plus a blank line put the guard on the ELEVENTH line, one outside a window counted from the line the match BEGAN on. The guard being missed is a textbook `if (empty($registerId) === true || empty($schemaId) === true) { $this->logger->warning(...); return; }` (AiService.php:580, :967). same-line guard `'ai_api_key_set' => ...getValueString(APP_ID, 'ai_api_key', '') !== ''` handles the empty default ON the match line, and the window started after it (AiService.php:710). The window now anchors to the END of the call expression — parentheses balanced forward from the `(` — and includes the remainder of that line. A single-line read keeps exactly the ten lines it always had. Caught by a before/after sweep of 12 fleet repos: 26 of 121 verdicts changed, 25 of them PASS -> NOT APPLICABLE (the truthfulness correction), and this was the only one that changed to FAIL. procest is PASS again, correctly. Three arms added: the multi-line shape, the same-line shape, and the reverse control — the same multi-line shape with the guard DELETED must still FAIL, so the window cannot have been widened until the gate finds nothing. Also: shellcheck SC2181 in gate-45's new status check, and a file-scoped SC2016 suppression for the acceptance suite, whose PHP fixtures are single-quoted on purpose.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 10, 2026
… 19, 13, 20 (#328) * fix(gate-45): read stylesheets — the gate had never opened a .css file Gate-45 (prefers-reduced-motion) scanned <style> blocks inside markup and nothing else, so every green it has ever produced is a statement about markup, not about CSS. In a Nextcloud app the app-wide motion lives in css/, because that is what Util::addStyle() loads. Measured before this commit: nldesign 3 stylesheets with motion, 0 guards -> gate-45 PASS openregister css/main.css, 7 motion decls, 0 guards -> gate-45 PASS The un-blinding is paired with four false-positive controls, because widening a gate fleet-wide is exactly the change that turns it into a noise generator: * the guard regex now recognises a full media prelude (@media screen and (prefers-reduced-motion: reduce)), which the old 'immediately followed by (' pattern could not have matched * comments are masked (#294's lesson), // only in SCSS dialects and never the // of a url(https://...) * transition/animation: none is how a fallback is WRITTEN, not motion * a repo-wide UNIVERSAL reset in one file guards every other file, so the gate accepts the fix people will actually write rather than reporting every other stylesheet the day that reset lands * generated output is skipped by CONTENT (a >500-char line), which catches webpack's css/main-<hash>.chunk.css that has no .min in its name Fleet-wide yield measured over 40 repos: 37 new findings, 14 of them in the GitHub fleet (nextcloud-vue 8, nldesign 3, opencatalogi 1, openconnector 1, openregister 1). The existing markup arm is unchanged at 143 findings, so nothing was widened by accident. test_gate_45_stylesheet_scope.sh proves both directions and goes 3 red against the pre-fix runner. Closes #287 * fix(gate-46): resolve @SPEC targets in tests/ — the enumerator skipped them Gate-46's scope was `find lib src`. It had never opened a test file, and tests/ is where a large share of the fleet's @SPEC tags live, because a test is the natural place to name the requirement it proves. Measured over the 21 apps carrying an openspec/: 272 unresolved targets under tests/, in 16 repos, that no run has ever reported. The textbook case is procest — tests/Unit/BackgroundJob/DsoDeadlineJobTest.php annotates openspec/changes/dso-omgevingsloket/tasks.md#T14 against a tasks.md numbering T01-T08 and V01-V10. There is no T14 and never was. The identical tag in lib/ would have failed this gate since #246. Landing as a HARD FAIL, matching the existing lib/src arm. The gate is diff-scoped under ADR-020, so pre-existing debt in an untouched test file never blocks a PR — the finding surfaces on the PR that edits that file, which is exactly when the tag should be re-checked. A named WARN would reproduce the failure mode this band exists to remove: a gate emitting something other than FAIL over a real defect. openspec/ is DELIBERATELY NOT added. Measured: 292 findings, dominated by documentation TEMPLATES that quote the syntax rather than use it (openspec/changes/{name}/tasks.md#task-N, <slug>, ...) in context-briefs and proposals across shillinq, pipelinq and others. Auditing them would bury real findings under placeholders. Correction recorded in the docblock: #322 as filed reports that tasks.md targets are never existence-checked (353 on doriath). That premise does not hold on this package. A planted openspec/changes/does-not-exist-at-all/tasks.md#task-1 IS reported as 'target file not found', and #task-99999 against a real tasks.md IS reported as 'anchor not found'. The 353 all resolve through build_archive_index, which exists for the archived-under-a-date-prefix case the issue describes, and the archived file exists at the very commit the issue measured. What produced PASS there was ADR-020 diff scoping. test_gate_46_tests_scope.sh proves both directions and goes 5 red against the pre-fix runner while its lib/ arm stays green. Refs #322 * fix(gate-19): a file no Playwright project runs cannot prove a scenario The gate counted every *.spec.ts under tests/e2e/** as a running test. Playwright does not: a file excluded by testIgnore, living outside testDir, or matched by no project's testMatch is never executed, and a scenario referenced only from such a file has no automated proof at all. The gate could already see describe.skip (#239) but not the config that does the same thing to a whole directory. Reproduced by planting an @e2e anchor in a CI-ignored directory: the uncovered count dropped 271 -> 270 with the scenario reported COVERED. The live fleet shape is openregister's tests/e2e/api-direct/search-views-presentation.spec.ts, under a **/api-direct/** that is excluded at top level AND repeated in every project, because a project-level testIgnore REPLACES the top-level one rather than merging with it. A GLOB LIST WOULD HAVE BEEN THE BUG. **/visual/** and **/docs-screenshots.spec.ts sit in a testIgnore in fourteen fleet configs and are pulled BACK IN by the visual / docs-capture projects via testMatch. Treating 'named in some testIgnore' as dead would have stripped coverage credit from every visual and docs spec in the fleet. A file is dead only when NO project would run it. Validated against all 21 real fleet configs: every one parses, and the only dead files anywhere are the api-direct trees excluded on purpose (openregister 25, openconnector 6). Visual and docs specs: 0 dead. Fleet-wide finding count: ZERO. Those api-direct files carry only prose mentions of @e2e plus one whole-spec tag with no slug, so no ref is currently claimed from an unrun file. The hole is real and proven by a planted true positive; nothing in the fleet is exploiting it today, so this lands with no churn. Conservative by construction: no config, an unparsable config, an extglob/brace pattern, a non-literal testMatch or testDir all resolve to LIVE, i.e. to the pre-existing behaviour. Comments are masked first — every fleet config explains the replace-not-merge rule in prose containing 'testIgnore:'. 11 new unit tests; the 4 that assert the fix go red against the pre-fix helper while the 7 anti-widening arms stay green. Suite: 105 -> 116. Closes #308 * fix(gate-13): see a dialog tag opened across several lines The test was grep -qE '<NcModal[ \t>/]|<NcDialog[ \t>/]'. grep matches line by line, so a tag with its props on following lines — which is how Vue components with more than a prop or two are actually written, and what every formatter produces — <NcDialog :open="showConfirm" name="Delete lead"> has nothing after <NcDialog on its own line. The character class cannot match end-of-line, so the tag was invisible. Measured on pipelinq: 0 of 9 real violations seen, while the gate passed its own planted true positive the whole time — a plant is written on one line and a real dialog is not. That is the trap: a minimal plant and a real defect differing in precisely the feature the regex depends on. The delimiter is WIDENED to include end-of-line, not dropped, so <NcDialogHeader> and <NcModalFooter> still do not match. Comments are masked first, which also removes a FALSE POSITIVE the old pattern had: it reported a <NcModal> written inside a /* */ block comment. Fleet-wide finding count: 8 -> 92 files (+84), across 10 repos — nextcloud-vue 50, procest 13, pipelinq 9, doriath 6, docudesk 4, openregister 3, softwarecatalog 3, decidesk 2, app-versions 1, hermiq 1. nextcloud-vue is the shared component library and accounts for over half; its findings are dialog components sitting outside src/dialogs/ rather than modals inlined in a parent. Gate-13 is diff-scoped under ADR-020, so none of this blocks a PR that does not touch the file. Landing as a hard FAIL, unchanged from what gate-13 already is. Comment masking suppresses zero findings across the fleet today; it is here so a <!-- <NcDialog … --> in a TODO cannot become one later, which is exactly how gate-20 acquired its commented-out call (#294). The checker now reports a crashed interpreter as wiring instead of leaving an empty log this gate would call clean (#147/#249/#262). test_gate_13_multiline_dialog.sh proves both directions and goes 3 red against the pre-fix runner while 4 anti-widening arms stay green. Closes #321 * fix(gate-20): mask comments before searching — a commented-out call is not a call The first thing the un-blinded gate reported in the fleet was not a call. It was openconnector lib/Service/SearchService.php:189: // $directory = $this->objectService->findObjects(filters: [...]); grep has no idea what a comment is, and a gate whose first live finding is false is a gate people learn to ignore. Applies the pass gate-5 received in #196: source_scope.py --mask php, which blanks //, # and /* */ while PRESERVING offsets and newlines, so the reported line number still addresses the real file. #[ is left alone — it opens a PHP 8 attribute, not a comment, and swallowing it would delete #[NoAdminRequired], the line these calls sit directly under. The log now prints the ORIGINAL source line rather than the blanked mask, so a reader sees the code that is actually at that line. The mask inherits this gate's own rule: if it cannot run, the gate reports wiring and NOT a pass. Falling back to raw text would silently restore the false positive; treating empty mask output as clean would make gate-20 green everywhere — the 2026-08-08 failure mode in a new costume. Fleet-wide finding count: 2 -> 1. The one removed is openconnector's commented-out line; the one kept is shillinq lib/Controller/BookingNotificationController.php calling findObject() on a container-resolved OCA\OpenRegister\Service\ObjectService, which is the real yield #271 identified. test_gate_20_comment_masking.sh proves both directions and goes 2 red against the pre-fix runner while all four true-positive arms — including #271's receiver anchoring and the #[Attribute] non-swallow — stay green. Closes #294 * style(test): fixed-string grep so the $ needs no escape (SC2016) * style(test): escape $ inside double quotes — SC2016 fires on single quotes --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the gate half of #228. The sniff half is being fixed in the app repos separately.
Cause
Gate-46 was a treadmill. The repo's own blocking phpcs
SpecTagSniffinstructs@spec openspec/changes/{name}/tasks.md#task-N, and a change directory is by definition temporary. 260 of portaliq's 385 tags did as told and dangled the moment their change was archived.But repointing tags at
openspec/specs/— the project rule — was not safe either, and that is the part that made this a treadmill rather than a one-off cleanup. A capability'sspec.mdoccupies three homes over its life:openspec/changes/<change>/specs/<cap>/spec.mdopenspec/changes/archive/<date>-<change>/specs/<cap>/spec.mdopenspec/specs/<cap>/spec.mdbuild_archive_indexalready carried a tag from (1) to (2), keyed on the change name. Not enough, twice over:openspec/specs/. procest carries 18 tags atopenspec/specs/process-mining-bottlenecks/spec.md, written exactly as the corrected sniff instructs, whose spec only ever existed at (2). Correct advice, dangling reference.realtime-updates-uicapability lives under the changeadopt-live-updates-ui, so no change-keyed index can reach it. 10 more findings.Keying on the capability spans all three homes and is stable across both the archive move and the promotion that may follow. That is what "a rule that survives archiving" has to mean.
Separately,
#T3/#T02— the shorthand spelling of "Task N" — had never resolved. portaliq writes#T3against### Task 3: Fail-closed trust re-checks…; procest writes#T02against## 2. Controller + routes. 117 findings, latent from the day they were written.This widens WHERE the gate looks, never WHAT counts as resolved
The capability index is consulted only after every literal spelling has failed, so a tag whose own path exists is still judged against that file — a stale archived copy cannot vouch for a requirement the canonical spec has dropped. The fragment must still name a heading that exists.
T<n>is deliberately not wired into the positional-checkbox rule:#task-Nalso means "the Nth checkbox", and givingT<n>that second reading would let#T99resolve against any file with 99 checkboxes.Measured — full-tree, repos at
origin/developmentdoriath is the control and it did not move. Its 77 findings are tags naming requirements —
Listing and download,Masked presentation,Lease management API— that appear nowhere in the repo, in any of the three homes. Those are true positives and this change rescues none of them. doriath does not even ship the sniff; its cause is different and its findings are real.Every survivor was checked by hand:
#T05— against a file whose sections stop at 4. A natural control for the new shorthand rule:#T02now resolves,#T05still fails.#task-2-5×10 — against a list that runs 2.1, 2.1b, 2.2, 2.3, 2.3b, 2.4, 2.4b. No 2.5.example-change,scaffold-v2,template-manifest-v1,portal-file-upload,portal-schema-endpoint,retrofit-2026-05-26-preferences-api: deleted, not archived, present in no home.manifest-v2-vue-scaffold, in no home at all.A finding count is not a defect count
portaliq's 100 findings were 29 distinct targets — one dangling target annotated on 15 methods emits 15 lines. That arithmetic is what drove people to grind tags one file at a time when the actual work is one repoint per target. The summary line now prints both numbers.
Tests — 37 → 50, all mutation-checked
TaskShorthandRelaxed(5) andCapabilityResolutionSurvivesArchiving(8). The latter includes the acceptance proof for #228: the same tag resolves at all three lifecycle stages, including after archiving.Mutation results:
has_anchorreturns True unconditionallybash hydra-gates/tests/run-helper-suites.sh→ 27 passed, 0 failed, 2 pre-existing quarantines.