doctor --fix: nested-runtime panic + behind==0 short-circuit + stale-agent prune target #295

Closed
opened 2026-06-29 07:09:49 +02:00 by clawdie · 1 comment
Owner

colibri doctor --fix + colibri upgrade shipped in #294, but the fix-dispatch
path is never exercised by the gates, so two bugs merged green. Both defeat the
exact scenario the feature exists for (osa's stale-baked binary after the
v0.12.0 tag). Plus the originally-planned second dispatch target.

All in crates/colibri-client/src/bin/colibri.rs.

1. Nested-runtime panic (crash) — blocker

fix_fixable is sync and called from the async fn run arm, but does:

let rt = tokio::runtime::Runtime::new().unwrap();
rt.block_on(cmd_upgrade(false, true))?;

block_on from inside the #[tokio::main] (multi-thread) runtime panics
"Cannot start a runtime from within a runtime." So doctor --fix aborts the
moment it finds a fixable check to act on. The "nothing to fix" path never hits
it — which is why tests pass.

Fix: make fix_fixable async and .await cmd_upgrade(false, true)
directly; drop the nested Runtime.

2. behind == 0 short-circuit (logic) — blocker

cmd_upgrade returns "already at latest" when the source tree is at HEAD:

let behind = git_behind_count(&src_dir).unwrap_or(0);
if behind == 0 { eprintln!("✓ already at latest"); return Ok(()); }

But binary_vs_git_version fires when the binary is stale vs the source
orthogonal to the source being behind origin. That's osa right now: checkout
at HEAD (0 behind), installed binary still bakes v0.11.0-383. So --fix
→ "already at latest" → leaves the WARN. The git-behind gate conflates "source
behind origin" with "binary behind source."

Fix: for the binary_vs_git_version case, force rebuild+swap regardless of
behind-count (the binary, not the repo, is what's stale).

3. Minor (fold into the same PR)

  • No rollback. If the freshly-built daemon fails to start, the old binary
    is already overwritten — a bad build bricks the daemon. cp current binary
    to .bak before swap, restore on failed post-restart verify.
  • @{u} vs origin/main mismatch. git_behind_count measures HEAD..@{u}
    while the pull targets origin/main. On a detached/non-main checkout these
    diverge or @{u} fails → None0 → silent "already at latest." Pin both
    to origin/main.
  • Triple git fetch (one in cmd_upgrade, two in git_behind_count) — harmless noise.

4. Planned: second --fix dispatch target — stale-agent prune

Originally deferred to after #292/#294. Give doctor --fix a target beyond
colibri upgrade: prune store agents that have no live handle
(registration_linkage WARN — osa showed 17 stale). Either colibri agents prune wired into the fixable set, or a dedicated check with
with_fixable() whose fix is the prune. Mark the new check fixable: true so
--fix dispatches to it the same way binary_vs_git_versioncolibri upgrade.


Bugs 1 and 2 compound to make doctor --fix non-functional on first real use.
The manual stop → cp → start flow still works in the meantime, so nothing is
operationally blocked.

`colibri doctor --fix` + `colibri upgrade` shipped in #294, but the fix-dispatch path is never exercised by the gates, so two bugs merged green. Both defeat the exact scenario the feature exists for (osa's stale-baked binary after the v0.12.0 tag). Plus the originally-planned second dispatch target. All in `crates/colibri-client/src/bin/colibri.rs`. ### 1. Nested-runtime panic (crash) — blocker `fix_fixable` is sync and called from the `async fn run` arm, but does: ```rust let rt = tokio::runtime::Runtime::new().unwrap(); rt.block_on(cmd_upgrade(false, true))?; ``` `block_on` from inside the `#[tokio::main]` (multi-thread) runtime panics *"Cannot start a runtime from within a runtime."* So `doctor --fix` aborts the moment it finds a fixable check to act on. The "nothing to fix" path never hits it — which is why tests pass. **Fix:** make `fix_fixable` `async` and `.await cmd_upgrade(false, true)` directly; drop the nested `Runtime`. ### 2. `behind == 0` short-circuit (logic) — blocker `cmd_upgrade` returns "already at latest" when the source tree is at HEAD: ```rust let behind = git_behind_count(&src_dir).unwrap_or(0); if behind == 0 { eprintln!("✓ already at latest"); return Ok(()); } ``` But `binary_vs_git_version` fires when the *binary* is stale vs the *source* — orthogonal to the source being behind *origin*. That's osa right now: checkout at HEAD (0 behind), installed binary still bakes `v0.11.0-383`. So `--fix` → "already at latest" → leaves the WARN. The git-behind gate conflates "source behind origin" with "binary behind source." **Fix:** for the `binary_vs_git_version` case, force rebuild+swap regardless of behind-count (the binary, not the repo, is what's stale). ### 3. Minor (fold into the same PR) - **No rollback.** If the freshly-built daemon fails to start, the old binary is already overwritten — a bad build bricks the daemon. `cp` current binary to `.bak` before swap, restore on failed post-restart verify. - **`@{u}` vs `origin/main` mismatch.** `git_behind_count` measures `HEAD..@{u}` while the pull targets `origin/main`. On a detached/non-main checkout these diverge or `@{u}` fails → `None` → `0` → silent "already at latest." Pin both to `origin/main`. - Triple `git fetch` (one in `cmd_upgrade`, two in `git_behind_count`) — harmless noise. ### 4. Planned: second `--fix` dispatch target — stale-agent prune Originally deferred to after #292/#294. Give `doctor --fix` a target beyond `colibri upgrade`: prune store agents that have no live handle (`registration_linkage` WARN — osa showed 17 stale). Either `colibri agents prune` wired into the `fixable` set, or a dedicated check with `with_fixable()` whose fix is the prune. Mark the new check `fixable: true` so `--fix` dispatches to it the same way `binary_vs_git_version` → `colibri upgrade`. --- Bugs 1 and 2 compound to make `doctor --fix` non-functional on first real use. The manual stop → cp → start flow still works in the meantime, so nothing is operationally blocked.
Author
Owner

Cross-host evidence for the stale-agent prune target. registration_linkage WARN seen on both platforms after live runs: osa (FreeBSD) 17 stale, debby (Linux) 4 stale. Confirms the second doctor --fix dispatch target (colibri agents prune / a fixable registration_linkage check) is worth building — it is the most-evidenced remaining item here.

**Cross-host evidence for the stale-agent prune target.** `registration_linkage` WARN seen on both platforms after live runs: osa (FreeBSD) **17** stale, debby (Linux) **4** stale. Confirms the second `doctor --fix` dispatch target (`colibri agents prune` / a fixable registration_linkage check) is worth building — it is the most-evidenced remaining item here.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
clawdie/colibri#295
No description provided.