Skip to content

fix(server): a secret that could not be read no longer leaves the provider signed out - #163

Merged
yordis merged 2 commits into
mainfrom
yordis/fix-unreadable-secret-recovers
Oct 25, 2026
Merged

yordis merged 2 commits into
mainfrom
yordis/fix-unreadable-secret-recovers

Conversation

@yordis

@yordis yordis commented Oct 25, 2026

Copy link
Copy Markdown
Member
  • A provider instance whose secret store could not be read at startup (locked vault, slow 1Password) launched without its credential and kept reporting "not authenticated" even after the store was unlocked, so the only way out was restarting the app.
  • The card blamed the provider login instead of the secret store, sending users to run `claude auth login` for a problem that login cannot fix.
  • A successful secret check in Settings should be enough to bring the affected instances back, without another round of biometric prompts.

…vider signed out

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 37 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 9a15450 and 29a81f7.

Files selected for processing: 11
  • apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts
  • apps/server/src/provider/ProviderInstanceRegistry.test.ts
  • apps/server/src/provider/ProviderInstanceRegistry.ts
  • apps/server/src/provider/ProviderRegistry.test.ts
  • apps/server/src/provider/ProviderRegistry.ts
  • apps/server/src/provider/ProviderSecretResolverLive.test.ts
  • apps/server/src/provider/ProviderSecretResolverLive.ts
  • apps/server/src/provider/providerMaintenanceRunner.test.ts
  • apps/server/src/provider/testUtils/providerRegistryMock.ts
  • apps/server/src/ws.ts
  • docs/fork/0016-provider-secrets-live-in-1password.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@cursor

cursor Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes provider boot/env wiring and secret caching behavior; incorrect parking or rebuild logic could leave instances stuck unavailable or briefly run with wrong credentials, but scope is limited to secret-backed configuration.

Overview
Fixes provider instances that could not read a 1Password (or other) secret at startup from launching with missing credentials and showing a misleading not authenticated state.

Unreadable secrets now block instance creation: the registry marks the provider unavailable with a message that names the variable (e.g. CODEX_TOKEN) and asks to unlock the store and refresh. The old behavior of stripping unresolved variables and inheriting the server process env is removed.

Parking & prompts: Instances stuck on a locked vault are tracked in secretsUnreadable; unchanged settings reconciles reuse the parked snapshot instead of hitting the secret store again. The secret resolver cache no longer retains failed reads (zero TTL on failure), so unlock + retry can succeed without restarting the app.

Recovery paths: ProviderRegistry.checkSecretSource runs the Settings “check,” then rebuilds only unavailable instances that reference that secret (using the value the check just cached), without invalidating the whole cache. The WS handler routes serverCheckSecretSource through the registry instead of the resolver alone. Full provider refresh still invalidates and batch-primes secrets as before.

Reviewed by Cursor Bugbot for commit 29a81f7. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions Bot added size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Oct 25, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1865209. Configure here.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB +7 B (+0.1%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB +7 B (+0.6%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 9a15450 · PR result: 29a81f7 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

…s save

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis merged commit 6cb3f22 into main Oct 25, 2026
28 checks passed
@yordis
yordis deleted the yordis/fix-unreadable-secret-recovers branch October 25, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant