fix(config): keep an existing refreshInterval when the version probe fails - #537
Open
taehwanis wants to merge 1 commit into
Open
Conversation
installStatusLine captures the existing statusLine.refreshInterval before replacing the statusLine object, but only re-applies it when supportsRefreshInterval is true. getClaudeCodeVersion() returns null - and isClaudeCodeVersionAtLeast() therefore false - whenever `claude --version` cannot be run at all: not on PATH, the 5s timeout elapses, the output does not match the version regex, or the process exits non-zero. In those cases a re-install silently drops a refreshInterval the user had set, even when the installed Claude Code does support the setting. Keep the captured value in that branch and only apply the default of 10 on supported versions, matching the behaviour documented in sirmalloc#297 ("preserve existing value on re-install"). Add the missing case to the install matrix: existing value + unsupported. The three existing tests cover supported/unsupported on a fresh install and supported on re-install, so this regression had no test to catch it.
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
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.
Refs #297 (the PR that added
statusLine.refreshIntervalsupport — no separate issue is open for this)Summary
installStatusLinecaptures the existingstatusLine.refreshIntervalbefore replacing thestatusLineobject, but only re-applies it whensupportsRefreshIntervalis true.getClaudeCodeVersion()returnsnull— andisClaudeCodeVersionAtLeast()thereforefalse— in four cases that are not "unsupported version":claudeis not onPATHexecSynctimeout elapses/^(\d+\.\d+\.\d+)/(e.g. something is printed before the version)In all four, a re-install silently removes a
refreshIntervalthe user had set, on a Claude Code that may well support the setting. (The TUI reports all four the same way too, as "requires Claude Code >=2.1.97".)#297 documents the intended contract as "preserve existing value on re-install", so the version gate appears to reach one case further than intended: it should gate the default, not the preservation.
Change
Only the default of
10stays gated onsupportsRefreshInterval; an existing value is written back either way.101055555Only the last row changes. Nothing new is written for users who never had the key, so the concern behind the original gate — not writing an unknown key into an older Claude Code's
settings.json— still holds.Tests
The three existing tests in
installStatusLine refreshIntervalcover the first three rows; the fourth had no test, which is why this went unnoticed. Added it.bun run lintis clean.bun testreports 1867 pass / 2 fail — the same 2 failures reproduce on an unmodified checkout ofmainon Windows (config utilities > saves through a symlinked settings file without replacing the linkandglobal command resolution > silences child stderr on best-effort probes so failures cannot leak to the terminal). Both look environment-dependent and are unrelated to this change; the pass count goes 1866 → 1867.Feedback I'm looking for
settings.statusLine.refreshIntervaluntouched rather than rebuilding thestatusLineobject and restoring the field.false, which also drives the TUI's "requires Claude Code >=2.1.97" message. Happy to split that into a separate issue if it is out of scope for this PR.