Repository navigation
Store-backed resume: Keychain for a custom config dir, a scrubbed preserved dir, no batch lost to a cancelled flush - #159
Merged
Merged
Conversation
Store-backed resume runs `security find-generic-password` whenever the caller has no CLAUDE_CONFIG_DIR and no env auth. session_resume_spec.rb stubbed that bridge in five places only, so on a default macOS setup eight ordinary examples reached it through the public resume flow: 11 real `security` invocations per run, a copy of the developer's credentials (minus the refresh token) written to a temp dir, and results that depended on the local Keychain. With CLAUDE_CONFIG_DIR set, the same run made none and the bridge had no coverage at all. session_resume_spec.rb now stubs read_keychain_credentials for every example that does not opt in with `keychain: true`. session_resume_keychain_spec.rb tests the bridge itself, reached through materialize_resume_session with capture_with_timeout replaced by a recorder: the argv and service name, a hit (redacted, 0600), the precedence over ~/.claude/.credentials.json, a missing or empty entry, a timeout, env auth, and a non-macOS host. The host OS and the environment variables the bridge reads are pinned, so the examples also run on Linux CI and in a shell that exports ANTHROPIC_API_KEY. capture_with_timeout gets examples against a real `ruby -e` child: output, output larger than a pipe buffer, a non-zero exit, a child killed and reaped at the deadline, and a command that cannot be spawned. Specs only; no library change. Exploration finding: F-C1 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
On macOS the CLI keeps OAuth credentials in the Keychain. With a custom CLAUDE_CONFIG_DIR it stores them under "Claude Code-credentials-<first 8 hex of SHA-256(config dir)>" and writes no .credentials.json. Store-backed resume consulted the Keychain only for the default config dir, and only under the fixed service name, so with a custom dir the materialized config dir got no credentials: the resumed CLI answered "Not logged in · Please run /login", and the failed attempt was mirrored into the store as a junk turn. copy_auth_files now reads the Keychain for a custom config dir too, deriving the service name from the directory the way the CLI does (the environment value as given, NFC-normalized). It does so only when that directory has no .credentials.json: which source the CLI prefers when both exist is not established, so a file that is there keeps winning, as before. Unchanged: ANTHROPIC_API_KEY / CLAUDE_CODE_OAUTH_TOKEN skip the bridge, the default config dir behaves as before, a non-macOS host never runs `security`, and the seeded copy has no refresh token and mode 0600. A user can notice: a store-backed resume on macOS with a custom config dir and a Keychain login now authenticates. It runs one `security find-generic-password`, which can raise a Keychain access prompt, as the default-dir lookup already could. The specs go through materialize_resume_session with capture_with_timeout stubbed; the expected service names are digests computed outside Ruby. They also pin that the output of a failed lookup is never used. A real-CLI integration example mirrors one turn into an InMemorySessionStore, removes the local transcript and resumes from the store. Exploration finding: B-P4 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
When the transcript mirror dropped batches, teardown keeps the materialized config dir (it holds the only copy of the dropped turns) and removes the credential copies from it. That scrub was a deny-list of four names: .credentials.json, .claude.json, settings.json and cowork_settings.json. The CLI also writes into the directory. At startup it saves the seeded .claude.json, which can hold MCP header secrets, as backups/.claude.json.backup.<ts>, and that copy stayed behind next to cache/ and sessions/. preserve_transcripts is now an allow-list: projects/ stays, every other entry of the temp dir is deleted, whatever its name. A symlink is removed, not followed. A user can notice: a preserved directory contains nothing but projects/. The warning that names it is unchanged. Exploration finding: B-P8 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
TranscriptMirrorBatcher#drain took the pending buffer first and then waited for the lock. A #flush cancelled while it was queued behind an in-flight append (its task stopped, a timeout around the caller) took that batch down with it: the entries never reached the store, a later #close could not deliver them, and no MirrorErrorMessage was produced. The loss was only counted in #batches_dropped?. #drain now detaches once it holds the lock. A drain cancelled while it waits has taken nothing, so its frames stay buffered for the next drain. Ordering is unchanged: the lock is FIFO and each holder takes everything pending at that moment. A batch whose drain is cancelled inside the append is still lost and still counted. A user can notice two things. Entries buffered behind a cancelled flush now reach the store. And a frame that arrives while #close is waiting behind an in-flight append goes out with #close's own batch instead of staying buffered and being reported as dropped, so fewer teardowns preserve the materialized resume dir. That second case was pinned the other way round by one example in transcript_mirror_batcher_spec.rb; its expectation now says the frame is delivered. The case that remains a drop, a frame arriving after #close detached, gets its own example next to the new regression specs, which force the interleaving with gates and no clock. Upstream: Python PR #1289. Exploration finding: U-P17 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
Inside a reactor the transcript mirror's retry backoff called Async::Task#sleep. async deprecated that method in favor of Kernel#sleep, and current versions warn about it when warnings are on: under `ruby -w` every retried append printed "`Async::Task#sleep` is deprecated, use `Kernel#sleep` instead." Kernel#sleep is scheduler-aware (it parks the fiber under a reactor and blocks the thread without one), so the backoff calls it in both cases. Behavior is otherwise unchanged; on the async floor (2.10.0) Task#sleep already was a plain call to Kernel#sleep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
ClaudeAgentOptions fills load_timeout_ms (60_000) and env ({}) in its
constructor only. Both attributes are nilable, and clearing one
afterwards (options.dup_with(env: nil), options.load_timeout_ms = nil)
leaves nil in place. A store-backed resume then raised NoMethodError
from inside the SDK: "undefined method '/' for nil" while computing the
store-call timeout, or "undefined method 'merge' for nil" while pointing
the options at the materialized config dir.
Both use sites now read nil as the constructor default: 60 s per store
call, and no environment overrides. The readers still return nil, and an
explicit load_timeout_ms of 0 is still honored.
Seeding the temp config dir was already nil-safe for env (it resolves
the child's environment through the parent's); the new spec pins that.
Exploration finding: C-P6
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
The Postgres reference adapter kept transcript entries in a jsonb
column. jsonb rejects the JSON escape for U+0000, and transcripts carry
it: a NUL character in binary tool output is serialized that way. One
such entry made the whole multi-row INSERT fail with
PG::UntranslatableCharacter, so the mirror dropped every entry of that
batch, the clean ones included, and a later store-backed resume replayed
a transcript with that turn missing.
The adapter now declares the column as json and casts to json on
insert. json keeps the text as given; the adapter only reads whole
entries, so it needs nothing jsonb offers.
create_schema is CREATE TABLE IF NOT EXISTS, so a table created by an
earlier copy of the adapter keeps its jsonb column and keeps rejecting
those entries. The adapter's header and README now say so and give the
migration:
ALTER TABLE claude_session_store
ALTER COLUMN entry TYPE json USING entry::json;
Example code only; the gem is unaffected. A new live-gated spec (it
needs the pg gem and SESSION_STORE_POSTGRES_URL, like the existing one)
covers a batch with a NUL entry, and an old jsonb table before and after
that migration.
Exploration finding: B-P7
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
docs/sessions.md lists what a store-backed resume does not see from the caller's config dir (user CLAUDE.md, agents/, skills/, plugins/) and says everything else is invisible as well. The project's auto-memory was not named. It lives under projects/<key>/memory/ in the config dir, the materialized temp config dir has no such directory, and the temp dir is deleted at disconnect, so such a session neither loads existing memory nor keeps what it writes. One sentence added to that paragraph. Documentation only: nothing is copied or symlinked into the temp dir. Exploration finding: U-P16 (refuted as a defect; the docs gap is real) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
The default stub of SessionResume.read_keychain_credentials lived in session_resume_spec.rb, so it covered that file only. Three examples in other files materialize a store-backed resume without stubbing the bridge: callback_wrapper_spec.rb:899, session_store_inline_spec.rb:343 and session_import_spec.rb:142. Since the bridge covers a custom CLAUDE_CONFIG_DIR, they ran `security find-generic-password` on macOS whatever the developer's setup, and with a custom config dir that has a Keychain login two of them read that real entry in the middle of the suite. spec/support/keychain_bridge_stub.rb (loaded by spec_helper.rb's support glob, so by the Rails specs too) now stubs the bridge to nil before every example that does not carry `keychain: true`, on itself or on its group. The file-level stub in session_resume_spec.rb and the one in session_resume_nil_options_spec.rb are redundant and gone. session_resume_keychain_spec.rb keeps its `keychain: true` tag and gains two examples that give the tag up again, to pin the default where the bridge would otherwise run (macOS, no env auth; default and custom config dir). Specs only. The full unit suite under an interceptor that records every `security` invocation now reports none, with and without CLAUDE_CONFIG_DIR in the environment (it was 3 in both before). Exploration finding: F-C1 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
When the transcript mirror dropped batches, preserve_transcripts deletes every entry of the materialized config dir except projects/. It reached that directory by path: if the path had been replaced before teardown, by a symlink or another directory at the same place (anything that knows CLAUDE_CONFIG_DIR can do that), File.directory? and Dir.children followed it and the loop deleted the target's entries instead. A probe with the root symlinked to an unrelated directory lost unrelated-dir/innocent/keep.txt; the four-name scrub before this branch never touched it. materialize_resume_session now records the directory's identity (File.lstat device and inode) right after mkdtemp and hands it to MaterializedResume, which takes it itself when it is built without one. preserve_transcripts checks that the path still leads to that directory (a real directory, not a symlink, same device and inode) before it lists the entries and again before each deletion. If the check fails it deletes nothing and the preservation warning says so. A symlink inside the directory is still unlinked, never followed. A user can notice: when the directory was replaced or removed, the warning adds "... is no longer the directory the SDK created (it was replaced or removed), so the SDK deleted nothing in it". docs/sessions.md says so too. Exploration finding: B-P8 (codex review round 1, B1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
preserve_transcripts deleted each entry with FileUtils.rm_rf, which swallows errors. An entry it could not remove therefore survived silently: with an owner-owned read-only (0500) backups/ directory, which can hold the seeded .claude.json with MCP header secrets, backups/ stayed in the preserved directory and the warning read as if the scrub had worked. Each entry is now removed with FileUtils.remove_entry, which raises instead of skipping and lstat's every entry, so a symlink is still unlinked and never followed. When that fails, the directories in the entry's subtree that this user owns are made accessible (0700), without following or changing symlinks, and the removal is tried once more. Afterwards preserve_transcripts lists the directory again: if anything besides projects/ is left, the preservation warning says that scrubbing failed and names what is left, with the first error. A user can notice: a read-only directory left by the CLI is now removed, and a warning that would have read as success now ends with "Scrubbing failed: could not remove <names> (<error>) - delete them yourself; they can hold copies of your credentials and settings." docs/sessions.md says so. Exploration finding: B-P8 (codex review round 1, B2) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
preserve_transcripts checks that its path still leads to the directory the SDK created before it lists the entries, and again before each deletion. Nothing pinned the second check: with it removed, a directory replaced in the middle of the scrub got the rest of the listed names deleted from the replacement, and every spec stayed green. The new example replaces the directory with a symlink right after the first deletion, gives the replacement an entry for every listed name, and expects all of them to survive and the warning to say that nothing was deleted in it. Specs only. Exploration finding: B-P8 (codex review round 1, B1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
The backoff example set MIRROR_APPEND_BACKOFF_S to [0, 0]: it proved the
Async::Task#sleep deprecation warning was gone, but it stayed green with
sleep_backoff turned into sleep(0), i.e. a retry loop with no backoff at
all against an overloaded store.
A second example configures [0.25, 0.5], lets the store fail twice, and
intercepts Kernel#sleep on the batcher (so nothing actually waits): the
two intervals must arrive, in order. It fails for sleep(0)
("expected: [0.25, 0.5] got: [0, 0]"), for the Async::Task#sleep code
this branch replaced, and when the backoff call is removed.
Specs only.
Exploration finding: U-P17 (codex review round 1, B3)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
Two of the new preserve_transcripts examples replace the temp dir with a symlink to another scratch directory, and the inner after hook removes that directory first. The outer after hook then asked File.directory?, which is false for a symlink whose target is gone, so every run left two dangling claude-resume-* symlinks in $TMPDIR (they pointed at deleted scratch dirs and held nothing). The hook now also removes the path when it is a symlink. Three runs of the file add no entries to $TMPDIR. Specs only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
preserve_transcripts checked that its path still led to the directory the SDK created and then deleted by path. Two races were left: - the root could become a symlink after that check and before FileUtils.remove_entry(root/backups) ran, as an intermediate path component, so a file outside was deleted while the warning said nothing had been; - the EACCES retry lstat'ed a child and then chmodded it by path, so a child swapped for a symlink in between got the directory it pointed at chmodded (0555 -> 0700). The CLI's tools can write into the temp config dir, sandboxed ones too, so the SDK's own cleanup must not become a way to reach outside it. A path that was checked is no longer resolved again: 1. The directory is moved, in one rename, into a fresh private (0700) directory created next to it. A rename moves a symlink itself, never its target. A read-only directory cannot be moved, so it is made writable first, through a no-follow handle that has to be the directory recorded at creation (device and inode). 2. What was moved is validated: a real directory with the recorded device and inode. Otherwise, or if it could not be moved, nothing is deleted and the warning says scrubbing was skipped and why. 3. SessionResume::Scrubber deletes the entries. It renames each one into a private trash directory before it looks at it, so a swap after the look has nothing to redirect; a symlink is unlinked, a directory is emptied the same way one level at a time. Permissions are repaired only with fchmod on a handle opened O_NOFOLLOW, after the handle proved to be the directory lstat had just seen and one this user owns, and the path is compared again afterwards. If it changed, the scrub stops and the warning says so. A user can notice: the preserved transcript is no longer at the temp dir's path but under claude-preserved-resume-*/<temp dir name>/projects, next to where it was; the warning names that path and the directory to remove afterwards, and config_dir returns it. When the scrub is cut short by a cancellation, a warning still says where the transcript is. docs/sessions.md describes the move. The specs force each interleaving by wrapping the call that comes next (no thread, no clock): the root swapped right before the move, a symlink planted at the old path, an entry swapped right before it is taken, backups/ swapped between the lstat and the chmod and right after the chmod. Directories outside keep their files and their mode. Exploration finding: B-P8 (codex review round 2, B1 and B4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
A teardown can run twice. Client#disconnect lets go of its
MaterializedResume only once preserve_transcripts returned; when that
was cut short by something that is not a StandardError (a cancellation
while the warning is written, a signal), a later disconnect finds the
query handler gone. Nothing then remembers that the mirror dropped
batches, so it calls #cleanup on the same object, which deleted the
directory the warning had just pointed at — the only copy of the
dropped turns.
MaterializedResume#cleanup now does nothing once preserve_transcripts
was called, however that went: finished, skipped, or interrupted before
the directory was even moved. Copies that were not scrubbed yet stay
where the warning says the transcript is; losing the transcript is the
worse outcome. A cleanup that is not preceded by preserve_transcripts
removes the temp dir as before.
Three examples: #cleanup after a finished preservation, after a scrub
interrupted by a non-StandardError, and after an interruption before
the move. Each of the first two lost the transcript before this change
(`got: {}`); the third lost the whole directory.
Exploration finding: B-P8 (round 3 follow-up, ruling r4 item 1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
Scrubber#remove called itself once per directory level. Teardown runs on the reactor, where a fiber's stack is small: a tree a few hundred levels deep (one `mkdir -p` inside the temp config dir) ended the scrub with SystemStackError. That is not a StandardError, so it left preserve_transcripts and the teardown, replaced an exception in flight, and the entries not reached yet stayed where they were. remove now keeps a list of what is still to be removed. Emptying a directory already moved its entries into the trash, which is flat, so the list holds trash paths whatever depth they came from; nothing recurses and no path grows. What it does to each entry is unchanged: take it out, look at it, unlink it or — for a directory — repair it if needed, take its entries out (up to three passes) and rmdir it. A failure anywhere under an entry still ends that entry's removal and is reported under its name. Specs: a chain of 1000 directories scrubbed inside the reactor (SystemStackError before this change). The chain is built in pieces of 150 levels, and whatever a failing run leaves is flattened before the usual teardown, which recurses and would hit the path length limit. Two examples pin behavior the rewrite could have dropped silently: an entry that arrives in a directory while it is being emptied is taken out too, and a directory that could be moved but not searched (0600) is made accessible before it is emptied. Exploration finding: B-P8 (round 3 follow-up, ruling r4 item 2) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
The examples that force an interleaving wrap a File or Dir method and make their swap when a given call comes. Stubs are still in place in an `after` hook, and the teardown makes the same calls. When an example failed before its wrapper had fired, the wrapper fired during the teardown instead: it swapped backups/ for a symlink under the code that was unlocking the tree, a read-only directory stayed read-only, the removal failed and the scratch directory was left in $TMPDIR. It took a broken scrub to get there (seen under a mutation run: one `preserve-spec-*` directory left per run), so no green run was affected. The hook now stubs those methods back to the originals first. With it, a run where 26 of the 28 examples fail (the spec against the round-2 library) and each of 33 mutation runs leave nothing behind. Spec only; no library change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
Cut short by something that is not a StandardError (a cancellation, a
signal), preserve_transcripts warned where the session transcript is and
nothing else. Since #cleanup leaves the directory alone once
preserve_transcripts was called, that warning is all the user gets: it
has to say that copies which were not removed yet — credentials among
them — may be left, and which directory to remove once the transcript
is imported.
It now does, and names the directory: the private one the temp dir was
moved into when the move had happened (the trash is in there too), the
temp dir itself when it had not. Never the temp dir's parent, which
would be the system's temp directory. Whether the move happened is read
from config_dir, which changes exactly then.
... Scrubbing was interrupted; the session transcript is under
<dir>/projects. Copies of your credentials and settings that were
not removed yet may be left under <to remove> — import the
transcript into your session store, then remove <to remove>.
Two examples replace the one that only checked the transcript's
location: interrupted after the move (names the private directory, not
the directory in it) and before the move (names the temp dir, not its
parent).
Exploration finding: B-P8 (round 3b follow-up)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
A directory moved into the trash can still receive an entry from something that holds it open. The scrub allowed for that by listing the directory again after taking its entries out — but it stopped at the first empty listing and then called rmdir once. An entry that arrived right after an empty listing was therefore never looked for: rmdir failed with ENOTEMPTY, the entry was reported as not removed and the trash stayed behind. Which case a run hits depends on the order the filesystem lists entries in. The spec that covers a late arrival wrote its file after the first listing of a trash directory: on APFS that was a directory with entries, on ext4 the empty sessions/, so the example passed on macOS and failed on Linux CI. rmdir is now what decides. Each pass lists the directory, takes its entries out and tries rmdir; ENOTEMPTY (EEXIST on some systems) gives it another pass, EMPTYING_PASSES in all, and the last refusal is reported as before. A directory nothing writes into costs one listing less than it did. Specs: the examples that wrap Dir.children now run with the names sorted, in reverse order and shuffled (with the run's seed), so their result no longer depends on the machine. A late arrival is checked after a listing that had entries and after one that was empty; rmdir answering EEXIST is treated the same; a directory that keeps receiving entries is given up on after exactly EMPTYING_PASSES passes and named in the warning. The two swap examples that used to force backups/ to the front now read off the listing what the scrub must have left untouched. Exploration finding: B-P8 (Linux CI failure on PR #159) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018TunHCEUfBYcKbMTdjR8Xw
ya-luotao
added a commit
that referenced
this pull request
Oct 2, 2026
client_receive_after_stream_end_spec.rb defined a helper named receive in its example group. That hides RSpec's own receive matcher for every example in the group, so any before hook that stubs with allow(...).to receive(...) calls the helper instead — as a suite-wide stub added by the store-backed resume PR (#159) does, which made all eight examples fail with "wrong number of arguments" once both PRs are in. The helper is receive_within_deadline now; what it does is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CNgMt2RjeBLyRP3CWFWNy2
The method was one line over Metrics/MethodLength (30) on this branch and carried a disable for it. The sessions PR (#157) shortens the same method by two lines, so once both are merged the disable is redundant and RuboCop fails on Lint/RedundantCopDisableDirective. The two early returns are one condition now, which brings the method under the limit on its own and lets the disable go, with or without #157. Also: the directory identity is read inside the begin block, so a failure while reading it removes the new temp dir like any other failure after mkdtemp. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CNgMt2RjeBLyRP3CWFWNy2
4 tasks done
Collaborator
Author
|
@codex review |
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. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
4 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Fixes around resuming a session from a
SessionStore, from the October exploration report (findingsF-C1,B-P4,B-P8,U-P17,C-P6,B-P7, and the docs note left byU-P16). Every fix has a spec that failed before it.CLAUDE_CONFIG_DIR(B-P4). There the CLI keeps itsOAuth credentials in the Keychain under
Claude Code-credentials-<first 8 hex of SHA-256(config dir)>and writes no.credentials.json. The SDK bridged only the default entry, so the resumed CLI said "Not logged in" and the failedattempt was mirrored into the store. The bridge now covers a custom config dir, only when that directory has no
.credentials.json(the file keeps winning). Env auth still skips it; nothing changes off macOS.projects/(B-P8). After a mirror failure the SDK keeps thematerialized dir and used to delete four known files, leaving the CLI's
backups/.claude.json.backup.<ts>— a fullcopy of the seeded
.claude.json, MCP header secrets included. The cleanup is an allow-list now, and it cannot beturned against anything outside the SDK's own directory: the CLI's tools can write into that directory, so the SDK
first moves it, in one rename, into a fresh private (0700) directory next to it and checks that what it moved is
the directory it created (device and inode recorded at
mkdtemp); otherwise it deletes nothing and says why.Entries are renamed into a private trash directory before they are looked at, a symlink is unlinked and never
followed, permissions are repaired only through a no-follow handle whose identity is checked before and after, and
the warning names whatever could not be removed. The preserved transcript therefore lives at a new path
(
claude-preserved-resume-*/<temp dir name>/projects), which the warning names. A teardown that runs again afteran interrupted one no longer deletes what was preserved, the scrub does not recurse (a deep tree cannot exhaust a
fiber's stack), and when preservation is cut short the warning says that copies may be left and which directory to
remove once the transcript is imported.
U-P17, Python#1289). The batcher detaches its buffer inside the lock instead of before queueing on it. One existing example
asserted the old outcome for a frame that arrives while
#closeis waiting (kept buffered, reported dropped); it isdelivered now, and the expectation says so. The retry backoff uses
Kernel#sleep(noAsync::Task#sleepdeprecation warning under
ruby -w).load_timeout_ms/envset back tonilno longer raiseNoMethodErrorin a store-backed resume (C-P6).json, notjsonb(B-P7, example code):jsonbrejects the escape fora NUL character, which failed the whole
INSERTand made the mirror drop the batch. The README gives the migrationfor existing tables (
ALTER TABLE … ALTER COLUMN entry TYPE json USING entry::json).F-C1). On a default macOS setup elevensecurity find-generic-passwordcalls per run came out of ordinary resume examples. A support file(
spec/support/keychain_bridge_stub.rb) stubs the bridge before every example not taggedkeychain: true, and thebridge has its own examples below a stubbed
capture_with_timeout.docs/sessions.mdnames the project's auto-memory among the things a store-backed resume does not see, and the newlocation of a preserved transcript.
Verification:
bundle exec rspecon the lock, latest and floor bundles, each with a Keychain interceptor that failsany
securitycall (0 attempts); RuboCop;rake rbs:validate rbs:test; the Rails specs; a real-CLI handshake (nomodel turn) with preservation forced. Three rounds of independent adversarial review, including gated race tests
(the root swapped for a symlink right before removal, a child swapped between
lstatandchmod) and mutation runsof the cleanup (each run in its own scratch
TMPDIR).Not in this PR
B-P12— CLI exit-time entries not mirrored forClient(a design task).B-S1), slicing drains (B-S3), sweeping orphaned temp dirs (B-S6); new publicAPI or warnings.
MaterializedResume#cleanupon the normal (non-preserving) path still recurses throughFileUtils.remove_entry(pre-existing; a very deep tree can raise
SystemStackErrorin a fiber). The work list the scrub now uses wouldfix it.
F-C1: examples that materialize a resume without stubbing still read the default configdir's file sources (
.credentials.json,.claude.json,settings.json) whenCLAUDE_CONFIG_DIRis unset — atest-hygiene follow-up.
CHANGELOG.md: this PR's entries are in the batch's combined CHANGELOG PR (kept separate so the fix PRs do notconflict with each other).
Checklist
bundle exec rakepasses (specs + RuboCop)## [Unreleased]— in the batch's combined CHANGELOG PRdocs/sessions.md,examples/session_stores/README.md); no public signature changedspec/integration/store_backed_resume_integration_spec.rbis new and has not been run against a real model:please run
RUN_INTEGRATION=1 bundle exec rspec spec/integration/store_backed_resume_integration_spec.rboncebefore merging (the CLI wire protocol itself is unchanged)
🤖 Generated with Claude Code
https://claude.ai/code/session_01CNgMt2RjeBLyRP3CWFWNy2