feat: detect host tools for the onboard recipe, and rename info's tools key to harnesses - #394
Conversation
|
@claude /review |
|
Claude finished @thecodedrift's task in 5m 52s —— View job Review mode: incremental — read 0 prior review thread(s) before reviewing.
Findings1. 2. Not flagged (checked, no issue found)
|
Finding 1 (Windows Finding 2 ( — AI Coding Agent |
|
Closing the loop on the Owner chose the
All four checks were green before this push; will confirm again once CI settles. — AI Coding Agent |
|
@claude /review |
|
Claude finished @thecodedrift's task in 5m 36s —— View job Review mode: incremental — read 2 prior review thread(s) before reviewing.Both prior threads are resolved and carry a closing reply ending in
Findings1. [New] Not flagged (checked, no new issue found)
|
Finding 1 ( On the out-of-scope observation: agreed, and deliberately not touched. The other
— AI Coding Agent |
The onboard recipe assumes `gh` in prose only, tells the agent to run a probe the CLI can answer for free, names Linear as the expected issue tracker, and offers PR mining in a repository that has no pull requests. Deltas for the four capabilities that move: `cli` (the `info --json` `tools` key becomes `harnesses`, and `tools` takes on the CLI binaries found on PATH), `cli-onboard` (the source menu, plus the onboard.txt -> onboard.md drift the Vale-coverage change left behind), `cli-agent` (a third substitution flavor for whole conditional passages), and `cli-knowledge-prompts` (the hostTools option and its full-menu default). The embedding requirement's title named a file that has not existed for two changes, so it is a REMOVED plus an ADDED rather than a MODIFIED: a MODIFIED block is matched by title and a renamed one applies nothing. Verified by archiving against a throwaway commit and diffing the requirement and scenario lists: cli 73 -> 76 scenarios, cli-onboard 27 -> 31, cli-agent 39 -> 41, cli-knowledge-prompts 30 -> 34, with no prior scenario lost anywhere.
`info --json`'s `tools` array has always carried agent harnesses -- Claude Code, Codex, Cursor, OpenCode -- which is the wrong noun for it and is the name the host-tool detection in the next commit needs. The array is unchanged in shape; only the key moves. Every consumer in this repository moves with it: the schema, the command (JSON and human output), the `info` recipe's example payload, and cli.test.ts. No consumer outside this repository's control reads it from source; the release note carries the rest.
`onboard.md` told the agent to run `command -v gh` for an answer the CLI already has, and offered PR-comment mining in repositories with no pull requests to mine. It now reports what was found. - `src/detect/host-tools.ts`: presence of `gh`, `git` and `jq` via `findOnPath`, which walks PATH and `existsSync`es a candidate. Nothing is executed, nothing is hashed, no version is read. A hash tier was measured and dropped: GitHub publishes digests for `gh`'s release archives and installers rather than for the extracted binary, and a Homebrew `gh` 2.97.0 matched 0 of 21 official digests. - `applicable` is a second axis, from `resolveRepositoryContext` rather than from a new is-this-GitHub probe that could disagree with it, and it outranks `present`: a repository with no GitHub origin is told there are nothing to mine, never told to install `gh`. - The renderer substitutes whole passages -- bullet marker, step title and connective included -- the way `DETECT_EVIDENCE` already does, and never post-strips. With no `hostTools` supplied it renders the full menu, so `@taskless/cli/prompts` is unaffected. - `agent` detects only for topics whose template contains one of these variables, asked of sprintf's own parse rather than of a topic list, so unrelated topics pay no git subprocess. - `info --json` gains the detected tools under the freed `tools` key. - The issue-tracker bullet stops naming Linear as the expected answer. Topic v3 -> v4.
`findOnPath` matches a PATH entry joined with the literal string and consults no PATHEXT, so every other caller wraps the name in `executableName` first (platform-binary.ts:250). `detectHostTools` did not, so on Windows `gh.exe`/`git.exe`/`jq.exe` were never found and a user with the GitHub CLI installed was told to install it -- the one instruction that cannot help them, arrived at from the other direction than the `applicable` case this change already guards. `executableName` was private; it is exported now so the pairing with `findOnPath` lives in one place rather than being re-derived per caller. CI runs ubuntu-latest only, so the regression is invisible without stubbing the platform. The new test writes a `gh.exe` fixture, sets `process.platform` to win32, and asserts it is found; it fails against the bare `findOnPath(name)` it replaces.
This PR is the tip of its stack, so it is the last branch that can archive before the change reaches main. The 'OpenSpec Label' job said so in a warning annotation; landing unarchived would need a second PR to correct, since main takes pull requests only. Verified against the pre-archive dry run rather than trusted: the requirement and scenario titles in all four rewritten specs are byte-identical to what the dry run produced, and no prior scenario was dropped anywhere. cli 73 -> 76 scenarios, 20 requirements cli-onboard 27 -> 31 scenarios, 6 requirements cli-agent 39 -> 41 scenarios, 15 requirements cli-knowledge-prompts 30 -> 34 scenarios, 13 -> 14 requirements The one title that disappears is the intended REMOVED plus ADDED rename of 'Onboard recipe is embedded from help/onboard.txt', which a MODIFIED block could not have performed: those are matched by title, so a rename applies nothing at all.
`toolLine` printed "this repository has no GitHub origin" for any tool with `applicable: false`. Unreachable through `detectHostTools`, which marks only `gh` inapplicable, but this is a public render over a caller-supplied array: a caller marking `jq` inapplicable for an unrelated reason got a confident GitHub explanation. `HostTool.reason` now carries it, set by the detector that knows it and printed verbatim by the render, which never supplies one of its own. Absent a reason the line names no cause at all, because a renderer that fills one in is how the GitHub sentence came to be printed for tools with nothing to do with GitHub. OPTIONAL rather than required. The type is published surface via `@taskless/cli/prompts`, so a required field would break every caller already building the array, and an applicable tool has nothing to explain — it carries no `reason` rather than an empty one a consumer would have to test for. The field is content, not just a guard against a wrong sentence. `applicable: false` tells an agent to drop a capability; the reason tells it whether any action by the user would change that, which is the difference between staying quiet and proposing a fix that cannot work. So it also rides on `info --json` and the human `info` output. Standing specs updated rather than left stale: `cli` and `cli-knowledge-prompts` gain the field and three scenarios. The change directory on this branch is already archived, so the archived delta is updated in step with the standing spec and the two still agree.
NOT A NORMATIVE CHANGE. The requirement 'onboard topic is registered in the agent index' names the file the CLI embeds, and that file has been `packages/cli/src/agent/onboard.md` since the Vale-coverage change gave the recipes a markdown extension. The spec kept saying `onboard.txt`, which no longer exists. Corrected in place. This requirement is NOT one this branch's change wrote a delta for -- that delta touches only 'Recipe substitution uses sprintf-js named arguments' -- so there is no archived copy to keep in step, and nothing here is paired the way `HostTool.reason` was. A later reader should read this as a factual correction to stale prose, not as an undocumented requirement edit: the obligation is unchanged, only the filename it names is now the real one. Occurrences under `openspec/changes/archive/` are deliberately left alone. Those are the historical record of what each change said when it landed, and rewriting them would make the archive describe a past that did not happen. `openspec/specs/` is now free of the stale name.
`prReviewSource` asks `toolState` about `gh` by name and renders the unmeasured default when the supplied array does not mention it. `hostToolsStep` gated only on `tools.length === 0`, so any non-empty array took the "Taskless already looked" branch and asserted "the list above is the answer" over whatever it happened to contain. On a partial array the two passages then contradicted each other: the menu bullet reported `gh` unmeasured while the adjacent step implied the enumeration was exhaustive, so a reader infers "not installed" from "not listed". That is a verdict read out of silence -- the same category as the `toolLine` defect fixed in cfdbd82, in a different function. Unreachable through either in-repo caller, since `detectHostTools` always returns all three tools. But `hostTools` is published surface via `@taskless/cli/prompts` and the mechanism is meant to be reused by recipes carrying their own subsets, so the guard belongs at the render rather than at the one caller that happens to be exhaustive today. The step now scopes its claim to the tools it was given and says a name absent from the list was not looked for, which is not the same as not installed. The full-array case still reads as a firm answer.
NOT A NORMATIVE CHANGE. Every file under `packages/cli/src/agent/` has been `.md` since the Vale-coverage change, and the build globs `../agent/*.md`; verified there are 22 recipe files and zero `.txt` among them. The specs still described the mechanism in terms of `.txt`, so the obligations are unchanged and only the filenames they name are now the real ones. 25 occurrences across five standing specs: cli-agent (recipe lookup, embedding, template and anonymous-variant requirements), cli (the Vite embedding scenario), cli-knowledge-prompts (purpose, export, anonymous variants, topic membership), cli-rules (the improve-rule recipe) and skills (where recipes live). NONE of them sit in a requirement this branch's change wrote a delta for. Checked per line rather than assumed: the deltas touch 'CLI info subcommand outputs version as JSON', 'Recipe substitution uses sprintf-js named arguments', the three cli-onboard requirements, and an added cli-knowledge-prompts requirement, and no corrected line falls in any of them. So there is no archived copy to keep in step here. DELIBERATELY LEFT: `requirements.txt` in cli-detect. That is a Python dependency manifest, which really is a `.txt` file, and rewriting it would introduce an error rather than remove one. `openspec/changes/archive/` is untouched, as before: it records what each change said when it landed.
ebf3286 to
089c7bb
Compare
The
onboardrecipe assumed the GitHub CLI in prose, told the agent to probe forit, named one issue tracker as the expected one, and offered PR-comment mining in
repositories that have no pull requests. It now reports what the CLI found.
The consumer-visible part:
info --jsonrenamestoolstoharnessestoolshas always carried agent harnesses — Claude Code, Codex, Cursor,OpenCode — with each one's installed skills and their staleness. That is the
wrong noun for it, and it is the name this change needs. The array is unchanged
in shape; only the key moves:
toolsthen takes on what the word says: the command-line binaries found onPATH, as{ name, present, applicable, path? }forgh,gitandjq.Every consumer inside this repository moves with it, and they are all here:
src/schemas/info.ts,src/commands/info.ts(JSON and human output),src/agent/info.md's example payload, andtest/cli.test.ts. Nothing else inthe repo reads the key — no
reference.jsonfield, no recipe, no telemetrypayload, and there is no dashboard or generator package in this tree.
Detection is presence, and only presence
src/detect/host-tools.tsasksfindOnPathwhether a file of each name sits onPATH. ItexistsSynces and executes nothing: no spawn, no--version, nohash. The recipe is worded to match — "
ghis on your PATH; Taskless did not runit" — because "
ghis available" would be a claim about a working install thatnobody checked.
Verification by hash was considered and rejected on measurement, not on
principle. GitHub publishes sha256 digests for
gh's release archives andinstallers, not for the extracted binary, and the Homebrew-installed
gh2.97.0on the development host matched 0 of the 21 official digests. A tier that
reports "unverified" for the ordinary macOS install path is worse than no tier,
because a reader takes that word to mean "suspicious" rather than "not
checkable".
applicableis a second axis, and it outranksabsentghin a repository with no GitHuboriginis inapplicable however well it isinstalled: there are no pull requests to read. That comes from
resolveRepositoryContext, the same resolution behindinfoand telemetry,rather than from a new is-this-GitHub probe that could disagree with it.
Precedence matters because collapsing the two produces the one instruction that
cannot help: a GitLab user with
ghinstalled being told to installgh. Anon-GitHub repository is told there are no pull requests to mine, and is not
offered the source even with
ghpresent.An omitted source always says why in one line, mirroring the reasoning
already written into
route.mdfor the remote tier it declines to offer: areader who is not told reads the omission as an oversight and asks for it, which
costs a turn and arrives back where the recipe already is.
The mechanism is general;
onboardis its only consumer hereRecipeOptions.hostToolscarries the state into the renderer, which substituteswhole passages — bullet marker, step title and grammatical connective
included — exactly as
DETECT_EVIDENCEandLOGIN_EVIDENCEalready do, andnever post-strips rendered text. With no
hostToolssupplied every passagerenders its default, so
@taskless/cli/promptsgets the full menu unchanged: aWorker consumer has no
PATHworth describing, and a recipe trimmed againstthis host's tooling would be describing the wrong machine.
Detection runs at the point
invocationis already detected, in both servingpaths.
agentruns it only for topics whose template actually contains one ofthese variables, asked of
getRawRecipe(...).variables(sprintf's own parse)rather than of a hardcoded topic list, so unrelated topics pay no
gitsubprocess and the next recipe to adopt the mechanism needs no edit there.
Also in here
Linear are named as examples of the class; whether one is reachable depends on
the agent's MCP roster, which the CLI cannot see, so it stays the agent's
judgement.
openspec/specs/cli-onboard/spec.mdstill saidonboard.txtthroughout. Thefile has been
onboard.mdsince the Vale-coverage change, and the requirementwhose title named the old path is a REMOVED plus an ADDED rather than a
MODIFIED, because a MODIFIED block is matched by title and a renamed one
applies nothing at all.
onboardv3 → v4.Verification
The spec delta was checked the way
CLAUDE.mdprescribes — archive against athrowaway commit, diff the requirement and scenario lists, reset — because
openspec validate --strictpasses on a delta that silently drops scenarios:clicli-onboardcli-agentcli-knowledge-promptsGates:
pnpm typecheckclean,pnpm lintclean (pnpm cli checkreports noissues),
pnpm test1713 passed across 103 files.Changeset is
patch: the package is0.y.z, where semver puts added surfaceoutside the stability guarantee. The rename is what the release note leads with.
Fixes #393