|
| 1 | +--- |
| 2 | +"@objectstack/cli": minor |
| 3 | +--- |
| 4 | + |
| 5 | +fix(cli): every author-time rule that can gate runs on all three commands (#4409) |
| 6 | + |
| 7 | +`os validate`, `os build` and `os lint` each hand-wired their own subset of the |
| 8 | +author-time rules. Nothing connected the three lists, so "which rules run here?" |
| 9 | +was answerable only by diffing three 800-line files by eye — and the answer |
| 10 | +drifted every time a rule landed. The audit found 23 of 26 rules running on some |
| 11 | +strict subset, nine of them able to emit `error`. |
| 12 | + |
| 13 | +The worst direction was the least obvious. `os build` — the command that |
| 14 | +PUBLISHES — was the weakest gate of the three: a flow whose expression approver |
| 15 | +does not parse (`approval-expression-invalid`) built and published green, and |
| 16 | +only `os lint` stopped it, while CI usually runs the other two. `os lint` |
| 17 | +disagreed in *both* directions at once, running one gating rule neither other |
| 18 | +command ran and missing six that both of them ran, which is worse than no |
| 19 | +pre-flight — the remaining options are re-verifying everything or learning to |
| 20 | +distrust the signal. |
| 21 | + |
| 22 | +This is the same failure mode's fifth appearance (#3583, #3782, #4384/#4394, |
| 23 | +#4402). Each earlier repair removed an instance and left the MODE: a rule's |
| 24 | +command coverage was whatever its author remembered to type, and forgetting was |
| 25 | +silent. #4402's guard could not catch the rest — it filtered on the current |
| 26 | +member names of one suite, so a rule hand-wired into two commands from outside |
| 27 | +that suite passed it without a word. A name list only guards the names on it. |
| 28 | + |
| 29 | +**The registry.** `AUTHORING_RULES` declares all 26 rules as data: tier |
| 30 | +(`gating`/`advisory`), which stack tier they read (pre-parse `normalized` vs |
| 31 | +`parsed`), which commands run them, and a written reason for the one narrowing. |
| 32 | +All three commands consume it through `runAuthoringRules()`, so adding a rule is |
| 33 | +a one-line edit that reaches every command at once. The three command files |
| 34 | +shrink by ~1000 lines between them. |
| 35 | + |
| 36 | +**The ratchet.** The wiring guard is no longer a name list: a `gating` rule on |
| 37 | +fewer than three commands fails, a narrowed rule with no reason fails, a command |
| 38 | +that calls or imports a registry rule directly fails, and an `advisory` claim is |
| 39 | +checked against the rule's own source — so a gate cannot wear an advisory label |
| 40 | +to buy itself partial coverage. That last check is the one #3760 needed, having |
| 41 | +promoted a `lintFlowPatterns` rule from advisory to gating with nothing anywhere |
| 42 | +asking whether its coverage should follow. Remaining direct calls are listed |
| 43 | +with reasons, and a stale entry fails too, so the ratchet cannot rot into a |
| 44 | +permanent permission slip. |
| 45 | + |
| 46 | +**The verdict, not just the wiring.** A separate test plants one defect per |
| 47 | +previously-blind gating rule and asserts all three commands gate on it, plus the |
| 48 | +issue's own repro driven end-to-end through the real CLI: exit 1 on all three |
| 49 | +where it was 1/0/0. |
| 50 | + |
| 51 | +Two behaviour changes fall out of reporting every failing rule in one run |
| 52 | +instead of exiting at the first failing gate: an author with three unrelated |
| 53 | +problems now sees all three in one pass, and `--strict` covers every advisory |
| 54 | +rather than the roughly half that happened to be printed inline. |
| 55 | + |
| 56 | +Also closes the same hole one gate over: `collectAndLintDocs` failed `os build` |
| 57 | +and never ran on `os validate`, invisible because the parity guard keyed on the |
| 58 | +`lint*`/`validate*` naming convention and that gate is called `collect*`. The |
| 59 | +guard now names each shared non-registry gate explicitly instead of |
| 60 | +pattern-matching for them. |
| 61 | + |
| 62 | +Cost is not what argued against any of this. The heavy dependencies |
| 63 | +(`typescript` ~9 MB, `sucrase`) are already lazy and load only when a stack |
| 64 | +carries the metadata that needs them, and the heaviest rule of the set has run |
| 65 | +on all three commands as a reference-integrity suite member since #4340 without |
| 66 | +anyone noticing. The one narrowed rule, `lintUniqueDeclarations`, is scoped |
| 67 | +because `os lint` already reports it through `lintDataModel` — coverage |
| 68 | +recorded, not coverage missing. |
0 commit comments