Skip to content

feat(prompts): export route from @taskless/cli/prompts - #251

Open
thecodedrift wants to merge 4 commits into
mainfrom
feat/export-route-prompt
Open

feat(prompts): export route from @taskless/cli/prompts#251
thecodedrift wants to merge 4 commits into
mainfrom
feat/export-route-prompt

Conversation

@thecodedrift

Copy link
Copy Markdown
Member

The platform generator asked for this, and their evidence is the argument.

Why it was withheld, and why both reasons fail

route sat in INTERNAL_TOPICS on two grounds, both recorded in src/prompts/index.ts:

  1. It contains local mechanics (taskless detect, on-device authoring) a Worker cannot run.
  2. A consumer could get each destination's scope from the three exported authoring recipes and "adjudicate genuinely ambiguous calls itself."

The first is true and does not defeat it. A service consumer ignores the mechanics, which is a smaller and more honest adaptation than restating the criteria.

The second was tried in the field and failed. The generator hand-wrote the same judgement, its classifier reached static | runtime with nowhere to put vale, and every prose rule was generated as an ast-grep rule — while its own delivery layer could already serve a Vale one. It shipped a tier it was incapable of producing.

The file already contains the argument against itself:

Exporting a chooser without its destinations reproduces, for the platform generator, the dead end this surface exists to remove.

We shipped the mirror image. Destinations without the chooser strand a consumer that can author but not route, and it writes its own chooser rather than stopping.

What lands

route moves from INTERNAL_TOPICS to TOPICS, and the reasoning in both doc comments is rewritten to record why rather than leaving the old justification standing. The completeness check (TOPICS + INTERNAL_TOPICS accounts for every canonical recipe on disk) holds unchanged.

Verified against the built entry, not just the source, since the two render options the consumer named are load-bearing:

renders:              true
in TOPICS:            true
header suppressible:  true

header: false matters to them because the version line would churn their prompt-cache key on every nightly publish.

One thing found on the way, filed not fixed

The first run failed with:

src/prompts/index.ts imports which engine: expected false to be true

My doc comment contained the phrase a different axis from "which engine", and the source-level import scanner read from "which engine" as an import.

That is the false positive .conventions/STYLEGUIDE-CODE.md documents by name in its worked example. The resolution recorded there — move the check into a vite plugin, delete the test — was applied to the built-output scanner. A source-level regex scanner survived and reproduces the same defect, on the same phrase.

Reworded the sentence here so this PR stays about one thing. Filed separately, because the next person to write that phrase hits it again and the styleguide currently reads as though it cannot happen.

Checks

pnpm typecheck, pnpm lint, full suite (1181 passed), pnpm cli check with only the 4 known pre-existing no-hedging warnings.

@thecodedrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude finished @thecodedrift's task in 3m 54s —— View job


Review

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains no reviewThreads, reviews, or comments, so this is the first review of this PR; the whole diff is assessed fresh, nothing to mark as already-addressed.

  • Read .prior-review.json
  • Fetch PR diff (gh pr diff) and metadata
  • Read whole files touched / referenced (src/prompts/index.ts, src/prompts/recipes.ts, src/agent/route.txt, test/prompts.test.ts)
  • Cross-check against openspec/specs/cli-knowledge-prompts/spec.md
  • Check changeset semver classification against project precedent
  • Post findings

The diff itself (route moves from INTERNAL_TOPICS to TOPICS, plus rewritten doc comments) is mechanically safe: the completeness check (TOPICS + INTERNAL_TOPICS == every canonical recipe on disk) still holds, route.txt is an existing, unchanged recipe already served by taskless agent route, and the generic TOPICS-driven tests in test/prompts.test.ts (byte-for-byte CLI parity, header suppression, raw/rendered round-trip, PROMPTS map) automatically pick up route with no test changes needed. No correctness bug in the mechanical change.

Three things worth your attention:

[New] Spec left inconsistent with the code (not part of this diff, but governs it). openspec/specs/cli-knowledge-prompts/spec.md:162-168, Requirement "Exported topics cover every engine a rule can be routed to," still says:

TOPICS SHALL export the authoring recipe for each engine — create-sg-rule, create-vale-rule, and create-runtime-rule.
route is not exported here — it still contains local mechanics a Worker cannot run — so until it is, a consumer gets each destination's own scope from these three and adjudicates a genuinely ambiguous call itself.

That's now factually wrong: route is exported after this PR, for exactly the reason the spec said it wasn't. There's also no #### Scenario asserting route is present in TOPICS, matching the Scenario: Every engine's authoring path is reachable from the export pattern already used for the other three topics. This PR doesn't touch that spec file, and there's no in-flight openspec/changes/* proposal that does either (checked — only runtime-demo-path and runtime-fixture-runner exist, neither touches cli-knowledge-prompts). Per this repo's own guidance on spec drift, an inconsistency like this doesn't fail any check and won't be caught later — worth reconciling in this PR or a fast follow before it's forgotten.

[New] Changeset semver classification — inline comment posted on .changeset/export-route-prompt.md:2. Marked patch, but this adds a new member to the exported TOPICS/PromptTopic surface, which is the same category of change the original @taskless/cli/prompts export shipped as a Minor Change for (packages/cli/CHANGELOG.md:7-21). Worth a second look against that precedent.

[New, lower confidence] route.txt's content doesn't itself branch for a service consumer. The PR's stated rationale is "a service consumer ignores the [local] mechanics" (taskless detect --json, taskless info --json in Steps 1-2, and "Name the command" in Step 8 telling the reader to run %(TASKLESS_CLI)s agent <topic>). That reasoning lives in the doc comment and PR description, but route.txt itself (unchanged by this PR, packages/cli/src/agent/route.txt:20-57, 252-258) is written as an unconditional imperative sequence with no branch or note for a reader that can't spawn a subprocess — every downstream step (the evidence-then-destination logic in Steps 3-4) depends on the output of Steps 1-2. A Worker-hosted consumer rendering this recipe verbatim gets instructions it structurally cannot follow, with nothing in the text itself signaling "skip this if you're not a local CLI session." Not a regression from this PR (the file is pre-existing), but it's the first time this recipe is reachable by the audience the gap actually affects — worth a conscious call on whether that's acceptable as shipped or wants a follow-up to the recipe text.

Nothing else stood out — the diff's own logic (list membership, doc-comment rewrite) is correct and internally consistent with itself.

Comment thread .changeset/export-route-prompt.md
The MODIFIED block is matched to the standing requirement by title. Rename it
and nothing is applied: the standing text survives untouched, the delta is
discarded, and `openspec validate --strict` passes on both sides.

Found by doing it. The delta for `cli-knowledge-prompts` renamed a requirement
to say it covers the chooser as well as the destinations, and the archive left
the old requirement exactly as it was, with zero occurrences of the new text.

This is the quieter half of the trap already recorded here. Dropping a scenario
at least changes something; a rename looks like a substantial edit and does
nothing. A title that should change is a REMOVED plus an ADDED.

Also corrects the verification procedure, which bit me twice while measuring
this. Reset to the scratch commit's own SHA, not to `HEAD~1`: resetting past it
deletes the change under test, and `git clean` then removes what the reset left
untracked.
@thecodedrift
thecodedrift force-pushed the feat/export-route-prompt branch from 5cf0082 to 337fbea Compare September 3, 2026 04:34
`route` sat in INTERNAL_TOPICS on two grounds. It contains local mechanics a
Worker cannot run, and a consumer could adjudicate ambiguous calls from the
three authoring recipes it already had.

The first is true and does not defeat it: a service consumer ignores the
mechanics, which is a smaller and more honest adaptation than restating the
criteria. The second was tested in the field and failed. The platform generator
hand-wrote the same judgement, reached `static | runtime` with nowhere to put
`vale`, and generated every prose rule as an ast-grep rule while its own
delivery layer could already serve a Vale one.

That is the dead end this surface exists to remove, mirrored. The file already
warned that exporting a chooser without its destinations strands a consumer
that can route but not author; destinations without the chooser strand one that
can author but not route, and it writes its own chooser rather than stopping.

Reworded one sentence to avoid the phrase `from "which engine"`, which the
source-level import scanner in test/prompts.test.ts reads as an import. That is
the false positive the code styleguide documents by name, still live in the
source-level half of a check whose built-output half was replaced by a vite
plugin. Filed separately rather than fixed here.
…lean absence

Exporting `route` shipped a recipe whose first two steps tell a reader to run
`taskless detect --json` and `taskless info --json`. A service consumer cannot,
and `invocation` does not answer it: that option substitutes a binary NAME
inside a command, so setting it to a phrase renders

  Run:
  ```
  no CLI is available detect --json
  ```

which is an instruction to execute something that does not parse. Worse than
either honest answer, and it worked only because a forgiving model skipped past
it to the criteria.

`mechanics: false` replaces both steps with what the caller must supply. It
does not drop them: the routing criteria are stated in terms of linters,
languages, rule styles, `loggedIn` and `ghOwner`, so a consumer that reads
nothing there loses the inputs rather than the commands.

Rendered through the variable table as whole blocks rather than stripped after
the fact, so the default can be proved rather than asserted. Measured
byte-identical at 18161 bytes, `diff` clean. `TASKLESS_CLI` and the new blocks
now share one invocation resolver so the command inside a step cannot disagree
with the command beside it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant