fix: error taxonomy — every failure is a fact plus one next step (N3) - #78
Conversation
Sweeps the user-facing failure paths in both clients into house copy: a plain sentence stating the fact, one next step, and the cause in parentheses — never a bare wrapped chain. CLI: a new failure.go rewrites the high-traffic causes (machine off, relay down, wrong/expired pairing code, self-update); `mir list` and machine resolution say when the registry could not be checked instead of degrading silently; keychain and doctor failures carry an unlock hint. Doctor gains a clock-skew warning against the relay's Date header, since renames and revocations resolve last-writer-wins. SPA: the last browser alert() calls become in-app notice sheets; a failed registry fetch renders a one-line "showing saved machines" notice cleared by the next good fetch; the failed security check and the camera error now state the way forward. Wording only: every fail-closed path stays fail-closed. Closes #77 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7467eb041
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } // not signed in / relay unreachable — keep the local list | ||
| // Relay unreachable: keep the local list, but say so — a silently stale | ||
| // list would read as live. pollForMachine keeps retrying either way. | ||
| if (local.length && !discoveryPaused) { discoveryPaused = true; renderMachines(root, local, []); } |
There was a problem hiding this comment.
When saved machines exist and the registry request fails after the user has already opened a terminal or pairing screen, this catch path calls renderMachines without checking mountGen, replacing the newer view with the machine list. The polling path already uses a generation check for this reason; capture and verify the current mount before rendering the failure notice as well.
Useful? React with 👍 / 👎.
| sessions, cleanup, err := client.AttachAll(ctx, resolved, idn, iceList, *relayOnly) | ||
| if err != nil { | ||
| return err | ||
| return humanAttachErr(a.binary, strings.Join(names, ", "), err) |
There was a problem hiding this comment.
For mir attach a b, if only b is unavailable, AttachAll returns an error containing attach b, but this wrapper supplies the joined string and reports machine "a, b" is unreachable. That incorrectly identifies both machines as one failed machine and obscures which target needs mir up; preserve the failing machine name from AttachAll or avoid the singular-machine rewrite here.
Useful? React with 👍 / 👎.
| mc, closeConn, err := pairing.DialPair(ctx, signalURL, pairing.RoomID(token)) | ||
| if err != nil { | ||
| return err | ||
| return humanRelayErr(a.binary, err) |
There was a problem hiding this comment.
When /pair rejects the WebSocket upgrade for a reachable-relay condition such as the repository's HTTP 429 rate limit (which includes Retry-After: 60), DialPair returns an error here and the new wrapper says the relay is unreachable and recommends mir doctor. That advice cannot resolve the failure and hides the actionable wait period; inspect the HTTP response/status before classifying every dial error as a connectivity outage.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c7467eb. Configure here.
| // list would read as live. pollForMachine keeps retrying either way. | ||
| if (local.length && !discoveryPaused) { discoveryPaused = true; renderMachines(root, local, []); } | ||
| if (!local.length) pollForMachine(root); | ||
| } |
There was a problem hiding this comment.
Failed fetch stomps the current view
Medium Severity
The new viewMachines failure path calls renderMachines without checking mountGen. renderMachines remounts the root, so a late registry or revocation failure replaces pairing or a live terminal if the user left the list after the first paint.
Reviewed by Cursor Bugbot for commit c7467eb. Configure here.
…opy (N3) (#81) BETA.md was written while both slices were still in flight; #76 and #78 landed the guided retirement flow and the failure-path pass. Claude-Session: https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Closes #77. The beta gate this serves: zero raw errors reach a user.
The inventory
Swept every user-facing failure path in
go/internal/cli(+ what bubbles up frominternal/client) andweb/src. Paths already at the bar (U1/U2/N1/N2/R1 copy, refusals whose wording is pinned for semantics) were left alone. Rewritten:mir attach/run, machine offerror: signaling: agent unavailable/ locator chainmachine "box" is unreachable — … (cause: …)mir up, or checkmir listerror: dial signaling: Post "https://…": connection refusedthe relay is unreachable — … (cause: …)mir doctormir pair <code>, wrong/expirederror: pairing handshake failed (wrong code?): …pairing failed — the code is wrong or expired (codes last 5 minutes) … (cause: …)mir self-updateerror: github releases: 403 Forbiddencould not update — … (cause: …)mir list, relay downnote: the relay is unreachable — showing saved machines only; discovery resumes when you are back onlineunknown machine "x" — neither paired locally nor online in your encrypted registry(misleading)macOS Keychain: owner secret "…" is unavailablemir doctorchecks thisalert()⚠ The relay is unreachable — showing saved machines; discovery resumes when you are back online.(cleared by the next good fetch)camera unavailable: NotAllowedError…The camera is unavailable — … or type the code instead.Doctor
Dateheader (>5 min). Renames and revocations resolve last-writer-wins on timestamps, so a skewed clock silently loses every merge — now doctor says so.Decisions
go/internal/cli/failure.goholds the rewrites as pure functions; call sites stay one-line wraps. Unknown causes pass through untouched — the taxonomy rewrites known causes, it does not blanket-wrap.Tests
failure_test.gopins the rewrites (table) + passthrough +clockSkewtable; fullgo test ./...green,gofmt -lclean.errors-copy.test.jspins: zeroalert(in app.js, the discovery-paused notice + its clearing, rename/security/camera copy. 152/152 green.testdata/vectors touched.🤖 Generated with Claude Code
https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y
Note
Low Risk
Changes are wording and UX around existing error paths; security semantics and fail-closed refusals are explicitly preserved, with tests pinning the new messages.
Overview
Introduces a shared N3 error taxonomy: user-facing failures become a plain fact, one next step, and
(cause: …)at the end—without changing fail-closed behavior.CLI (
failure.go) centralizes rewrites for attach/run (machine unreachable vs relay down), pair (wrong/expired code), and self-update. Attach, run, and pair call sites route errors through these helpers; unknown errors still pass through unchanged.Discovery honesty when the relay is down:
mir listprints a stderr note that only saved machines are shown; resolveMachines distinguishes “unknown machine” from “registry couldn’t be checked offline.” The web app mirrors this with adiscoveryPausedbanner that clears on the next successful fetch.Doctor adds a clock-skew warning (>5 min vs relay
Date) for timestamp-based rename/revocation merges, plus keychain unlock hints (also on keychain read failures).Web SPA replaces
alert()with in-appnoticeSheetfor rename validation/failures, expands security-check and camera-denied copy, and adds source tests pinning the copy.Reviewed by Cursor Bugbot for commit c7467eb. Bugbot is set up for automated code reviews on this repo. Configure here.