Skip to content

fix(trg): keep op from answering reads out of its daemon cache - #254

Merged
yordis merged 1 commit into
mainfrom
yordis/fix-op-cache-disabled
Oct 15, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/fix-op-cache-disabled

Conversation

@yordis

@yordis yordis commented Oct 15, 2026 •

Copy link
Copy Markdown
Member
  • op caches by default on Unix, and that cache lives in an op daemon the CLI spawns on its own and leaves running after trg exits. From a background exec run that daemon is where a macOS permission prompt nobody is watching for stalls a read, and where Linux collects zombies.
  • A backend reads an item once per run, so the cache never saves a read and only adds a process trg did not spawn and cannot account for.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Summary by CodeRabbit

  • Improvements
    • Secret-management commands now bypass the CLI cache, helping ensure commands use current account data.
    • The command timeout remains 10 seconds.

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

cursor Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Localized change to how op is spawned for secrets reads; behavior should be more predictable with slightly less caching, not a change to secret resolution or auth logic.

Overview
Every 1Password CLI subprocess now passes --cache=false via a shared OP_NO_CACHE constant in OnePasswordBackend::run, so item get, account list, and account get no longer use op's default Unix cache/daemon path.

The intent is to avoid a background op daemon that outlives trg and can stall unattended reads (e.g. macOS permission prompts) or leave zombie processes on Linux, without gaining much because this backend typically reads each item once per exec run. Stub tests were updated to assert the new flag in recorded argv.

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

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: TrogonStack/rusty-monorepo/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 63bde343-cf86-4aa7-9b72-6d2ae3bc5200
📥 Commits

Reviewing files that changed from the base of the PR and between 6e70927 and b875c46.

📒 Files selected for processing: 1
  • crates/trg/src/secrets/onepassword.rs

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

📝 Walkthrough

The 1Password CLI runner now adds --cache=false to each op invocation. The run documentation and argv expectations for item reads and account commands reflect this argument.

Changes: 1Password CLI invocation

Layer / File(s) Summary
Runner arguments and tests
crates/trg/src/secrets/onepassword.rs
The runner adds --cache=false after the subcommand words. Documentation and expected argv for item reads and account commands include the new argument.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b875c

The runner now requests uncached 1Password CLI calls. Whether the CLI accepts this flag placement remains unverified, but no concrete invocation failure is established.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preventing op from serving reads from its daemon cache.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • 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.

@yordis
yordis merged commit 1c54b95 into main Oct 15, 2026
14 of 15 checks passed
@yordis
yordis deleted the yordis/fix-op-cache-disabled branch October 15, 2026 18:43
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.

1 participant