Repository navigation
docs(agents): two laws — done means verified, and MCP parity is the contract - #687
Merged
Merged
Conversation
…ire it
v0.69.0 shipped with two items under "Not verified": the MCP twin of the credits
path, and 5 of 19 skill-benchmark tasks. Asked why we were shipping blind, both
were verified within the hour. Neither had ever been blocked. Both had been
labelled instead of run, and both found something.
The MCP twin took 58 seconds and $0 -- and it needed its own test, because the
excuse for skipping it ("the CLI path is verified and they share
services/credits.py") pointed at the one part that was never the risk. The shared
service was covered; the adapter was not. It is now
tests/e2e/test_credits_mcp_e2e.py: a live balance with the full key contract
asserted, all_profiles preserving partial results across 9 profiles with every
failed row carrying a reason, and an unknown profile refused structurally rather
than raising across the tool boundary. 3 passed in 58.15 s.
The 5 tasks ran on a model whose quota was still open and scored 1/5, avg 0.280
-- against a control of 4/5, avg 0.800 for tasks that had scored 1.00 on the
first model, which rules out model weakness. Four real skill defects that
"recorded, not omitted" had made invisible: t2v reached for where a staged object
needs i2v, re-run scope on an edited manifest, an overnight batch refused without
naming the foreground alternative, and a lip-sync detector trusted without being
proved against a known delay. Filed as #685.
## What changed
AGENTS.md gains § "The Iron Law" at the head of the working-discipline section --
one place, cited rather than copied, per the rule this repo already learned about
duplicated checklists. Touch a command, a feature or a Flow surface and you run
its e2e test; no run is not done. If no test covers it, writing one is part of
the change.
Three escape hatches are closed. The e2e bullet's "if you cannot run it, a
maintainer will" is now available only after a blocker is named -- a handoff is
not a verification. The "no Flow surface" carve-out must be claimed explicitly.
And "it's LIKELY -- not CONFIRMED" is replaced: a claim you have not run is a
guess with formatting, and grading it as low-risk is not evidence. Confidence
labels were the mechanism of this failure, not a mitigation for it.
The law permits exactly one exception -- an external blocker you cannot remove
(exhausted quota, hardware you lack, an account you do not control, a cohort you
are not in) -- and names the excuses that are not blockers, including the one
used here.
## Wiring, because a documented rule is what just failed
check_release_artifacts.py now requires every bullet under a "Not verified"
heading in the release ledger to name a blocker or carry a tracking issue.
Against the ledger as shipped this morning it flags nine entries, including the
MCP excuse. Debt is still allowed, but only when tracked: an issue someone can
close, not a parking space.
Satisfying it changed the ledger for the better rather than only adding words.
Four i2v entries turned out to have real blockers that were never stated (spends
credits on a second account; a guard reachable only offline; a modal neither
account raises any more). Five were genuine untracked debt and are now #686. One
was not an unverified item at all -- a note comparing two runs -- and moved out of
the section.
Note the gate's own bug, caught by its test: "credits" as a blocker marker
matched any mention of the credits *feature*, so the real excuse passed. Narrowed
to "spends credits" / "credit budget".
The existing gate is satisfied by a *mapping*: every CLI leaf must name an MCP tool or a reasoned exemption. It stays green while a mapped leaf's options never reach that tool -- a hole AGENTS.md already described in prose and nothing enforced. Its first run found #689: --reference-entity / --reference-entity-name exist on image t2i, image i2i, video t2v and video r2v, and no MCP tool exposes the concept at all. With the whole character group exempt as 'not yet ported', the identity axis of the product is unreachable over MCP -- an agent can neither create a character nor attach one -- while parity reported full compliance. Three checks: every mapped leaf's options reach the tool (or are CLI-only by nature, or recorded); no exemption outlives the option it describes; and any entry admitting a GAP carries an issue number, so an admission cannot decay into an excuse. Aliases are matched as 'either spelling satisfies parity' rather than a rewrite, because a rename is per-tool: refs means reference_images on the generate tools and stays refs on instructions_add. Getting that wrong is what the stale-exemption test caught while this was being written.
…on-level teeth The rule already existed, under "Code style", enforced by a gate that checked only that every CLI leaf was *mapped* to a tool. AGENTS.md even described the hole in prose -- "stays green while an option goes unmirrored" -- and nothing enforced the admission. An option-level sweep found what that hole was hiding. --reference-entity and --reference-entity-name exist on image t2i, image i2i, video t2v and video r2v, and no MCP tool exposes the concept anywhere; grep for reference_entit across src/gflow_cli/mcp/ returns nothing. The whole gflow character group is separately exempt as "not yet ported". So the product's identity axis -- create a character, attach a character -- is unreachable over MCP, while parity reported full compliance. An agent cannot discover this: it finds out when a face comes back different. Filed as #689. The rule is therefore promoted out of Code style into the working-discipline section, beside the Iron Law, where CLAUDE.md's @-import guarantees it is in context. Every user-facing CLI capability has an MCP twin or a recorded reason it does not. Parity holds at the leaf, its options, the queued-payload keys, and the docstring's claims -- the six mirror axes, cited from check/SKILL.md rather than copied, because a duplicated checklist drifts. The clause that ties the two laws together: a twin is a separate surface, so the Iron Law applies to it separately. Verifying the CLI path does not verify the MCP path. They share a service and the service was never the risk -- the adapter is. That is exactly the reasoning that let gflow_get_credits ship unrun this morning. The Code style block keeps its per-phase duty table but no longer claims the gate is command-level only, because that stopped being true in the previous commit.
ffroliva
changed the title
…characters discoverable Fixes the two real gaps behind #689, after correcting the issue's own headline claim. ## The correction first I filed #689 saying an agent could not attach a character over MCP. That was false and I should have traced the path before asserting it: @name mentions resolve to referenceEntities, and worker/daemon.py:206,331 call services/mentions.resolve_and_apply on the queued task, so attach-by-name has always worked. I found an absent parameter and inferred an absent capability. Corrected on the issue and retitled. ## Gap 1 -- attach by entity id --reference-entity is the "you have the id and want no name ambiguity" route; display names are not unique, which is the ambiguity it exists to remove. It had no MCP equivalent. The instructive part is where it was missing. worker/codec.py already read reference_entities and reference_entity_names off the payload, and both request DTOs already carried them -- the whole downstream path was wired. Only the tool signature was absent, so the keys were never written. That is the mirror image of #628: not a key written and never read, but a key read and never written, and just as silent. The tests therefore assert the round trip through the real tools rather than the signature; a tool that accepts the argument and drops it would pass a signature check and still do nothing. Wiring it exposed a second-order bug. The r2v guard demanded reference_images, while GenerateVideoRequest accepts "reference_images, ref_names, OR reference_entities" -- the guard was stricter than the domain rule it fronts, so an agent holding only an entity id could not reach r2v at all. That was invisible while entities were unreachable anyway. ## Gap 2 -- discovery Attach-by-name only helps an agent that was told the name. character list and show were exempt, so there was no way to enumerate a project's characters or their ids. Both are now tools. The exemption they replace records how the last attempt failed: "the old MCP stub returned a misleading empty list (#499)". An empty list is indistinguishable from a project with no characters, and no offline test catches it -- a stub returning [] passes them all. So the evidence is a live e2e that drives BOTH surfaces against one project and asserts they agree, and fails outright if the project has no characters, because a test that cannot tell a working tool from the stub is not evidence. 3 passed in 53 s against a project with real entities, $0. Serialisation mirrors Character's own rule -- stable identifiers only, no signed CDN URLs -- asserted offline and again on the live payload. ## Parity The eight GAP entries added to _OPTION_EXEMPT this morning are gone, so the option-level gate now proves the hole is closed rather than recording it. Both leaves move from _MCP_EXEMPT into CLI_TO_MCP. docs/MCP.md gains the two tools and one paragraph stating the identity axis once: @name, reference_entities by id and reference_entity_names all reach the same wire and dedupe, while reference_images is a different axis that buys a look rather than an identity.
This was referenced Sep 6, 2026
…h real reasons "Character mutations — not yet ported" was doing the work of a justification in three exemptions. Challenged on it, none of the three held as written. `character voices` is not a mutation. It reads the in-process VOICES tuple: no network, no browser, no profile, no cost. It was exempt on a description that is wrong on both counts, and it is the lookup an agent needs to choose a --voice for character create, so the omission compounded the one next to it. Now gflow_character_voices, with a test that patches FlowApiClient to raise, so a future change that quietly opens a session fails instead of merely being slower. `character create` has no principled reason and now says so. The implied argument is that a quota-spending mutation needs human consent, but gflow_generate_image is exposed over MCP and spends the same per-model image quota; no principle admits one and refuses the other. Compare the exemptions that do hold their weight: `video extend` carries three independent reasons, `auth login` is genuinely interactive, `data prune` is irreversible. This one is simply unported. Filed as #691 with the two things the port must decide rather than assume — worker-enqueue vs inline (#481), and whether it honours GFLOW_MCP_NO_SPEND — and the exemption is now a tracked GAP, which the gate requires to carry an issue. `character rm` keeps its exemption because it earns one, but for the right reason. It is irreversible deletion of a user-owned Flow entity, the same rule as data prune, and the CLI gates it behind a confirmation an agent cannot give on the user's behalf. The note records that it is FREE — cost is not the reason, irreversibility is — so nobody re-derives the wrong rationale later.
13 of 19 tasks
kittinan
added a commit
to kittinan/gflow-cli
that referenced
this pull request
Sep 6, 2026
…es still exit 36 Eight places still told users and agents that r2v exits 36 on the migrated host. `mcp/tools.py` is the important one — it is the literal text an agent reads before calling the tool. The line has to be drawn precisely rather than by deleting "r2v", because ffroliva#687 relaxed the MCP r2v guard to accept `reference_entities` alone. That path could not be reached before, and now can: it passes the guard, reaches the transport, and `_unported_form` returns "character references" -> exit 36. Correct behaviour, but it means the two forms differ: r2v with local `reference_images` -> served on the migrated host r2v with `ref_names` / entities -> still exit 36 Updated in `mcp/tools.py`, `docs/MCP.md`, `docs/USAGE.md` (the t2v callout, the exit-36 table row, and the `gflow video r2v` section, which had no migrated-host callout where i2v has one), `docs/CONFIGURATION.md`, `docs/INDEX.md`, `KNOWN_ISSUES.md` and `skills/gflow-cli/SKILL.md`, with the `website/docs/` mirror regenerated rather than hand-edited. CHANGELOG: the merge from develop landed this feature's entry inside the released 0.69.0 section, which both produced the duplicate `### Added` and stated it shipped in a release it is not in. Moved to `[Unreleased]`, and the entry now carries the correction about the unreachable body assertion, since that claim was made publicly. Spike doc: bullets under "What this does NOT settle" that Rounds 6-7 of the same document had settled — multiple references, local files, the `MZZa6b` response shape, i2v recon — are retired with a note saying why, because a superseded open question in a doc agents read as ground truth is how a settled surface gets re-mined. The `veo_3_1_lite_lower_priority` contrast is relabelled inferred: no capture in this repo has observed a migrated t2v body at that tier. Wire-lessons ledger gains an r2v slice-2 section so the next session does not re-mine a 405-line spike, including the dead-wiring lesson this review found. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MK6VHisb4JifQPAmvm724e Signed-off-by: Kittinan <tunxedge@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two laws into
AGENTS.md§ Working discipline, whereCLAUDE.md's@-import keeps them in context. Each is stated once and cited elsewhere, and each is wired, because a documented rule that nothing enforces is what failed in both cases.Law 1 — done means verified by a run
v0.69.0 shipped with two items under "Not verified". Challenged, both were verified within the hour. Neither had ever been blocked; both had been labelled instead of run — and both found something.
services/credits.py") pointed at the part that was never at risk. Nowtests/e2e/test_credits_mcp_e2e.py, 3 passed live.Three escape hatches closed. "A maintainer will run it" now requires a named blocker first. The "no Flow surface" carve-out must be claimed explicitly. And "it's LIKELY — not CONFIRMED" is gone: a claim you have not run is a guess with formatting. Confidence labels were the mechanism of the failure, not a mitigation.
Wiring:
check_release_artifacts.pyrequires every "Not verified" bullet to name a blocker or carry a tracking issue. Against this morning's shipped ledger it flags nine entries, including my own excuse. Satisfying it improved the ledger: four entries had real blockers never stated, five were untracked debt (now #686), one wasn't an unverified item at all.Law 2 — MCP parity is the contract
The rule existed under Code style, enforced by a gate that only checked leaves were mapped. AGENTS.md described the hole in prose — "stays green while an option goes unmirrored" — and nothing enforced the admission.
An option-level sweep found what it hid:
--reference-entity/--reference-entity-nameon four leaves, and no MCP tool exposes the concept at all. Withgflow characterexempt as "not yet ported", the product's identity axis is unreachable over MCP — an agent can neither create a character nor attach one — while parity reported full compliance. → #689Wiring:
tests/mcp/test_cli_parity.pygains option-level checks — every mapped leaf's options reach the tool (or are CLI-only by nature, or recorded); no exemption outlives the option it describes; anyGAPentry carries an issue number so an admission cannot decay into an excuse. All three mutation-tested.The clause joining the laws: a twin is a separate surface, so Law 1 applies to it separately. Sharing a service is not coverage — the adapter is the risk. That is precisely the reasoning that let
gflow_get_creditsship unrun.Honest notes
"credits"as a blocker keyword matched any mention of the credits feature, so the real excuse sailed through. Narrowed to"spends credits".AGENTS.md's "that CI gate is command-level only" is corrected, because this PR made it false.Gates
repo hygiene · doc links · PII · website mirror · council memory · release artifacts · ruff · format · pyright 0/0 · 177 MCP tests · 236
tests/scriptsTracks #685, #686, #689 — none are closed here; this PR measures and fences them.