Skip to content

[medium] fix(install): keep unreadable Claude settings instead of overwriting them - #627

Merged
sirmalloc merged 1 commit into
sirmalloc:mainfrom
elhoim:fix/install-keep-unreadable-settings
Oct 9, 2026
Merged

sirmalloc merged 1 commit into
sirmalloc:mainfrom
elhoim:fix/install-keep-unreadable-settings

Conversation

@elhoim

@elhoim elhoim commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

BLUF

  • Priority: medium.
  • If ~/.claude/settings.json exists but can't be read or parsed (a trailing comma, EACCES/EBUSY, or a read that races Claude Code's own write), install replaced it with a file holding only statusLine. Permissions, hooks, env, model and every other key were lost.
  • Install now stops with an error that names the file and the reason, and leaves the file untouched. A missing file still installs normally, starting from {}.
  • The TUI now shows that error in an "Install Failed" notice. Before, it showed a generic 2-second "✗ Install failed" flash.

Details

Failure scenario. installStatusLine caught any error from loadClaudeSettings, logged a warning, set settings = {} and saved. The backups did not save the user:

  • saveClaudeSettings writes .bak, but the widget-hook sync (syncWidgetHooks) runs right after and loads and saves again, so .bak ends up holding the stripped file.
  • Only .orig kept the original, and the next install overwrites it.

The TUI also calls console.error while Ink is rendering, so the warning was easy to miss.

Fix.

  • loadClaudeSettings already returns {} when the file doesn't exist. So on this path, any error it throws means the file exists but can't be used. Install now rethrows it as an Error that names the path and the underlying reason, keeping the original error as cause.
  • The .orig backup now runs after a successful read. An aborted install therefore can't replace an earlier good .orig with the broken file.
  • This matches uninstallStatusLine, which already refuses to change unreadable settings.
  • App.tsx: the install catch now uses the existing flowNotice pattern (the same one "Export Failed" uses), so the user sees the message and returns to the install menu.

Test. The existing test installStatusLine should warn and recover when existing settings are invalid checked for the old behaviour. It is replaced in place by installStatusLine should abort without touching existing settings that cannot be parsed. The new test checks that install rejects with the settings path in the message, that the file is byte-for-byte unchanged, and that no .bak or .orig is written. The missing-file case is already covered by the buildCommand via installStatusLine tests, which start without a settings file.

Checks

  • Red first: on unpatched main with only the test changed, bun test src/utils/__tests__/claude-settings.test.ts gave 93 pass / 1 fail (the promise resolved instead of rejecting). After the fix: 94 pass / 0 fail.
  • bun run lint (tsc + eslint, --max-warnings=0): clean.
  • bun run build: succeeds, and dist/ccstatusline.js is produced.
  • bun test (full suite): 2317 pass / 41 fail. Every failure is a timing-sensitive test outside this change: fetchUsageData error handling (5 s timeouts), custom command capture, and several Ink TUI component tests. The machine running them was under heavy load (load average around 30–40). Running the same 10 test files on unmodified main under the same load gave 54 failures in the same tests, so they come from the environment and not from this change. No failure touches install or claude-settings.

🤖 Generated with Claude Code

…them

installStatusLine caught any error from loadClaudeSettings (a JSON parse
failure such as a trailing comma, EACCES/EBUSY, or a read racing Claude
Code's own write), printed a warning, fell back to {} and then saved a
settings.json containing only statusLine. Permissions, hooks, env, model
and every other key were lost: the .bak written by saveClaudeSettings is
immediately overwritten by the widget-hook sync, and the .orig backup is
overwritten by the next install.

loadClaudeSettings already returns {} when the file does not exist, so
any error it throws means the file exists but cannot be read or parsed.
In that case install now aborts with an error naming the file and the
reason, leaving it (and any previous .orig) untouched; the .orig backup
is taken only after a successful read. This matches uninstallStatusLine,
which already refuses to modify unreadable settings. The TUI shows the
error text in an "Install Failed" notice instead of a 2-second flash.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sirmalloc
sirmalloc merged commit 475e401 into sirmalloc:main Oct 9, 2026
3 checks passed
@elhoim
elhoim deleted the fix/install-keep-unreadable-settings branch October 10, 2026 22:33
axisrow added a commit to axisrow/ccstatusline that referenced this pull request Oct 12, 2026
…xes on macOS + isolate the mutation sandbox (#58)

* fix(verify): isolate the mutation sandbox from the real home directory

Running Stryker over config.ts/claude-settings.ts/powerline.ts executed
upstream's claude-settings.test.ts, whose install tests reset the config
path to the real ~/.config/ccstatusline/settings.json and, after one test
deletes CLAUDE_CONFIG_DIR without restoring it, write the real
~/.claude/settings.json too. A mutant that broke config validation
materialised default settings over the live statusline config, and an
install test pointed statusLine at a client inside .stryker-tmp — which
Stryker deletes afterwards, so the Claude Code status line went empty
(upstream issue sirmalloc#716).

globalSetup now redirects HOME and CLAUDE_CONFIG_DIR to a fresh temp
directory for every sandboxed run. Verified by rerunning the same
mutation scope with mtime guards on both real config files: stryker exit
0, both untouched ("BOTH-REAL-CONFIGS-UNTOUCHED").

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(settings): reproduce upstream 4c62e31 on macOS

Upstream 4c62e31 keeps private settings private when saving and backing
up: the atomic-save temp file inherits the replaced file's permissions
(a 0600 settings came back 0644), and the .bak/.orig backups of Claude
Code's settings.json get the settings' permissions instead of the
umask default. Upstream CI only runs Linux; this proves the fix holds
on macOS.

Red: against the pre-fix tree (75f07d6) both cases fail — a 0600
ccstatusline settings comes back 0644 after a save, and the .orig backup
of a 0600 Claude settings is 0644.

Green: bun test 4/4 and vitest/Node 4/4 (three upstream-repro files).

Mutation (StrykerJS over config.ts/claude-settings.ts/powerline.ts):
the backup-mode lines' survivors are killed by covering the .bak
existence after a save, the recreation of an existing .orig with loose
permissions (writeFile alone keeps 0777), and a re-install over an
existing status line. Remaining survivors on fix lines are equivalent:
writeFile with encoding '' writes the same utf-8 bytes, and readFile
returning a Buffer rounds-trips identically.

Note: this file family runs installs against real home directories in
upstream's claude-settings.test.ts (issue sirmalloc#716);
the mutation sandbox is isolated from the real home by the preceding
commit's globalSetup.

Upstream commit: sirmalloc@4c62e31

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(install): reproduce upstream 475e401 on macOS

Upstream 475e401 (sirmalloc#627) makes installStatusLine abort when the existing
Claude settings cannot be read or parsed, instead of overwriting them
with a settings.json holding only statusLine (permissions, hooks, env
lost). Upstream CI only runs Linux; this proves the fix holds on macOS.

Red: against the pre-fix tree (baf37e1) the install promise resolves
(the overwrite happens) where the fixed code rejects.

Green: bun test 3/3 and vitest/Node 8/8 (three upstream-repro files).

Mutation: the abort path's survivors are killed by asserting the exact
abort reason ("left unchanged") and covering both keepExisting branches
(keep-existing over an unreadable file, keep-existing over a valid
statusLine). Remaining survivors on fix lines are equivalent
(logErrors: false → true only changes console noise; a truncated
message tail).

Upstream commit: sirmalloc@475e401

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(powerline): reproduce upstream 6aefa6b on macOS

Upstream 6aefa6b downloads Powerline fonts into an mkdtemp folder — a
random name created 0700 — instead of the predictable
os.tmpdir()/ccstatusline-powerline-fonts-<timestamp>, which another
account on a shared /tmp could create first and repopulate with its own
install.sh. Upstream CI only runs Linux; this proves the fix holds on
macOS.

Red: against the pre-fix tree (9246095) the observed download folder is
the predictable timestamp name (ccstatusline-powerline-fonts-<13
digits>), failing the random-name assertion. The mocked clone recreates
what git clone would, so the folder's name and permissions are observed
mid-install (the installer removes the folder in its finally).

Green: bun test 1/1 and vitest/Node 8/8 (three upstream-repro files).

Mutation: the mkdtemp line has zero surviving mutants in the batch scope
(powerline.ts:19x). The file's broader low score comes from code paths
this fix does not touch (font directory selection, install script
variants), outside the fix-line criterion.

Upstream commit: sirmalloc@6aefa6b

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(usage): shared subprocess probe harness for usage-fetch tests

The usage fetch code paths (proxy selection, lock handling, request
deadlines) can only be exercised in a child process: bun's os.homedir()
ignores HOME, so in-process tests would read and write the live
~/.cache/ccstatusline, and node:https is not patchable through bun's
ESM namespace. The harness bundles a probe script with bun (extensionless
imports resolve) and runs it with node, whose homedir() honors HOME; the
https.request spy is installed by a --require patch that runs before the
bundle (a CJS-require mutation alone would not reach the bundle's ESM
import bindings). Subprocesses go through Bun.spawnSync under bun
(sibling suites mock node:child_process and that mock leaks across files
in the shared runner); vitest/Node keeps file isolation and takes the
real spawnSync.

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(usage): reproduce upstream e8ef3cb on macOS

Upstream e8ef3cb connects usage and status-page requests directly to
hosts NO_PROXY excludes: the usage request carries the account's OAuth
token, and a user keeping api.anthropic.com off a proxy (for example a
TLS-inspecting corporate one) had it sent through the proxy anyway.
Upstream CI only runs Linux; this proves the fix holds on macOS.

Red: against the pre-fix tree (4c62e31) the probe observes a proxy agent
configured for the request even though NO_PROXY excludes the host (both
spellings covered); the control case without an exclusion still sees the
agent. The probe runs in a subprocess (usage-probe.ts harness).

Green: bun test 3/3 and vitest/Node 5/5 (three batch files).

Mutation (StrykerJS over no-proxy.ts): 94.23%, the 3 survivors are
equivalent — splitting on one whitespace/comma character instead of runs
yields empty entries that the `domain !== ''` check already rejects, and
empty-domain mutants compare against a target that is never empty.

Upstream commit: sirmalloc@e8ef3cb

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(usage): reproduce upstream 0bcc7eb on macOS

Upstream 0bcc7eb (sirmalloc#624) makes a successful fetch clear usage.lock only
when it still holds that render's own in-flight record: previously a
success unconditionally removed the lock, so with two concurrent renders
and one 429 the other's later success deleted the rate-limit lock and the
next render fetched inside the server's backoff window. Upstream CI only
runs Linux; this proves the fix holds on macOS.

Red: against the pre-fix tree (475e401) the probe's rate-limit lock
(written by a "concurrent render" mid-request) is gone after the
observed success — lockExistsAfter expected true, received false.

Green: bun test 1/1 and vitest/Node 5/5 (three batch files).

Mutation: usage-fetch.ts is exercised through the subprocess harness,
whose coverage the mutation runner cannot see (the probe is a child
process), so the fix lines report as uncovered rather than surviving;
the red/green pair above is the regression proof for this fix.

Upstream commit: sirmalloc@0bcc7eb

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(usage): reproduce upstream 610dcc9 on macOS

Upstream 610dcc9 bounds the usage fetch with an absolute deadline: the
request's `timeout` only covers an idle socket, and behind HTTPS_PROXY
the request has no socket until the proxy answers CONNECT — a proxy that
accepts and never answers held the render forever, and its agent kept
the process alive afterwards. Upstream CI only runs Linux; this proves
the fix holds on macOS.

Red: against the pre-fix tree (ae9721e) the probe against a local TCP
proxy that accepts and never answers never finishes — the 20s probe
budget kills it (spawnSync ETIMEDOUT), where the fixed code finishes
with an api-error in about 6s (the 5s deadline plus overhead).

Green: bun test 1/1 and vitest/Node 5/5 (three batch files).

Mutation: usage-fetch.ts is exercised through the subprocess harness,
whose coverage the mutation runner cannot see (the probe is a child
process); proxy-agent.ts survivors are equivalent in any environment
that has AbortController (the typeof fallback targets ancient runtimes,
where controller?.abort() and controller.abort() are the same).

Upstream commit: sirmalloc@610dcc9

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(usage): honor probe timeoutMs under the bun runner

The Bun branch of runProcess dropped options.timeout, so a hung probe
failed the stalled-proxy contract by hanging instead of failing; pass
it through like the node branch does. Bun.spawnSync kills the process
with SIGTERM on timeout (verified on Bun 1.3.13), which surfaces as a
non-zero exit here.

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(powerline): scope font-dir observation to the clone target

Prefix-scanning the shared tmpdir broke under leftovers from killed
runs (they landed in observed and failed toHaveLength(1) on every
later run) and the blind rmSync could delete other accounts'
directories. Observe and clean up only the target the execFileSync
mock already extracts from the installer's own clone call; drop the
dead git-clone branch in the execSync mock, since powerline.ts clones
via execFileSync.

Co-Authored-By: Claude Code <noreply@anthropic.com>

* test(sandbox): fix setup comment and clean up the mutation home

globalSetup runs once per run, not per test file — say so; return a
teardown that removes the sandbox home after the run.

Co-Authored-By: Claude Code <noreply@anthropic.com>

---------

Co-authored-by: axisrow <axisrow@users.noreply.github.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
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.

2 participants