feat(mdcode): generate an Agent Skill from a semantic model

[libei]

kcmd skills-generate writes a semantic model out as an Agent
Skill folder — a SKILL.md an agent loads by
itself, plus one references/<action>.md per action.

skills/commerce/
├── SKILL.md                      67 lines, 4,554 bytes
└── references/
    └── issue-credit.md           60 lines, 3,292 bytes

The generator renders, it does not derive

generateSkill() is a renderer over modelTools() — the same call the
agent tools and the action runner go through. It adds layout and nothing
else. A skill therefore cannot describe a tool different from the one
that runs: the guards, the parameters, and the affects rows on a
reference page are the values the executor reads, not a second
derivation of them.

The example command line is the same: runLine() is promoted out of
commands.ts into runtime/run_action.ts, so what a skill tells an
agent to type and what kcmd action list prints are one function.

Router, not catalog

SKILL.md is a table of actions pointing at reference pages. The body
does not inline parameter tables. A model with thirty actions costs the
same at agent startup as a model with one — frontmatter loads eagerly,
the body on activation, a reference page only once the agent has picked
that action.

This is the correction to the shape MCP Toolbox's skills-generate
produces, which inlines every parameter table into the body and offers
no third tier.

One section depends on the deployment

The skill is generated from the logical model. Exactly one section of
SKILL.md describes where a run lands.

Generating the commerce model under the spanner and alloydb profiles
— different table names, a differently named column, a different SQL
dialect — leaves references/issue-credit.md byte-identical. The
verified diff is in docs/semantic-model/skills.md; only the profile
name, the store line, the --profile argument, and a Spanner-only
gcloud block move.

The judge is not a second axis, and an earlier revision of this PR wrongly
made it one. A rule stated in words is settled by asking a judge, and the
runtime asks it before the transaction opens — never the agent making the
call, which would be the constrained thing certifying itself. So a guarded
action only ever runs against a runtime that has a judge, and that is the
runtime every generated skill is written for: the command line says
--judge and the paragraph under it says what the flag settles.

No physical name reaches the skill. A test asserts the table named in
the DML appears nowhere in the output.

What it refuses to do

• Names that would not install. The frontmatter carries only the two
  fields the spec requires, and name must match the directory it is
  written to. In a scan of 726 installed SKILL.md files, 466 (64%)
  have a frontmatter name that differs from their directory name. The
  generator cannot produce one. It is also quoted: a model named no,
  on or 2024 is not a string under YAML 1.1, which most parsers
  outside JavaScript still read.
• A command line that does not run. An action with no parameters
  used to end the example on a dangling \. Fixed, with a regression
  test, because this is the one place the skill tells an agent what to
  type.
• Overwrite. An existing skill directory needs --force, and
  --force replaces rather than layers: a reference page for an action
  the model no longer declares is deleted and reported, because staged
  loading means an agent will open a page nothing points at.
• Write a skill that can run nothing, silently. A profile that binds
  no store, or an executor the runtime won't wrap, can leave every action
  unrunnable — which was two files and exit 0 saying nothing. It now
  warns before writing, pointing at the section that gives the reason for
  each action.
• Resolve a name collision by picking one. Two models whose names
  normalize to one skill name are refused, naming both.
• Claim a read it cannot do. instructionFor() tells an agent to
  "find it with the lookup tools", but no CLI command invokes a lookup.
  Rather than fork that string, ## Finding a record says so and gives
  the gcloud spanner databases execute-sql line that works.

Advisory guards (on_violation: warn) are listed on the reference page
and marked (advisory). toolDescription() still omits them —
different audience, and the reference page has room for the distinction.

What the review round changed

The first commit put tool.unavailable at the top of each action's
reference page. That reads naturally and it made every page
binding-specific, falsifying the claim above. Moving it into "Running an
action" in SKILL.md makes the invariant hold and says more, since that
section can name which guards the action is waiting on.

The command line in that section had been re-derived here rather than
asked for, and got three things wrong: it omitted --judge-reads-store
where a judgment reads the store, listed optional parameters, and
printed <value> where the type belongs. Sharing runLine fixes all
three and closes the drift.

Also: budget the description from the front so a long model description
is cut rather than the action names; name the action in the router row
as authored rather than as its snake_case tool name; sanitize and
de-duplicate reference filenames so an action name cannot write outside
the skill directory; escape table cells; shell-quote the action name in
the example.

--judge came back off

skills-generate originally took --judge, mirroring kcmd agent tools.
That was wrong, and the confusion it caused is the reason it's gone: a
skill is read by a model, so "a skill for an agent that holds no judge"
sounds like it's asking whether the reader can reason. It isn't, and it
shouldn't have been asking anything.

modelTools({judge}) is coherent in agent_tools.ts — there the judge is
the live object that will settle the call, and advertising a guarded tool
that then refuses mid-call is exactly the bug it prevents.
skills-generate constructs no runtime. It writes a document read later,
by an unknown agent, against an unknown runtime, so the parameter became a
prediction the generator can't make, and a model-level fact made to depend
on a kcmd flag.

It was doing no work beyond suppression, which the goldens prove:
runLine() already derives --judge from the action's guards, nothing
calls the judge during generation, and swapping the flag for an assumed
judge left all three remaining goldens byte-identical.

Also here

storeLine() moves from commands.ts to runtime/store.ts, so the CLI
listing, --store, and a generated skill give one answer for where a
run would land.

reference.md gains a skills-generate entry — every other verb has
one. That commit also drops a flag table sitting between the agent
section and "What gets created in BigQuery" with no heading of its own,
duplicating the row above it. It predates this PR, but a headingless
table would now read as belonging to the section added below it.

Coupling with #448

#448 renames action run → action-run and action list →
action-list. A generated skill embeds that command name, so when #448
lands, nine literal strings change: one in skills.ts, four in
docs/semantic-model/skills.md, four in the tests. runLine itself also
moves here, from commands.ts to runtime/run_action.ts, so #448's edit
to it lands in a different file. #448 is currently CONFLICTING against
main; happy to rebase this behind it in whichever order you want to land
them.

Tests

43 tests in tests/libts/semantic/skills.test.ts for the document, and
5 in tests/tool/skills_generate.test.ts for what reaches the
filesystem — the directory a skill lands in, pruning, and the collision.

Three of those 43 are a golden corpus. actions_place_order.yaml, the
only fixture in the tree carrying actions and constraints, is generated
under three bindings: a SQL executor with a store, the same with no store,
and the authored MCP executor. Each gets its own SKILL.md golden, and
all three are checked against one references/place-order.md golden.
That shared file is the claim at the top of this PR as an artifact rather
than a paragraph: break it and exactly one assertion fails, naming the
binding that moved the page.
UPDATE_GOLDENS=1 re-blesses the shared page from the first case only,
so a later case can't paper over a divergence by overwriting it.

Every other emitter here — bigquery, spanner, osi, pull,
knowledge_catalog — already goldens its output over a shared corpus.
This was the one that didn't, and the review round's defects were all of
the kind a golden catches: a change to the document that reaches a
reviewer as a diff of string concatenation.

It paid for itself on the first run. The sql_unbound golden shows the
gcloud read snippet under "Finding a record" disappearing along with
the command line, so "everything the binding decides is gathered into one
section" was an overclaim in the module comment and in reference.md —
the guide's own pasted diff had shown that snippet moving all along, but
its summary sentence hadn't. The narrower claim the goldens actually
check is that nothing under references/ is about where an action runs.

Suite 1,155 tests; tsc --noEmit clean. Two to four fail in my sandbox,
all in store.test.ts on an expired ADC token — they shell out to
gcloud and race a 5s timeout, so which ones trip varies per run. That
file isn't touched here.

Every diff and listing in the doc came from the built binary, with one
exception now flagged: the zero-runnable warning example used to be
produced by omitting --judge, and with the flag gone that trigger no
longer exists. Rather than paste a listing I couldn't re-run (ADC), that
one is now prose. Worth regenerating against a store-less profile before
this merges.

Second review round

The three SKILL.md goldens are named for the two things that vary across
them -- the executor the action carries, and whether the profile bound a
store -- so the axis reads off the filename (9fd8a7f, pure rename):

sql_bound     sql executor, store bound     -- runs
sql_unbound   sql executor, no store        -- nothing to run against
mcp           mcp executor, store bound     -- store fine, write not wrappable

mcp_unbound is absent on purpose: an unbound profile refuses every action
whatever its executor, so it would only re-check sql_unbound.

Five defects found and fixed in 98a7565, each with a regression test:

• A description whose action list alone passed 1,024 characters was cut to
  length, dropping the "Use when..." sentence a client routes on and ending
  the list part-way through a name. Names now come off the end with an exact
  count and "and N more".
• The command block was rebuilt by splitting the rendered line on ' --',
  which splits an action name too. An action named Place --Order produced a
  bogus --Order flag. runFlags() now hands the flags over separated.
• A model with actions and no entities lost "Finding a record" entirely while
  keeping the instruction to use lookup tools the skill does not have.
• Warnings were labelled with the slugged skill name, errors with the model
  name. Both are the model name now, matching reference.md.
• runLine's comment, left in commands.ts when the function moved, had come
  to read as documentation for runOneAction.

The three goldens did not move under any of these, which is what says the
first two were rendering faults and not changes to what the emitter decides.

b001fe9 is separable and unrelated to skills — it fixes a bug in a621a20,
already on main: cac serves --help for a command it never matched, so
kcmd bogusverb --help exited 0 and never reported the unknown command. It
also raises the timeout on the two push --version tests, which were failing
on bun's 5s default rather than on what they assert. Happy to split it out.

Not in this PR

Reads, metrics, and a full Agent Plugin (plugin.json + mcp.json).
Listed under "What it doesn't generate yet" in the doc.

[libei]

Reviewed PR #451 — the architecture (modelTools + runFlags + storeLine single source of truth, router-plus-references structure, and the 3-to-1 shared reference golden proving deployment invariance) is super clean. Left 4 inline comments on minor edge cases.

[libei]

Partial write on skill-name collision in multi-model scopes:

Because collision detection (written.get(generated.name)) runs inside the same loop that writes dir to disk, if two models in a scope normalize to the same skill name (e.g. Sales Orders and sales_orders) and the target directory does not yet exist:

1. The first model writes skills/sales-orders/ to disk (Wrote skills/sales-orders/...).
2. The second model triggers the collision error and exits 1, leaving the first model's skill written on disk.

Since generateSkill() is pure and in-memory, generating all SkillPackages (and checking name collisions / existing directories) in a first pass before writing to disk will ensure nothing is written when two models collide.

[libei]

Confirmed and fixed in 2481df0. Reproduced first by asserting on disk in the existing collision test — skills/sales-orders/ was there after the run that refused the scope:

expect(fs.existsSync(path.join('skills', 'sales-orders'))).toBe(false);
  Expected: false
  Received: true

Two passes now, as you describe. generateSkill is pure, so the first pass generates every model, resolves the name each one claims, and checks its directory; nothing is written until the whole scope comes back clean.

One judgement call beyond the collision case: I made every pre-write fault stop the scope, not just a name collision. A generate error and an existing directory without --force are discovered in the same pass and now behave the same way — if (failed) return 1; before the write loop. The alternative is a rule that reads "a collision writes nothing, but an occupied directory writes everything else", which is harder to predict than all-or-nothing and leaves the same half-written scope by a different route. Say the word if you would rather the existing-directory case stay per-model.

Genuinely unwritable paths are still per-model and still continue — that one cannot be checked in advance, and the comment now says so.

That assertion stays in the test as the regression.

[libei]

actsSentence can lengthen the sentence on the 1 more step:

When the last action name is shorter than "and 1 more" (10 chars), replacing , <lastName>. with , and 1 more. makes sentence longer than the unabridged list. For example, if names has two short names (["Pay", "Void"]) and budget is slightly smaller than "Declares 2 actions: Pay, Void." (30 chars), the loop sets sentence to "Declares 2 actions: Pay, and 1 more." (36 chars), exits the loop, and truncate() cuts the longer "and 1 more" string rather than the original sentence.

Keeping the initial full string for the final truncate(full, budget) fallback (or skipping steps where the and N more string is longer than full) avoids truncating a string made longer by the suffix.

[libei]

Confirmed and fixed in 2481df0. The loop now only takes the step when it actually shortens the sentence, so the full list survives to the cut when and N more would be longer:

const dropped = `${head}${names.slice(0, shown).join(', ')}, and ${
    names.length - shown} more.`;
if (dropped.length < sentence.length) sentence = dropped;

Worth recording what it takes to see this from outside, because it is narrower than it first looks. Both forms share the prefix head + names[0] + ', ', so they only differ inside the cut when the first name nearly fills the budget on its own — and the second still has to be short enough (< 10) to trigger the step and long enough to push the list over. That pins it to a window: with the budget at 954, len(names[0]) between 922 and 925.

Simulated both versions over that range to find a case that actually diverges rather than guessing one:

L0=925 L1=7
  before: "AAAAAAAAAAAAAAAA, and 1…"  (953)
  after:  "AAAAAAAAAAAAAAAAAAAAAA,…"  (947)

The old output ends part-way through and 1 more — an abridgement marker reading as the tail of a name, which is the same failure mode the function was written to avoid. The regression test uses those sizes and explains why they are what they are; it fails against the previous code.

[libei]

Minor: Unescaped pipe in tool.actionName inside GFM table cells:

Since action.name is an unconstrained string (z.string()), if tool.actionName contains a |, GFM tables split the row even when the pipe is inside backticks (the same edge case handled for p.default on line 626). Passing cell(tool.actionName) here (and for a.concept in affectsRow) keeps the Markdown table intact for arbitrary names.

[libei]

Confirmed and fixed in 2481df0, and widened slightly — the same cell was unescaped in four places, not two. All of them take cell() now:

• tool.actionName in the router table (here)
• a.concept, a.operation and the fields list in affectsRow
• p.name and p.type in the argument table, which are as unconstrained as the rest

One deliberate exception: the Reference column keeps paths.get(tool.actionName) raw. That value is a slug from skillNameFor, whose alphabet has no pipe in it — the same property that stops ../../../.bashrc reaching the filesystem. Escaping it would be dead defence implying the slug is arbitrary text.

Two tests added, one per table; both fail against the previous code.

[libei]

CLI unknown-verb check when --help precedes the unknown verb:

Note why kcmd action list --help had action removed from cli.args: cac only removes the command name from cli.args when it matches a command, in which case cli.matchedCommand is truthy (c.name === "action"), so execution skips both if (!cli.matchedCommand) blocks anyway and exits 0 naturally.

When !cli.matchedCommand is true, cac did not match any command and left any positional arguments in cli.args (as used on line 268: if (cli.args.length > 0)). Because typed.startsWith("-") only inspects process.argv[2], running kcmd --help bogusverb sets typed = "--help", making verbIsKnown === true and exiting 0 instead of reporting Unknown command 'bogusverb'.

You can simplify this to:

if (!cli.matchedCommand && cli.args.length === 0 &&
    (cli.options.help || cli.options.version)) {
  process.exit(0);
}

which handles kcmd --help, kcmd action list --help, kcmd bogusverb --help, and kcmd --help bogusverb.

[libei]

Confirmed and fixed in 6dbfdb1 — but with the scan rather than the cli.args.length === 0 gate, because that gate regresses the sub-argument case.

kcmd --help bogusverb exited 0. Real bug, exactly as described.

On the premise, though: cac strips the matched command's name from cli.args and clears matchedCommand, in the same breath. Both are true at once, so !cli.matchedCommand does not imply "nothing matched". Instrumented at the gate:

kcmd --help                  matched=null args=[]
kcmd action --help           matched=null args=[]
kcmd action list --help      matched=null args=["list"]
kcmd owl import x.ttl --help matched=null args=["import","x.ttl"]
kcmd bogusverb --help        matched=null args=["bogusverb"]
kcmd --help bogusverb        matched=null args=["bogusverb"]

So cli.args.length === 0 would send action list --help and owl import x.ttl --help down the error path and print Unknown command 'list' / Unknown command 'import'. That is also why cli.args[0] cannot decide it on its own: list and bogusverb are both words that name no command.

What separates them is only recoverable from process.argv. The fix keeps that and drops the wrong assumption inside it — the verb is the first token that is not a flag, not the first token:

const typed = process.argv.slice(2).find(arg => !arg.startsWith('-'));
const verbIsKnown =
    typed === undefined || cli.commands.some(c => c.name === typed);

Scanning is exact here because the only options cac takes ahead of a command are --help and --version, and neither swallows a value that could be mistaken for the verb.

All nine invocations, after:

invocation	exit
--help	0
--version	0
action --help	0
skills-generate --help	0
action list --help	0
owl import x.ttl --help	0
--help action	0
bogusverb --help	1	Unknown command 'bogusverb'
--help bogusverb	1	Unknown command 'bogusverb'

Two tests added for the new pair. Kept in its own commit so it can still travel with b001fe9 if that gets split out of this PR.

[libei]

Merged. One change went in at the merge that you did not review, and it is a content change rather than a mechanical one, so it is worth flagging.

ace6f20 (#452) deleted entity-reference parameters and the in-transaction resolve. This skill promised that resolve to the agent reading it:

An action argument typed as an entity will also accept text that identifies exactly one record, and the call fails when nothing matches or more than one does.

Merging that as-written would have shipped a skill telling an agent to pass a name where a key is now wanted — the expensive direction for a falsehood to point.

What replaced the resolve is said in its place, because it answers the question an agent has once it is told to go and find a key:

A key that matches no record costs you the call rather than the data: a statement that writes no rows fails the action and rolls the whole transaction back, so nothing is half-applied and nothing is silently skipped.

That is a property of the sql executor this runtime performs itself, so it is gated on the kind — the MCP golden drops the sentence, the two SQL goldens carry it. The reference page follows the fixture: customer projects from customer.c_custkey, reads as an integer described by the field, and the run line asks for <Integer>. Still one page shared by three bindings, which was the claim under test.

Three tests changed with it, none deleted: the entity-argument assertion became a projected-vs-declared one, and the no-entities pair became a does-this-runtime-perform-the-write pair.

npm test is green on all four legs (1046 semantic, 62 tool) and npm run build compiles.

Two things still open from the review, neither blocking:

• Whether the existing-directory case should stay per-model rather than stopping the whole scope. Reversible in one if.
• The zero-runnable warning example in docs/semantic-model/skills.md is still the hand-written one; regenerating it against a store-less profile needs ADC.