feat(consent): local source discovery + per-source opt-in (task 002)#2
Conversation
Sources are now discovered (metadata only) and gated behind explicit consent (ADR 0012). Nothing reads a credential until the user opts a source in; the already-shipped Claude Code source connects through this flow instead of fetching unconditionally. What changed - core: `sources` module (descriptors + catalog + the present&&enabled gate), plus `SourceProbe` (metadata-only presence) and `ConsentStore` ports. `discover_sources`/`active_sources` are pure over the ports. - adapters: `ClaudeCredentials::is_present` (stat the creds file / Keychain item WITHOUT `-w` — never reads the token), `LocalSourceProbe` dispatch, and `FileConsentStore` (JSON in app config dir; consent is not a secret, so it never touches the keychain; persists across restart). - app: refresh loop + `fetch_claude_usage` are consent-gated; new `list_sources` / `set_source_enabled` commands. Opting in kicks an immediate refresh — effective without a restart. - ui: a Sources/consent screen (presence badge, plain-language access note shown before opt-in, per-source switch) and a connect-gated usage view. Risk & fallback - A corrupt consent file fails closed (every source opted-out) — it can never accidentally enable a read. Discovery is decoupled from reading: a present-but-garbage creds file reads as "detected" yet the secret path rejects it (covered by a test). Verification - `make check` green (fmt, clippy, core-purity, 26 tests, cargo-deny), `cargo machete`, core coverage 88% (>=80), Biome, svelte-check (0/0), `pnpm build`. Manual QA (tray + network/keychain confirmation that nothing is read pre-opt-in) is the remaining human step per the DoD.
📝 WalkthroughWalkthroughThis PR adds a complete source discovery and consent system, allowing users to discover login-reuse sources (currently Claude Code) by checking metadata-only presence, opt in per-source, and gate API calls on both consent and presence. The implementation spans core contracts, adapter implementations, Tauri backend refactoring, and frontend UI changes. ChangesSource discovery and consent system
Sequence DiagramsequenceDiagram
participant User
participant Frontend as Svelte UI
participant Tauri as Tauri Backend
participant Probe as SourceProbe
participant Consent as ConsentStore
participant Fetch as API Fetch
User->>Frontend: Mount page / list sources
Frontend->>Tauri: listSources()
Tauri->>Probe: is_present("claude-code")
Tauri->>Consent: is_enabled("claude-code")
Tauri->>Frontend: SourceState[]{present, enabled}
Frontend->>User: Show sources list with checkboxes
User->>Frontend: Enable source
Frontend->>Tauri: setSourceEnabled("claude-code", true)
Tauri->>Consent: set_enabled("claude-code", true)
Tauri->>Tauri: spawn_refresh_loop
Tauri->>Probe: is_present (enabled only)
Tauri->>Fetch: fetch usage
Tauri->>Frontend: emit usage-updated
Frontend->>User: Show usage snapshot
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/adapters/src/claude.rs`:
- Around line 27-29: The keychain probe (keychain_present) runs a blocking
std::process::Command::output() directly from the async path (is_present called
by LocalSourceProbe::is_present), which can stall the tokio runtime and has no
timeout; change the probe to execute off the async executor and enforce a hard
timeout: either (preferred) rewrite keychain_present to use
tokio::process::Command (spawn the child, await output) and wrap the await in
tokio::time::timeout with a short bound, killing the tokio::process::Child via
child.kill() on timeout, or (if keeping std::process) call it inside
tokio::task::spawn_blocking and wrap that future in tokio::time::timeout but
ensure you spawn the child so you can kill it if the timeout fires; update the
call site in is_present (claude.rs) to await the new async-safe probe (or await
the spawn_blocking) and preserve the file_present check.
In `@src/routes/`+page.svelte:
- Around line 88-101: toggleSource can set loading=true after setSourceEnabled
even if an asynchronous usage update already populated snapshot via
onUsageUpdated, causing the UI to incorrectly show the loading state; update
toggleSource to only set loading=true when enabling a source if snapshot is
currently null/undefined (i.e., check snapshot before assigning loading=true)
and/or ensure you compare the current source.id/state to avoid overriding a
fresh snapshot (refer to toggleSource and onUsageUpdated to coordinate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c2c5142-211b-4af4-99ca-77f6010a5173
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
crates/adapters/Cargo.tomlcrates/adapters/src/claude.rscrates/adapters/src/consent.rscrates/adapters/src/lib.rscrates/adapters/src/sources.rscrates/core/src/lib.rscrates/core/src/ports.rscrates/core/src/sources.rsdocs/PRD.mddocs/tasks/002-source-discovery-consent.mdsrc-tauri/src/lib.rssrc/lib/usage.tssrc/routes/+page.svelte
| pub fn is_present() -> bool { | ||
| file_present() || keychain_present() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '\b(spawn_blocking|timeout)\b' --type=rust
rg -nP -C3 'is_present\s*\(' --type=rustRepository: ogrodev/MLT
Length of output: 3990
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect claude adapter presence helpers around keychain_present/file_present
rg -n "fn (file_present|keychain_present|is_present)\b" crates/adapters/src/claude.rs
rg -n "keychain_present|security|find-generic-password" crates/adapters/src/claude.rs
wc -l crates/adapters/src/claude.rs
# Print relevant parts without dumping entire file
# (Try to stay within ~200 lines around the match)
for pat in "fn keychain_present" "fn file_present" "fn is_present" "security" "find-generic-password"; do
echo "----- context for $pat -----"
rg -n -C5 "$pat" crates/adapters/src/claude.rs || true
done
# 2) Confirm callers don't add timeout/spawn_blocking wrappers
# Search for probe.is_present callsites + any timeout/spawn_blocking around them in the async call chain.
rg -n "probe\.is_present\(&" -S crates src-tauri || true
rg -n "sources\.probe\.is_present\(&" -S src-tauri crates || true
# 3) Specifically scan sources.rs to see how LocalSourceProbe is used (no wrapper inside it?)
rg -n "impl SourceProbe for LocalSourceProbe|LocalSourceProbe::is_present|timeout|spawn_blocking" crates/adapters/src/sources.rs
sed -n '1,220p' crates/adapters/src/sources.rsRepository: ogrodev/MLT
Length of output: 6499
Presence probe can block tokio runtime and lacks per-probe timeout
crates/adapters/src/claude.rs (keychain_present, ~147-153) runs /usr/bin/security find-generic-password ... via synchronous std::process::Command::output() with no timeout; it’s called directly from the async LocalSourceProbe::is_present (crates/adapters/src/sources.rs) without spawn_blocking/bounded wait. If security stalls, it can block runtime worker threads. Errors are mapped to false (unwrap_or(false)), but hangs aren’t bounded—ADR 0015 requires a per-probe timeout/failure gate for upstream probes. Move the blocking subprocess off the async executor and enforce a hard timeout (and kill the child on timeout).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/adapters/src/claude.rs` around lines 27 - 29, The keychain probe
(keychain_present) runs a blocking std::process::Command::output() directly from
the async path (is_present called by LocalSourceProbe::is_present), which can
stall the tokio runtime and has no timeout; change the probe to execute off the
async executor and enforce a hard timeout: either (preferred) rewrite
keychain_present to use tokio::process::Command (spawn the child, await output)
and wrap the await in tokio::time::timeout with a short bound, killing the
tokio::process::Child via child.kill() on timeout, or (if keeping std::process)
call it inside tokio::task::spawn_blocking and wrap that future in
tokio::time::timeout but ensure you spawn the child so you can kill it if the
timeout fires; update the call site in is_present (claude.rs) to await the new
async-safe probe (or await the spawn_blocking) and preserve the file_present
check.
There was a problem hiding this comment.
Verified, but pushing back on doing this in this PR.
The finding is real in the narrow sense (a blocking subprocess awaited on the async executor, no timeout), but two things matter:
- This specific probe is the low-risk one.
keychain_presentcallssecurity find-generic-passwordwithout-w— it reads the item's attributes, never decrypts the secret, so it does not trigger a Keychain ACL prompt and returns promptly. The variant that can actually block on a user prompt isread_keychain(-w), which is pre-existing and on the consentedloadpath. - The blocking-in-async pattern is codebase-wide and pre-existing, not introduced here:
read_keychain(-w),detect_user_agent(claude --version), andClaudeCredentials::load'sstd::fs::read_to_stringall run synchronously inside async fns. The same refresh loop that calls this probe also callsclaude_usage → load → read_keychain, so wrapping only the new probe inspawn_blocking+ timeout would create a divergent pattern and false assurance while the larger exposure (the fetch path) stays unbounded.
ADR 0015's per-probe timeout / failure-gate is a mandated-but-not-yet-implemented resilience pattern. It should be applied uniformly across the adapter's blocking calls and the usage fetch as a dedicated resilience change, not spot-welded onto this one probe. Deferring it there rather than diverging here.
There was a problem hiding this comment.
@ogrodev, that's a well-reasoned pushback — accepted. The distinction between keychain_present (attributes-only, no ACL prompt) and read_keychain (-w, secret decrypt) is the key point I missed, and the argument for uniform ADR 0015 treatment rather than a divergent spot-fix is sound. Applying spawn_blocking + timeout to only this probe while leaving read_keychain, detect_user_agent, and fs::read_to_string in load() unbounded would create inconsistency without meaningfully reducing the actual exposure.
Would you like me to open a follow-up issue to track the ADR 0015 resilience work (uniform spawn_blocking + bounded timeout across all blocking adapter calls)?
✏️ Learnings added
Learnt from: ogrodev
Repo: ogrodev/MLT PR: 2
File: crates/adapters/src/claude.rs:27-29
Timestamp: 2026-06-01T00:56:24.222Z
Learning: In the `ogrodev/MLT` repository (`crates/adapters/src/claude.rs`), the blocking-in-async pattern (synchronous `std::process::Command` and `std::fs` calls inside async fns) is pre-existing and codebase-wide, covering `read_keychain` (with `-w`), `detect_user_agent`, and `ClaudeCredentials::load`. The `keychain_present` helper intentionally omits `-w` so it reads only Keychain item attributes and never triggers an ACL/user prompt. ADR 0015 mandates a per-probe timeout/failure-gate that must be applied uniformly across all blocking adapter calls as a dedicated resilience change, not spot-fixed on individual probes.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Thanks — I went ahead and opened #3 to track the uniform ADR 0015 pass (spawn_blocking + bounded timeout across read_keychain, detect_user_agent, load's fs read, and this probe), so no need for the bot to file one.
set_source_enabled emits usage-updated from a spawned refresh while its invoke promise resolves independently (Tauri gives no ordering guarantee between the two). If the event populates the snapshot before toggleSource runs its post-await line, flipping loading=true masked the live snapshot until the next 60s poll. Guard with `&& !snapshot` so opt-in only shows the loading state when there's nothing to show yet. Addresses CodeRabbit review on PR #2.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/adapters/src/sources.rs">
<violation number="1" location="crates/adapters/src/sources.rs:22">
P2: `ClaudeCredentials::is_present()` is synchronous (calls `std::process::Command::output()` for the macOS Keychain check) but is invoked directly from the async `SourceProbe::is_present`. This blocks the tokio worker thread if the `security` subprocess stalls. Wrap the call in `tokio::task::spawn_blocking` and add a bounded timeout (e.g., `tokio::time::timeout`) so a hung child process cannot starve the runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| impl SourceProbe for LocalSourceProbe { | ||
| async fn is_present(&self, id: &ProviderId) -> bool { | ||
| match id.as_str() { | ||
| "claude-code" => ClaudeCredentials::is_present(), |
There was a problem hiding this comment.
P2: ClaudeCredentials::is_present() is synchronous (calls std::process::Command::output() for the macOS Keychain check) but is invoked directly from the async SourceProbe::is_present. This blocks the tokio worker thread if the security subprocess stalls. Wrap the call in tokio::task::spawn_blocking and add a bounded timeout (e.g., tokio::time::timeout) so a hung child process cannot starve the runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/adapters/src/sources.rs, line 22:
<comment>`ClaudeCredentials::is_present()` is synchronous (calls `std::process::Command::output()` for the macOS Keychain check) but is invoked directly from the async `SourceProbe::is_present`. This blocks the tokio worker thread if the `security` subprocess stalls. Wrap the call in `tokio::task::spawn_blocking` and add a bounded timeout (e.g., `tokio::time::timeout`) so a hung child process cannot starve the runtime.</comment>
<file context>
@@ -0,0 +1,36 @@
+impl SourceProbe for LocalSourceProbe {
+ async fn is_present(&self, id: &ProviderId) -> bool {
+ match id.as_str() {
+ "claude-code" => ClaudeCredentials::is_present(),
+ _ => false,
+ }
</file context>
There was a problem hiding this comment.
Verified, but deferring (same as CodeRabbit's note on the same probe). It is a real blocking-in-async call, but:
- This probe runs
security find-generic-passwordwithout-w— an attribute lookup, no decrypt — so it does not trigger a Keychain ACL prompt and returns promptly. The prompt-capable-wread isread_keychain, on the consentedloadpath. - The blocking-in-async pattern is pre-existing and shared by
read_keychain(-w),detect_user_agent(claude --version), andload'sstd::fs::read_to_string— all driven by the same refresh loop. Wrapping only this probe inspawn_blocking+ timeout would diverge and give false assurance while the larger exposure stays unbounded.
ADR 0015's per-probe timeout/failure-gate is mandated but not yet implemented; it should land as one uniform resilience pass over the probe and fetch paths, not a spot-fix here. Tracking it there rather than diverging in this consent PR.
There was a problem hiding this comment.
Tracked as #3 — uniform spawn_blocking + bounded timeout across the probe and fetch paths (per ADR 0015), rather than a spot-fix here.
…shness - consent (FileConsentStore): persist the toggle *before* committing it to the in-memory map, rolling back on write failure, so runtime consent never diverges from disk. Fail closed — a source is only treated as opted-in once the choice is durably recorded. Adds a deterministic persist-failure test. - ui: gate the source switch on `present || enabled` instead of `present`, so an opted-in-but-now-absent source can still be turned OFF (revoked); only enabling a never-detected source stays disabled. Adds a matching hint. - ui: re-discover sources when the popover regains focus, so presence reflects logins/logouts that happened since the webview loaded (it was fetched once). Addresses cubic review (P1 + two P2) on PR #2.
1 participant
Implements task 002 — local source discovery + consent screen. Closes the consent half of PRD §4 / [§9].
ADR: 0012 — metadata-only discovery → per-source opt-in.
What changed
sourcesmodule: source descriptors + catalog, the pure consent gate (active = present && enabled), anddiscover_sources/active_sourcesover two new ports. NewSourceProbe(metadata-only presence) andConsentStoreports incore::ports. Stays IO-free (purity gate green).ClaudeCredentials::is_presentstats the creds file / queries the Keychain item without-w, so discovery never reads the token;LocalSourceProbedispatches by id;FileConsentStorepersists opt-in as JSON in the app config dir (consent is not a secret, so it never touches the keychain) and fails closed on a corrupt file.fetch_claude_usageare now consent-gated; newlist_sources/set_source_enabledcommands. Opting a source in kicks an immediate refresh, so it takes effect without a restart. Claude Code now connects through this flow instead of fetching unconditionally.Acceptance criteria
discover_sourcesover the catalogclaude_usage → creds.load) is gated byactive_sources; the probe only stats. Test: a present-but-garbage creds file reads as detected whileparse_credsrejects itset_source_enabledper id; per-source independence + persistence testedrefresh_active; the loop re-reads consent each tickTests / fixtures added
sources: gate logic, discover/active pairing, consent-first short-circuit, honest-note assertion.Verification
make checkgreen: fmt, clippy (-D warnings), core-purity, 26 tests, cargo-deny (advisories/bans/licenses/sources). Pluscargo machete, core line coverage 88% (sources.rs97%, floor 80%), Biome,svelte-check0/0,pnpm build.make qa— confirm with a network/Keychain monitor that nothing is read before opt-in. Default on a fresh install is Claude off until opted in (per ADR 0012).Summary by cubic
Added local source discovery (metadata-only) and per-source opt-in so secrets are only read when a source is present and enabled. Claude Code now connects through this consent gate, meeting PRD §4/§9.
New Features
sourcesmodule with catalog, consent gate (present && enabled), anddiscover_sources/active_sources; newSourceProbeandConsentStoreports.LocalSourceProbe;ClaudeCredentials::is_presentchecks file/Keychain presence without reading the token;FileConsentStorepersists JSON consent using an in-processparking_lotMutex.fetch_claude_usage;list_sourcesandset_source_enabled; opt-in triggers an immediate refresh without a restart.present || enabledso a now-absent source can be revoked; usage view is connect-gated.Bug Fixes
Written for commit 3b8dee3. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Documentation