Phase 3: give the errors the frontend branches on a machine-readable code - #279
Merged
Conversation
Introduce IpcError { code, message } as the shared boundary struct (D-01),
pin the four contract codes with a Rust unit test that fails on rename
(D-03/ERR-02), and add a serde_json round-trip test for the wire shape
(D-09). Migrate evidence_binder_mutate's revision-conflict path from a
bare "evidence_binder_revision_conflict" string to a typed IpcError whose
message gains the sanctioned human-readable suffix.
On the frontend, mirror the four codes in src/lib/types.ts, add
normalizeIpcError as the single normalization point every invoke funnel
runs a rejection through (idempotent, fail-safe on unknown codes), wrap
mutateEvidenceBinder's whole body so both the browser mock and the real
invoke rejection normalize the same way, and switch
EvidenceBinderPane's recovery branch from message.includes(...) to
err instanceof IpcError && err.code === "...".
This is the tracer: one command, one emit site, one branch site, proving
the mechanism before 03-02/03-03 repeat it across the other six commands.
Records the ERR-04 pre-migration baseline B = 1138, measured before the first signature flipped, which 03-02 and 03-04 assert against. Checkpoint outcome recorded precisely: the gate was approved by the user, but no per-step smoke observations were returned, so the SUMMARY does not claim the migrated path was observed working in the real app. Nothing in CI exercises WKWebView, so that gap stays open until 03-04's full-gate run. Two accepted conditions carried into wave 2: - `cargo clippy -- -D warnings` is transiently red on TODAY_CONFLICT, TASK_CONFLICT and DOCUMENT_CONFLICT, which have no consumer until 03-02 migrates the other six commands. No suppression attribute was added; the phase prohibition holds. 03-02 owns clearing it. - The 7-command branch-on scope is ratified over CONTEXT's "four commands" wording. Same four codes; graph_link_apply stays display-only and OUT. The executor subagent ended at the checkpoint, so the orchestrator wrote this SUMMARY after independently re-running every verify command against the committed tree. No production code came from the orchestrator.
Flip check_revision, today_mutate, today_finalize_setup,
today_calendar_publish, load_context, task_transition, and task_trash
to Result<_, IpcError>, constructing TODAY_CONFLICT / TASK_CONFLICT at
every contract emit site (the two today_mutate inline guards, one of
which lives in apply_mutation's SetPlan branch rather than today_mutate
itself). Every other error path in these functions keeps its exact
legacy text via IpcError's From<String> conversion (explicit .into()
at direct return-Err sites; automatic via ? elsewhere).
today_ai.rs's today_apply_plan_result and today_calendar.rs's
task_calendar_set_sync stay Result<_, String> (ERR-04, no frontend
branch) via a .map_err(|e| e.to_string()) adapter at their today_mutate
call sites, mirroring evidence_binder_mutate's tracer shape.
web_actions.rs is not in this plan's file list but calls today_mutate
and task_transition directly; its two call sites needed the same
adapter, and web_actions_import_top's internal conflict branch moved
from err.starts_with("today_conflict") to err.code == "today_conflict".
Nine prefix-string test assertions beyond the four the plan enumerated
also needed migration, since every unwrap_err() on a flipped command
now yields IpcError regardless of which error path fired.
Flip assert_expected_revision and save_document to Result<_, IpcError>, constructing DOCUMENT_CONFLICT at all three conflict emit sites (the shared guard plus save_document's two inline missing-file checks). The "Cannot read document" read-failure paths and the read_document tail call keep their exact legacy text, the latter via an explicit .map_err(Into::into) since it's a direct (non-?) tail call into an unmigrated Result<_, String> function. update_frontmatter_field, a caller of assert_expected_revision that stays out of the branch-on set, gets the same .map_err(|e| e.to_string()) adapter already used for today_apply_plan_result and task_calendar_set_sync. This clears the last of the three not-yet-consumed contract-code constants 03-01 defined with no consumer; `cargo clippy --lib -- -D warnings` is green. Post-migration ERR-04 count: 1128 (B-10, exact).
web_actions_import_top's only Rust-side branch on a contract code compared err.code against the raw literal "today_conflict" instead of the TODAY_CONFLICT constant, defeating ERR-02's rename-fails-the-build protection at exactly the site it exists to guard: renaming the constant would fail ipc_error_codes_are_stable but leave this literal silently unmatched, breaking the conflict-retry recovery path with no build error. The test assertion at web_actions.rs:1863 stays a literal by design - pinning the wire value is what makes a rename observable.
Documents the defect fixed in 98f655b. web_actions_import_top compared err.code against the raw literal "today_conflict" instead of the TODAY_CONFLICT constant. A grep over src-tauri/src confirms this is the only Rust-side branch on a contract code in the tree, which makes the gap consequential rather than cosmetic: renaming the constant's value leaves ipc_error_codes_are_stable red, but a deliberate rename that updates that test alongside it would silently stop this branch matching, killing the conflict-skip recovery path with no build error. That is the exact failure ERR-02 exists to prevent, reproduced in a new site. Missed because web_actions.rs was absent from 03-02's files_modified list, so it never got the per-file PATTERNS.md treatment the enumerated files received. Also records the consequence for 03-04: web_actions.rs:860 is the only Rust-side contract-code branch, so the ERR-02 rename drill must exercise it or it proves only the TypeScript half of the two-sided gate.
Add the review-caught web_actions.rs TODAY_CONFLICT fix as deviation 5, correct the follow-up line numbers, reconcile a duplicate note that landed concurrently, and update actuals for the extra commit.
- todayInvoke wraps its whole body (e2e-override + real invoke branches) in try/catch, rethrowing normalizeIpcError(err) so fixture throws and real Tauri rejections reach branch sites through the same shape. - isTodayConflict / isTaskConflict now check `err instanceof IpcError && err.code === "..."` against the exported contract codes instead of parsing a message prefix. - todayErrorCode deleted (ERR-02: a rename now fails typecheck instead of silently missing a substring match); TodayReview's saveReflection catch branches on err.code via IpcError instead of the retired parser. - today.test.ts and TodayExecute.test.tsx updated to construct IpcError instances instead of prefix strings.
… conflict branch - saveDocument wraps its whole body (override + invoke branches) in try/catch, rethrowing normalizeIpcError(err), matching the today.ts funnel shape. - reportInsert's isConflict now takes the caught error and checks `err instanceof IpcError && err.code === "document_conflict"` instead of a message substring match; its caller passes the raw error object. - e2e/helpers/todayFixtures.ts's three today_conflict throw sites and e2e/smoke.spec.ts's save_document mock now throw the raw wire shape (prefix-free message + separate `code` field, per D-02) so the funnels' whole-body normalizer converts them into real IpcErrors exactly once. - reportInsert.test.ts's conflict case rejects with a real IpcError instance; the normalized message keeps the document_conflict text.
…a full green make verify
…guard Review caught that the three rename drills each prove one side pins itself, not that the two sides agree with each other. A Rust-only rename (constant value + its own ipc_error_codes_are_stable assertion, updated together) leaves cargo test and tsc -b both green while a frontend recovery path silently stops matching anything the backend sends - the exact failure ERR-02 exists to prevent. Add src/lib/types.test.ts, following the existing agentCapabilities.test.ts pattern: read ipc_error.rs via Vite's ?raw, extract the four pub const values, assert they equal IPC_ERROR_CODES. Correct the now-false doc comment at types.ts that implied this cross-check already existed.
…summary Documents the review-caught gap honestly rather than rewriting history: the original three-drill set (Rust value, Rust name, TS union) each proved self-pinning only. Records the exact reproduction (matched Rust-only rename passes both cargo test and typecheck), the fix (src/lib/types.test.ts), and the re-verification that the new guard - and only the new guard - catches it.
…KWebView gap Independently re-derived every ERR-01..04 claim against the tree rather than trusting SUMMARY text, including re-running the ERR-02 rename drill from scratch (matched Rust-only rename: cargo test and typecheck stay green, the new cross-language guard in src/lib/types.test.ts goes red as designed, then reverted clean). All four roadmap success criteria and all four ERR requirements verify true. Status is human_needed, not passed: 03-01's real-app WKWebView smoke checkpoint was approved without per-step observations reported back, and no later plan closed that gap, so it is carried forward here instead of being silently dropped.
A Codex adversarial review of PR #279 returned three medium findings. All three were checked against the tree and are accurate. None is a live defect in this phase's output; all three say the phase is less closed than its artifacts suggest. Two become v2 requirements rather than widening an already 32-file PR: - ERR-05: the contract constrains declarations, not emissions. IpcError is a pub struct with a pub code: String, so any module can mint an arbitrary code, and types.test.ts inventories pub const declarations without ever inspecting construction sites. An unregistered code passes the Rust pin, tsc, and the regex guard alike, then normalizeIpcError downgrades it to a plain Error and no recovery branch runs. - ERR-06: today_apply_plan_result and task_calendar_set_sync reach today_mutate and flatten its typed error back to String. Confirmed no caller branches on either path today, so nothing regressed and 03-02's justification holds - but the boundary was drawn by current frontend usage, which gives a future author no compile-time signal that the advertised recovery is unavailable there. The third finding sharpens the existing human-verification item instead of adding to it: e2e/smoke.spec.ts:1261 reads like native conflict coverage but injects a JS Error with a synthetic code via addInitScript, never crossing Rust serialization or the WKWebView bridge. Recorded so it is not mistaken for evidence when that gap is next assessed.
…e UI thread Both defects were found by the Phase 3 real-app smoke, and neither was observable from unit tests or the mocked e2e layer. Deserialization: EvidenceBinderMutation carried serde(tag, rename_all) which renames variants but not their fields, so the frontend's camelCase payload (candidateId, bindingId, sectionBindings, ...) was rejected as a missing snake_case field and every binder mutation failed at the bridge. Adding rename_all_fields = "camelCase" fixes it, and a regression test now pins every mutation wire shape the TypeScript union can send. Responsiveness: evidence discovery deep-inspected every candidate on the main thread, opening Office archives and scanning HWPX XML members before any bound was applied. On a 64,744-file workspace that blocked above 98% CPU for over 40 seconds. Now: - evidence_binder_read/mutate are #[tauri::command(async)], off the UI thread - discovery stays metadata-only and selects the newest MAX_CANDIDATES before any deep inspection, so an unscoped document never inspects the whole inbox - a failed validation skips the second HWPX unzip that follows it - the bounded inspection set runs in parallel via rayon - inspections are cached by path/size/mtime, bounded at 512 entries Measured after: a cache-empty 200-candidate load finishes in 4s with UI observation responsive at 0.25s.
The native WKWebView smoke that 03-01's checkpoint left unobserved has now been run, and it earned its place: the first binder attempt surfaced a real camelCase deserialization defect that no unit test or mocked e2e run could see, plus a severe main-thread stall on a large workspace. Both are fixed in the preceding commit. That is the concrete answer to the Codex review's third finding, which argued the Playwright conflict canary only injects a JavaScript Error with a synthetic code and never crosses Rust serialization or the WKWebView bridge. It was right, and the gap was hiding a live bug rather than a theoretical one. status: human_needed -> passed, with the observations recorded.
Verification passed, so the phase checkbox and progress row close and STATE advances to phase_complete with Phase 4 next. Two lessons recorded rather than left in the phase directory: - The real-app WKWebView smoke is not a formality. It found two live defects nothing else could see, one of which meant every evidence-binder mutation was rejected at the bridge. Later phases should treat the missing native gate as real coverage loss. - Codex's adversarial review produced ERR-05 and ERR-06 in the v2 requirements: the contract pins declarations but not emission sites, and commands able to emit reserved codes still flatten to String.
This was referenced Aug 23, 2026
Merged
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.
Four plans, 16 commits. The frontend stops parsing error messages to decide what
to do, and a rename now breaks a build instead of a recovery path.
What changed
src-tauri/src/ipc_error.rsintroducesIpcError { code, message }withDisplay,std::error::Error, andFrom<String>. Seven commands migrate:today_mutate,today_finalize_setup,today_calendar_publish,task_transition,task_trash,save_document,evidence_binder_mutate.graph_link_applydeliberately stays display-only and out.From<String>carries every non-contract error path intomessagewith anempty
code, andDisplayrenders an empty-code error as its message alone,so display-only text stays byte-identical.
src/lib/types.tsmirrors the codeset;
src/lib/ipcError.tsnormalizes rejections at the funnel; all fivefrontend branch sites read
err.code.todayErrorCodeis deleted.Two defects caught in review, not by the plans
web_actions.rscompared the code as a raw string literal. A grep confirmsit is the only Rust-side branch on a contract code in the tree, so the gap was
consequential:
err.codeis aStringin Rust, so a literal comparisoncompiles against anything. Renaming the constant would have left this
conflict-skip recovery path silently dead. Fixed to compare against
TODAY_CONFLICT(98f655b).The TypeScript side is different and it matters:
IpcError.codeis typed as theIpcErrorCodeunion, so a literal comparison there IS checked (TS2367). Gettingthose two backwards is what produced the bug, so the distinction is written down
in the plan summaries.
ERR-02 was marked complete while still unsatisfied. The phase shipped three
rename drills, and all three proved only that each language pins itself: the
Rust test pins Rust literals, the TS union pins TS literals. Nothing asserted
the two agree with each other. Reproduced: rename the Rust constant's value
along with its own stability assertion, exactly as a deliberate rename does, and
Rust emits
today_conflict_v2, TypeScript still expectstoday_conflict, bothbuilds green, and the recovery branch is dead. That is verbatim the failure
ERR-02 exists to prevent.
Closed by
src/lib/types.test.ts(eb154f1), following the existingagentCapabilities.test.tsdrift-guard pattern: it readsipc_error.rs?rawthrough Vite and asserts the two sets are equal. Re-run against the drill, that
guard is the only thing that goes red. The comment at
types.ts:64claimed across-check that did not exist and was corrected rather than left aspirational.
Verification
make verifygreen end to end: test-rust 1217 passed / 0 failed / 3 ignored,test-ts 190 files / 1858 tests, clippy
-D warningsclean, fmt clean, bundlebudget within limits
union, cross-language guard), each broken, each observed red, each reverted
set, inside the
[1118, 1138]band. The baseline B = 1138 was measured beforethe first signature flipped, since the tracer itself moves the count
todayErrorCodegrep both empty#[allow(oreslint-disablein the phase diff; non-contract legacyerror strings verified byte-identical
Open gap, recorded rather than closed
03-VERIFICATION.mdisstatus: human_needed, notpassed, for one reason:there is still no evidence the
{ code, message }wire shape round-tripsthrough the real native bridge. Every gate above is cargo test, vitest, tsc,
or Playwright's mocked IPC layer.
make test-e2eserves plain vite, nottauri dev, sowindow.__TAURI_INTERNALS__never exists.This is the pre-disclosed
native-tauri-e2e-runner-missingblind spot Phase 1'sown verification handed forward to Phase 3. Plan 03-01's checkpoint existed to
close it; it was approved without per-step observations being reported back, and
no later plan closed it either. The verification report carries a concrete
smoke checklist for whoever runs it.
Not a phase-goal failure - the serde round-trip tests and unit coverage are
strong indirect evidence - but a real hole in the evidence chain, so the
ROADMAP phase checkbox stays unchecked until a real-app pass happens.