docs(openspec): propose the agent command and the Vale authoring path - #102
Conversation
`add-vale-rule-engine` shipped a chooser without a destination. `engine-selection` teaches an agent to conclude a rule belongs to `vale`, and then there is nowhere to go: `static.txt` is ast-grep authoring, and its only mention of Vale tells the reader to confirm `sg` was right. The `runtime` answer dead-ends the same way. The exclusion was recorded as a deliberate non-goal at the time, which was defensible while nothing chose Vale -- the chooser is what makes the gap reachable. The same work exposed that the surface is shaped for a human. `help` names the command after a human's reason for typing it, and longform addressing makes an agent know both the words and their order. Single hyphenated tokens read as literal strings to copy rather than phrases to paraphrase, which is the failure this surface cannot afford. Proposes: `help` -> `agent`, flattened single-token topics, `static` -> `create-sg-rule` and `existing` -> `create-legacy-rule`, new `create-vale-rule` and `create-runtime-rule`, `route` merged with `engine-selection` into one front door, and a section-less scaffold. Two decisions carry the most weight. Merging `route` and `engine-selection` removes a second fetch and a handoff to answer one question, but it deletes a topic the platform generator consumes -- so the criterion distributes to the destinations, each `create-*-rule` stating the evidence that makes its engine right. That is what keeps it exportable, since a consumer with no `route` step and no `detect --json` could never use a chooser anyway. And the section-less scaffold ships only paired with surfacing Vale's stderr on a zero-exit run: with no section to copy, the likely first edit is a rule assignment at top level, which Vale ignores and reports on stderr, which we currently discard -- reintroducing the silent disable one level up. Pre-1.0, every backwards-incompatible item here is a MINOR bump. Delivery shape: single PR stacked on #100. A half-renamed command surface is not shippable, since recipes cross-reference each other by literal command string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
…mote-rule Settles the two open questions. `route` reads login state early, because it determines which destinations exist and discovering it late means classifying against the wrong set. It does not follow that the agent should open by asking "do you want remote generation?" -- at that point neither it nor the user knows whether the rule is a two-line pattern or something local authoring cannot express, so the question costs a turn and cannot be answered well. Service generation is offered where it is genuinely a choice: locally expressible AND logged in. Not logged in, or not locally expressible, are not choices and are not posed as one. That narrows the biased-local requirement rather than reversing it. The bias survives for the case it was written about -- local authoring that works is not abandoned for the service -- while a logged-in user stops being steered away from a path they already pay for. No recipe delegates to another. A logged-in runtime request routes straight to `create-remote-rule`; `create-runtime-rule` becomes the logged-out path and owns the gate explanation. Routing runtime through a topic that forwards would reintroduce the second fetch D1 removes, and split one explanation across two files. `remote.txt` and `rule-create.txt` merge into `create-remote-rule` -- a content merge, not a rename, since split across a boundary statement and a procedure an agent fetches one only to learn it needs the other. Adds a verification step worth more than prose review: rehearse each recipe against a subagent with no prior context, ask what steps it would take, and check that account against what the recipe intends. A recipe that reads correctly to its author and produces the wrong plan is exactly what reading it again cannot catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Closes both open questions, and corrects an overclaim in D1 while doing it. D1 justified merging the chooser partly on the criterion distributing to the destinations, each stating the evidence for its own engine. That is the same duplication the merge exists to remove, one level down: five copies of one test, and the first edit to any of them diverges silently. The criterion now lives once, in `route`'s destination table, which is where the comparison is actually made. Destinations orient instead. Each opens with a fixed line -- what topic this is, what it helps you write, revisit routing if that is not what you need. Its job is self-correction: an agent that arrived wrongly, by guessing or by a user naming a topic directly, should find out in the first line where recovery is cheap, not after authoring the wrong artifact. Scope only, never the comparison. The cost is worth stating rather than hiding. A consumer reading only `create-vale-rule` gets its scope but not the boundary cases that settle hard calls. Enough to pick between destinations; not enough to adjudicate an ambiguous rule. If the generator needs the full test it consumes `route` or keeps its own classifier -- recorded as the one consumer this change leaves with less than it had, rather than papered over by copying prose into five files. `create-runtime-rule` points at `auth` rather than restating login. An extra CLI turn is cheap when each turn delivers something concrete, and a copy of the login procedure inside a rule-authoring recipe goes stale the first time login changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
…oute claim D1 asserted that exporting `route` is wrong because it is built on local mechanics. That is too strong: `route` is expected to be exported in a later change. It is not exported *here* because its local mechanics need separating from its reasoning first, and doing that inside a change that already renames a command and five topics is how a rename becomes unreviewable. Recorded as deferred rather than rejected. That also closes the last open question. The gap this change leaves -- destinations carry scope, not the boundary cases -- closes when `route` is exported: the service holds the route prompt, needs no escalation path because it is the escalation, and supplies its own runtime prompt for its own agentic flow. Worth stating why the prompts are exported at all, since it changes what "enough" means. The goal is consistency between the local and remote paths, not transferring a capability the service lacks. The service can classify without us; what it must not do is classify differently. A rule routed to `vale` locally and `sg` server-side is one request answered two ways, and that is what the shared surface exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The subcommand is named for its reader: it serves agents fetching a procedure, not humans asking for help. Topics are now addressed by exactly one token. Joining positionals made `taskless help rule create` resolve `rule-create.txt`, which invited an agent to reorder or paraphrase a topic name and still get a hit. A single hyphenated token is a literal string to copy, so extra positionals are an error rather than something to guess at. The `cli_help` telemetry event name is deliberately unchanged — dashboards key on it, and renaming it alongside the upcoming `TOPICS` break would take them dark for an unrelated reason. Recipe text still says `taskless help`; that sweep is a later task group. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The recipes are the deliverable of this change, and the failure mode is one review cannot see: prose that reads correctly to its author and produces the wrong artifact. So execute them instead of reading them. A fresh subagent gets the recipe text, a sandbox path, and a rule intent in plain words -- and no repository access, because with it the agent finds the existing `no-simply.yml` and copies it, and the loop tests our fixtures rather than our writing. Then check the artifacts mechanically and, the part that actually decides it, whether `check` reports the finding and `verify` passes. `build:dev` is the right target rather than `dist/`: it bakes an absolute path into the recipe text, so the agent reads the command it will really run. Against `dist/` we would be testing a recipe no reader receives. Three intents, not one. Everything in this repo today is `existence`, so a recipe drafted from our own examples teaches token blocklists -- and Vale has eleven extension points, most prose rules being substitutions or capitalization. `create-sg-rule` runs through the same harness as a control: it documents a flow that already works, so a failure there means the harness is wrong rather than the recipe. Converged when an agent, given an intent the recipe never names, produces a rule that fires on its fail fixture and stays quiet on its pass fixture first try. The iteration log is kept, because it is the only evidence a reviewer can check without rerunning the loop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Handoff for a context reset. `tasks.md` is the authority on what is done; this records what a fresh reader cannot recover from the artifacts -- which worktree the work lives in, the environment traps that each cost an hour last session, the Vale facts already measured so they are not re-derived, and the landmines group 2 walks into. Points at the two decisions most likely to be re-litigated by accident: the engine criterion is stated once in `route` rather than copied into the destinations, and the section-less scaffold ships paired with stderr surfacing or it reintroduces the silent disable the Vale work exists to remove. Not part of the artifact set; delete it when the change is archived. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Group 2 of agent-command-and-vale-authoring: the recipes themselves. `route` absorbs `engine-selection` (D1). One fetch now goes from a rule-authoring request to a runnable command, with the engine criterion stated once, in route's destination table, and the runtime row split on login state (D7). Destinations open with a fixed orientation banner that self-corrects a misrouted reader without restating the criterion (D9). Topics flatten to verb-noun single tokens. `static` -> `create-sg-rule` also drops a leak: "static" is a trust tier, not an engine, and naming the ast-grep path after the tier taught the confusion the criterion exists to correct. `remote` + `rule-create` merge into `create-remote-rule` (D8); `rule-create.anonymous` is deleted rather than renamed, since it duplicated `static.txt` and a "local variant of the remote recipe" is the contradiction `route` resolves. Two recipes are new. `create-vale-rule` teaches the three artifacts, and above all the section: the scaffold ships section-less, so the first rule in a project also writes the first scope, and an assignment outside a section is ignored rather than rejected. `create-runtime-rule` is the logged-out path, explaining the gate as a property of executing code and deferring to `auth` (D6, D10). Groups 5.1 and 5.2 land here rather than later, deliberately: they are preconditions of testing the Vale recipe honestly. The scaffolded .vale.ini drops its `[*]`, and runVale surfaces a zero-exit stderr as a notice so the mistake that scaffold invites is legible instead of silent. Measured end to end — a top-level `rules.<id> = YES` now prints Vale's W101 and still exits 0. Also corrects three spec deltas that still referenced `engine-selection` as a live topic, and re-homes the two engine-reasoning requirements that survive the merge onto `route`. 572 tests pass; typecheck and lint clean.
Run B (substitution) converged first try and independently confirmed run A's three worst findings. Its most valuable addition: both `tokens` and `swap` keys are Go RE2 patterns presented in the recipe as literal strings, and a wrong guess there fails as a silent non-match — the exact failure the recipe spends a section warning about. Also records one finding rejected as a harness artifact, so a later reader does not "fix" build:dev's absolute-path rewriting.
Round 1 of 2b: three sandboxed agents authored a real Vale rule from the rendered recipe with no repository access. Two converged first try; the third took a retry and found the two defects that mattered. The field table was factually wrong. It said `%s` interpolates the match; for `substitution` a single `%s` interpolates the *replacement*, so "Use GitHub not %s" against "Github" rendered "Use GitHub not GitHub". This is the worst kind of defect — it passes every check the recipe tells you to run, and only a human reading the message sees it. The extension-point table also pointed product names at `capitalization`, which applies `match` to a whole scope and cannot express "this word, wherever it appears". Measured, it flags entire sentences. Product-name spelling is a `substitution`. Beyond those: tokens and swap keys are Go RE2 patterns presented as literals (no lookaround, implicit word boundaries, live metacharacters, first-wins on overlap); `success` and the exit code do not answer the question the fixtures ask, only `results[].ruleId` does; `check <path>` is not scoped to the rule under test; MinAlertLevel was missing from the debug ladder, which had no branch for the over-broad case at all; and the pass bucket is near-misses, not correct prose. Claims of CLI enforcement are removed rather than softened. Nothing validates the fixture layout today, and saying otherwise taught a reader to rely on a check that does not run. One agent recommendation was rejected on measurement: `ignorecase: true` does not flag text already equal to its replacement, so the proposed warning would have been false. 572 tests pass. Round 2 is running against the revised text.
Both round-2 agents converged first try, including `capitalization` on the use case round 1 got wrong — so 2b.6 is met across three extension points. They still found nine gaps, three of them in prose written during round 1's revision. The biggest: the field table listed the five fields every rule shares and omitted every field a rule actually needs. `tokens`, `swap`, `match` and `exceptions` appeared only inside examples, so the one field run E's rule depended on was undocumented. There is now a per-extension-point field table. `scope` was likewise an "e.g." with two values for the field that decides where a rule looks; all sixteen markdown scopes are listed. Measured and written down rather than guessed: `match: $sentence` means first word capitalized and everything else lowercase, proper nouns included, so `exceptions` is load-bearing — `Getting started with Kubernetes` fires unless Kubernetes is listed, while an exception's plural and a leading exception are both fine. Link text is prose and the URL is not, so `scope: link` narrows to it. Word boundaries wrap the whole pattern, multi-word included. Two defects were my own from the last pass. A nested-fixture warning described "counting against either bucket", machinery no reader has been shown and a leftover from assuming verifyValeRule was reachable. And the `BasedOnStyles =` rationale claimed it stops a later edit from enabling a style, which is not a mechanism — replaced with the honest reason. 572 tests pass.
Both harness rounds converged, but every run invented its rule shape from three examples, and the recurring note in all five reports was "I guessed and happened to be right". The recipe explained the mechanics well and showed almost nothing. Nine worked rules, one per extension point covered, each paired with the near-miss that fails and why. The YAML blocks were extracted from the *rendered* recipe and executed verbatim, so what ships is what was tested rather than something adjacent to it — all nine fire with the expected message, and the negative cases stay quiet (the correct `GitHub` is not flagged; `scope: link` spares the same phrase in prose; a defined acronym licenses its later uses). Writing them surfaced a failure mode no agent hit: an unquoted `[^\s]+` in a repetition rule's tokens matches nothing — no error, no diagnostic, zero findings. It is a YAML escaping problem that presents exactly like a Vale scoping problem, and it is now example 7's "goes wrong". Three others are behaviors an agent would not guess: `consistency` enforces internal consistency rather than picking a winner, `occurrence` counts per scope, and `conditional`'s first/second invert the rule when swapped. 572 tests pass; lint clean.
`verifyValeRule` has existed since the Vale engine landed and had no CLI caller — `rule verify <id>` routed to the ast-grep verifier only, so there was no way to verify a Vale rule from the CLI. Every harness run noticed, because the recipe had to describe a layout nothing enforced. Baking it in now rather than later: once people write Vale rules against an unenforced layout, changing it is a migration. `rule verify <id>` now dispatches on which engine owns the rule, decided by where the file sits — the same rule `dispatch` follows, so a rule cannot be verified by one engine and run by another. An id present under both engines is an error naming both paths rather than a silent pick. The two reports stay separate shapes behind an `engine` discriminant. ast-grep verification is three layers over a rule and its test cases; Vale verification is one question asked of two fixture buckets. Mapping the second onto schema/requirements/tests would invent two empty layers and lose `fixtures`, which is the field that catches a rule that was never really verified — one bucket proves half of what a rule needs. Enforced, measured end to end: both buckets populated or it fails without running Vale; every fail/ document must fire; no pass/ document may; a nested fixture directory is rejected by name rather than skipped. Adds ENGINE_UNAVAILABLE to the error envelope, kept apart from SCAN_FAILED because 'install something' and 'fix something' are different asks. 8 new tests, 580 passing.
|
@claude /review |
|
Claude finished @thecodedrift's task in 5m 55s —— View job Review:
|
There was a problem hiding this comment.
Pull request overview
This PR renames the CLI’s agent-facing knowledge surface from taskless help to taskless agent, switches topic addressing to single-token names, and fills the previously missing Vale authoring destination by adding new authoring recipes plus related CLI/test updates. It also adjusts Vale scaffolding and runtime behavior (stderr-on-success surfaced as a notice) and extends rule verify to dispatch by owning engine (ast-grep vs Vale) to make the authoring/verification loop reachable for both engines.
Changes:
- Replace the
helpsubcommand withagent, remove positional-join topic resolution, and update telemetry/tests accordingly. - Add/rename rule-authoring recipes (
create-sg-rule,create-vale-rule,create-runtime-rule,create-remote-rule,create-legacy-rule) and merge engine-selection reasoning intoroute. - Extend rule verification to route by engine ownership, add Vale verify output shape, and surface Vale stderr diagnostics as non-blocking notices; update OpenSpec change docs/specs.
Reviewed changes
Copilot reviewed 43 out of 47 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/test/rule-verify-dispatch.test.ts | Adds coverage for rule verify dispatch by owning engine and Vale fixture validation behaviors. |
| packages/cli/test/prompts.test.ts | Updates prompt/topic typing and parity checks to the renamed topics and agent command. |
| packages/cli/test/onboard.test.ts | Updates onboarding test expectations to use taskless agent recipes. |
| packages/cli/test/help-telemetry.test.ts | Switches telemetry coverage from help command module to agent command module and updates topic capture expectations. |
| packages/cli/test/help-routing-telemetry.test.ts | Adjusts routing telemetry tests for the new command (but still referenced removed topics; flagged in comments). |
| packages/cli/test/help-extensions.test.ts | Updates behavioral tests for topic listing/resolution, removed-topic behavior, and new routing/authoring topics. |
| packages/cli/test/cli.test.ts | Updates “no args” behavior expectation to reflect agent index output. |
| packages/cli/test/cli-run.test.ts | Updates resolveCommandName coverage to treat agent like the old help (command name only). |
| packages/cli/test/anonymous-flag.test.ts | Updates anonymous-flag guidance strings to point to new taskless agent … topics. |
| packages/cli/src/types/errors.ts | Adds ENGINE_UNAVAILABLE error code to distinguish missing engine binaries from execution failures. |
| packages/cli/src/telemetry-run.ts | Updates command-name derivation docs/behavior from help to agent. |
| packages/cli/src/schemas/rules-verify.ts | Adds an engine discriminant to sg verify output and introduces a distinct Vale verify output schema. |
| packages/cli/src/rules/vale/run.ts | Captures Vale stderr on zero-exit runs and returns it as an optional notice on ok outcomes. |
| packages/cli/src/rules/owner.ts | New helper to determine rule ownership by rule file location and format actionable paths for errors. |
| packages/cli/src/rules/dispatch.ts | Forwards Vale “notice” (stderr diagnostics on success) through dispatch results without affecting exit codes. |
| packages/cli/src/prompts/recipes.ts | Renames schema-bearing topics (rule-create/rule-improve → create-remote-rule/improve-rule). |
| packages/cli/src/prompts/index.ts | Updates exported TOPICS/INTERNAL_TOPICS to match the renamed authoring surface and removed engine-selection export. |
| packages/cli/src/index.ts | Registers agent subcommand and updates non-interactive routing from help to agent. |
| packages/cli/src/help/verify-rule.txt | Adds a verify recipe (but needed updates to match single-token topic naming and new output shape; flagged in comments). |
| packages/cli/src/help/static.txt | Removes the old static authoring recipe (replaced by create-sg-rule). |
| packages/cli/src/help/rule-create.txt | Removes old service-generation recipe (merged into create-remote-rule). |
| packages/cli/src/help/rule-create.anonymous.txt | Removes old anonymous create recipe (folded into create-sg-rule guidance). |
| packages/cli/src/help/route.txt | Rewrites routing to classify into the new destinations and embeds engine reasoning directly. |
| packages/cli/src/help/remote.txt | Removes old “remote boundary” recipe (merged into create-remote-rule). |
| packages/cli/src/help/improve-rule.txt | Adds API-backed improve recipe (but had stale taskless help references/topic header; flagged in comments). |
| packages/cli/src/help/improve-rule.anonymous.txt | Adds anonymous/local improve recipe (but had stale taskless help references/topic header; flagged in comments). |
| packages/cli/src/help/engine-selection.txt | Removes standalone engine-selection recipe (merged into route). |
| packages/cli/src/help/delete-rule.txt | Adds delete recipe (but had stale taskless help references/topic header; flagged in comments). |
| packages/cli/src/help/create-vale-rule.txt | Adds comprehensive Vale authoring recipe (rule + .vale.ini scoping + fixtures). |
| packages/cli/src/help/create-sg-rule.txt | Adds renamed local ast-grep authoring recipe replacing static. |
| packages/cli/src/help/create-runtime-rule.txt | Adds logged-out runtime authoring/gate explanation recipe. |
| packages/cli/src/help/create-remote-rule.txt | Adds merged, single-procedure service-generation recipe. |
| packages/cli/src/help/create-legacy-rule.txt | Renames/retitles legacy-linter authoring recipe (was existing). |
| packages/cli/src/filesystem/migrations/0004-vale-engine.ts | Makes scaffolded .vale.ini section-less to avoid unscoped default linting. |
| packages/cli/src/commands/rules.ts | Dispatches rule verify by owning engine; adds Vale verification command path and JSON output support. |
| packages/cli/src/commands/agent.ts | Implements agent command (index + single-token topic resolution + telemetry), replacing old help behavior. |
| openspec/changes/agent-command-and-vale-authoring/tasks.md | Plan/checkboxes for this OpenSpec change set. |
| openspec/changes/agent-command-and-vale-authoring/specs/cli-vale-rule-engine/spec.md | Specifies section-less scaffold + stderr-on-success notice requirements for Vale. |
| openspec/changes/agent-command-and-vale-authoring/specs/cli-rule-routing/spec.md | Specifies merged routing+engine reasoning and renamed destinations. |
| openspec/changes/agent-command-and-vale-authoring/specs/cli-knowledge-prompts/spec.md | Specifies topic export surface stability and pre-1.0 MINOR semantics. |
| openspec/changes/agent-command-and-vale-authoring/specs/cli-help/spec.md | Updates spec to agent command semantics and no positional-join behavior. |
| openspec/changes/agent-command-and-vale-authoring/specs/cli-agent-authoring/spec.md | Adds requirements ensuring each engine has a reachable authoring recipe. |
| openspec/changes/agent-command-and-vale-authoring/resume.md | Handoff notes for continuing the change set work. |
| openspec/changes/agent-command-and-vale-authoring/proposal.md | Proposal narrative for the rename + Vale authoring destination. |
| openspec/changes/agent-command-and-vale-authoring/iteration-log.md | Execution log of recipe harness runs used to validate authoring prose. |
| openspec/changes/agent-command-and-vale-authoring/design.md | Design decisions (D1–D10) supporting the new command/topic model. |
| openspec/changes/agent-command-and-vale-authoring/.openspec.yaml | OpenSpec change metadata for the proposal directory. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -40,10 +40,10 @@ describe("help routing topics emit cli_help intent telemetry", () => { | |||
| it.each(["route", "existing", "static", "remote", "engine-selection"])( | |||
| it("returns the canonical recipe when --anonymous is omitted", async () => { | ||
| const result = await runCli(["help", "rule", "create", "-d", cwd]); | ||
| expect(result.stdout).toContain("# Topic: rule create"); | ||
| const result = await runCli(["agent", "improve-rule", "-d", cwd]); | ||
| expect(result.stdout).toContain("# Topic: rule improve"); | ||
| expect(result.stdout).not.toContain("(anonymous)"); | ||
| }); |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /** Where a rule of each engine lives, for an error message that can be acted on. */ | ||
| export function ruleFileLocation(engine: EngineName, ruleId: string): string { | ||
| const layout = ENGINE_LAYOUTS[engine]; | ||
| return `.taskless/${layout.rulesDirectory}/${ruleId}.yml`; |
There was a problem hiding this comment.
ruleFileLocation reconstructs the path purely from ENGINE_LAYOUTS[engine].rulesDirectory (the canonical location), but rulefileOwners above matches against astGrepRuleFileCandidates, which also includes the legacy .taskless/rules/<id>.yml path. If ownership is ambiguous because the ast-grep rule actually lives at the legacy path, the "exists for more than one engine" error built from this function (see commands/rules.ts around the owners.length > 1 branch) names a canonical path that doesn't exist, and never mentions the real conflicting file.
Since rulefileOwners already knows which candidate matched (for (const path of paths) { if (await isFile(path)) { ... } }), the simplest fix is to have it return the matched { engine, path } pairs directly instead of just EngineName[], and drop ruleFileLocation — that also removes the possibility of these two functions drifting out of sync.
| return `.taskless/${layout.rulesDirectory}/${ruleId}.yml`; | |
| export async function rulefileOwners( | |
| cwd: string, | |
| ruleId: string | |
| ): Promise<Array<{ engine: EngineName; path: string }>> { |
(illustrative — the body and call sites would need to follow suit; not meant as a literal drop-in.)
| cwd: string, | ||
| ruleId: string | ||
| ): Promise<EngineName[]> { | ||
| const candidates: Array<[EngineName, string[]]> = [ |
There was a problem hiding this comment.
The candidate list is hardcoded to sg and vale rather than derived from ENGINES/ENGINE_LAYOUTS, so rulefileOwners silently omits runtime. A rule filed under .taskless/runtime/rules/<id>.yml returns zero owners here, and verifyCommand falls through to the ast-grep verifier, which reports a "missing rule file" failure under engine: "sg" instead of a real answer about the runtime engine. Every future engine added to ENGINES needs a matching manual edit here or it mis-routes the same way.
| // fixtures go unverified while it reported a pass. | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| if (json) { | ||
| console.log(JSON.stringify(makeErrorEnvelope("INVALID_INPUT", message))); |
There was a problem hiding this comment.
This hand-rolls console.log(JSON.stringify(makeErrorEnvelope(...))) where writeJsonError (packages/cli/src/types/errors.ts) already wraps exactly this call and is used elsewhere (e.g. commands/auth.ts). The same pattern repeats at the "Vale never ran" branch and the owners.length > 1 branch further down — three call sites in this file now diverge from the shared helper, so a future change to envelope serialization (pretty-printing, a trailing newline, a common field) only applies where writeJsonError is called, not here.
The existing migration tests assert where files land and that their bytes survive. Both can be perfect while the migrated project reports nothing, which is how the Vale silent-disable shipped: 0004 enables a check named `rules.<id>`, 0005's StylesPath resolves the same rule as `<id>.<id>`, and carrying the old assignment forward leaves every Vale rule present, valid, enabled against a check that does not exist, and quiet. It was caught by hand, by noticing ast-grep still reported while Vale had gone silent. So this seeds a version-4 project and runs the real CLI over it after migrating: both static engines still report, the assignment is retargeted, every rule still verifies, the tests that moved still run, runtime capture bytes are unchanged, and a second run is a no-op. The ast-grep half is the control, so a silent Vale shows up as a difference between engines rather than a plausibly-empty run. The changeset also described a layout this stack has since replaced, and promised an `engine-selection` topic that #102 merged into `route`. Rewritten for what actually ships. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
7.1-7.6 verified. 7.7 is blocked on #102: a change archives once on the tip, and the gate wants openspec/changes/ empty, but agent-command-and-vale-authoring still has 14 open tasks. Archiving it early would drop the spec deltas for work nobody has done. #102's delta also has to reach specs/ before this change's delta has a target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
…topics The surface an agent reads was shaped for a human. `taskless help` names the command after a human's reason for typing it, and addressing was longform, so an agent had to know both the words and their order. Sweeps the shipped surface: 17 files, every recipe, both READMEs, the skill, and the prompts doc comment. Nested topics flatten to their filenames (`rule improve` → `improve-rule`). `static`, `existing`, and `engine-selection` are removed rather than renamed, because `route` now applies the criterion itself and names a concrete destination. Left alone deliberately: CHANGELOG.md, openspec/changes/archive/**, and this change's own proposal and design, which quote the old name to describe the rename. Three cross-reference tests, and each caught something. Five recipes still announced themselves by their old multi-token name, so `taskless agent delete-rule` printed "# Topic: rule delete". The See Also check found a dangling entry left by the sweep collapsing two topics onto `route`. The `taskless help` check is the cheap total one the design asked for. Verified by rehearsing `route` against fresh agents with no repository access, one per destination: all five route correctly, and the two judgment branches (present-both when logged in and locally expressible, split on login state for runtime) behave as written. Also adds the W101 notice test and its no-matcher counterpart, which is what keeps the section-less scaffold from reintroducing a silent disable. BREAKING CHANGE: `taskless help <topic>` is now `taskless agent <topic>`, topics are single-token, and `@taskless/cli/prompts` no longer exports `engine-selection`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Merges the deltas into specs/ and creates cli-agent-authoring. The archive refused three times, each for a real defect in the deltas rather than a tooling complaint: `cli-help` MODIFIED "Routing recipes name a destination", a header that does not exist because the requirement is a rename of "Routing recipes reference engine selection". MODIFIED matches by header, so a rename has to be a REMOVE plus an ADD. The delta also never removed "The engine-selection topic is registered in the help system", which registers a topic this change deletes. `cli-rule-routing` MODIFIED "Trust tier is not an engine-selection input", which is new, so it belongs under ADDED. Then `cli-help` MODIFIED requirements the parser could not see. The spec carried an orphaned duplicate of "Help text files follow a consistent format" whose `### Requirement:` header and opening code fence had both been lost, so a documentation template parsed as real `## Goal`/`## Preconditions` headings, ending the Requirements section and stranding every requirement below it. The stray closing fence was the tell. Pre-existing on main. Removed the duplicate; the live copy is unchanged. Also brings along the requirements the rename left describing a command that no longer exists: the onboard topic, routing topic registration, anonymous variants, embedded schemas, and both telemetry requirements. The telemetry ones were already wrong before this change — they specify per-topic `help_<topic>` events, and the code emits one `cli_help` event with a `topic` property. Corrected to what the code does. Renames two sibling requirements the delta had skipped while renaming their peer: "Existing recipe" and "Remote recipe" now name create-legacy-rule and create-remote-rule. `cli-rules` and `cli-update-engine` still fail --strict; both fail on main and neither is touched here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The stack's second and final archive, which is why #102 had to go first: this change's `cli-agent-authoring` delta modifies a requirement that one introduces, so it needed a target in specs/. The archive refused twice, both times on a rename expressed as a MODIFIED. Five of the six MODIFIED entries across cli-rule-format and cli-vale-rule-engine renamed their requirement, which openspec matches by header, so each became a REMOVE of the old name plus an ADD of the new. cli-rule-format's MODIFIED section emptied out entirely. Two requirements the deltas never retired, both now describing code this change deletes: - "Both the legacy and engine-partitioned layouts are readable" promises a legacy read path that task 1.5 removes. The legacy constant now names the same string as the rules root, so a stale read path would resolve into the live tree (design D9). - "The scaffolded Vale config carries no section" describes a file that no longer exists. Verified against a fresh `init`: the scaffold contains no `.ini` at all, because scope is declared per rule. `cli-rules` and `cli-update-engine` still fail --strict; both fail on main and neither is touched by this stack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
|
@claude /review |
|
Claude finished @thecodedrift's task in 5m 2s —— View job Review:
|
| .array(z.string()) | ||
| .describe("pass/ fixtures the rule flagged and should not have"), | ||
| }); | ||
|
|
There was a problem hiding this comment.
valeVerifyOutputSchema has no field for Vale's stderr notice (the W101-class diagnostic runVale/dispatch.ts now capture on a zero-exit run). That's not just a schema gap — verifyValeRule (packages/cli/src/rules/vale/verify.ts, not touched by this PR) reads outcome.results but never outcome.notice, so rule verify <id> --json on a Vale rule silently drops the same diagnostic that taskless check surfaces for the identical repo state.
This directly undercuts D4's stated rationale (surfacing the notice is what makes the section-less scaffold safe) for the rule verify entry point specifically: an author who runs verify after writing a rule with a misplaced rules.<id> = YES gets a clean pass with no indication the rule isn't scoped to anything. packages/cli/test/rule-verify-dispatch.test.ts doesn't exercise this case either. Same gap was flagged on the prior review pass of this PR and appears to still be open.
| cwd: string, | ||
| ruleId: string | ||
| ): Promise<EngineName[]> { | ||
| const candidates: Array<[EngineName, string[]]> = [ |
There was a problem hiding this comment.
Still hardcoded to ["sg", "vale"] rather than derived from ENGINES/ENGINE_LAYOUTS (ENGINES = ["sg", "vale", "runtime"] in packages/cli/src/rules/engines.ts), so rulefileOwners silently omits runtime. A rule filed at .taskless/runtime/rules/<id>.yml returns zero owners here; commands/rules.ts's verifyCommand then falls through to the ast-grep verifyRule, which reports a "missing rule file" failure under engine: "sg" instead of a real answer about the runtime engine.
This was flagged on the prior review of this PR and is still present. Every future engine added to ENGINES needs a matching manual edit here or it mis-routes the same way — worth generalizing (e.g. iterate ENGINES/ENGINE_LAYOUTS and build the candidate list, including the legacy ast-grep location, from that) rather than adding cases by hand.
| | `rule delete` | `delete-rule` | | ||
| | `rule verify` | `verify-rule` | | ||
| | `rule meta` | `rule-meta` | | ||
| | `static`, `existing` | `route` | |
There was a problem hiding this comment.
This row's mapping doesn't match the actual rename. Per the diff:
static.txtwas deleted and its content replaced by the new, exportedcreate-sg-rule.txt(TOPICSinpackages/cli/src/prompts/index.tsnow includescreate-sg-rule) — notroute.existing.txtwas renamed (git detects it) tocreate-legacy-rule.txt, an internal topic — notroute.- Only
engine-selectionactually merged intoroute(its reasoning now lives there, per D1).
As written, this table tells a consumer that static/existing became route, which would send them looking for ast-grep/legacy-linter authoring guidance inside the routing recipe instead of at create-sg-rule/create-legacy-rule. Since this changeset is the public breaking-change note (and TOPICS crosses the deploy boundary the PR body calls out), worth correcting before this ships:
| | `static`, `existing` | `route` | | |
| | `static` | `create-sg-rule` | | |
| | `existing` | `create-legacy-rule` | | |
| | `engine-selection` | `route` | |
| @@ -1,4 +1,4 @@ | |||
| # Topic: rule verify (CLI v%(CLI_VERSION)s / topic v1) | |||
| # Topic: verify-rule (CLI v%(CLI_VERSION)s / topic v2) | |||
There was a problem hiding this comment.
This recipe's content is still ast-grep-only (Goal, Preconditions, the three-layer Steps, Errors table, and the JSON example all describe only the sg path) even though commands/rules.ts's verifyCommand now dispatches rule verify <id> by engine ownership and the actual JSON output has diverged in this PR: verifyOutputSchema gained an engine discriminant field (not shown in the example here), and a Vale-owned id gets a completely different valeVerifyOutputSchema shape (fixtures/missingFailures/unexpectedFindings, no schema/requirements/tests). The new "rule exists for more than one engine" error (owners.length > 1 in commands/rules.ts) isn't in the Errors table either.
This matters because create-vale-rule.txt (lines ~30, ~43, ~369 in that file) treats verify-rule as the authority on what rule verify checks for Vale rules ("verify-rule checks all three, and refuses a rule that has only one..."), but an agent that actually fetches taskless agent verify-rule to read the full contract gets guidance that contradicts what it just saw and omits the Vale-specific precondition/output/errors entirely. The topic version bump to v2 suggests the content changed meaningfully, but the diff here is only the rename + cross-reference updates.
| @@ -40,10 +40,10 @@ describe("help routing topics emit cli_help intent telemetry", () => { | |||
| it.each(["route", "existing", "static", "remote", "engine-selection"])( | |||
There was a problem hiding this comment.
This still iterates the removed/renamed topics (existing, static, remote, engine-selection), flagged on the earlier Copilot review pass too. The test still passes, but only because agent.ts fires telemetry.capture("cli_help", { topic: key }) in both the found and not-found branches (lines ~154 and ~158 of commands/agent.ts) — so the assertion capture was called with { topic } holds whether or not the topic actually resolves.
The describe block's name ("agent routing topics emit cli_help intent telemetry") and the test's own framing ("captures cli_help for %s") no longer describe what's being exercised: these four topics now hit the unknown-topic error path, not routing. Worth updating the topic list to the current destinations (route, create-legacy-rule, create-sg-rule, create-vale-rule, create-runtime-rule, create-remote-rule) so the test pins the intended behavior rather than silently degrading into "unknown topics still emit telemetry."
Stack (root → tip):
Proposal only — no implementation. Draft so the archive gate stays quiet until the work is stacked beneath it.
add-vale-rule-engineshipped a chooser without a destination.engine-selectionteaches an agent to conclude a rule belongs tovale, and then there is nowhere to go —static.txtis ast-grep authoring, and its only mention of Vale tells the reader to confirmsgwas right.runtimedead-ends the same way. That exclusion was a recorded non-goal at the time, defensible while nothing chose Vale; the chooser is what made it reachable.What it proposes
taskless help <topic>taskless agent <topic>taskless help rule createtaskless agent create-rule(single token, no positional join)staticcreate-sg-ruleexistingcreate-legacy-rulecreate-vale-rule,create-runtime-ruleroute+engine-selectionroutePlus a section-less scaffolded
.vale.ini, so a project lints nothing until someone scopes it deliberately.The two decisions worth reviewing
D1 —
routeandengine-selectionmerge. One decision, made once from one reading of the evidence. Whether a rule is expressible locally and which engine can express it are answered from the same signals, so splitting them cost a second fetch and a handoff without adding information.The consequence needed solving rather than hand-waving: the platform generator consumes
engine-selectionthroughTOPICSand has noroutestep, so merging can't just delete what it reads. Exportingrouteinstead is wrong — it's built ontaskless detect --jsonand local authoring, neither of which a Worker can do. So the criterion distributes to the destinations: eachcreate-*-rulestates the evidence that makes its engine right, androuteapplies those same tests. That keeps every exported topic one a service-side consumer can actually act on, and removes the drift class where a chooser and its destinations disagree.D4 — the section-less scaffold ships paired with surfacing Vale's stderr. Measured: with no section to copy, the natural first edit is
rules.<id> = YESat top level, which Vale reports asW101 ... is ignoring it— on stderr, exit 0, valid{}on stdout.runValereads stderr only on a non-zero exit, so today that's discarded and the author gets a rule that verifies, runs, and reports nothing. Shipping the scaffold alone would reintroduce the exact silent-disable class this engine's design exists to prevent, one level up. They are one requirement in the spec, not two.Also settled
create-remote-rule. "Remote" names who generates, not an engine, and the one place it bites — the user is logged out — is wherecreate-runtime-rulealready has to speak. One topic, one gate explained once.cli-knowledge-promptscurrently requires a topic rename be MAJOR. The delta corrects that to pre-1.0 semantics, keeping the obligation that actually matters — naming the rename in the changeset, sinceTOPICScrosses a deploy boundary and the generator breaks on upgrade rather than at our build time.Open questions in the design
remote.txt/rule-create.txtstill implement service generation but are no longer named byroute. Keep, fold intocreate-sg-rule's escalation path, or merge? Worth deciding before the cross-reference sweep.create-runtime-ruleexplain the login gate itself, or defer toauth?Scope
~306 occurrences of
taskless helpacross 77 files — but concentrated: only one skill file, the rest are recipe cross-references and tests. Delivery shape is a single PR stacked on #100, because recipes cross-reference each other by literal command string, so a half-renamed surface points at commands that don't exist.pnpm openspec validate --strictpasses.Refs #100