Skip to content

feat(mdcode)!: trim the action command lines to what curation needs - #448

Draft
libei wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
libei:kcmd-actions-trim
Draft

feat(mdcode)!: trim the action command lines to what curation needs#448
libei wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
libei:kcmd-actions-trim

Conversation

@libei

@libei libei commented Sep 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

kcmd is for curating a semantic model, not for dispatching its actions. The
action command lines had drifted into the second job, and this PR trims them
back to the first.

What goes

Every flag that made kcmd behave like a runtime:

Flag Was on Why it goes
--judge [model] action-run, agent-tools Hires a Gemini model from a debugging CLI
--judge-location <region> action-run Configuration for a judge there is no longer
--judge-reads-store action-run A judge issuing SQL, from a CLI

What is left is three flags — --profile, --store, --arg — with no
combinations between them: action-list (--profile, --store), action-run
(--arg, --profile), agent-tools (--profile). Six distinct flags over
eight command-flag pairs becomes three over five.

Flat verbs

action <command> [name] and agent <command> become action-list,
action-run and agent-tools. cac has no nested subcommands, so the space
form was a positional verb dispatched by hand — the same shape owl <action> <file> still has, and it was being paid for in the help text: five of the six
flags on action ended in a parenthetical naming which subcommand they applied
to, which is cac's flag-to-command binding rewritten in English and enforced by
nobody. kcmd action list --judge-reads-store parsed clean.

Flat verbs give each command its own flags, its own --help, and a misspelling
caught by the parser rather than by a hand-written if forty lines into
commands.ts.

What action-run does now

It binds the arguments, opens one transaction, applies the statements. Before
it writes, it names the guards it is passing over, so nobody reads a committed
write as a checked one:

$ kcmd action-run IssueCredit --arg order=12346 --arg amount=3.00 --arg memo="Coupon applied late"
Running 'IssueCredit' on projects/my-project/instances/my-instance/databases/semantic_agent_demo...
  NOT CHECKED: CreditWithinOrderTotal, CreditUnderReviewThreshold, CreditMemoNamesAServiceFailure, CreditIsNotSplitToAvoidReview -- this command settles no guard, and the write still happens
Committed at 2026-09-20T18:51:11.964483Z.

That is the trade, and it is the useful half for curation: whether an action
binds its arguments, writes the line it says it writes and leaves the store
consistent is a question about SQL, and one command against your own database
answers it with no judge to stand up first.

What agent-tools does now

It derives the tool set with skipGuards, so a guarded action is offered
rather than marked [NOT RUNNABLE]. Who settles a rule belongs to whoever
dispatches the call, and this command cannot know what that will be, so
marking the action unrunnable would describe a caller rather than the model.
The rules are still printed in the tool's own description, wording and all.

A tool is still withheld for what no judge would repair: no executor under the
binding, an executor no handler runs, a guard naming a rule the model never
declares, or a guard naming one that quotes nothing.

The skills generator

Rebased onto #451, which moved runLine/runFlags into run_action.ts and
uses them to build the command line in a generated Agent Skill. Those no longer
emit a judge flag, and the skill's paragraph under the line now says the line
settles no guard — so an agent that tries the call and watches it commit does
not read that as the rules having held. The flat verbs reach the skill too, so
it says kcmd action-run. The golden fixture is regenerated.

Guards are untouched

Judge, GeminiJudge, modelJudgeStore and runAction's judge option are
all unchanged. The demo agent in demo/semantic-model/agent/ settles all four
of its rules against Gemini, reads the store for the one that needs a number on
record, and reaches all three on_violation outcomes live. Only the command
lines stopped pretending to be that application. Putting a judge back behind a
command later is a decision this PR leaves open.

One defect the live run found

Under skipGuards the unsettled-guard loop still warned once per rule, burying
the outcome of the write under a list the caller had just written. The loop is
now gated on !opts.skipGuards, with a test.

Docs

actions.md §7 and the agent README's steps 3 and 4 are rewritten. Every CLI
listing in both was regenerated from a live run — the README's against the
demo's Spanner store, actions.md's against the payments model rebuilt from
that page's own snippets until action-list matched it byte for byte.

Verification

tsc --noEmit clean; full test suite passes (1049 + 64 + 76 + 21 + 2, 0 fail);
npm run build OK; cross-document anchor check 11 files / 0 problems;
reference.md's flag tables diffed against live --help.

@libei libei mentioned this pull request Sep 20, 2026
Merged
libei added a commit that referenced this pull request Sep 20, 2026
@libei
feat(mdcode): generate an Agent Skill from a semantic model (#451)
da81e55 
`kcmd skills-generate` writes a semantic model out as an [Agent
Skill](https://agentskills.io) 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
feat(mdcode)!: trim the action command lines to what curation needs
da429a0 
kcmd is for curating a semantic model, not for dispatching its actions, and
the action commands had drifted into the second job. `--judge` hired a Gemini
model from a debugging CLI, `--judge-reads-store` let it issue SQL,
`--judge-model` and `--judge-location` configured it, `--skip-guards` existed
to undo it, and `--json` was a second output format nobody consumed. Every
listing then had to say which combination a reader was looking at.

All of them are gone. What is left is four flags with no combinations between
them: `action-list` (--profile, --store), `action-run` (--arg, --profile),
`agent-tools` (--profile).

`kcmd action-run` binds the arguments, opens one transaction and applies the
statements. Before it writes, it names the guards it is passing over, so
nobody reads a committed write as a checked one:

  $ kcmd action-run IssueCredit --arg order=12346 --arg amount=3.00 \
      --arg memo="Coupon applied late"
  Running 'IssueCredit' on .../databases/semantic_agent_demo...
    NOT CHECKED: CreditWithinOrderTotal, CreditUnderReviewThreshold,
    CreditMemoNamesAServiceFailure, CreditIsNotSplitToAvoidReview --
    this command settles no guard, and the write still happens
  Committed at 2026-09-20T18:51:11.964483Z.

That is the useful half for curation: whether an action binds its arguments,
writes the line it says it writes and leaves the store consistent is a
question about SQL, and one command against your own database answers it with
no judge to stand up first.

`kcmd agent-tools` derives the same set with `skipGuards`, so a guarded action
is offered rather than marked `[NOT RUNNABLE]`: who settles a rule belongs to
whoever dispatches the call, and this command cannot know what that will be.
What still marks a tool unrunnable is what no judge would repair -- no
executor under the binding, an executor no handler runs, a guard naming a rule
the model never declares, or one that quotes nothing.

`runFlags` no longer emits a judge flag, so the suggested line in
`action-list` and in a generated Agent Skill (GoogleCloudPlatform#451) carries the arguments and
nothing else. Where the action states a rule, the skill now says that the line
settles none of them, so an agent that tries the call and watches it commit
does not read that as the rules having held. The verbs are also flat now, so
the skill and its golden fixture say `kcmd action-run` rather than `kcmd
action run`.

Guards reach a runtime unchanged. `Judge`, `GeminiJudge` and `modelJudgeStore`
are untouched, the demo agent in demo/semantic-model/agent/ settles all four
of its rules against Gemini, and `runAction`'s `judge` option is still how an
application passes one in. Only the command lines stopped pretending to be
that application. A later change can put a judge back behind a command if one
turns out to be wanted.

One defect the live run found: under `skipGuards` the unsettled-guard loop
still warned once per rule, burying the outcome of the write under a list the
caller had just written. The loop is gated, with a test.

Docs: actions.md section 7 and the agent README's steps 3 and 4 rewritten.
Every CLI listing in both was regenerated from a live run -- the README's
against the demo's Spanner store, actions.md's against the `payments` model
rebuilt from the page's own snippets until `action-list` matched it byte for
byte.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@libei