Repository navigation
Expose the engineering record over MCP without exposing execution (E07) - #434
Merged
Merged
Conversation
…on (E07)
Adds `codecarto_change`, the first point where the engineering record system
is reachable from outside this process. It records; it does not act.
What the surface deliberately refuses:
- Execution. No exec, no spawn, no target-code write, no network call. The
server gained a way to record what a host did, not a second way to do
things. A test greps the module for those symbols, and the guard was
verified to bite by injecting `child_process` with a real anchor.
- Self-approval. `approve`/`accept` are refused with a reason rather than
falling through as unknown actions, and `state` cannot be set directly.
- Self-attestation. A proof arriving through a tool call is `claimed`, never
`observed`: authority is derived from collector and attestation, and the
adapter overwrites provenance rather than merging it. A payload declaring
`authority: "observed"` is stored faithfully and discharges nothing.
Deliberately NOT refused: a payload that declares derived fields on a proof
is ignored rather than rejected. Refusing would teach callers to strip the
fields and change nothing about what is trusted; the claim must be inert.
The gate reports `needs-human-acceptance`, never `may-accept`: an MCP tool
call cannot obtain a human decision, so it says so instead of implying a
capability the transport lacks. A mutation flipping that to `true` initially
survived — nothing asserted the host capability — which is the same coarse-
test failure class as E06.
Retry-safety uses the store's own idempotency key rather than a `request_id`
record field the schema rightly refuses. Because the store compares bytes,
the change id and timestamp are derived from the request when a key is
supplied; a fresh uuid per call would make two retries of one request differ
and be reported as a conflict.
Also fixes a real defect in E06 found while testing transport: the
always-stated limitation ("a gate cannot show the change is correct") was
missing from the early-return path, so the one disclosure the module promises
unconditionally was absent from exactly the outcomes a confused caller is
most likely to read. Now a named constant used on every path.
Tests: 17 unit tests plus 3 that drive the compiled server over real stdio
JSON-RPC in a separate process, because a surface that works in-process and
not over the wire is not a surface. All 9 mutations bite (3 initially
survived: the refused-action list, the derived-field refusal, and the host
capability above). Suite 1161/1161 on two consecutive runs.
Contract shapes corrected against the real definitions rather than guessed:
CHANGE_MODES has no "behavior", BaselineReference takes `vcs` not `kind`, and
E01 requires a full commit hash whenever vcs is `git` — so a create without
one records `vcs: "none"` rather than a git baseline naming nothing.
Refs #405
Six mutations survived a green suite. The pattern: every `update` test asserted the REFUSAL path (a stale revision is rejected) and none asserted the WRITE path, so an update that stored nothing at all would have passed. Dropping the title write, dropping the outcome write, and never bumping the revision were all invisible. Added: - update actually persists title and outcome, read back through a separate show call so a reported-but-unstored write fails - an omitted field is left alone rather than cleared - show reports the record's real state (seeded from a fixture whose state is not `draft`, since a hardcoded "draft" is indistinguishable from a working implementation on a freshly created change) - whitespace-only required fields are refused - a replay reports retried: true and a first write reports false - a non-object proof is refused All nine mutations now bite. Suite 1167/1167. Refs #405
…face
Adversarial review returned REQUEST_CHANGES with nine findings. Two were real
integrity defects, reproduced personally before any fix.
F-1 (HIGH): an ACCEPTED change could be rewritten. updateChange never looked
at record.state, and `title`/`requested_outcome` are exactly the two fields an
AcceptancePresentation carries. Because validateRecord recomputes
presentation_digest from the approval's OWN embedded copy, the approval kept
validating while the change stated an outcome nobody approved. Reproduced: a
fixture forced to `accepted`, then update returned "revision 3" with the state
still accepted and the new title stored. Terminal states are now refused.
F-2 (HIGH): the compare-and-swap was opt-in. `ifRevision` was populated from
the record the adapter had just read, so it only ever compared the adapter
against itself -- last-write-wins with extra steps, and a concurrent writer's
work vanished with no error. `revision` is now required on update.
F-3: created_at was a digest of the request: a fixed 2020 epoch plus a bounded
offset, so a workspace had at most 1000 distinct creation times and anything
ordering by them got arbitrary order. Retry-safety now comes from reusing the
first call's real timestamp, recovered from the record it wrote.
F-4: ordinary bad arguments (unknown mode, non-hash baseline, oversized or
traversing request_id, idempotency conflict) surfaced as InternalError
(-32603), which hosts treat as a server bug and retry. Retrying an invalid
enum fails identically forever. Request-blaming store codes now map to
InvalidParams.
F-5: REFUSED_ACTIONS was a plain object literal, so `action: "constructor"`
printed native function source as the refusal reason. Null prototype plus an
Object.hasOwn guard.
F-8: a NUL byte and ANSI escapes reached the reported text unescaped, letting
a caller repaint how a record reads. Control characters are stripped from
displayed text; stored bytes are unchanged.
Tests, including three the review showed were proving nothing:
F-6: the laundering test sent attested_by "adapter", which E01 refuses on
SHAPE for a host-observed collector -- so with the provenance overwrite
removed it failed on validation and never reached the authority assertions. It
could not tell a working overwrite from an unrelated refusal. Now sends
host-tool-result, the value that actually launders, and asserts the STORED
provenance.
The storage-boundary disclosure ("these records could have been rewritten by
the agent whose work they describe") was missing from the gate's early-return
path -- the same defect class as the correctness limitation fixed in the
previous commit, found because a test finally asserted the specific string.
Both are now emitted by one helper used on every path.
Also added: two successive updates advance the revision by exactly one (the
`ifRevision: 1` and `+2` survivors both needed a second successful update to
observe), mode and baseline_commit are recorded as supplied, a proof is filed
against the change in the REQUEST not the payload, and InvalidParams codes are
asserted over the wire because the server's catch wrapper -- not the adapter --
is what assigns InternalError.
Kept deliberately: the prototype guards, documented as untested by
construction. The action allow-list runs after them and rejects every
inherited name anyway, so mutating either away leaves the suite green. They
are defence in depth against a reordering, not dead code -- deleting a guard
because a mutation survived is the error that let a baseline proof discharge
in E05.
Suite 1180/1180 on two consecutive runs. Every mutation bites, once the
harness was fixed to rebuild dist/ (the roundtrip tests spawn the compiled
server, so source-only mutations were invisible) and to mutate all three
asCallerError sites rather than the first.
Refs #405
This was referenced Sep 22, 2026
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.
Closes part of #405.
Adds
codecarto_change— the first point where the engineering record system is reachable from outside this process. It records what a host did; it never acts.What it refuses, and why that is the feature
exec/spawn/target-code write/network call. Recording and doing must stay separate.approve/acceptauthority: "observed"discharges nothing.statewritesDeliberately not refused: a payload declaring derived fields is ignored, not rejected. Refusing would teach callers to strip the fields and change nothing about what is trusted — the claim has to be inert, not forbidden.
The gate returns
needs-human-acceptance, nevermay-accept: this transport cannot obtain a human decision, so it says so rather than implying a capability it lacks.An E06 defect found while testing transport
The always-stated limitation — a gate cannot show the change is correct — was missing from the early-return path. The one disclosure the module promises unconditionally was absent from exactly the outcomes a confused caller is most likely to be reading. Now a named constant applied on every path.
Verification
can_obtain_human_decisiontotruemade the surface claim it could ask a human, and nothing failed. Same coarse-test class as E06.child_processwith a real anchor (a non-existent anchor is how this project has twice proved nothing).--tarball.npm run smokeinstalls the published package, so it reports this tool missing until a release carries it; that caveat is now in the quickstart. The first two smoke failures were exactly this, not a packaging defect, confirmed by querying the compiled server's inventory directly.Contract shapes, corrected rather than guessed
CHANGE_MODEShas nobehavior;BaselineReferencetakesvcs, notkind; and E01 requires a full commit hash whenevervcsisgit. A create without one recordsvcs: "none"rather than a git baseline that names nothing. Retry-safety uses the store's existing idempotency key, not arequest_idfield the schema rightly refuses.