[malibio]

Closes #3540. Implements ADR-093 §1 (get a skill, the list version) and §2 (a tool carries its command-line form).

What changes

A tool's CLI command lives on its registry node. tool-native declares an optional text field, cli_command: the nodespace command and subcommand that does what the tool does (create_relationship → nodespace relationship create). Every built-in tool with a CLI equivalent is seeded with one, from Tool::cli_command, an exhaustive match, so a tool added to the registry has to say which it is.

A fetched skill comes with the commands of the tools it names. For each skill a fetch returns, the daemon returns the command of every registry tool the skill lists in tool_whitelist or names in its body. This holds for built-in, user-edited and user-written skills. The body is not rewritten.

• A name is matched as a whole identifier, so update_node is not named by update_nodes_from_markdown, and the prose "update node" is not a tool name.
• A tool with no command is left out, and the fetch still succeeds.
• An archived tool supplies no command.
• A built-in tool's command is served only from that tool's own seeded node, the same rule the built-in agent applies to a tool's definition.

nodespace skill get "<name or id>" fetches one skill. It is its own subcommand, backed by a new GetSkill RPC, and returns what a match returns for that skill: instructions, the schemas it is linked to through applies_to, tool commands, and the same provenance envelope. It needs no embedding model. A name no skill has fails with a message naming it; a name two skills share fails with their ids.

The skill list has a version. nodespace skill guidance with no task carries version (top level in --json, in the first line of human output). It is a digest of every skill's id, name, description, tool list and body, computed on read: nothing is stored and no counter is kept.

Node type change

• Type: tool-native, a core subtype of the abstract base tool; category flat.
• Field added: cli_command (text, optional, no default), stored in the tool-native bucket. No relationship or structural rule changes.
• Wire type and typed update: tool-native has neither. A tool travels as the generic node with its chain's fields inside properties, and is written through the flat update, which the same validation pipeline checks. bun run gen:types produces no diff.
• Behaviour: cli_command, when set, must be text.
• Seeds: existing databases need a reset to pick up the commands; there is no backfill.

Two things to check

1. route_clarify has no command. The issue asks for a test that fails when a tool a seeded skill names has no command. Node Creation and Graph Editing list route_clarify, which hands a question back to the user and has no CLI equivalent. The test therefore allows a tool with no command only when it is listed in NO_CLI_EQUIVALENT with its reason, and fails if a listed tool later gains a command.
2. resolve_query maps to nodespace query, the same command as search_nodes. The CLI has no resolver of its own: its caller writes the query the reference resolves to.

The version also covers a skill's tool list, which the issue does not name. A changed tool list changes the commands a fetch returns, so a client holding the old list should be told.

Unchanged

• The built-in agent's rendering of every skill, and the prompt and routing golden snapshots.
• The per-audience rule forms.

Tests

• skill_guidance_tool_commands (agent, no model): every built-in skill by name and by id with its commands; a user-written skill; a user-edited built-in skill; word-boundary matching; a tool with no command; an archived tool; an unknown and a shared name; the version across add, rename, redescribe, body edit, archive, delete and an unrelated node.
• cli_integration: GetSkill and skill get end to end; the listing's version over gRPC.
• skill_pipeline: every tool a seeded skill names has a command; each seed records its tool's command.
• tool_subtypes_test: the field's bucket, flat update, clear, wire shape, wrong type refused.
• Printer tests for tool commands, the version and a named fetch, in human and --json output.
• Live (run locally with the embedding model, both pass): a user-written skill naming a built-in tool is fetched by task with its commands, and the returned nodespace relationship create is parsed by the CLI and run, creating the link.

bun run test:changed and bun run quality:fix pass.

Docs

In nodespace-docs, pushed to main just before the merge: components/node-types.md (the tool-native sheet), components/tool-registry.md (the field and how commands are returned), architecture/prompt-skill-system.md (what a fetch returns, fetch by name, the version).

packages/skill/SKILL.md, references/cli.md and references/graph-authored-guidance.md cover skill get, the returned tool commands and the version.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review: PR #3553 (issue #3540)

Review Type: Initial Review (no prior reviews or comments on the PR)

Summary: The change is cohesive and follows the existing layering: core finds the tools a skill names, the daemon admits a built-in tool's command only from its seeded node, and the CLI prints it inside the skill's provenance banner. The node type sequence is followed for the one new field. I found no blocking defects. One hardening gap on the new field and a few smaller points are below.

Verified by reading the diff and the surrounding code; scripts/check-node-type-literals.ts passes. I did not re-run the test suites (bun run test:changed was reported as passing).

Requirements Check

	Acceptance criterion	Notes
✅	tool-native declares an optional text field, through the node type sequence	cli_command in core_schemas.rs (Text, required: false, no default, Core protection); behaviour check in ToolNativeNodeBehavior::validate; node-types.md sheet edited in the docs working tree. tool-native has no wire struct and no typed update in nodespace-types (confirmed: only CoreNodeType entries), so there is nothing to regenerate.
✅	Every seeded tool with an equivalent carries its command; a test fails if a tool a seeded skill names has none	Tool::cli_command is an exhaustive match; every_tool_a_seeded_skill_names_has_a_cli_command covers whitelist and body, with NO_CLI_EQUIVALENT holding route_clarify and its reason. The exception list is a reasonable reading of the criterion.
✅	A fetch returns the command of every registry tool a skill lists or names	skill_tool_commands in skill_ops.rs, used by both find_skill_guidance and get_skill_guidance.
✅	Same for user-written and user-edited skills, with a test	a_user_written_skill_gets_the_commands_of_the_tools_it_names, a_user_edited_built_in_skill_gets_the_commands_of_the_tools_it_names.
✅	Word-boundary matching	names_identifier treats _ as a word character; unit test covers prefix, suffix, digit and prose cases.
✅	A tool with no command is left out and the fetch succeeds	registry_tool_commands skips it; test on Node Creation / route_clarify.
✅	Built-in agent rendering and golden snapshots unchanged	No snapshot or prompt code in the diff; available_tools reads only description and parameter_schema.
✅	Per-audience rule forms unchanged	skill_rules.rs untouched.
✅	Fetch by exact name or id, same content and envelope; unknown name fails naming it	GetSkill RPC and nodespace skill get; NOT_FOUND asserted over gRPC. A shared name fails with the ids.
✅	Listing carries a version in --json and human output	Top-level version; first line of human output, including the empty-graph line.
✅	Version changes on add/remove/archive/name/description/body; not on unrelated nodes	the_list_version_changes_with_the_skills_and_with_nothing_else and the unit test, which also covers field-boundary shifts.
✅	Two listings with no change return the same version	Sorted by id and length-prefixed, so read order does not matter.
✅	SKILL.md and references/cli.md updated, including when to fetch by name	Both updated; SKILL.md is 15,465 chars. skill:check not re-run by me.
✅	SKILL.md says the returned commands are how a tool-naming step is carried out	"Where a step names a tool, run the command returned for it."
✅	Live test: a user-written skill naming a built-in tool, and the command is run	live_skill_guidance_fetch plus cli_integration, which parses the returned command with the real CLI parser and runs it. Note: the test harness runs the command, not a model. Both need the embedding model on disk, so the merge gate does not exercise them.
✅	tool-registry.md and prompt-skill-system.md updated	Present as uncommitted edits in the docs working tree, to be pushed before merge.
✅	quality:fix, test:changed	Reported by the author; not re-run.

Code Review Findings

🟡 Important

1. The seed gate checks which node a command comes from, not what the command says
📁 packages/core/src/behaviors/mod.rs:2022-2029, packages/core/src/ops/skill_ops.rs:1004-1018, packages/daemon/src/services/node_service.rs:2855-2862

Tool::is_seeded_as admits a command only from the tool's seeded node, which stops an impostor node. The value on the seeded node is still ordinary graph data: validate accepts any string, registry_tool_commands only trims it, and the CLI's sanitize_for_terminal keeps \n. So anyone with write access can set delete_node's command to nodespace node delete --yes, or to several lines of text, and it is printed under "where a step above names one of these tools, run its command". SKILL.md tells the agent "That command is how you carry out the step".

This stays inside the provenance banner and the documented trust boundary, so it is not a blocker. But the contract ("command and subcommand, with no arguments") is enforced only for seeds, by a test (seed_tool_nodes_record_each_tools_cli_command), not for writes.

Recommendation: enforce the shape in ToolNativeNodeBehavior::validate: a single line, starting with nodespace , no word starting with -. That is the same check the seed test already makes, moved to where every write passes through it. Principle: validate at the trust boundary; a field whose value an agent is told to execute should not accept more than its contract allows.

🟢 Suggestions

2. A listing now reads every skill's subtree, and a failed read changes the version
📁 packages/core/src/ops/skill_ops.rs:1058-1080 (and the doc comment at :142-150)

list_skill_guidance was one query; it now calls render_skill_instructions once per skill, serially, with no bound. ADR-093 has a plugin compare the version on each prompt, so this is the hot path of the feature. On a local SQLite database with tens of skills this is fine today. Two follow-ons:

• The comment on render_skill_instructions ("Acceptable under MAX_SKILL_LIMIT = 10") no longer describes all its callers.
• render_node_subtree returns an empty string when the subtree read fails (it logs a warning). The digest then hashes an empty body, so a transient read failure reports a different version and a client would announce a skill change that did not happen. Returning the error from the listing would be more honest than a wrong version.

Principle: a value clients compare for equality should fail loudly, not degrade silently.

3. The version covers less than what a fetch returns
📁 packages/core/src/ops/skill_ops.rs:1028-1051

The digest covers id, name, description, tool list and body. It does not cover a skill's applies_to links or a tool's cli_command, both of which change what skill get returns. This meets the acceptance criteria as written, and the PR already extends the digest to the tool list for the same reason. Worth a sentence in prompt-skill-system.md so a plugin author does not treat an unchanged version as "every fetched skill is still current".

4. skill get error text and status handling
📁 packages/core/src/ops/skill_ops.rs:1108-1112, packages/cli/src/commands/skill.rs:352-366

• OpsError::NotFound { id } carries a sentence in its id field. With ops_error_to_status the user sees: Fetching the skill "X" failed: Not found: no skill has the name or id "X". ..., which says the same thing three times.
• run_get maps every status to that message and drops the code. When the daemon is unreachable, or the name is shared by two skills, the hint "nodespace skill guidance with no task lists every skill by its exact name" does not help. Appending the hint only for Code::NotFound would fix both.

Nitpicks

• Nit: packages/agent/src/local_agent/tools.rs:4847 repeats the rule that Tool::is_seeded_as now names. Using the new helper there keeps one definition of "is the seed" (DRY).
• Nit: packages/core/src/ops/skill_ops.rs:1004-1011 reads the tool-native bucket by hand; get_namespaced_prop_str in behaviors does the same lookup.
• Nit: For an unedited built-in skill the daemon serves the CLI-form body, while the commands are computed from the stored body and whitelist. The human output then lists tools "a step above names" that the printed body never names. This follows the acceptance criterion; it is only a small amount of extra output per built-in skill.

CLAUDE.md adherence

• No backward-compatibility or backfill code; the PR states a reset is needed for seeds.
• No literal node_type comparison (checker passes); tools resolve through ToolOrigin::of on the type chain.
• No lifecycle_status read; archived tools and skills are excluded through query_nodes_by_type(_, false).
• No Pro wording, no lint suppression, no issue numbers in comments, no new docs in the repo.

Recommendation

APPROVE. Finding 1 is worth folding into this PR because it is a few lines in the validator the PR already touches; the rest are optional.

[malibio]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[malibio]

Review addressed

Addressed (4 findings, 1 nit)

1. 🟡 The command's shape is now validated on every write. tool-native refuses a cli_command that is not nodespace followed by one or two subcommand words on one line. A flag (--yes), an argument, a second line or shell syntax is refused. Unit test covers accepted and refused values.
2. 🟢 A listing fails when a skill's body cannot be read, in place of hashing an empty body and reporting a version that says the skill changed. The stale comment on render_skill_instructions is corrected.
3. 🟢 Docs say what the version covers. prompt-skill-system.md states that it is the list's version: a changed applies_to link or tool command leaves it as it was. tool-registry.md states the shape rule from finding 1.
4. 🟢 skill get errors. The not-found message names the skill once, and the "list every skill" hint is printed for NOT_FOUND only. Other failures keep their status in the error chain.
5. Nit: available_tools uses Tool::is_seeded_as.

Skipped (2 nits)

• Reading the tool-native bucket through behaviors' helper: that helper is private, and exporting it for one call widens the module for no gain.
• Tool commands listed for an unedited built-in skill whose CLI-form body names no tool: the issue asks for the command of every tool a skill lists in tool_whitelist, built-in skills included.

bun run quality:fix and bun run test:changed pass. The two live tests pass locally with the embedding model.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: The review approved the PR with no blocking finding. The changes are small and local: a pure validation function with its own unit test, one error-propagation change, error-message wording, and one mechanical substitution. None changes the design the review read.