Enable the SDK's ETag response cache - #374
Conversation
The CLI has run with CacheEnabled: false since the SDK migration (#23), so no request ever revalidated with If-None-Match and every read refetched full bodies the server had already told us hadn't changed. The SDK cache is a revalidation cache: every read still asks the server, conditionally, and only a 304 is answered from disk — so enabling it can never serve stale mail. The cache lives under the CLI's cache directory at hey-cli/http, and logout drops it so cached mail does not outlive the credentials that fetched it. When no cache location resolves, caching stays off. Generated operations start revalidating once the SDK dependency carries basecamp/hey-sdk#140; until then the cache covers the hand-written request paths and is inert elsewhere.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Pull request overview
Enables SDK ETag revalidation caching and removes cached responses during logout.
Changes:
- Configures an XDG-aware HTTP cache.
- Clears cached mail on logout.
- Adds cache configuration and logout tests.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
internal/cmd/sdk.go |
Enables and manages the SDK cache. |
internal/cmd/auth.go |
Clears the cache during logout. |
internal/cmd/sdk_cache_test.go |
Tests cache setup and cleanup. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c944997ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Login over standing credentials — token, cookie, or OAuth, setup's flow included — replaces them and orphans whatever the old ones cached, so the replacement clears the cache without waiting for a logout. A clear that fails no longer vanishes: the warning names the directory left to remove by hand, after the credential change it trailed has already happened.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c79b05e2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The CLI has run with
CacheEnabled: falsesince the SDK migration (#23), so no request ever revalidated withIf-None-Matchand every read refetched full bodies the server had already told us hadn't changed — while HEY serves ETags broadly and answers conditionals with 304s.The SDK cache is a revalidation cache: every read still asks the server, conditionally, and only a 304 is answered from disk — so enabling it can never serve stale mail. Specifics:
hey-cli/http(XDG-aware); when no cache location resolves, caching stays off.Safe to merge independently of the SDK: the cache is inert where ETags aren't wired. Generated operations start revalidating once the SDK dependency carries basecamp/hey-sdk#140; until then the cache covers the hand-written request paths.
Card: https://app.basecamp.com/2914079/buckets/48521764/card_tables/cards/10240004704
TMPDIR=/tmp/t make checkgreen.Summary by cubic
Enables the SDK's ETag response cache so reads revalidate with
If-None-Matchand serve 304s from disk instead of refetching unchanged bodies. Replacing credentials—logout or login over standing credentials—clears the cache, so cached mail never outlives the credentials that fetched it.hey-cli/httpunder the CLI's XDG-aware cache directory; when no cache location resolves, caching stays off.basecamp/hey-sdk#140; until then, the cache covers the hand-written request paths only.Written for commit 1c79b05. Summary will update on new commits.