Skip to content

fix(server): one failed Claude probe no longer sticks as an auth warning - #13838

Open
StiensWout wants to merge 1 commit into
pingdotgg:mainfrom
StiensWout:t3code/claude-failed-probe-keeps-status
Open

StiensWout wants to merge 1 commit into
pingdotgg:mainfrom
StiensWout:t3code/claude-failed-probe-keeps-status

Conversation

@StiensWout

@StiensWout StiensWout commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #13635

One stalled Claude capability probe turned a working Claude into "Could not verify Claude authentication status from initialization result." The probe mapped every failure (a timeout, an SDK error) to undefined, the driver cached that result for five minutes, and nothing logged the cause. The warning also drops Claude out of the new-thread picker, even though its running threads keep working.

Fix

  • The probe fails with its real error and logs the error tag. It does not log the SDK message, which can quote the CLI's stderr.
  • When a probe fails, the last good probe answers once more. Claude stays ready with the same account and slash commands. The stand-in drops its usage windows, so the published limits keep their real age instead of being restamped as current.
  • A second failure in a row, or a failure before any good probe, shows a warning that names the cause: Timed out after 25s while checking Claude account status. or Claude Agent CLI failed while checking account status. The cache never keeps that failure, so the next status check probes again.

There is no new timer. Retries use the existing refresh loop and its client-demand gate, as the triage asked. A stalled claude --version still reports an error, which #7232 covers. #13643 adds only the log line; this PR logs the same tag.

Because the last good probe carries its slash commands, one failed probe no longer empties the / menu either (#7111).

Before

One stalled refresh, with the Claude CLI hanging on init, turns Claude into an auth warning:

Before: one stalled refresh shows Could not verify Claude authentication status

After

The same stalled refresh keeps Claude authenticated:

After: Claude stays authenticated after one stalled refresh

A second stalled refresh in a row says what failed:

After: a second failure in a row says the probe timed out

Captured with Playwright against a local dev server and a stand-in Claude CLI that stops answering initialize on demand.

Testing

  • ClaudeCapabilitiesProbe.test.ts: a new test walks through a first failure, a good probe served from cache, a failed probe answered by the last good account (without usage), and a second failure that reaches the caller and is retried on the next check. It fails if failures are cached, if the fallback never runs out, if success is not cached, or if the fallback is removed.
  • ProviderRegistry.test.ts: the warning names a timeout or a CLI failure.
  • tsc --noEmit for apps/server, plus vp lint and vp fmt --check on the changed files.

Prepared for Wout by claude-opus-5-5 in Claude Code.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Claude account checks when the CLI is slow or unavailable. Timeout and other probe failures now show a warning with an appropriate error message instead of an ambiguous status.
    • When a usage request fails, account capability information can still be displayed without usage details. After a successful check, a later probe failure can temporarily fall back to the last known capabilities; repeated failures are reported rather than treated as successful checks.

One stalled capability probe replaced a ready Claude snapshot with
"Could not verify Claude authentication status", cached that result for
five minutes, and dropped the cause.

The probe now fails with its real error and logs the tag. When a probe
fails, the last good probe answers once more, so one stall keeps Claude
ready with the same account and slash commands. A second failure in a row,
or a failure before any good probe, shows a warning that says whether the
probe timed out or the CLI failed. The cache never keeps that failure.

Fixes pingdotgg#13635

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 26, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This production change modifies Claude authentication-status reporting and capability caching, including retaining authenticated state after a failed probe and changing behavior after repeated failures. The scope and tests are focused, but the authentication implications require human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a977d782-1854-4008-a80b-e78e95fa544d

📥 Commits

Reviewing files that changed from the base of the PR and between 95030dc and 9b9c924.

📒 Files selected for processing (4)
  • apps/server/src/provider/Drivers/ClaudeDriver.ts
  • apps/server/src/provider/Layers/ClaudeCapabilitiesProbe.test.ts
  • apps/server/src/provider/Layers/ClaudeProvider.ts
  • apps/server/src/provider/Layers/ProviderRegistry.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Claude capability probes now use a five-minute success cache. Failed refreshes can use the last good capabilities without usage data. Provider status reports probe failures as warnings with unknown authentication status. The Claude driver uses the cache for status checks and invalidation.

Changes

Claude capability probe

Layer / File(s) Summary
Capability cache and fallback
apps/server/src/provider/Layers/ClaudeProvider.ts, apps/server/src/provider/Layers/ClaudeCapabilitiesProbe.test.ts
Successful probes are cached for five minutes. A failed refresh can return the last good capabilities once without usage windows. Tests cover cache reuse, fallback, and retry behavior.
Provider status on probe failure
apps/server/src/provider/Layers/ClaudeProvider.ts, apps/server/src/provider/Layers/ProviderRegistry.test.ts
Probe failures produce a warning with unknown authentication status and a timeout-specific or general error message. Tests cover timeout and unknown failures.
Driver cache integration
apps/server/src/provider/Drivers/ClaudeDriver.ts
The driver creates the capability cache and uses its lookup and invalidation effects.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 9b9c9

No merge-blocking issue was established for the Claude probe resilience change; it is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 9b9c9

A failed account check can temporarily preserve and publish an earlier account state as ready. This improves resilience to transient failures, but can also delay visibility of a revoked, changed, or otherwise invalid account state.

Retained concerns

  • Medium · security · inferred: The capability cache can convert a failed live probe into a cached successful capability result, causing prior account metadata and ready/authenticated status to remain visible for the success TTL. A credential or account change within the same resolved home can therefore temporarily reuse prior identity-derived state.
Security review details

Security Blast Radius

  • observed — The affected state feeds the Claude provider snapshot, including authentication metadata, account-derived capability information, and slash-command availability.

Security Findings and Attack Paths

  • inferred — If revocation, logout, or an account change causes the live capability probe to fail, the first failure can publish the prior account as ready/authenticated instead of exposing unknown authentication. The state can remain cached until the normal success TTL expires.

Trust Boundaries and Controls

  • observed — The inspected caller supplies a driver-owned cache getter rather than caller-supplied account state, and no new external caller or trust-boundary transition was found in the focused driver-to-status relationship.

Resilience and Maintainability Implications

  • observed — Timeout and SDK failures are bounded and normally retryable because raw failures are not retained; however, classifying fallback as success delays a fresh probe and obscures whether the published capability state is live or degraded.

Hardening Proposals

  • proposed — Represent degraded fallback separately from a fresh successful capability result, prevent it from extending the normal success TTL for authentication state, and invalidate or re-key account-derived capabilities when credential identity changes.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changes address several requirements in [#13635]. They preserve the last successful account and slash commands for one failed probe, remove usage windows from the fallback, avoid failure caching, … Add a demand-independent retry policy for failed probes, or otherwise guarantee the required short-backoff retry when no provider-status demand exists. Implement and test stale-snapshot age display if it is not already present.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main fix: a failed Claude probe no longer persists as an authentication warning.
Description check ✅ Passed The description explains what changed, why it changed, the fallback and retry behavior, user-visible outcomes, testing, and UI screenshots. It does not include the template's Checklist section, but th…
Out of Scope Changes check ✅ Passed The changed production files implement Claude probe caching, fallback status construction, error reporting, and integration with the provider driver. The changed tests verify those behaviors. Slash-co…
Full details: Linked Issues check

Explanation

The changes address several requirements in [#13635]. They preserve the last successful account and slash commands for one failed probe, remove usage windows from the fallback, avoid failure caching, log only the error tag, and return cause-specific warning messages with unknown authentication. The new tests cover fallback, consecutive failures, retry, and warning messages. However, [#13635] requires a failed probe to retry on a short backoff without client demand. The PR keeps the existing demand-gated refresh loop and adds no demand-independent retry. The reviewed changes also provide no evidence that the required stale-snapshot age is shown in the UI.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

1 participant