Skip to content

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

Merged
libei merged 11 commits into
GoogleCloudPlatform:mainfrom
libei:kcmd-actions-trim-rebased
Sep 21, 2026
Merged

libei merged 11 commits into
GoogleCloudPlatform:mainfrom
libei:kcmd-actions-trim-rebased

Conversation

@libei

@libei libei commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Replaces #448, which cannot be updated in place: this branch is a rebase and the
head of that PR cannot be moved without a force-push.

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

And one flag moves rather than goes — --store, to profiles --print-store
(below).

What is left on the three commands is two flags with no combinations between
them: action-list (--profile), action-run (--arg, --profile),
agent-tools (--profile). Across those three, six distinct flags over eight
command-flag pairs becomes two over four; profiles gains two.

--store moves and is renamed

action-list --store printed where a run would land and exited, suppressing
the listing entirely — a mode flag on a listing command, wearing the name of a
target flag. Nothing on any kcmd command line selects a store: --profile
picks a binding, and the binding's deployment target decides where writes land.
So a flag called --store that takes no value and only prints is one you have
to run before you can tell which direction it goes.

It becomes kcmd profiles --print-store — the command that owns bindings, under
a name that says which way it points. Output is byte-identical, so the demo's
setup script changes only the command it calls:

IFS=/ read -r PROJECT INSTANCE DATABASE <<<"$(kcmd profiles --print-store)"

profiles also gains --profile [name], which narrows the report to one
profile and is what picks the store --print-store prints; without it the move
would drop what action-list --store --profile alloydb already did. Naming a
profile the model does not declare exits 1, because an empty report reads as
"this profile withholds nothing" — the opposite of what a typo means.

profiles had no flags and no section in reference.md; it now has both.

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.

action-list takes no positional at all. The old action <command> [name]
carried one because action run <name> needed it and action list did not;
once the verbs are flat, action-list [name] is a promise the command never
kept — it accepted the name and listed everything. It lists everything, and
says so.

What action-run does now

It binds the arguments, opens one transaction, applies the statements. It
settles none of the guards, and the outcome says so, 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_skill_demo...
Warning: guards were not checked: CreditWithinOrderTotal, CreditUnderReviewThreshold, CreditMemoNamesAServiceFailure, CreditIsNotSplitToAvoidReview -- this run was told to skip them, and the write was made anyway
Committed at 2026-09-21T02:33:20.747065Z.

That sentence is a property of the run, not of this command line. It rides on
ActionOutcome.warnings, so an agent handed the same action as a tool by its
own framework reads it in the tool's result rather than a bare applied: true.
See the review round below — that is what the review changed.

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, and still tested. Only the command lines stopped pretending to be
the application that hires them. Putting a judge back behind a command later is
a decision this PR leaves open — see the note below, because after #453 nothing
in the repository hires one.

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.
reference.md gains a profiles section, which it never had. The skill demo's
README is rewritten as above; every listing in it is from a live run against a
Spanner store stood up for this revision and dropped afterwards.

The demo #453 just shipped

#453 merged while this was open. It deleted demo/semantic-model/agent/ — so
every link here into that directory was dead — and it shipped a 906-line
demo/semantic-model/skill/README.md built on kcmd action run --judge --judge-reads-store: twelve command lines, nineteen judge flags, and two
sections whose entire subject is a flag pair this PR removes.

Section 4 is rewritten around what the command line does now, against a live
store. A $3.00 credit commits, carrying the warning. Then the same command asks
for $20.00 against an $18.00 order — the case CreditWithinOrderTotal exists to
stop:

$ gcloud spanner databases execute-sql "$DATABASE" \
    --instance="$INSTANCE" --project="$PROJECT" \
    --sql='SELECT order_id, total FROM Orders WHERE order_id = 12346'
order_id  total
12346     -5

An order with a total of negative five dollars, committed, with the rule that
forbids it sitting in the model. That is what the trim costs, shown rather than
asserted, and it makes the case for where guards belong better than the prose it
replaces. Those two writes are real, so the page now resets 12346 before
section 7, which runs against the seed from section 3.

The guarded transcripts are kept and marked — they are the only record of one
action carrying four rules through all three on_violation outcomes — and they
are pasted as they were printed, two-word verb and all, because a transcript
nobody ever saw is not evidence. The agent runs in sections 7–9 get the same
note: those calls were refused because the generated command line carried a
judge at the time, and the skill as it generates today would commit them.

Three claims the demo made are now false, and the page says so rather than
letting them stand: that the rules are enforced underneath whatever reads the
skill, that an agent ignoring the description cannot get past them, and that the
split is impossible because of the runtime rather than because the harness
behaved. Limits leads with the honest version.

Worth a decision before this merges. This PR and #453 between them leave no
caller in the repository that settles a guard: the CLI declares them unchecked,
and the one application that hired a judge was the ADK agent #453 removed. The
judge, the routing and the rules are all still library code and still tested —
what is gone is anything wired to them. That is the "add back later" half of the
trim, and it is now the demo's top limit.

One silent break the merge did not flag

#453 added a skills.ts import of readableEntities, which this PR had
unexported. Both sides merged cleanly and only tsc caught it. Re-exported: the
skills generator calls it to name the tables a generated skill tells an agent it
may read, which is a different use from the judge's and a legitimate one.

First review round

A review of the branch raised eight findings. Seven held up; one did not, and
checking it found a regression the review had not.

Three were the same mistake in three places: a name nobody declares got
answered as if it were a question about an empty set.

  • action-list <name> took the positional and ignored it, listing everything
    in scope. The ignoring predates the branch, but renaming the command promoted
    the argument into an explicit promise. Fixed here by making it filter; the
    second round dropped it instead (below).
  • profiles --profile <typo> exited 0 against a model with no profile
    documents — the "no binding profiles" line returned before the name was
    checked, so a script keying off the exit code read a misspelling as success.
    Confirmed on a live run, not just by reading.
  • The fix for that one then broke profiles --profile default. default is the
    sentinel for the inline bindings rather than a profile document — a push
    rejects a file that claims the name — which makes it the one profile name
    always valid against every model. It reports the inline bindings now.

The most consequential finding was in the skill generator. The caveat that
kcmd action-run settles no guard was emitted only when the action the example
command line happened to pick was itself guarded. Pick order is declaration
order, so a model whose first runnable action is unguarded shipped a SKILL.md
with the caveat missing — while its reference pages still told the agent "Each
is settled before anything is written." The one place the skill corrects that
claim was the place that dropped out. It keys on the model now.

Also: skipGuards no longer asks a judge when one is handed over (it used to
spend the call and suppress the verdicts into warnings, so the two inputs
disagreed with no way to tell from the result which won); a demo model document
stopped describing a judge flag kcmd no longer has; and one test comment
described behaviour that is not what its assertion checks.

The finding that did not hold claimed profiles --print-store --profile default
succeeds where the report path errors. Both exit 1 on a live run; only the
messages differ.

Separately, an editor formatter had run over src/tool/commands.ts and mangled
new Map<string, {...}>() into new Map < string, {...} > (). Rather than
patch the one visible break, the file was restored from its pre-format state and
the branch's changes re-applied on top — churn fell from 256/256 to 181/161
lines — and the value type is now named, so there is no object literal inside a
generic argument left for a formatter to break.

Second review round

Eleven inline comments. All eleven were correct; two were already fixed by
commits pushed half an hour after the review, and the other nine are addressed
in 9ed8854, fe72c37, 98eb162 and ff19a55.

The one that changed the design. The doc comment on skipGuards claimed the
run still reports every guard it did not check — and the code below it did the
opposite, suppressing the whole loop under skipGuards on the reasoning that a
caller who asked for the skip knows it asked. True of the caller and of nobody
else: describeOutcome hands an outcome to an agent as the answer to its tool
call, and an agent told applied: true and nothing further has been told the
write met every rule the model states, which is the one thing it did not. The
CLI had been papering over this with a line of its own.

So the fact moved onto the outcome rather than the sentence being deleted.
runAction names every guard a skipGuards run passed over in warnings, in
the order the action states them, one warning for the whole skip rather than one
per rule; everything downstream gets it without having to remember. That also
settled a second comment: the CLI's pre-run line was printed before runAction
had decided there would be a run at all, so a call with a missing argument
announced that the write happens and then errored without opening a
transaction. It is a fact about a write that was made, so it is now printed from
the outcome and only when there is one.

action-list drops its positional rather than filtering, as the review
suggested — this change is trimming the surface, and a listing command that
always lists everything is one fewer thing to know. That backs out the
unknown-name check the first round added for the filter.

Three documents claimed this repository settles a guard. reference.md,
model_spec.md and actions.md all said the commerce demo is where the guards
are actually settled, live. That was true when the generated command line was
kcmd action run --judge --judge-reads-store. The transcripts are still there
and still the only place a guard is shown holding an agent to something, but
they record what hiring a judge takes, not what this repository does.

The demo's own sequence was broken by its new transcripts. The two runs in
section 4 now commit, leaving order 12346 at -$5.00, while section 7 opens
by saying the four runs are consecutive against the seed from section 3 and
shows 12346 at $18.00. A reset was added before section 5. Both action-run
transcripts were re-recorded against a live Spanner database rather than edited,
since the warning text changed.

Plus four leftover kcmd action references in prose and code comments.

Verification

tsc --noEmit clean; full test suite passes (1052 + 73 + 76 + 21 + 2 = 1224, 0
fail), including nine profiles tests covering the filter, the typo against
both a model with profile documents and one without, the default sentinel, the
two non-naming --profile forms and both --print-store backends, and a new
agent_tools test proving a skipped guard reaches the agent rather than only
the command line; npm run build OK; cross-document anchor check 12 files / 0
problems; the flag table of every one of the five documented commands diffed
against live --help; the demo's action-list block and both new action-run
transcripts pasted from a live run against a Spanner database stood up for this
revision and dropped afterwards; every generated-skill excerpt in section 5
diffed against a fresh skills-generate.

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.
`action-list --store` printed where a run would land and exited, suppressing
the listing entirely: a mode flag on a listing command, wearing the name of a
target flag. Nothing on any kcmd command line selects a store -- `--profile`
picks a binding and the binding's deployment target decides where writes land
-- so a flag called `--store` that takes no value and only prints is one a
reader has to try before they can tell which direction it goes.

It moves to the command that owns bindings, under a name that says so:
`kcmd profiles --print-store`. The output is byte-identical, so the demo's
setup script changes only the command it calls.

`profiles` also gains `--profile [name]`. It narrows the report to one profile
and is what picks the store `--print-store` prints; without it the move would
drop what `action-list --store --profile alloydb` could already do. Naming a
profile the model does not declare exits 1, because an empty report reads as
"this profile withholds nothing", which is the opposite of what a typo means.

`action-list` is left with one flag.
The trim unexported it, on the grounds that letting a model compose
queries against a caller's data is a decision for whoever embeds the
runtime rather than something the curation CLI offers. That reasoning
still holds for the judge, but GoogleCloudPlatform#451's skills generator calls the same
function for a different purpose: naming the tables a generated skill
tells an agent it may read. The two PRs were in flight together, so the
textual merge was clean and only the type checker caught it.
GoogleCloudPlatform#453 shipped this demo hours before the trim removed the flags it runs
on. Twelve command lines and nineteen judge flags in one README, and two
sections whose entire subject was a flag pair that no longer exists.

Section 4 is rewritten around what the command line actually does now,
with two live runs: a credit that commits under the NOT CHECKED banner,
and the same command asked for twenty dollars against an eighteen dollar
order -- the case CreditWithinOrderTotal exists to stop -- committing and
leaving the order at a total of negative five dollars. That is the cost
of the trim, shown rather than asserted, and it is a better argument for
where guards belong than the prose it replaces.

The guarded transcripts are kept and marked. They are the only record of
one action carrying four rules through all three on_violation outcomes,
and they are pasted as they were printed, two-word verb and all, because
a transcript nobody ever saw is not evidence. The agent runs in sections
7 through 9 get the same treatment: those calls were refused because the
generated command line carried a judge at the time, and the skill as it
generates today would commit them.

Three claims the demo made are now false and say so instead of quietly
standing: that the rules are enforced underneath whatever reads the
skill, that an agent ignoring the description cannot get past them, and
that the split is impossible because of the runtime rather than because
the harness happened to behave. Limits leads with the honest version --
no caller in this repository settles a guard today, because the CLI
declares them unchecked and the one application that hired a judge was
the ADK agent GoogleCloudPlatform#453 removed. The library is untouched and still tested;
what is missing is something wired to it.

@libei libei left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed PR #454. The split into flat verbs (action-list, action-run, agent-tools), profiles --print-store / --profile, and skipGuards for curation commands are clean and well-tested.

Left inline comments on a few issues to address before merge:

  1. Stale docs/comments after removing --judge and rebasing over #453 (commerce.yaml, reference.md, actions.md, model_spec.md).
  2. CLI & runtime edge cases:
    • action-list [name] declares a [name] positional argument that actionList silently ignores.
    • profiles --profile <unknown> exits 0 instead of 1 when a model has no .profiles/*.yaml files.
    • RunActionOptions.skipGuards doc comment contradicts if (!opts.skipGuards) in run_action.ts (and ActionTool.call() with skipGuards: true drops guard warnings).
    • runOneAction prints NOT CHECKED: ... and the write still happens before runAction checks whyRefusedWithoutRunning and bindParameters.
  3. Minor nits (actions.md, main.ts, commands.ts, action.test.ts, and resetting Order 12346 after Section 4 in demo/semantic-model/skill/README.md).

# therefore takes a judge that has been given one -- a `store` on the judge the
# agent hires. Giving a judge your tables belongs to whoever embeds the
# runtime, so no kcmd flag offers it, and `kcmd action-run --judge` stops on
# that rule rather than guessing at the number it names.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kcmd action-run no longer has a --judge flag (and does not stop on this rule — it runs with skipGuards: true). This comment looks like a leftover from an intermediate draft where only --judge-reads-store was removed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and already fixed -- in 4c7d58a, pushed about half an hour after
you left this. The comment now says kcmd action-run settles no guard at all,
names them and writes anyway, rather than describing a flag that is gone.

author can exercise a model they are curating, and find out whether the
statements do what they meant, without first standing up an agent. To see the
guards actually settled, run the model through something that embeds the
runtime: the [commerce demo](../../demo/semantic-model/skill/README.md) hires

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence described demo/semantic-model/agent/README.md before #453 deleted it. Now that the link points to demo/semantic-model/skill/README.md, it contradicts that page (which explicitly states that there is no agent in the directory and that the executor it hands the agent does not currently settle guards).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Fixed in 98eb162. The paragraph now says settling the guards takes
something that embeds the runtime and hires a judge for it, that nothing in this
repository does today, and that the demo keeps recorded runs of what it takes
while its own executor is this command, which settles none.

The rules below run against the commerce model under `demo/semantic-model/skill`
— the [credit policy worked through earlier](#a-credit-policy-worked-through),
rebuilt around what a runtime can settle today. A profile binds `IssueCredit` to
a `sql` executor, and the action names four rules in `guards`. One of the four

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small doc mismatches here after #453:

  1. Lines 1132–1133 say the commerce demo (demo/semantic-model/skill/README.md) is "where the guards are actually settled, against the same model, live", whereas demo/semantic-model/skill/README.md now states that no caller in the repo settles guards today and only keeps recorded transcripts.
  2. Line 1149 says "These three were recorded through kcmd action-run, back when it took a judge" — when it took a judge, the command was kcmd action run (as demo/semantic-model/skill/README.md notes).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both correct, both fixed in 98eb162.

The "actually settled ... live" claim now says the demo is where what it takes
to settle them is shown, in runs recorded while a command line still hired a
judge. And the blockquote below it now says kcmd action run -- the two-word
verb those transcripts were actually recorded under -- rather than renaming them
to a command that would not have produced them.

refused rather than run past its rules. No command line here does that:
`kcmd action-run` checks no guard, because who settles one belongs to whoever
dispatches the call in earnest. The [commerce
demo](../../demo/semantic-model/skill/README.md) is where it is shown. Rules

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same note as reference.md: demo/semantic-model/skill/README.md no longer embeds a runtime with a live judge (only historical transcripts from before #453 and this PR).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Fixed in 98eb162, same rewrite as reference.md: the demo shows what
hiring a judge takes, in recorded runs, and nothing in this repository embeds one
today.

Comment thread toolbox/mdcode/src/tool/main.ts Outdated
.option(
'--arg <name=value...>',
'Bind one action parameter; repeat the flag for each one (`run` only)')
'action-list [name]',

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

action <command> [name] only had [name] because action run <name> required an action name while action list did not. Now that action-list is its own command, [name] is advertised here (and in reference.md:99) and accepted by cac, but actionList(_name, options) in src/tool/commands.ts:1291 ignores _name completely (listActions also keeps an unused options: ActionOptions parameter). Running kcmd action-list IssueCredit or kcmd action-list NoSuchAction silently ignores the argument and prints all actions with exit code 0.

Suggest dropping [name] from action-list (cli.command('action-list', ...)).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and taken as suggested -- 9ed8854 drops [name] from action-list
rather than making it filter.

I had it filtering first. Dropped it instead: this change is trimming the
surface, and a listing command that always lists everything is one fewer thing
to know. listActions lost the unused options parameter with it.

That also backs out 1d55914, which added a scope-wide unknown-name check for
the filter -- with no name to pass there is nothing to be wrong about. The repo
squash-merges, so the history flattens.

Comment thread toolbox/mdcode/src/tool/commands.ts Outdated
const available = layout.profileDocuments(doc.name);
if (!available.length) {
const declared = layout.profileDocuments(doc.name);
if (!declared.length) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because if (!declared.length) continues before checking only, running kcmd profiles --profile <unknown> against a model with only inline bindings (declared.length === 0) prints the inline binding note and exits 0 instead of setting missing = true and exiting 1 (whereas kcmd profiles --print-store --profile <unknown> exits 1, and reference.md states that naming an undeclared profile is an error).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and already fixed -- 1d55914, pushed about half an hour after this
review. The typo check now runs before the inline-bindings line, so an unknown
name exits 1 whether or not the model declares any profile documents.

Worth flagging what that fix nearly broke, since it is the same class of bug:
default is not a profile document and never can be -- a graph-enabled push
rejects a file by that name -- so the check called the one profile name that is
always valid undeclared, and kcmd profiles --profile default started exiting
1. There is now an explicit branch for the sentinel above the check, with a
test for it under both a model with profiles and a model with only inline
bindings.

// supply and would find every guarded action unrunnable; the alternative is
// deleting the guards to test the write, which is worse. The run still
// reports each guard it did not check, so a caller reading the output is
// never told the write passed rules nothing consulted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last sentence of this doc comment ("The run still reports each guard it did not check, so a caller reading the output is never told the write passed rules nothing consulted") contradicts lines 202–207 below (if (!opts.skipGuards)), which suppresses unsettledGuards warnings in runAction() when skipGuards: true.

Also note that ActionTool.call() in src/libts/semantic/runtime/agent_tools.ts:206 passes skipGuards: opts.skipGuards to runAction() and formats the result with describeOutcome(outcome) — because outcome.warnings is empty when skipGuards: true, calling such a tool returns Committed ... with no mention that guards were skipped.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and this was the most useful comment in the set -- the contradiction was
real and the code was the wrong half of it.

The carve-out's premise was "the caller asked for the skip, so the caller knows".
True of the caller and of nobody else. describeOutcome hands an outcome to an
agent as the answer to its tool call, and an agent told applied: true and
nothing further has been told the write met every rule the model states, which is
the one thing it did not.

So fe72c37 moves the fact onto the outcome instead of deleting the sentence.
runAction now names every guard a skipGuards run passed over, in warnings,
in the order the action states them -- one warning for the whole skip rather than
one per rule -- and everything downstream gets it without having to remember.
tests/libts/semantic/runtime/agent_tools.test.ts has a new case proving the
agent path specifically.

Comment thread toolbox/mdcode/src/tool/commands.ts Outdated
const guards =
(runtime.model.actions ?? []).find(a => a.name === name)?.guards ?? [];
if (guards.length) {
console.log(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because NOT CHECKED: ... -- this command settles no guard, and the write still happens is printed before runAction() runs whyRefusedWithoutRunning and bindParameters, any call that fails pre-flight validation (e.g., missing/invalid --arg, an mcp/rest executor, or a guard with an empty judgment) prints "and the write still happens" right before Error: ... even though no transaction was opened and no write happened.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Fixed in fe72c37, by deleting the banner rather than moving it.

It is a fact about a write that was made, and it now rides on
ActionOutcome.warnings (see the thread on run_action.ts), so it is printed
from the outcome -- which only exists once there was a run. A call that fails
pre-flight now errors without ever having announced that the write happens.

tests/tool/action.test.ts gained says nothing about guards until there is a write to say it about, which is the case you described.

### Calling it from code

`kcmd agent tools` prints these tools; `modelTools` returns them. Both take a
`kcmd agent-tools` prints these tools; `modelTools` returns them. Both take a

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: leftover kcmd action reference (createSemanticRuntimes assembles them the way kcmd action does -> kcmd action-list / kcmd action-run).

A few similar leftover comment references in code:

  • src/tool/commands.ts:1363: `agent tools` -> `agent-tools`
  • src/tool/main.ts:267: `action list --help` -> e.g. `action-run IssueCredit --help`
  • tests/tool/action.test.ts:848: `action run` -> `action-run`

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All four fixed -- the prose in 98eb162, the three code comments in 9ed8854
and fe72c37.

An order with a total of **negative five dollars**, committed, with the rule
that forbids it sitting right there in the model. The banner is not a formality.

### Where the rules are settled instead

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, all four runs in Section 4 were refused (Nothing was written), so Order 12346 was still at $18.00 entering Section 7. Now that these two live kcmd action-run commands actually commit and leave Order 12346 at -$5.00, a reader following the README sequentially will have 12346 at -5 instead of 18 when they reach Section 7 (## 7. Run it, which states "The four runs are consecutive against the seed from section 3" and shows 12346 at 18). Worth adding a quick reset statement or note here before Section 5/7.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct -- section 7 opens by saying the four runs are consecutive against the
seed from section 3, and after these two commits they are not.

Added the reset before section 5: two gcloud spanner databases execute-sql
statements that delete the credit line and put 12346 back to 18.00. Both
action-run transcripts in section 4 were also re-recorded against a live
Spanner database rather than edited, since the warning text changed with
fe72c37.

Three ways the trimmed command lines answered a question nobody asked.

`action-list <name>` took the positional and ignored it, printing every
action in scope. The ignoring predates this branch, but renaming the
command promoted the argument into an explicit promise, so it now checks
the name across the whole scope before anything prints and names what is
declared when it finds nothing.

`profiles --profile <typo>` exited 0 on a model with no profile
documents: the "no binding profiles" line returned before the name was
ever checked, so a script keying off the exit code read a misspelling as
success. The check moves ahead of that line.

`profiles --profile default` then broke, because 'default' is the
sentinel for the inline bindings rather than a profile document -- a
push rejects a file that claims the name -- so it is the one profile
name always valid against every model. It reports the inline bindings
instead of being called undeclared.

Also restores the formatting of `catalogOnly`, which an editor formatter
had mangled into `new Map < string, {...} > ()`, and names its value
type so there is no longer an object literal inside a generic argument
for a formatter to break.
The caveat that `kcmd action-run` settles no guard was emitted only when
the action the example command line happened to pick was itself guarded.
Pick order is declaration order, so a model whose first runnable action
is unguarded shipped a SKILL.md with the caveat missing -- while its
reference pages still told the agent "Each is settled before anything is
written." The one place the skill corrects that claim was the place that
dropped out.

It now keys on whether any action in the model declares a guard, and
says so: the command line settles no guard for this action or any other.

Also drops a comment in the demo model document that still described
`kcmd` as offering a judge flag, and re-syncs the copy of the caveat
pasted into the demo README.
A caller passing both a judge and skipGuards reached the judge anyway
and then had its verdicts suppressed into warnings. That spends a model
call to produce a refusal nobody acts on, and it means the two inputs
disagreed about whether guards were being checked with no way to tell
from the result which one won. skipGuards is the answer: it is the
statement that this run settles nothing.
The verb was declared `action-list [name]` and the positional was accepted
and thrown away: naming one action on a model that declares several printed
them all and exited 0, which reads as the answer to the question that was
asked.

Two ways to settle that. Make the name filter, or drop it. Dropped: this is
a listing command in a change that is trimming the surface, and a listing
that always lists everything is one fewer thing to know. Nothing is lost --
a scope's actions are few enough to read, and the name is still how
`action-run` is addressed.

Removes the scope-wide unknown-name check that landed with the filter, since
there is no longer a name to be wrong about.
`skipGuards` said, in its own documentation, that the caller knows it asked
and so needs no telling. That is true of the caller and of nobody else.
`describeOutcome` hands an outcome to an agent as the answer to its tool
call, and an agent told `applied: true` and nothing further has been told
the write met every rule the model states, which is the one thing it did
not. A skipped guard was reaching the command line, which printed its own
line for it, and stopping there.

So it travels with the outcome. `runAction` names every guard a `skipGuards`
run passed over in `warnings`, in the order the action states them, and
everything downstream gets it without having to remember: the command line,
`describeOutcome`, and whatever calls the runtime next.

One warning for the whole skip rather than one per rule. The rules went
unchecked for a single reason, and repeating it four times buries the
outcome of the write under a list that says the same thing each time.

Drops the pre-run banner that said this. It was printed before `runAction`
had decided there would be a run at all, so a call with a missing argument
announced that the write happens and then errored without opening a
transaction. It is a fact about a write that was made, so it is not said
until one has been.

Re-records both `action-run` transcripts in the skill demo against a live
Spanner database, and adds the reset the second half of that page needs to
start from the state it describes.
Three places said the commerce demo is where the guards are "actually
settled", "live", against the same model. That was true when the generated
command line was `kcmd action run --judge --judge-reads-store`. Those flags
are gone, the demo's executor is `kcmd action-run`, and it settles nothing.
The recorded transcripts are still there and still worth reading -- they are
the only place a guard is shown holding an agent to something -- but they are
a record of what hiring a judge takes, not of what this repository does.

Also names the two verbs where the text still said `kcmd action`.
Two sentences still pointed at the line that was printed before the run.
There is no such line; the fact is on the outcome now, and the two runs above
them show it as a warning beside the commit.
@libei
libei marked this pull request as ready for review September 21, 2026 03:01
@libei
libei merged commit fc8514e into GoogleCloudPlatform:main Sep 21, 2026
7 checks passed
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