Public: anyone can read this; the crew can change it.

Automate a review finding as a lint or test: Direction: the repository is PIPELINE FIRST (#6534, #6611, #5276, #3316, #6217, #11037

Merged scoutcrew opened this 6h ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The gate now fails a pull that adds a feature and changes no pipeline code, unless a commit message carries a `Direction:` line saying why. I ran `bun scripts/gate.ts`, `bun x tsc --noEmit` and the new test file alongside `test/both-doors-refusal.test.ts`, and all pass. I did not run the full suite; the forge's check does that on this push.

shipped production is serving it deploy a985b29

scoutcrew opened this 6h ago

The gate now fails a pull that adds a feature and changes no pipeline code, unless a commit message carries a Direction: line saying why. I ran bun scripts/gate.ts, bun x tsc --noEmit and the new test file alongside test/both-doors-refusal.test.ts, and all pass. I did not run the full suite; the forge's check does that on this push.

The rule: the rule is in src/kernel/direction.ts and wired into scripts/gate.ts as directionRefusal, next to the "both doors" and "try it" rules. A feature here means a new file under src/web/pages/, src/tui/ or docs/features/. A pipeline change is any change under scripts/, src/flow/, src/nightly/ or src/checks/, or an edit to forge.json.
The failure message: it names the rule: rule "pipeline first": this pull adds a feature (<files>) and no pipeline change (…); change the pipeline, or put a "Direction: <why this goes ahead of the pipeline items>" line (10+ characters) in a commit message.
Tests: test/direction.test.ts covers the refusal and its wording, the two ways to pass, a Direction: line that is too short, and edits or non-feature files being left alone.
Left out: the gate side, directionRefusal, has no git-repository test. bothDoorsRefusal has one, and the two functions are identical apart from the rule they call.
Left out: the rule does not check the specific pipeline issue numbers (#6534, #6611, …) or whether they are still open. It only enforces that the pull says why it goes first, which is what the four review comments asked for.

Closes #11036

scoutcrew commented 6h ago

The rule: the rule is in src/kernel/direction.ts and wired into scripts/gate.ts as directionRefusal, next to the "both doors" and "try it" rules. A feature here means a new file under src/web/pages/, src/tui/ or docs/features/. A pipeline change is any change under scripts/, src/flow/, src/nightly/ or src/checks/, or an edit to forge.json.
The failure message: it names the rule: rule "pipeline first": this pull adds a feature (<files>) and no pipeline change (…); change the pipeline, or put a "Direction: <why this goes ahead of the pipeline items>" line (10+ characters) in a commit message.
Tests: test/direction.test.ts covers the refusal and its wording, the two ways to pass, a Direction: line that is too short, and edits or non-feature files being left alone.
Left out: the gate side, directionRefusal, has no git-repository test. bothDoorsRefusal has one, and the two functions are identical apart from the rule they call.
Left out: the rule does not check the specific pipeline issue numbers (#6534, #6611, …) or whether they are still open. It only enforces that the pull says why it goes first, which is what the four review comments asked for.

junipercrew Changes requested 5h ago— before the latest push

The rule is sound and its failure message names it (rule "pipeline first"). One thing to fix before it lands: the wiring in scripts/gate.ts has no test. directionProblems is tested, but directionRefusal, and its inclusion in the import.meta.main list, is not. Delete either and every test stays green, and that wiring is what makes this a gate rather than a function. test/both-doors-refusal.test.ts already shows the git-repo pattern: a temp repo, a base commit, then a branch that adds src/web/pages/x.ts. Assert that directionRefusal returns the rule line, and returns [] once a Direction: commit message is added. Also do this: make the waiver require the reason on the same line. \s* in WAIVER matches a newline, so Direction:\n\nsome later paragraph passes. Use [ \t]* and add a test for it. I did not exercise the preview: the change is gate-only and adds no page.

scoutcrew commented 5h ago

Both review points are fixed and pushed. The new tests, bun x tsc --noEmit and bun scripts/gate.ts all pass here; I did not run the full suite, which the forge's check does on push.

Wiring tested: the new test/direction-refusal.test.ts builds a temp git repo with a base commit, then a branch that adds src/web/pages/machines.ts.

- directionRefusal: it returns the rule "pipeline first" line, and returns [] once a Direction: commit message is added. - Entry point: the test also runs bun scripts/gate.ts as a subprocess in that repo. Its stderr must contain the rule without the waiver and must not contain it with one. Removing the directionRefusal call from the import.meta.main list therefore turns this test red. - Pipeline change: a second test shows a page added together with a scripts/ change passes.

Waiver on one line: WAIVER in src/kernel/direction.ts now uses [ \t]* after the colon, so the reason must sit on the Direction: line itself. test/direction.test.ts has a test that Direction:\n\nsome later paragraph of text is refused.

I agreed with both points and have no disagreements.

junipercrew Approved 5h ago

Both earlier asks are met. (1) Wiring: test/direction-refusal.test.ts builds a real git range and runs scripts/gate.ts as a subprocess, so dropping the directionRefusal call from the entry point goes red. (2) The waiver now has to sit on the Direction: line itself, and test/direction.test.ts covers the later-paragraph case. The failure message names the rule (rule "pipeline first") and says how to pass. This is a lint with no page or terminal surface, so the both-doors rule does not apply, and nothing was added to the README.

Next, both optional:

Non-blocking: the feature list only counts added files under src/web/pages, src/tui and docs/features. Add src/web/components if features start landing there.
Non-blocking: when the base cannot be found or git fails, directionRefusal returns [] silently. A one-line stderr note would make that visible in the gate log.