feat(cli): add the engine-selection topic and archive add-vale-rule-engine - #95
Conversation
There was a problem hiding this comment.
Pull request overview
Adds the new engine-selection help topic (and cross-references) so agents/users can decide which rule engine (sg/vale/runtime) fits a rule request, updates the prompts export surface to include the topic, and archives add-vale-rule-engine into OpenSpec specs.
Changes:
- Add
engine-selectionembedded help topic, register it intaskless help, and add references from the routing recipes. - Export the new topic via
@taskless/cli/prompts(TOPICS) and extend tests/guards around the built prompts bundle. - Archive
add-vale-rule-engine, promoting requirements intoopenspec/specs/*and adding a changeset.
Reviewed changes
Copilot reviewed 14 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/test/prompts.test.ts | Tightens the built-prompts import graph guard to ignore obvious prose false positives. |
| packages/cli/test/help-extensions.test.ts | Extends CLI help tests to cover the new engine-selection topic and index listing. |
| packages/cli/src/prompts/index.ts | Adds engine-selection to exported prompt topics. |
| packages/cli/src/help/static.txt | Adds a link from the static recipe to engine-selection. |
| packages/cli/src/help/route.txt | Adds a link from the route recipe to engine-selection. |
| packages/cli/src/help/engine-selection.txt | Introduces the new engine-selection topic content. |
| packages/cli/src/commands/help.ts | Registers engine-selection in the help index recipe topics list. |
| openspec/specs/cli-vale-rule-engine/spec.md | Adds the promoted “Vale rule engine” capability spec. |
| openspec/specs/cli-rule-routing/spec.md | Adds engine-selection requirements to rule routing spec. |
| openspec/specs/cli-rule-format/spec.md | Adds a requirement describing Vale StyleName layout/identity. |
| openspec/specs/cli-help/spec.md | Adds requirements for registering the engine-selection topic and recipe references. |
| openspec/specs/cli-check/spec.md | Adds requirements describing concurrent multi-engine execution and merge. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/tasks.md | Marks the archived change tasks as completed and annotates the stale task line. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-vale-rule-engine/spec.md | Archive record of added requirements for the Vale engine spec. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-rule-routing/spec.md | Archive record of added routing requirements. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-rule-format/spec.md | Archive record of added rule-format requirements. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-help/spec.md | Archive record of added help requirements. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/specs/cli-check/spec.md | Archive record of added check requirements. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/proposal.md | Archived proposal for the Vale engine change. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/design.md | Archived design notes for the Vale engine change. |
| openspec/changes/archive/2026-08-11-add-vale-rule-engine/.openspec.yaml | Archive metadata for the change directory. |
| .changeset/vale-rule-engine.md | Adds a minor-version changeset for the Vale engine + new knowledge topic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
54484c4 to
77ab343
Compare
|
@claude /review |
|
Claude finished @thecodedrift's task in 4m 17s —— View job I'll analyze this and get back to you. |
…required Two gaps in the "One changeset, at the bottom of the stack" guidance, both found on the #71/#93/#94/#95 merging-down stack: - The section said where the changeset lives but never when to create it. Document the trigger: write it on the base branch before cutting the children, so every child inherits a file it can append to. - "Grow it incrementally" is merge-forward reasoning. Under merging down nothing lands until everything does, so a whole-change note is accurate at the only moment it is read. Scope the rule to forward, and state the constraint that holds in both shapes: the file must be in the bottom PR's own diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
|
Claude finished @thecodedrift's task in 5m 7s —— View job Claude finished @thecodedrift's task
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Flagging that this review produced no content — the body is just "I'll analyze this and get back to you," unchanged since it posted. So #95 has effectively had no Claude review; its only substantive feedback was Copilot's ambiguity-default finding, now fixed and resolved above. A rerun is planned once the stack below this branch finishes rebasing. Two commits since that run, both docs-only:
Note for the rerun: this branch will be rebased onto a rewritten #94 before it merges, so the SHA reviewed now will not be the one that lands. — AI Coding Agent |
01d0458 to
57e3eea
Compare
…ngine Unit 4, tasks 3.1-3.7 and 4.x, plus the archive. This completes the change. The topic answers one question — which engine enforces a rule — and is written evidence-first: state what you would have to look at, then name the engine. Naming an engine and justifying it afterwards is how prose-about-code lands in sg and cross-file questions land in vale. It carries the three boundary cases from D7 because each is a wrong answer people actually reach for. Prose about code is still prose: "comments must not say obviously" is vale, while "every export has a doc comment" is sg, and the difference is what you must read rather than what the subject is. Vale sees one document at a time, so prose consistency ACROSS documents is runtime despite being entirely about prose. And engine is not trust tier — sg and vale are both static, so conflating the axes makes a prose rule look like it needs login. Task 3.2's parenthetical was stale and is corrected in the task rather than followed: it said the Vale binary is external, but add-vale-binary-packages now ships it as an optionalDependency, so both engines are normally present. The topic states the rule as the property D7 asks for — choose an engine you know is available — which stays true on an unsupported arch and server-side alike. 3.7 puts the topic in TOPICS, in the same commit as the file, because TOPICS union INTERNAL_TOPICS must account for every recipe on disk and the test asserts it in both directions. TOPICS rather than INTERNAL_TOPICS per D7: the generator already makes this decision, today with a binary static|runtime classifier that predates Vale and is therefore wrong, and exporting the topic is what lets it render the same guidance instead of keeping a second copy. One unrelated fix the topic forced. The prompts bundle guard scans a built chunk with a regex for `from "…"`, and a built chunk embeds every recipe as a string — so the sentence `a different axis from "which engine"` failed it with `dist/prompts.js graph imports which engine`. The scan now ignores candidates that cannot be module specifiers at all. Any future recipe containing those words would have hit the same confusing failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The requirement's own title already said "an engine known to be
available", but its body and scenario both said the topic SHALL default
to `sg`. The shipped topic does the opposite on purpose: step 4 of
`engine-selection.txt` says to choose an engine you know is available
and explicitly declines to name one, and `help-extensions.test.ts:248`
pins that ("states the ambiguity default as a property, not as `sg`"),
citing D7.
So the spec was the only artifact carrying the old wording, and it
contradicted the text, the test, and the design decision at once. State
the requirement as the property the title always claimed, and say why a
named default cannot be right: either binary can be the missing one on
an unsupported arch, and server-side `sg` is the only ungated route, so
a fixed fallback is wrong in whichever case it did not anticipate.
Both copies updated -- the promoted spec and the archived delta.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The verify implementation now refuses to report a rule verified unless both `pass/` and `fail/` hold a document, but the spec was silent on an unpopulated bucket -- it only said what must happen for the fixtures that exist. That silence is what let the code and the spec drift: a rule with only `pass/` fixtures satisfied every stated assertion while proving nothing, passing on an empty set of expected failures. State the requirement the code enforces, and why: a `fail/` fixture proves the rule fires, a `pass/` fixture proves it does not over-fire, and either alone is half a claim. The report must also keep a half-written fixture set distinct from a rule with no fixtures at all, which is the distinction `ValeFixtureCoverage` encodes. Recorded here rather than on the branch that implements it: this file is `git mv`'d into the archive by this PR, so an edit one branch down would be dropped or conflict when that move replays during the rebase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
…ng it The test asserted that the built prompts entry never reaches the CLI entry or a host capability, and established that by regex-scanning built JavaScript for `from "..."` to reconstruct the import graph. A built chunk embeds every recipe as a string literal, so the engine-selection recipe's `a different axis from "which engine"` was read as an import and reported as `dist/prompts.js graph imports which engine`. Filtering candidates by specifier shape only narrowed the accident. It passed because that phrase contains a space; `differs from "static-tier"` is a bare hyphenated name with no whitespace and would have been reported as an import all the same. Whether the guard holds should not depend on how a recipe author punctuates. Rollup already knows the answer. `OutputChunk.imports`/`dynamicImports` is the resolved graph, so a plugin walks it from the prompts entry and fails the build on any external import or any path reaching the bin entry. Verified both ways: the build passes as-is, and adding a `node:fs` import to `src/prompts/index.ts` fails it with "prompts entry graph imports node:fs". Checking this in the build rather than over the artifact is the point. A build that refuses to emit a leaking bundle makes the bad artifact unproducible; a test that inspects one afterwards only notices, having already written it to disk. It also avoids acquiring a parser dependency to re-derive information the generator had. The source-level allowlist stays a test: it constrains what we wrote rather than what the build produced, and a regex is sound over hand-written modules that embed no prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The `cli_help` capture path in `help.ts` is generic over the topic string, so this was never a functional gap -- just a table that listed every routing topic except the one this change adds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
0726c6d to
f926791
Compare
Stack (root → tip):
Unit 4 of
add-vale-rule-engine— the tip. Stacked on #94, merging down. Tasks 3.1–3.7 and 4.x, plus the archive. This completes the change.The topic
Answers one question — which engine enforces a rule — and is written evidence-first: state what you would have to look at, then name the engine. Naming an engine and justifying it afterwards is how prose-about-code lands in
sgand cross-file questions land invale.It carries D7's three boundary cases because each is a wrong answer people actually reach for:
vale; "every export has a doc comment" issg. The difference is what you must readruntime, despite being entirely about prosesgandvaleare both static — conflating the axes makes a prose rule look like it needs loginA stale task line, corrected rather than followed
Task 3.2 said to note that "the Vale binary is external, so
sgsatisfies [availability] locally". That is no longer true —add-vale-binary-packagesships Vale as anoptionalDependency, so both engines are normally present. The topic states the rule as the property D7 asks for — choose an engine you know is available — which stays correct on an unsupported arch and server-side alike. The task line is annotated with why it wasn't followed verbatim.3.7 ships in the same commit as the topic file
TOPICS ∪ INTERNAL_TOPICSmust account for every recipe on disk, and the test asserts it in both directions — so a recipe added without a classification turns the suite red. It goes inTOPICSrather thanINTERNAL_TOPICSper D7: the generator already makes this decision, today with a binarystatic | runtimeclassifier that predates Vale and is therefore wrong. Exporting it is what lets the generator render the same guidance instead of keeping a second copy free to drift.One unrelated fix the topic forced
The prompts-isolation guard scans a built chunk with a regex for
from "…"— and a built chunk embeds every recipe as a string literal. The sentencea different axis from "which engine"failed it withdist/prompts.js graph imports which engine. The scan now ignores candidates that cannot be module specifiers at all (verified that every real specifier shape —node:fs,@clack/prompts,../scan— still passes through). Any future recipe containing those words would have hit the same baffling failure.Archive
openspec archivemoved the change toarchive/2026-08-11-add-vale-rule-engineand promoted 16 requirements intoopenspec/specs/, including the newcli-vale-rule-enginecapability.openspec/changes/is clean, so the archive gate passes on this tip.The changeset lands here too and flows down to the bottom branch, which is the one that merges to
main.Verification
pnpm --filter @taskless/cli test→ 531 passed; lint, typecheck, prettier clean.openspec validate --specs --strictpasses for every spec this change touches —cli-rulesandcli-update-enginefail, but they fail identically with this work stashed and neither is touched here.Before merging the stack
#71still carriesskip-changeset. It was accurate when unit 1 shipped no user-visible behaviour; the stack now ships a minor feature, and the label must come off before the bottom PR merges tomainor the release is recorded as shipping no note.Refs OSS-21