feat(errors): machine-readable error taxonomy + truthful apply outcome (NRN-150/183)#92
Merged
Conversation
…(NRN-150/183)
Turn opaque apply failures into a structured, machine-branchable contract shared
by the CLI (--format json) and the MCP server, so an agent can decide
retry-vs-reread-vs-giveup by comparing a stable CODE instead of string-matching
prose. Two coupled tasks over the one ApplyReport machine-contract surface.
NRN-150 — error taxonomy:
- ApplyError::code()/path()/is_precondition() on the rich standards::apply enum
(+ ContainmentError::code()/target()); every variant maps to a stable
kebab-case code (stale-document-hash, expected-old-value-mismatch,
post-image-verification-failed, content-op-after-vacate, containment-*, …).
- apply_report::ApplyError becomes the {code, message, path?} envelope with
from_rich / from_anyhow constructors (downcasts through ApplyError,
ContainmentError, CacheError; falls back to internal-error).
- CLI: a --format json failure on migrate/move/delete/rewrite-wikilink now prints
the envelope to STDOUT (prose still to stderr for records/TTY).
- MCP: to_mcp_error carries the envelope in the JSON-RPC error `data`; the mutate
tools stopped rewrapping the error so the downcast survives.
- Return-report-on-refusal (MMR-202): a validation-phase precondition refusal is
returned to an MCP caller as the ApplyReport — offending op status:failed with
error.code, untouched ops not_run, outcome=refused, vault byte-identical —
instead of a bare internal_error. Gated by ApplyContext::refuse_as_report (set
by the MCP mutate tools; CLI keeps the Err→envelope path). Only
is_precondition() errors convert; post-write failures still propagate as Err.
NRN-183 — outcome signal:
- ApplyReport gains an `outcome` field (applied/failed/refused, + reserved
`rebased` for NRN-152) and an exit_code() helper, exposing the process-exit
tri-state identically over --format json and the MCP structuredContent. The MCP
confirm path emits report.exit_code() so a failed/refused apply is no longer
invisible behind isError=false.
Docs: new docs/errors.md documents the exit-code contract, the outcome field, and
the full code taxonomy; linked from docs/commands.md. CHANGELOG under Added.
Existing skip_reason codes are already kebab (no change). Parity gate needs no new
entries — the additions are output fields, not input params.
Tests: ApplyError::code() per-variant kebab assertion + precondition
classification; envelope shape / from_rich; outcome kebab + legacy-default
round-trip; CLI e2e that a --format json failure emits the envelope on stdout;
MCP handler test that a precondition refusal returns the report (op failed +
error.code, outcome refused), not a bare error. Two cli_output tests updated from
the old stderr-prose contract to the new stdout-envelope contract.
…variant (NRN-150/183)
The refused-vs-failed decision keyed off the per-VARIANT ApplyError::is_precondition()
flag, which structurally cannot be correct: the same variant (stale-document-hash /
unknown-path) is raised from BOTH a pre-write site (Phase A1 content CAS) AND a
post-write-possible site (the Phase B delete pass, after Phase A2 already wrote other
ops). So a post-write precondition failure surfaced as outcome:refused ("vault
byte-identical, nothing written") over an already-mutated vault — the byte-identity
lie — and on the CLI every apply Err mapped to exit 2, so a partial folder-move
(op0 renamed, op1 MoveDestinationExists) exited 2 as if nothing happened. The
Failed/exit-1 outcome was also unreachable.
Fix: thread a runtime write-state fact (wrote_any) through
apply_repair_plan_with_context — flipped the instant the first atomic_write / rename /
remove / dir-create / backlink-rewrite lands — and gate on it:
- failure, no write yet -> outcome:refused, exit 2, byte-identical (unchanged).
- failure after any write -> outcome:failed, exit 1, TRUTHFUL partial report
(already-applied ops applied, failing op failed + error.code, rest not_run) on
BOTH surfaces (MCP Ok(report); CLI report.exit_code()).
This makes Failed/exit-1 reachable, kills the byte-identity lie, and makes
MissingNewValue's phase correct-by-construction (the gate no longer reads the
variant). is_precondition() is kept only as a coarse taxonomy hint.
- repair_apply: add wrote_any: Option<&mut bool>, mark at every FS write site.
- applier: gate on wrote_any; new build_partial_failure_report derives per-op status
from the telemetry events already emitted before the abort.
- CLI (migrate/move/delete/rewrite-wikilink): exit on report.exit_code(), not the
hardcoded Ok(2).
- docs/errors.md: exit-1 = partial-apply failure (reachable); refused vs failed is a
runtime write fact, not a variant.
- CHANGELOG: reclassify the stderr-prose -> stdout-envelope change as a breaking
change; keep the additive taxonomy in Added; add the correctness fix under Fixed.
Tests (all red before the fix): applier + MCP apply_plan 2-op byte-identity-lie
scenario (op0 set writes X, op1 delete untracked Y -> unknown-path in Phase B ->
outcome:failed not refused); CLI multi-op migrate partial move -> exit 1 not 2;
preserved clean pre-write single-op refusal -> refused/exit 2, byte-identical.
Adversarial-Review: run engine=code-review tier=high findings=5 fixed=5 dismissed=0 deferred=0
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.
Summary
Makes apply failures machine-branchable on both surfaces (the last non-breaking v0.45 contract work):
ApplyError::code()returns a stable kebab code per variant (stale-document-hash,expected-old-value-mismatch,post-image-verification-failed, …), emitted from the start in the canonical form NRN-190 enforces (no snake debt). CLI--format jsonfailures now emit a structured{code, message, path?}envelope on stdout; MCP routes failures off the opaqueinternal_errorinto structured error data. A precondition refusal returns theApplyReport(offending opstatus:failed+error.code, othersnot_run, vault byte-identical) instead of a bare error — satisfying Mimir's B7 CAS-retry ask (branch on a code, never prose).rebasedreserved for NRN-152.ApplyReport.outcome∈ {applied(0)/failed(1)/refused(2)/reservedrebased} withexit_code(), exposed identically on CLI and MCP (outcomefield, notisError— a refusal is a normal structured report, not a transport error).docs/errors.md: the exit-code contract + code taxonomy + outcome semantics — the surface that reconciles the divergent per-command exit-code meanings.Adversarial review
engine=code-review tier=high— correctness + taxonomy/docs finders + controller verification (I re-derived the headline finding against the raise sites and hand-checked everymark_wrote!placement). Taxonomy itself was clean (every variant mapped, all kebab, retryable/terminal grouped,rebasedreserved). 5 findings, all fixed:outcome:refused("nothing written, safe to retry") over a mutated vaultErr→ exit 2, so a partial-apply failure exited 2 ("refused") when files were already writtenreport.exit_code()outcome:failed/exit-1 was unreachable — the tri-state collapsed to {applied, refused}MissingNewValuereport-vs-error inconsistent (raised from both pre- and post-write sites)### Added### Breaking changessectionRoot fix: refused-vs-failed was gated on
is_precondition(), a per-variant flag — but the same variant is raised from both a pre-write and a post-write site, so it can't answer "was the vault mutated?" Replaced with a runtime write-state fact (wrote_any, flipped the instant the first write lands, persisting across theErrboundary): no write yet →refused/exit 2 (byte-identical, any variant); a write already landed →failed/exit 1 with a truthful partial report (applied opsapplied, failing opfailed+code, restnot_run) on both surfaces. Red→green on the exact multi-op scenario (MCP + CLI); the clean single-op pre-write refusal path is preserved and tested.Verification
check --locked/ 2191 passed 0 failed /clippy -D warnings/fmt --check);Cargo.lockuntouched; parity gate greencli_outputtests moved from stderr-prose matching to parsing the stdout envelope + asserting the kebab code (stronger, not weakened)