Treat auth-required probe failures as expected online RPC failures - #2294
Treat auth-required probe failures as expected online RPC failures#2294bradhallett wants to merge 2 commits into
Conversation
The startup provider.list_models probe for ACP providers lands before the agent's auth handshake settles, so the daemon warned "online host RPC failed" with "ACP agent is not authenticated." on every daemon start. The failure is a user state, not a daemon bug: its errorCode still reaches the server, which logs and surfaces it (model load error, turn error). Add auth_required to the expected-failure codes so only the duplicate daemon-side warn is silenced.
The regression test from the original commit passed bare Errors and leaned on the ACP_AUTH_REQUIRED_PATTERN message regex, which e42a4ef (get-bb#2325) deleted: on main the router sees a typed AgentRuntimeRecoveryError carrying code "auth_required". Rebuild the test on that shape at both layers — the classifier unit and the router path (typed errorCode still returned, debug accounting kept, no warn) — with a contrast case proving unclassified failures still warn. Also correct the allow-list comment: the probe execs the agent's list-models CLI with no ACP handshake, and the entry covers every online RPC that can surface the code.
Rework addendum (reply to review of #2291)Thanks for the thorough report — reworked per the REQUEST CHANGES verdict. Rebased onto current Test is dead on arrival / Merge conflict (Blockers) — Fixed. The rebase hit exactly the predicted conflict: Test exercises the wrong unit (Medium) — Fixed. Mirrored your §4a verification as router-level tests in Blast radius not stated (Low) — Fixed in the code comment: it now says the entry covers every online RPC that can surface the code (provider.usage, provider.installation.*, any plugin.host.call error carrying code "Auth handshake race" wording (Low) — Dropped. The new comment states the real mechanism (user-state outcome; typed code reaches the server, which logs and surfaces it; the daemon warn duplicates it with two nested stacks). Note the original commit message is preserved unsquashed per repo convention, so its stale race wording remains in that commit's message only; this addendum and the rework commit message correct the record. Verification — mutation-checked: deleting the
|
192a39a to
751194d
Compare
What was wrong
On every host-daemon start, the server's first execution-options resolution probes
provider.list_modelson the fresh daemon session (the probe memo is keyed by daemon session id, so a reconnected daemon always re-probes; failures are not memoized). The probe lands ~0.6–0.9 s into the daemon session — before the ACP agent's auth handshake/login state has settled — so the provider-acp bridge maps the agent CLI's auth-required failure to"ACP agent is not authenticated."(plugins/provider-acp/src/bridge/bridge.tsL520-L521, L798-L801). The daemon classifies that asauth_required, which at basefff3ae8is not an expected online-RPC failure code, soCommandRouter.handleOnlineRpcRequestlogs a warnonline host RPC failedon every start — 33 occurrences across two rotated daemon logs on the reporting host, each benign and each already mirrored by the server's own contextual warn + UI model-load error. Issue: #2291 (post-mortem Issue 4b of the OMP↔bb integration audit).What changed
apps/host-daemon/src/command-dispatch-support.ts— addedauth_requiredtoEXPECTED_ONLINE_RPC_FAILURE_CODES, following the existingfile_too_large/provision_cancelledpattern. An unauthenticated agent is a user-state outcome, not a daemon bug: the failure'serrorCodestill travels to the server in the RPC response, and the server still logsFailed to resolve provider modelsand surfaces the model-load error in the UI — persistent (genuinely logged-out) failures still surface exactly as before. Only the daemon-side duplicate warn is silenced; the failure-counting debug line inlogOnlineRpc(which recordsok: falsewith theerrorCode) still fires. No wire changes; no behavior change for any other error class.Deviation from the issue's alternatives: the issue offered retry-with-backoff or log-level demotion; demotion via the existing expected-failure set is the smaller change and the established repo pattern — the daemon has no notion of a "startup probe", and the transient case already self-heals because the server re-probes (failures are not memoized).
How you verified
apps/host-daemon/src/command-dispatch-support.test.ts(treats ACP model-probe auth failures as expected RPC failures): assertsisExpectedOnlineRpcFailureErroris true for both message shapes the router can see ("ACP agent is not authenticated."and the Cursor CLI'sAuthentication required … CURSOR_API_KEY/…text). Fails before the change by construction — the sibling test at the same site already pinsgetErrorCode(…)toauth_requiredfor both messages, andauth_requiredwas absent from the set — and passes after.~/.bb/logs/host-daemon.*.log/server.*.logtimestamps.pnpm exec turbo run test --filter=@bb/host-daemon— 561/561 passing.Mutation check: with
command-dispatch-support.tsreverted to base, the newtest fails (1 failed / 5 passed) and passes again with the entry restored.
pnpm exec turbo run typecheck lint --filter=@bb/host-daemongreen;pnpm exec oxfmt --checkgreen on both touched files.Fixes #2291