Skip to content

fix(server): one vault unlock covers every provider that reads from it - #29

Merged
yordis merged 2 commits into
mainfrom
yordis/fix-batch-1password-reads
Aug 20, 2026
Merged

fix(server): one vault unlock covers every provider that reads from it#29
yordis merged 2 commits into
mainfrom
yordis/fix-batch-1password-reads

Conversation

@yordis

@yordis yordis commented Aug 20, 2026

Copy link
Copy Markdown
Member
  • Approving a 1Password read is charged per CLI invocation, not per secret, and every provider instance resolved its own environment independently. A machine running several reference-backed providers therefore paid one biometric prompt per provider, at every server start and every provider refresh. The cost scaled with how many providers someone runs, which is backwards for a feature meant to make credentials less tedious.

  • The two places that fan out across instances are the settings watcher and the refresh path, so both resolve the whole set of references up front. Everything downstream reads what is already in memory.

  • Priming is deliberately best effort. 1Password resolves a template as a whole or fails it, so a single unreadable reference would take an entire batch down with it. A batch that does not come back leaves the cache untouched and the existing one-at-a-time path runs unchanged, which is where per-variable failure isolation lives and how someone still learns which reference is the broken one.

@cursor

cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches 1Password secret resolution and provider boot/refresh, including a new batch CLI path that parses secret output. Failure is designed to fall back to the existing per-reference reads.

Overview
1Password unlocks are charged per CLI invocation, not per secret. Each provider used to resolve its own environment, so a fleet of reference-backed instances prompted once per provider at boot and on refresh.

This adds a best-effort prime on ProviderSecretResolver that batch-reads uncached references with a single op inject (skipped for fewer than two). Boot hydration and the settings watcher prime the derived config map before reconcile; refresh primes via a new listEnvironments on the instance registry before rebuilds.

If the batch fails, the cache stays cold and resolve still reads one reference at a time, so a single bad item does not poison the rest.

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

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@yordis, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

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

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

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: db6c38a9-c3f8-4301-ace8-cb805e13cbab

📥 Commits

Reviewing files that changed from the base of the PR and between b76b900 and 0ced035.

📒 Files selected for processing (15)
  • apps/server/src/provider/Layers/ProviderAdapterRegistry.test.ts
  • apps/server/src/provider/Layers/ProviderInstanceRegistryHydration.ts
  • apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts
  • apps/server/src/provider/Layers/ProviderRegistry.test.ts
  • apps/server/src/provider/Layers/ProviderRegistry.ts
  • apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts
  • apps/server/src/provider/Layers/ProviderSecretResolverLive.ts
  • apps/server/src/provider/ProviderSecretReference.test.ts
  • apps/server/src/provider/ProviderSecretReference.ts
  • apps/server/src/provider/Services/ProviderInstanceRegistry.ts
  • apps/server/src/provider/Services/ProviderSecretResolver.ts
  • apps/server/src/textGeneration/TextGeneration.test.ts
  • docs/fork/0016-provider-secrets-live-in-1password.md
  • docs/internals/providers.md
  • docs/user/provider-secrets.md

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.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Aug 20, 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 1e83833. Configure here.

The settings stream carries later writes only, so the initial fleet was built without priming and every instance read its own reference. Boot is the run where nothing is cached yet, which made it the most expensive path rather than the one the change was aimed at.

Signed-off-by: Yordis Prieto <[email protected]>
@yordis
yordis merged commit a7cf32d into main Aug 20, 2026
10 of 14 checks passed
@yordis
yordis deleted the yordis/fix-batch-1password-reads branch August 20, 2026 20:45
@yordis
yordis restored the yordis/fix-batch-1password-reads branch August 21, 2026 00:28
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