fix(statusline): move the usage fetch off the render path - #1773
Open
asdf8675309 wants to merge 1 commit into
Open
fix(statusline): move the usage fetch off the render path#1773asdf8675309 wants to merge 1 commit into
asdf8675309 wants to merge 1 commit into
Conversation
The location and weather refreshes already run as detached background subshells. The comment above them explains why: a synchronous curl on the render path "killed the perf and got the feature ripped out on 2026-05-08." The usage fetch is the last one still inline. The mutex winner runs security find-generic-password and then a curl with a 3s timeout while the statusline renders. When that endpoint is slow the whole render blocks. Measured with a blackholed endpoint, a render costs 3.36-3.49s against a shipped refreshInterval of 1. Claude Code kills a render that overruns its interval, so those seconds are also a kill window. On macOS the keychain read is the part that hurts: security prompts through readpassphrase(), which opens /dev/tty directly and puts the terminal in canonical no-echo mode. Kill the render mid-call and the terminal can stay that way. Arrow keys emit escape sequences, backspace stops echoing, and typing goes to the password buffer instead of the prompt. Severing stdin with </dev/null removes that path entirely. Bash 3.2 has no BASHPID and $$ inside a subshell still names the parent, which exits in milliseconds. Writing $$ as the ownership token would let the next render's liveness check reap a lock whose fetch is still running. sh -c 'echo $PPID' returns the subshell's own pid, verified equal to the parent's $! on bash 3.2.57. Release moved to an EXIT trap so a killed fetch cannot leak the mutex, and it still verifies ownership first. _data_age no longer reaches zero on this render, so the winner now follows the same path as the mutex losers: last-known-good with an honest age, and the fresh data lands on the next tick. The forced-refetch bookkeeping restores _orig_age, which is correct, because from this render's point of view the attempt has not landed yet. Render latency with a 3s-timeout endpoint drops from 3.36-3.49s to 0.27-0.29s.
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.
What
Location and weather already refresh in detached background subshells. The comment above them says why:
The usage fetch is the last one still inline. Every 900s the mutex winner runs
security find-generic-passwordand acurl --max-time 3while the bar renders. If that endpoint is slow, the render sits there waiting.Measured against a blackholed endpoint so the curl burns its full timeout: 3.36–3.49s per render, against a shipped
refreshIntervalof 1.The part that bites on macOS
Those seconds are a kill window — Claude Code terminates a render that overruns its interval (#1767 covers what the killed renders leave behind).
securityis the bad one to get killed inside of. It prompts throughreadpassphrase(), which opens/dev/ttydirectly and flips the terminal to canonical no-echo mode. Kill the render mid-call and the terminal can stay that way. What you get is a session where arrow keys spray escape sequences, backspace stops echoing, and what you type goes into a password buffer instead of the prompt — so hitting enter answers a keychain challenge you can't see.Worth noting: the
2>/dev/nullon that line can't prevent any of it, because the prompt goes to/dev/tty, not stderr.</dev/nullon the detached subshell removes the path entirely. The fetch can't read the terminal because it no longer has one.Change
Wrap the fetch in the same
) </dev/null >/dev/null 2>&1 &+disownshape the weather block already uses. Two things had to change to keep @lmbagley's mutex from #1349 intact.Ownership token. Bash 3.2 has no
BASHPID, and$$inside a subshell still names the parent — which exits in milliseconds. Writing$$would let the next render's liveness check reap a lock whose fetch is still running.sh -c 'echo $PPID'returns the subshell's own pid:sh_c_PPIDmatches the parent's$!. That's the value that goes in the pid file, sokill -0still reads a live process.Release. Moved to an
EXITtrap so a killed fetch can't leak the mutex. It still verifies ownership before removing anything, same as before.Why it's safe
_data_agenever reaches 0 on this render now, so the winner takes the same path the mutex losers already take: last-known-good with an honest age, fresh data on the next tick. That path is already there and already exercised.The forced-refetch bookkeeping restores
_orig_age, which is correct — from this render's point of view the attempt hasn't landed yet, and the comment there already says a non-landed attempt should restore the true age.Everything else is untouched: the fail-closed
.five_hourprobe, the atomicfetched_at-stamped write, the symlink guard, the dead-owner reap, the 900s TTL and the scoped fast lane._tmp_cachenow keys off the fetch pid instead of$$, which is strictly more unique.Verification
macOS 15 (Apple Silicon), bash 3.2.57. Both arms get test-only cache and lock paths plus a blackholed endpoint, so nothing touches live state and the curl always burns its full 3s.
Lock behavior on the patched build:
bash -nexits 0.Portability of the ownership token. The token has to equal the parent's
$!or the liveness reap can kill a live fetch, so I checked it on both shells LifeOS runs on, with both common/bin/shimplementations:/bin/sh$!BASHPIDresolves to the same value on 5.3.15, so one code path is correct on both and there's no need for a platform branch. dash sets$PPIDcorrectly, which matters because it's/bin/shon Debian and Ubuntu. Everything else in the diff is POSIX apart fromdisown, which the weather block above already uses with the same2>/dev/null || trueguard.Linux gets a smaller version of the same win. It takes the
.credentials.jsonbranch, so there's nosecuritycall and no terminal to strand — but thecurlis still on the render path, and #1767's report came from a headless Ubuntu box where killed renders were the problem.Control for that last line. "Released cleanly" is a pass-by-absence, so I checked it can fail: strip the
EXITtrap, leave everything else, re-run. Mutation counted (trap lines 1 → 0), sabotaged script still parses, and the lock leaks. The check isn't blind.The stock arm is its own control for the latency number — same harness, same endpoint, 12x the wall time.
Scope
Left alone on purpose:
DeployComponents.tsand therefreshIntervaldefault. That's fix(installer): raise the shipped statusline refreshInterval from 1s to 30s #1772. This PR stands on its own — a render that doesn't block is worth having at any interval./tmp/pai-parallelscratch dirs. fix(statusline): bound the /tmp pai-parallel leak (follow-up to #1297) #1767 owns that. No overlap: different function, different region of the file.2>/dev/nullon the keychain read. Inside a detached subshell everything already goes to/dev/null, so changing it would be noise.USAGE_CACHE_TTLand the scoped fast lane. Not my call to retune, and detaching doesn't depend on it.One thing I noticed but didn't act on: the comment at L859 says "the 5s tick rate must never hammer it," so the script was written expecting a 5s tick while the installer ships 1. That mismatch is what #1772 is about.