MCP operate verbs behind a human-in-the-loop gate#262
Merged
Conversation
The MCP operate verbs (and later the deploy tool) need to stamp the same tool metadata and resolve the same rollback targets as the CLI, but both lived unexported in package cmd. Move them: - cli/internal/stamp (new): stamp.Ledger builds the remote.Ledger from an already-resolved env plus operation/version/root, with the GAFFER_REVISION/git and GAFFER_ACTOR/principal resolution moved along. cmd's toolLedger stays as a thin resolveLiveEnv wrapper. One deliberate behaviour change: an env with no connection now yields an empty actor instead of letting engine.Principal dig credentials out of .env for an env gaffer never connected with - unreachable from cmd today (connectResolved refuses an empty connection before any write), and the safer contract for new callers. - remote gains the rollback hash resolution: FindVersionByHash (each page bounded by the RPC timeout, same as cmd's rpc() did), VersionMatch (neutral name - hash-to-version resolution isn't rollback-specific), NormalizeHashPrefix, and RollbackRefusal. Pure-logic tests move with their code; a new cmd test locks the toolLedger wrapper's unresolvable-env contract (actor dropped, everything else stamped).
confirmWrite is the production gate the operate verbs (and later the
deploy tool) sit behind: a write that targets a production server - or
one flagged Always, for verbs with no undo - requires an MCP
elicitation confirm, answered by the human through the client's UI
mid-call, so the model cannot fabricate it. Accept is the only proceed
token; decline, cancel, an unknown or empty action, an elicit failure,
a client without the (form) elicitation capability, and a call with no
session all refuse without writing, naming the CLI equivalent as the
next step. Non-production writes don't elicit: every MCP tool call
already passes the client's tool-approval layer, and double-prompting
every resume trains click-through.
The elicit schema is raw {"type":"object","properties":{}} - the spec
requires the properties member and jsonschema.Schema's omitempty drops
an empty map (adding the package as a direct dep also downgraded the
MCP SDK via MVS; review caught it). The projection name is %q-quoted in
the question and refusals so a crafted name can't read as part of the
sentence the human decides on.
Tests run over real in-memory MCP transports with a fake human on the
client side: accept, decline/cancel/empty (locking the accept-only
allowlist), elicit failure, missing capability, nil session, the
always-tier off production, and the prodWhere phrasing table.
Six write tools behind the human-in-the-loop gate, wrapping the same
remote.Client calls as the CLI operate commands and echoing
{name, outcome, env, target, production}:
- deploy_pause / deploy_resume / deploy_abort mirror disable / enable /
disable --abort; prod-gated, no ledger (lifecycle writes are
metadata-less by design, like the CLI)
- deploy_delete mirrors gaffer delete: disable first, state and
checkpoint streams included, deleteEmitted optional - and it always
elicits, production or not, because delete is the one verb with no
undo story at all; a client without elicitation cannot delete via MCP
- deploy_recreate mirrors gaffer recreate: config + compile gate before
the destructive Disable -> Delete -> Create sequence (no validation
bypass exists here - the CLI's --no-validate is an operator's flag,
not an agent's), the emit re-emission caution folded into the elicit
wording, stamped operation: recreate
- deploy_rollback mirrors gaffer rollback: hash-prefix resolution via
remote.FindVersionByHash, the unchanged short-circuit, the in-place
refusal for create-time dimensions, stamped operation: rollback, full
target hash in the result
A shared runOperateVerb/operateSetup core does the project gate, the
system-projection refusal, connect, existence check, and target/
production resolution (operateTarget - the CLI's resolveOperateTarget
semantics); every RPC gets its own timeout budget like the CLI's rpc
helper. These are the first handlers to consume the request (the gate
needs req.Session); annotations mark pause/resume non-destructive and
the rest destructive.
- deploy_recreate now runs the diagnostics preflight the CLI runs by default (engine.Preflight): a source that compiles can still carry error-severity diagnostics known to fault on the server, and Delete runs before Create - without the gate a healthy projection could be destroyed and rebuilt into a faulting one, a state the CLI can only reach via an explicit --no-validate - recreate joins delete's always-elicit tier: it embeds a delete (state wiped; deleteEmitted destroys emitted-stream data), and the CLI's recreate confirm is unconditional too. Rollback deliberately stays production-only: in-place and roll-forward-able, the one recorded deviation from the CLI's confirm-always - the rollback elicit names the hash movement (current -> target), so the human confirms a specific change, not an abstract one - the CLI hand-off strings carry --delete-emitted when set, and the delete error messages match the CLI's could-not-delete phrasing - recreate's step closures drop their redundant timeout wrappers - deploy.Recreate bounds every step itself - resume and abort join the system-projection refusal test table
One lifecycle against a live KurrentDB: pause/resume/abort off production (no elicit, nil request), recreate declined-then-accepted (declined leaves the projection in place; accepted stamps a gaffer-attributed recreate in history), rollback to the recreated content by hash prefix plus the unchanged short-circuit, and delete refused for a no-elicitation client, refused on decline, then accepted and verified gone. The gate harness from the unit tests composes with the live fixture - the fake human is a real in-memory MCP client.
- The MCP docs page gains the verbs bullet, explaining the elicitation gate: production writes confirm through the client mid-call, recreate and delete ask every time, and a client without elicitation is pointed at the CLI - The server's initialize instructions mention the lifecycle verbs - Changeset for the release notes
Docs-review pass: split the gate sentence, add the per-verb nuances the tool descriptions carry (abort's skipped checkpoint, recreate's preflight, rollback's in-place refusal, delete's deleteEmitted), and fix the changeset's hyphen-as-dash.
George-Payne
added
enhancement
PR Summary by QodoAdd MCP deploy operate verbs with human-in-the-loop production write gate
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
CI's KurrentDB is a release image that ignores ledger metadata, so the recreate's stamped create reads back metadata-less and history classifies it as an external edit. Assert the gaffer-stamped recreate classification only when GAFFER_TEST_LEDGER says the server keeps metadata - the same gate remote's ledger integration tests use. The rest of the lifecycle (outcomes, refusals, rollback by content hash) holds on both server generations and stays asserted unconditionally.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
The gate's refusals tell the human which command to run instead, and the projection name in that string is model-controlled - a crafted name could smuggle shell syntax into a command someone copies and runs. Single-quote it (POSIX quoting disables all expansion; embedded single quotes close-escape-reopen). Rollback's hash prefix stays bare: it is validated hex before anything else runs.
An agent test-drove the verbs against a production-flagged database and
found the prompt under-signalled: "Pause "orders" on production
Navigator testing?" buries the risk mid-sentence, reads like a server
named "production Navigator testing", and never shows the env key the
operator actually chose. The new format front-loads it, plain text and
single-line throughout (Claude Code renders elicit messages without
markdown):
PRODUCTION [env.production]: pause projection "orders" on
"Navigator testing"? Stops after a final checkpoint; resume with
deploy_resume.
Delete projection "orders" on "local" [env.local]? Removes the
projection, its state, and checkpoints. No undo.
Each verb now supplies its complete lowercase action phrase (rollback
splits its verb around the name, so the gate can't assemble verb+name)
and a required consequence stating what the human is agreeing to -
notably abort's skipped checkpoint, the distinction status can't show.
The no-undo tier (delete, recreate) escalates on production: the
confirm requires typing the projection name into the form, checked
against the input; off production it stays accept/decline - friction
proportional to blast radius. The SDK's schema validation already
rejects a missing or mistyped field before the gate's value check.
deploy_status and deploy_plan now echo the resolved target and
production flag alongside env, so an agent reading status alone can
tell it is pointed at a production database before deciding to mutate.
Claude Code renders the field description at the input, so carrying
"Type the projection name ... to confirm" in both the message and the
field showed it twice. It now lives on the field alone, and the name
reads "orders" rather than ("orders") - the parens added nothing over
the quotes.
The confirm field was free text, so a client had nothing to validate against and the human could Accept a wrong name only to learn afterwards that nothing happened. The schema now carries pattern: ^<name>$ (regexp-escaped), so a client that enforces JSON Schema on elicitation input can gate its Accept inline. Claude Code currently doesn't (tested; it checks type and presence only), but the constraint is strictly better: enforcing clients close the seam, ignoring clients lose nothing, and the SDK validates the result against it in the round-trip either way - which the gate now reports as "the typed name must match ... exactly; nothing was changed" instead of a raw schema-validation error. pattern is an extension over the MCP elicitation string-schema subset (a one-item enum, the subset's only exact-match shape, would render as a picker and remove the friction). The server-side equality check stays as the real guard.
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
1 participant
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.
Adds the operate verbs to the
gaffer mcpserver behind a human-in-the-loop production gate:deploy_pause,deploy_resume,deploy_abort,deploy_recreate,deploy_rollback, anddeploy_delete, each mirroring its CLI command's semantics.cli/internal/stamp(new): the ledger-stamping core (toolLedger's revision/actor resolution) moved out of cmd so mcpserver stamps the same attribution; one deliberate change documented in the commit - an env with no connection yields an empty actor rather than scraping.envcredentials.remotegains the rollback hash resolution (FindVersionByHash, neutralVersionMatch,NormalizeHashPrefix,RollbackRefusal), each page bounded by the RPC timeout as before.deploy_gate.go: theconfirmWritegate. Production writes (and always-tier writes) require an MCP elicitation confirm answered by the human through the client mid-call - the model cannot fabricate it. Accept is the only proceed token; decline/cancel/error/no-capability/nil-session all refuse without writing and name the CLI command as the next step. The elicit schema is raw{"type":"object","properties":{}}(spec requiresproperties; the typed schema'somitemptydrops an empty map and pulling the package in downgraded the SDK via MVS). Projection names are%q-quoted in the question so crafted names can't read as part of the sentence the human decides on.runOperateVerb/operateSetupcore (project gate, system-projection refusal, connect, existence check,$server-infoprod detection matching the CLI). Tiers: pause/resume/abort elicit on production only; recreate and delete always elicit (both destroy state with no undo); rollback elicits on production, naming the exact hash movement. Recreate runs the compile and diagnostics preflight with no bypass - review caught that skipping the diagnostics gate could destroy a healthy projection and rebuild it into a faulting state. Recreate/rollback stampoperation: recreate/rollback; lifecycle verbs stamp nothing, like the CLI.deploy_history).non-production destructive
production non-destructive
production destructive