Repository navigation
fix(minimal): name the min stop race in plain language - #2314
norrietaylor merged 7 commits into
Conversation
An activation that races min stop loses the race and its RPC fails with the daemon's shutdown refusal, which surfaced as a raw RPC chain (a refused ConfigureLoadout/CreateSession wrapping the daemon-side 'Internal error: in shutdown') instead of a sentence. The daemon now marks every shutdown refusal with a shared constant in minimald-rpc, and the client matches on it to print one plain line: the daemon was shutting down, no session was created, run the command again once it is running. Other failures keep their chain, wrapped as before. Applies to min session activate, min task run, and the dashboard's activate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title is concise, uses the required Conventional Commit format, and clearly describes the main change: reporting the min stop race in plain language. |
| Description check | |
The description explains the change and includes verification commands, test results, and the known pre-existing failure. It omits the required ## Checklist section and uses ## Verification instea… |
| Docstring Coverage | |
Docstring coverage is 94.44% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 7 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description explains the change and includes verification commands, test results, and the known pre-existing failure. It omits the required
## Checklistsection and uses## Verificationinstead of## Testing, but it is otherwise substantially complete.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit hears the daemon say,
“The session cannot start today.”
Two RPCs name the shutdown race,
And other errors keep their place.
The burrow notes the shared reply,
Then hops along beneath the sky.
Comment @coderabbitai help to get the list of available commands.
The shutdown-race sentence no longer leads with the internal RPC name, and the refusal marker matches only as the end of the daemon's message, so the same words inside other text keep their chain.
…tarted A ConfigureLoadout refused in shutdown follows a CreateSession that worked, so 8000 the text no longer claims no session was created: it says the session was created but not started, and that the daemon deletes it when it starts again. The daemon test now pins the refusal text end to end: the manager's error is the marker, and a CreateSession after a forced shutdown ends its stderr with it.
|
/review |
|
Approved: no findings at Nothing to do. Clobber: none seen. |
The client reworded any error whose chain had a cause ending with the shutdown marker, so an unrelated error ending in the same words was hidden. The wire carries no structured refusal, so match the whole client-side error text for this RPC exactly, and keep the original error in the chain under the plain line. The daemon test now pins that exact stderr text.
|
/review |
Wrapping the original error put the RPC name back into the printed chain. Return the plain line alone again, and log the raw error at debug so the RPC name stays in the logs. The exact match is unchanged.
|
/review |
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @norrietaylor closed the last one. The review of 1c02cf07ac5d withheld this approval on the threads open then. Posted as a comment, as the review itself was; the check and the label carry it.
|
/review |
2 similar comments
|
/review |
|
/review |
There was a problem hiding this comment.
Approved on ARCH PASS at 1c02cf0.
There was a problem hiding this comment.
The match is deliberately exact: a single-link error whose whole text is the client's daemon-side wrapper around the shared SHUTDOWN_REFUSAL constant, and the tests pin the negative cases (the marker inside other text, another RPC's refusal, a wrapped one). The two sentences say what each lost race leaves behind, which is the useful part. The daemon-side test checks the exact stderr bytes the client keys on. One heads-up for whoever merges second: #2224 rewrites the DeleteSession handler around the third in shutdown site this PR changes, and adds two refusals of its own that should use the constant once both are in.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Commit pushed:
|
|
/review |
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABX3vc8g
An activation that races
min stoploses the race: its CreateSession or ConfigureLoadout RPC fails with the daemon's shutdown refusal, which surfaced as a raw RPC chain (a refused RPC wrapping the daemon-sideInternal error: in shutdown) instead of a sentence.The daemon now marks every shutdown refusal with a shared constant in minimald-rpc (informed by #613), and the client matches on it to print one plain line: the daemon was shutting down, no session was created, run the command again once it is running. Other failures keep their chain, wrapped exactly as before. Applies to
min session activate,min task run, and the dashboard activate.Verification
cargo fmt --all --check— cleancargo clippy --workspace --all-targets --locked -- -D warnings— cleancargo test --locked -p minimald-rpc -p minimald -p minimal— minimald-rpc and minimald fully green; minimal 525 passed, 1 pre-existing environment-dependent failure (tests::legacy_network_spellings_parse_with_hint), verified to fail identically on unmodified mainSummary by CodeRabbit