feat(setup): treat python -m claude_statusbar.cli as our own statusLine - #40
Conversation
📝 WalkthroughWalkthroughStatusLine handling recognizes module-based CLI invocations, preserves them during configuration repair, normalizes Windows executable extensions, and adds coverage for supported and foreign command cases. ChangesStatusLine command recognition
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/claude_statusbar/render_thin.py`:
- Around line 174-178: Update the command-name extraction before the extension
loop in the relevant rendering logic to normalize Windows backslash separators,
then perform basename extraction so paths like C:\Users\x\Scripts\cs.EXE resolve
to cs. Preserve the existing lowercasing and .exe/.cmd/.bat stripping behavior.
In `@src/claude_statusbar/setup.py`:
- Around line 113-122: Replace the substring check in _invokes_our_module with
token-based recognition of the exact “-m claude_statusbar.cli” sequence,
preserving ownership detection only for that module invocation. Apply the same
recognition in src/claude_statusbar/render_thin.py lines 170-173 before
suppressing the warning; add a negative test in tests/test_daemon.py lines
1053-1111 for a foreign command such as starship --config claude_statusbar.toml.
- Around line 241-242: Update the existing-command preservation logic around
_invokes_our_module so it only retains the module-form command when fast is
None, the daily-repair path. Ensure explicit --setup --inline requests with
fast=False proceed to write the inline command instead of becoming a no-op.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b872ac9c-95c2-445c-9160-45a9cd53dc93
📒 Files selected for processing (3)
src/claude_statusbar/render_thin.pysrc/claude_statusbar/setup.pytests/test_daemon.py
| if isinstance(existing_cmd, str) and _invokes_our_module(existing_cmd): | ||
| desired["command"] = existing_cmd |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Honor explicit --setup --inline requests.
This runs even when fast=False, so an existing python -m claude_statusbar.cli render command is preserved and the explicit inline request becomes a no-op. Preserve the module form only during the fast is None daily-repair path.
Proposed fix
- if isinstance(existing_cmd, str) and _invokes_our_module(existing_cmd):
+ if fast is None and isinstance(existing_cmd, str) and _invokes_our_module(existing_cmd):
desired["command"] = existing_cmd📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if isinstance(existing_cmd, str) and _invokes_our_module(existing_cmd): | |
| desired["command"] = existing_cmd | |
| if fast is None and isinstance(existing_cmd, str) and _invokes_our_module(existing_cmd): | |
| desired["command"] = existing_cmd |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/claude_statusbar/setup.py` around lines 241 - 242, Update the
existing-command preservation logic around _invokes_our_module so it only
retains the module-form command when fast is None, the daily-repair path. Ensure
explicit --setup --inline requests with fast=False proceed to write the inline
command instead of becoming a no-op.
Fork-local. This machine is Windows and installs with `pip install -e`, so
none of this is reachable here:
install.sh, web-install.sh, uninstall.sh POSIX shell installers
publish.sh upstream's PyPI release script
packaging/ PyInstaller spec for the
standalone macOS/Linux binary
.github/workflows/release-binaries.yml builds that binary; would break
once packaging/ is gone
Nothing in `src/` imports any of them. `updater.py`'s INSTALL_SH_URL points at
upstream's raw copy on GitHub, not the local file, so the frozen-binary hint
still resolves.
ci.yml is kept on purpose — it is the only signal that changes here still work
on the platform upstream targets, which matters while PRs leeguooooo#37-leeguooooo#40 are open.
Everything else non-Windows (service.py, the macOS HUD, the /proc + ps
branches in daemon.py, darwin paths, POSIX docs) is recorded in TODO.md with
the reason each was deferred: they all sit inside files upstream edits often,
so removing them makes every future merge a conflict inside live code.
923 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Line pip's console-script launcher spawns a second process on Windows, which roughly doubles per-render latency: python -m claude_statusbar.cli render : median 0.44s cs.exe render : median 0.92s bare python -c pass : median 0.41s So the module form is a reasonable statusLine command on slow Python startups — but nothing recognised it as ours: - `_displacement_suffix()` painted a red "statusLine 被 python.exe 占用" warning onto the bar, every render. - `cs doctor` reported "(not ours)". - the daily `ensure_statusline_configured()` pass rewrote it back to the console script. Recognise `claude_statusbar` in the command as ours, keep an existing module invocation instead of overwriting it, and match `render` as the last token so fast mode is still detected. `_displacement_suffix()` also gained the lowercase/.EXE normalisation `_is_our_statusline()` already had — a `cs.EXE render` entry was painting the same false warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Path(cmd).name` 在 POSIX 上不拆反斜杠,`C:\...\cs.EXE render` 整串被当成 程序名,exe shim 那条断言只在 Windows 上成立 —— ubuntu CI 上必挂。 手工按 `/` 和 `\` 取 basename,让判定在哪个平台跑都一样。
dab05fb to
6cb13c8
Compare
|
感谢这一组 PR — #37 / #38 / #39 都已合入 main。 这个 PR 我直接 rebase 到了新 main 并推回了你的分支（tests/test_daemon.py 尾部和 #38 的新用例撞在一起，保留双方即可），另外加了一个 commit： `_displacement_suffix()` 里 本地全量 941 passed。CI 绿了就合。 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_daemon.py (1)
1065-1066: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEscape the Windows paths in these test fixtures.
C:\Python\python.exeandC:\Windows\explorer.exeuse invalid Python escape sequences, which breaks parsing under strict warnings-as-errors. Use raw strings or doubled backslashes here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_daemon.py` around lines 1065 - 1066, Update the Windows path fixtures in the daemon test, including the stdout command and the C:\Windows\explorer.exe value, to use raw strings or doubled backslashes so Python parses them without invalid escape-sequence warnings.
🧹 Nitpick comments (1)
tests/test_daemon.py (1)
1116-1133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the actual repair path.
With
refreshIntervalset to1, this may only test that no rewrite is needed. Use a stale value, then assert that repair occurs, the module command remains unchanged, and the refresh interval is corrected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_daemon.py` around lines 1116 - 1133, Update test_daily_repair_does_not_rewrite_module_invocation to use a stale refreshInterval that triggers ensure_statusline_configured’s repair path. Assert that changed is True, the module command remains MODULE_STATUSLINE, and the repaired configuration has the expected current refresh interval.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_daemon.py`:
- Around line 1135-1142: Update _displacement_suffix() to recognize only the
exact module invocation defined by _invokes_our_module(), rather than any
command containing the module name. Extend
test_displacement_warning_silent_for_module_invocation with a foreign command
such as python -m claude_statusbar_wrapper render and assert that the
displacement warning is not suppressed.
---
Outside diff comments:
In `@tests/test_daemon.py`:
- Around line 1065-1066: Update the Windows path fixtures in the daemon test,
including the stdout command and the C:\Windows\explorer.exe value, to use raw
strings or doubled backslashes so Python parses them without invalid
escape-sequence warnings.
---
Nitpick comments:
In `@tests/test_daemon.py`:
- Around line 1116-1133: Update
test_daily_repair_does_not_rewrite_module_invocation to use a stale
refreshInterval that triggers ensure_statusline_configured’s repair path. Assert
that changed is True, the module command remains MODULE_STATUSLINE, and the
repaired configuration has the expected current refresh interval.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6913841d-f43d-4f98-9976-b5c2116e3eb6
📒 Files selected for processing (3)
src/claude_statusbar/render_thin.pysrc/claude_statusbar/setup.pytests/test_daemon.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/claude_statusbar/render_thin.py
- src/claude_statusbar/setup.py
| def test_displacement_warning_silent_for_module_invocation(monkeypatch, tmp_path): | ||
| settings = tmp_path / "settings.json" | ||
| settings.write_text(json.dumps({ | ||
| "statusLine": {"type": "command", "command": MODULE_STATUSLINE}, | ||
| }), encoding="utf-8") | ||
| monkeypatch.setattr(render_thin, "_USER_SETTINGS", settings) | ||
|
|
||
| assert render_thin._displacement_suffix() == "" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject foreign commands that merely contain the module name.
_displacement_suffix() currently treats any command containing claude_statusbar as owned. A foreign command such as python -m claude_statusbar_wrapper render would therefore suppress the warning. Add a negative regression test and require the exact module invocation, consistent with _invokes_our_module().
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 1136-1138: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"statusLine": {"type": "command", "command": MODULE_STATUSLINE},
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_daemon.py` around lines 1135 - 1142, Update _displacement_suffix()
to recognize only the exact module invocation defined by _invokes_our_module(),
rather than any command containing the module name. Extend
test_displacement_warning_silent_for_module_invocation with a foreign command
such as python -m claude_statusbar_wrapper render and assert that the
displacement warning is not suppressed.
2 participants
Why
pip's console-script launcher spawns a second process on Windows, which
roughly doubles per-render latency. Measured on a conda install, same payload,
6 runs each:
python -m claude_statusbar.cli rendercs.exe renderpython -c pass(interpreter floor)Nearly all of the console script's cost is the extra process — the module form
is within 30ms of the interpreter floor. On a machine where Claude Code kills
a slow statusLine, halving that matters.
Problem
The module form is a perfectly valid way to run us, but nothing recognised it:
_displacement_suffix()painted a red⚠ statusLine 被 python.exe 占用warning onto the bar on every render.
cs doctorreported(not ours).ensure_statusline_configured()repair pass rewrote it back tothe console script, so the choice would not survive a day.
Fix
_is_our_statusline()/_displacement_suffix(): a command containingclaude_statusbaris ours.ensure_statusline_configured(): keep an existing module invocation insteadof overwriting it (refreshInterval is still refreshed).
_existing_uses_render(): matchrenderas the last token, so fast modeis detected for both
cs renderand the module form._displacement_suffix()also gained the lowercase +.exe/.cmd/.batnormalisation
_is_our_statusline()already had — acs.EXE renderentrywas painting the same false warning (the tail of Windows: "not ours" false positive in cs doctor / --setup due to unstripped .EXE extension #32).
Tests
Five cases in
tests/test_daemon.py: module form is ours and reads as fastmode; the daily repair leaves it untouched; no displacement warning for the
module form or for
cs.EXE render; a genuinely foreign command(
starship prompt) still warns.tests/test_daemon.py+tests/test_setup.py: 93 passed, 1 skipped, 4pre-existing unrelated failures on this Windows checkout, unchanged by the diff.
🤖 Generated with Claude Code
Summary by CodeRabbit
statusLinecommands during configuration refreshes to avoid unwanted rewrites.statusLinecommand is configured.