Skip to content

fix(claude): re-read the Keychain once the remembered token expires - #168

Merged
jaywedgeworth22 merged 1 commit into
mainfrom
mm/claude-refresh-rereads-keychain
Oct 5, 2026
Merged

jaywedgeworth22 merged 1 commit into
mainfrom
mm/claude-refresh-rereads-keychain

Conversation

@jaywedgeworth22

@jaywedgeworth22 jaywedgeworth22 commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

PR #156 closed the consent loop by remembering Claude Code's Keychain payload in process memory — correct goal, but resolveSilently then returned that memory whenever it was non-nil:

Summary

Fixes a stale-credential bug in the Claude credential source where the app kept answering from a credential it had read into memory at launch, even after Claude Code had renewed its login and rewritten the same Keychain item. The result was a UI row pinned to "login idle" while Claude Code was actually signed in.

Problem

Memory of a Keychain read is a permission fact, not a freshness fact. The previous behavior treated a remembered payload as the permanent answer for the item. Because Claude Code rewrites that same Keychain item whenever it renews its own login, the bytes captured at launch go stale within the hour, and the app never re-read them.

Changes

Serve memory only while it is fresh (Sources/QuotaCore/ClaudeCredentialSource.swift)

  • A remembered credential is now returned from memory only when its own access token has not expired.
  • If the remembered payload is expired, or nothing is remembered, the source performs a silent read through /usr/bin/security. Once "Always Allow" has been granted, this read succeeds without a prompt and lets the app pick up a token Claude Code renewed on its behalf.

New freshness check

  • Added rememberedGrantIsFresh(_:now:), which requires an unexpired access token. This is intentionally stricter than the existing rememberedGrantStillUsable, which also accepts an expired-but-renewable record. The looser rule is appropriate for deciding whether to keep trusting the grant; it is wrong for deciding whether to keep sending the bytes, since an expired token returns 401 regardless of renewability.

Preserve the consent-loop fix

  • If the silent re-read yields nothing but a grant is already remembered, the source still returns the remembered payload rather than .unauthorized. A read failure on a loaded Mac is not a permission problem, and reporting .unauthorized there would re-arm the consent prompt that the earlier fix closed.
  • Only when nothing is remembered and nothing is readable does the source return .unauthorized, which correctly represents a fresh install that has never been granted access.

Tests

  • Updated the expired-remembered-grant test: an expired payload must now be re-read, and a failed re-read must still not produce a consent prompt.
  • Added an end-to-end regression test: launch reads an expired payload, Claude Code renews and rewrites the item, and the next refresh returns the new bytes, transitions the row to connected, and is then served from memory on the following cycle.
  • Added a test asserting rememberedGrantIsFresh is true only for unexpired tokens and is stricter than rememberedGrantStillUsable for a renewable-but-expired record.

Review notes (medium, unresolved)

  • Once a remembered token goes stale, the refresh loop can spawn a blocking /usr/bin/security child on every tick with no backoff, since neither a failure nor a repeat of the same expired payload clears the stale state.
  • Stale remembered grants therefore cause repeated security child-process spawns in the refresh loop, as failed re-reads are neither throttled nor memoized.

The Claude row could sit on "login idle" indefinitely while Claude Code was
signed in and working.  PR #156 fixed the consent loop by remembering the
Keychain payload in process memory, but `resolveSilently` returned that
memory whenever it was non-nil, so the item was never re-read for the life
of the process.  Claude Code rewrites that same item every time it renews
its own login, so the copy captured at launch went stale within the hour and
the frozen, expired token kept answering instead.

That also killed the self-healing path: after `renewClaudeLogin` ran and
Claude Code refreshed its item, the reader asked for the credential again
and got the same stale bytes back, so the renewal could never take effect.

Memory is a permission fact, not a freshness fact.  It records that macOS
already granted `/usr/bin/security` this item, which is what stops the
consent row re-arming on every launch.  So keep serving memory while its
own access token is unexpired, and re-read once it is not.  The re-read is
the same silent child, so nothing new prompts, and a denied or slow read
still reports what we already hold rather than `.unauthorized`, which is
the consent loop #156 closed.

Verified against the live Keychain on this Mac: the item holds a valid
`max` token expiring 2026-10-06T07:44Z, and with this change the same
silent read resolves to `.connected` instead of `.idle`.  The unified log
had shown only "silent attributes status 0" every five minutes since launch,
with zero "silent security CLI" lines, which is the short circuit itself.

331 tests pass, including a regression test that walks the real sequence —
stale payload remembered, Claude Code renews underneath it, next refresh
must return the new bytes.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@jaywedgeworth22
jaywedgeworth22 merged commit c39639c into main Oct 5, 2026
4 checks passed
@kody-ai

kody-ai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

​

@kody-ai

kody-ai Bot commented Oct 5, 2026

Copy link
Copy Markdown

kody code-review Kody Rules medium

Add a second space after usage. because the agent-written PR description must use two spaces between sentences.

Kody rule violation: Use two spaces between sentences in every human-facing string and agent-written paragraph

@kody-ai

kody-ai Bot commented Oct 5, 2026

Copy link
Copy Markdown

kody code-review Business Logic medium

🤔 Insufficient Task Context

I found a task linked to this PR, but it only contains minimal information (title only, no description or acceptance criteria). To perform a meaningful business rules validation, I need more details.

🔍 What I need to validate:

  • Business requirements and acceptance criteria
  • Expected behavior and business rules
  • Edge cases and constraints to consider

💡 How to improve the task context:

  • Add a description to the linked ticket
  • Include acceptance criteria or business rules
  • Describe the expected behavior after the change

⚠️ Important:

A task title alone is not sufficient to determine whether the implementation is correct or complete.


💡 This validation runs automatically only on the first review of a pull request. To run it again, comment @kody -v business-logic.

// "login idle" while Claude Code sat right there signed in — the
// Keychain item held a fresh token the app simply never re-read.
// So: serve memory only while its own access token is unexpired.
let remembered = rememberedCredential()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Performance medium

The silent re-read has no throttle or failure memo, so once the remembered payload is stale — the normal state whenever Claude Code is idle or not running — every provider refresh tick forks a /usr/bin/security child on the serial com.jays.usage-monitor.claude-keychain queue while holding the activeRead semaphore, forever. neither outcome lets the state recover: on .denied/.timedOut the stale memory is preserved by if let remembered { return .authorized(remembered) }, and on .found the same expired bytes are re-memorialized by remember(data), so rememberedGrantIsFresh is still false on the next tick; each spawn blocks that queue up to cliAttemptDeadline (12s) and makes a concurrent boundedAccess fail semaphore.wait(timeout: .now()) in access(), which LocalQuotaReader maps to the "Claude quota is temporarily unavailable." issue. Before this PR a remembered grant short-circuited after one child per process lifetime. memoize the last re-read attempt (timestamp) and only fork again after a minimum interval, or skip the re-read unless the remembered record is renewable/expiring soon.

// Re-read at most once per interval: a stale remembered payload
            // never becomes fresh on its own, so an unthrottled retry forks
            // /usr/bin/security on every refresh tick and blocks the serial
            // worker queue.
            if lastSilentReadAt.map({ Date().timeIntervalSince($0) < silentReadRetryInterval }) ?? false {
                if let remembered { return .authorized(remembered) }
                return .temporarilyUnavailable
            }
            lastSilentReadAt = Date()
            let outcome = probe.readViaSecurityCLI(attemptDeadline, { isAbandoned() })
            log.notice("claude keychain silent security CLI: \(outcome.logLabel, privacy: .public)")
            if case .found(let data) = outcome {
                remember(data)
                return .authorized(data)
            }
            if let remembered { return .authorized(remembered) }
Prompt for LLM

File Sources/QuotaCore/ClaudeCredentialSource.swift:

Line 309:

The silent re-read has no throttle or failure memo, so once the remembered payload is stale — the normal state whenever Claude Code is idle or not running — every provider refresh tick forks a `/usr/bin/security` child on the serial `com.jays.usage-monitor.claude-keychain` queue while holding the `activeRead` semaphore, forever. neither outcome lets the state recover: on `.denied`/`.timedOut` the stale memory is preserved by `if let remembered { return .authorized(remembered) }`, and on `.found` the same expired bytes are re-memorialized by `remember(data)`, so `rememberedGrantIsFresh` is still false on the next tick; each spawn blocks that queue up to `cliAttemptDeadline` (12s) and makes a concurrent `boundedAccess` fail `semaphore.wait(timeout: .now())` in `access()`, which LocalQuotaReader maps to the "Claude quota is temporarily unavailable." issue. Before this PR a remembered grant short-circuited after one child per process lifetime. memoize the last re-read attempt (timestamp) and only fork again after a minimum interval, or skip the re-read unless the remembered record is renewable/expiring soon.

Suggested Code:

// Re-read at most once per interval: a stale remembered payload
            // never becomes fresh on its own, so an unthrottled retry forks
            // /usr/bin/security on every refresh tick and blocks the serial
            // worker queue.
            if lastSilentReadAt.map({ Date().timeIntervalSince($0) < silentReadRetryInterval }) ?? false {
                if let remembered { return .authorized(remembered) }
                return .temporarilyUnavailable
            }
            lastSilentReadAt = Date()
            let outcome = probe.readViaSecurityCLI(attemptDeadline, { isAbandoned() })
            log.notice("claude keychain silent security CLI: \(outcome.logLabel, privacy: .public)")
            if case .found(let data) = outcome {
                remember(data)
                return .authorized(data)
            }
            if let remembered { return .authorized(remembered) }

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

// already granted. Return .authorized(data) so LocalQuotaReader
// and ClaudeLoginState report it as signedOut/idle, not
// needsPermission.
// Memory is a permission fact, not a freshness fact. It records

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Performance medium

Throttle or memoize the failed security re-read in resolveSilently so stale remembered grants do not spawn a security child process on every refresh cycle while the activeRead semaphore is held.

Throttle re-read attempts or memoize the failure to avoid spawning the security CLI on every cycle for stale remembered grants.
Prompt for LLM

File Sources/QuotaCore/ClaudeCredentialSource.swift:

Line 299:

Throttle or memoize the failed security re-read in resolveSilently so stale remembered grants do not spawn a security child process on every refresh cycle while the activeRead semaphore is held.

Suggested Code:

Throttle re-read attempts or memoize the failure to avoid spawning the security CLI on every cycle for stale remembered grants.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

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