Skip to content

Treat auth-required probe failures as expected online RPC failures - #2294

Open
bradhallett wants to merge 2 commits into
get-bb:mainfrom
bradhallett:fix/auth-probe-noise
Open

Treat auth-required probe failures as expected online RPC failures#2294
bradhallett wants to merge 2 commits into
get-bb:mainfrom
bradhallett:fix/auth-probe-noise

Conversation

@bradhallett

Copy link
Copy Markdown
Contributor

What was wrong

On every host-daemon start, the server's first execution-options resolution probes provider.list_models on 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.ts L520-L521, L798-L801). The daemon classifies that as auth_required, which at base fff3ae8 is not an expected online-RPC failure code, so CommandRouter.handleOnlineRpcRequest logs a warn online host RPC failed on 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 — added auth_required to EXPECTED_ONLINE_RPC_FAILURE_CODES, following the existing file_too_large / provision_cancelled pattern. An unauthenticated agent is a user-state outcome, not a daemon bug: the failure's errorCode still travels to the server in the RPC response, and the server still logs Failed to resolve provider models and 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 in logOnlineRpc (which records ok: false with the errorCode) 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

  • Regression test in apps/host-daemon/src/command-dispatch-support.test.ts (treats ACP model-probe auth failures as expected RPC failures): asserts isExpectedOnlineRpcFailureError is true for both message shapes the router can see ("ACP agent is not authenticated." and the Cursor CLI's Authentication required … CURSOR_API_KEY/… text). Fails before the change by construction — the sibling test at the same site already pins getErrorCode(…) to auth_required for both messages, and auth_required was absent from the set — and passes after.
  • Log forensics in the linked issue: the warn's race window (+640…+888 ms after the daemon ready banner) and its 1:1 pairing with the server-side warn were measured from ~/.bb/logs/host-daemon.*.log / server.*.log timestamps.
  • pnpm exec turbo run test --filter=@bb/host-daemon — 561/561 passing.
    Mutation check: with command-dispatch-support.ts reverted to base, the new
    test fails (1 failed / 5 passed) and passes again with the entry restored.
  • pnpm exec turbo run typecheck lint --filter=@bb/host-daemon green;
    pnpm exec oxfmt --check green on both touched files.

Fixes #2291

AGENT GENERATED

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.
@bradhallett

Copy link
Copy Markdown
Contributor Author

Rework addendum (reply to review of #2291)

Thanks for the thorough report — reworked per the REQUEST CHANGES verdict. Rebased onto current main (42658f987) and replaced the regression test as requested.

Test is dead on arrival / Merge conflict (Blockers) — Fixed. The rebase hit exactly the predicted conflict: e42a4ef48 (#2325) removed ACP_AUTH_REQUIRED_PATTERN and reduced command-dispatch-support.test.ts from ~150 to 15 lines. Resolved to main's structure and dropped the bare-new Error(...) test entirely. The new tests construct the error the way the runtime actually does — new AgentRuntimeRecoveryError({ code: "auth_required", message, recovery: { kind: "authRequired", providerId, retryable: false }, cause }) — so they fail if the classifier ever returns to message-shape matching.

Test exercises the wrong unit (Medium) — Fixed. Mirrored your §4a verification as router-level tests in apps/host-daemon/test/command/command-router.test.ts (placed beside the existing provision-cancellation precedent): provider.list_models through CommandRouter.handleOnlineRpcRequest with listModels throwing the typed error asserts (a) the typed { ok: false, errorCode: "auth_required", errorMessage } response reaches the server intact, (b) logger.warn is called zero times, and (c) the debug accounting line (Online host RPC, with commandType/errorCode/ok:false) still fires. Added the contrast case: an unclassified failure still warns exactly once with online host RPC failed.

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_required), not just the probe. On the two alternatives you offered: I kept the scope tight and did not add rate_limited — there's no report of that warn being noise, and each code can be added independently when warranted — and did not switch to instanceof AgentRuntimeRecoveryError, because the string-code allow-list matches the existing pattern beside it (file_too_large, provision_cancelled from 4c7d2a9) and stays decoupled from the daemon importing the runtime's error class. Happy to switch if maintainers prefer the type-based form.

"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 "auth_required" entry makes both new tests fail (unit: expected false to be true; router: warn called 1 time), restoring makes them pass. Scoped gates: tsc --noEmit clean, package lint clean, oxfmt --check clean on touched files, host-daemon suite 553/557 — the 4 failures (thread-stop-races.test.ts) reproduce identically on pristine main sources in this environment (esbuild cannot resolve @get-bb/plugin-sdk/provider-bridge because packages/plugin-sdk imports @bb/provider-bridge-acp/launch-spec without declaring that dependency, so its dist never builds under pnpm strict isolation) — unrelated to this PR and worth a look separately.

AGENT GENERATED

@bradhallett
bradhallett force-pushed the fix/auth-probe-noise branch from 192a39a to 751194d Compare August 24, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

provider.list_models startup probe logs "ACP agent is not authenticated." at warn on every daemon start

1 participant