Repository navigation
Re-advertise a vendor CLI version that changes under a running daemon - #803
Conversation
…#802) A failed re-probe keeps the previously advertised version: the server reads a null version as the vendor being gone. A refresh that changes nothing does not re-register, since every registration bumps the slot's connection generation and fails a reviewer launch pinned to the previous one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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. |
PR Summary by QodoRe-advertise vendor CLI updates without restarting the daemon
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![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: d83a0f79e1
ℹ️ 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".
| foreach (var (vendor, cliPath) in Watched) { | ||
| var current = StatBinary(cliPath); | ||
| if (!Changed(_baselines.GetValueOrDefault(vendor), current)) continue; | ||
| _baselines[vendor] = current; |
There was a problem hiding this comment.
If Tick calls Refresh while SingleFlightRefresh is between consuming _rerunRequested and clearing _passRunning (SingleFlightRefresh.cs:71-74), the trigger observes a running pass and sets the rerun flag, but that pass exits without observing it. Because this line has already advanced the fingerprint, subsequent polls see no change and never retry, so the updated CLI remains unadvertised until a reviewer launch fails. Keep the change pending until the refresh is guaranteed to run, or make the single-flight handoff atomic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5029d88 on the single-flight side: after releasing the gate the runner re-reads the rerun flag and re-takes the gate when a request landed late, so the watcher may keep advancing its baseline before requesting the refresh.
| .Select(vendor => (vendor, _factories[vendor].CliPath)) | ||
| .Where(pair => !string.IsNullOrEmpty(pair.CliPath)) | ||
| .ToArray(); | ||
| PrimeBaselines(); |
There was a problem hiding this comment.
The advertised versions are probed before host.StartAsync, whereas this baseline is captured only when the hosted service starts. If a vendor auto-updates in that interval—which can be substantial while other vendors consume their retrying version-probe budgets—the baseline records the new binary while _config.UnattendedVendorCapabilities still contains the old version. Every later tick then reports no change, so the initial connection advertises the stale version and the first reviewer launch is still rejected. Capture the fingerprint alongside the version probe or perform an initial capability reconciliation before treating the current files as the baseline.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 5029d88. Startup now fingerprints each advertised vendor binary immediately before the version probe (DaemonRunner.FingerprintUnattendedVendors, stored on the config) and the watcher primes from that record instead of the file it finds when the service starts. A_baseline_recorded_before_the_startup_probe_wins_over_the_file_at_start pins it.
A request landing between a pass's last rerun check and the gate release is served by re-taking the gate. A republish request folded into a running pass is a sticky flag, since the rerun executes the first caller's delegate. The watcher's first baseline is the fingerprint taken before the startup probe, so an update during the probe window still reads as a change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
Closes #802 — AI-2547
What & why
The vendor CLI version a daemon advertises is a startup probe cached for the process lifetime, so a Claude auto-update under a long-running daemon costs the next review-flow launch, and the rejection tells the operator to restart the daemon: the one action that tears down every hosted agent. The rejection already re-advertised on the live connection, so the remedy now says retry. A
VendorCliWatcherfingerprints each advertised vendor's binary (PATH-resolved, symlink chain followed, size, mtime) every 15 s and, on a change, re-probes and re-registers through the orchestrator's single-flight refresh, which the rejection path now shares.Where to look
DaemonRunner.RetainAdvertisedVersions: a re-probe that returns null keeps the previously advertised version, because the server reads null as the vendor being gone. An unchanged refresh skips the re-register, since every registration bumps the slot's connection generation and fails a reviewer launch pinned to the previous one; the rejection path forces a republish because the server's copy has just proven wrong, and that request is a sticky flag because a request folded into a running pass reruns the first caller's delegate. The watcher's first baseline is the fingerprint taken at startup before the version probe, so a vendor updating inside the probe window still reads as a change.SingleFlightRefreshre-takes its gate when a request lands between the last rerun check and the release, so a watcher request cannot be lost. The refresh probes the startup vendor set rather than re-classifying, so a vendor withheld at startup for a version floor stays withheld until a restart.Verification
Live: Claude flipped 2.1.259 → 2.1.263 on Sep 6 under a daemon started Sep 3; one spec-review launch was rejected at 08:31 on Sep 7 and
GET /api/daemonsthen showedcli_version: 2.1.263with the daemon never restarted, which is the self-heal this PR builds on.