From 8a341ad73e12fd923ecc34090e226595f85b825b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:50:32 -0400 Subject: [PATCH 1/2] fix(rate-limit-guard): reclaim leaked tee temp files and stop windowless clobber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code cancels an in-flight statusline script when a new update arrives while the previous one is still running, and a cancellation between the write and the rename left the atomic-write temp file behind permanently. No failed rm is needed to explain it: the process never reaches the reclaim line, and the only reclaim paths were write-failure and retry-exhaustion. 61 orphans were found clustered in one busy 27-hour window, which is the shape the correlation predicts — the rename retry loop holds the file open longest exactly when the target is contended, which is also when the session is busy enough to trigger a cancelling update. Two mechanisms, because neither is sufficient alone. A trap reclaims on exit and on a catch-able signal; an age-filtered sweep of leftover siblings on the next refresh recovers what a SIGKILL, a crash, or power loss leaves, which no trap can. Reproduced with an mv shim that parks so the kill lands inside the window: before, SIGTERM and SIGKILL each leak one file; after, SIGTERM leaks none and a SIGKILL orphan is reclaimed by the next refresh. The sweep costs nothing on a clean directory — a shell glob decides whether to spawn anything at all, so a normal refresh runs no extra process on a path that already sits at two to four times the 300 ms debounce interval. Its one-minute age floor cannot race a concurrent session's live temp, whose write-to-rename window is sub-second and bounded by the 300 ms retry loop. Separately, a session with no rate-limit windows overwrote a snapshot that had them. On a mixed-auth machine an API-key or enterprise session landed a snapshot with rate_limits absent and a FRESH captured_at, so consumers never saw "stale" — they saw a current snapshot with no data and dropped to whole-guard reactive-only, on a machine where a window-bearing session had good data available. Each landing could destroy up to the reader contract's full ten-minute staleness budget of usable proactive data. The tee now skips the write when this session has no rate_limits and the target already has them; both tests are substring checks, one on buffered stdin and one on the target read with $(<...), so the guard adds no process to the hot path. A windowless session still writes when the target has no windows either, so a machine with no window-bearing session keeps an honest staleness signal. The reader contract's directory inventory named stop-events.jsonl.lock and told tooling sweeping the directory to expect it while omitting the only litter actually found there; it now documents the temp shape, why it can outlive its writer, and that a cleanup tool should leave it alone. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C --- .../.claude-plugin/plugin.json | 2 +- plugins/rate-limit-guard/CHANGELOG.md | 48 ++++++++++++ .../reference/reader-contract.md | 15 +++- .../scripts/statusline-tee.sh | 77 ++++++++++++++++++- .../scripts/statusline-tee.test.sh | 76 ++++++++++++++++++ 5 files changed, 212 insertions(+), 6 deletions(-) diff --git a/plugins/rate-limit-guard/.claude-plugin/plugin.json b/plugins/rate-limit-guard/.claude-plugin/plugin.json index 1ffc4dc967..77b8e5280a 100644 --- a/plugins/rate-limit-guard/.claude-plugin/plugin.json +++ b/plugins/rate-limit-guard/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "rate-limit-guard", - "version": "0.3.6", + "version": "0.4.0", "description": "Shared rate-limit guard for loop lanes: a statusline wrapper tees the subscription rate-limit windows to a fixed machine-scope file, a StopFailure hook records rate-limit stops reactively, and a reader contract fixes how consuming sessions pause and resume.", "author": { "name": "Melodic Software", diff --git a/plugins/rate-limit-guard/CHANGELOG.md b/plugins/rate-limit-guard/CHANGELOG.md index d5d4f7a0a7..82fbf98f54 100644 --- a/plugins/rate-limit-guard/CHANGELOG.md +++ b/plugins/rate-limit-guard/CHANGELOG.md @@ -3,6 +3,54 @@ All notable changes to the `rate-limit-guard` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.4.0] + +### Fixed + +- **The statusline tee no longer leaks its atomic-write temp file when the harness cancels it + (#1807).** Claude Code + [cancels an in-flight statusline script](https://code.claude.com/docs/en/statusline) when a new + update arrives while the previous one is still running, and a cancellation between the write and + the rename left the temp file behind permanently — no failed `rm` was needed to explain it, the + process simply never reached the reclaim line. The only reclaim paths were write-failure and + retry-exhaustion. 61 orphans were found clustered in one busy 27-hour window, which is the shape + the correlation predicts: the rename retry loop holds the file open longest exactly when the + target is contended, which is also when the session is busy enough to trigger a cancelling update. + + Two mechanisms ship, because neither is sufficient alone. A trap reclaims on exit and on a + catch-able signal; an age-filtered sweep of leftover siblings on the next refresh recovers what a + SIGKILL, a crash, or power loss leaves, which no trap can. Reproduced with an `mv` shim that parks + so the kill lands inside the window: **before**, SIGTERM and SIGKILL each leak one file; + **after**, SIGTERM leaks none and a SIGKILL orphan is reclaimed by the next refresh. + + The sweep costs nothing on a clean directory — a shell glob decides whether to spawn anything at + all, so a normal refresh runs no extra process on a path that already sits at two to four times + the 300 ms debounce interval. Its one-minute age floor cannot race a concurrent session's live + temp, whose write-to-rename window is sub-second and bounded by the 300 ms retry loop. + +- **A session with no rate-limit windows no longer overwrites a snapshot that has them (#1807).** + On a mixed-auth machine an API-key or enterprise session would land a snapshot with `rate_limits` + absent and a **fresh** `captured_at`, so consumers never saw "stale" — they saw a current snapshot + with no data and dropped to whole-guard reactive-only, on a machine where a window-bearing session + had good data available. Each such landing could destroy up to the reader contract's full + ten-minute staleness budget of usable proactive data. + + The tee now skips the write when this session has no `rate_limits` and the target already has + them. Both tests are substring checks — one on the buffered stdin, one on the target read with + `$(<…)` — so the guard adds no process to the hot path. A windowless session still writes when the + target has no windows either, so a machine with no window-bearing session keeps an honest + staleness signal rather than an empty directory. + +### Changed + +- **`reference/reader-contract.md` inventories the temp-file shape.** The directory listing named + `stop-events.jsonl.lock` and explicitly told tooling sweeping the directory to expect it, while + omitting the only litter actually found there. It now documents + `.rate-limits.json.tmp..`, why it can outlive its writer, and that a cleanup tool + should leave it alone — one may belong to a live concurrent session, and the tee reclaims them + itself. The script header's atomicity comment says the same, rather than implying the rename is + the only outcome. + ## [0.3.6] ### Fixed diff --git a/plugins/rate-limit-guard/reference/reader-contract.md b/plugins/rate-limit-guard/reference/reader-contract.md index 72d3e34e11..72192fbb36 100644 --- a/plugins/rate-limit-guard/reference/reader-contract.md +++ b/plugins/rate-limit-guard/reference/reader-contract.md @@ -99,9 +99,18 @@ The hook is side-effect-only (the harness ignores StopFailure output and exit co payload carries no reset or quota data — a record means "a rate limit stopped a turn at this time", nothing more. The file is bounded (rotated to the newest 100 records past 200). -The contract directory holds one more file: `stop-events.jsonl.lock`, the advisory-lock sibling the -hook's serialized append and rotation use (present wherever `flock` exists). Readers ignore it; it -is part of the seam only in the sense that tooling sweeping the directory should expect it. +The contract directory holds two more shapes, neither of which readers consume, listed so tooling +sweeping the directory expects them: + +- `stop-events.jsonl.lock` — the advisory-lock sibling the hook's serialized append and rotation use + (present wherever `flock` exists). +- `.rate-limits.json.tmp..` — the tee's atomic-write staging file. Normally it exists + for well under a second between write and rename. It can outlive its writer: Claude Code + [cancels an in-flight statusline script](https://code.claude.com/docs/en/statusline) when a new + update arrives, and a cancellation inside that window leaves the file behind. The tee reclaims its + own on exit and on a catch-able signal, and sweeps siblings older than a minute on the next + refresh, which is what recovers from a SIGKILL, a crash, or power loss. A cleanup tool should + leave these alone: one may belong to a live concurrent session, and the tee reclaims them itself. ## Invariants and boundaries diff --git a/plugins/rate-limit-guard/scripts/statusline-tee.sh b/plugins/rate-limit-guard/scripts/statusline-tee.sh index 35088feb9f..c61a9e0559 100755 --- a/plugins/rate-limit-guard/scripts/statusline-tee.sh +++ b/plugins/rate-limit-guard/scripts/statusline-tee.sh @@ -42,6 +42,19 @@ # jq, unwritable path, failed rename — ever alters the wrapped statusline's # output or exit code. # +# TEMP-FILE RECLAIM: a temp file can outlive this process. Claude Code +# "cancels the in-flight script" when a new update arrives while this one is +# still running (https://code.claude.com/docs/en/statusline), and a +# cancellation between the write and the rename leaves the temp behind — no +# failed rm is needed to explain it, the process simply never reaches the +# reclaim line. Two mechanisms, because neither is sufficient alone: a trap +# reclaims on exit and on a catch-able signal, and an age-filtered sweep of +# leftover siblings recovers what a SIGKILL, a crash, or power loss leaves, +# which no trap can. The sweep costs nothing on a clean directory — a glob +# decides whether to spawn anything at all — and it cannot race a live +# sibling, since the normal write-to-rename window is sub-second while the +# age floor is a minute. +# # jq is required for the tee and for the standalone line; when absent the # wrapper stays transparent and appends a visible one-line notice instead of # silently dropping the feature. @@ -62,6 +75,45 @@ else IFS= read -r -d '' -t 5 INPUT || true fi +# Path of the temp file currently in flight, for the reclaim traps below. A +# global rather than the function's local: the trap body is evaluated when the +# trap fires, by which time the function's locals are gone. +TEE_TMP="" + +reclaim_tee_tmp() { + [[ -n "$TEE_TMP" ]] && rm -f "$TEE_TMP" 2>/dev/null + TEE_TMP="" + return 0 +} + +# The signal traps exit rather than reclaiming directly, so the EXIT trap stays +# the single reclaim path. Exiting is also the right response to a cancelling +# signal: this refresh's snapshot is already superseded by the update that +# cancelled it. +trap 'reclaim_tee_tmp' EXIT +trap 'exit 143' TERM +trap 'exit 130' INT +trap 'exit 129' HUP + +# Reclaim temp siblings left by a process that never got to clean up. Only +# files older than the age floor are touched, so a concurrent session's live +# temp — sub-second between write and rename — is never a candidate. +# +# The glob runs first and decides whether to spawn at all: on a clean +# directory, which is every refresh in normal operation, this costs zero +# processes on a path that already runs at two to four times the statusline +# debounce interval. +sweep_stale_tee_temps() { + local dir="$1" candidate + for candidate in "$dir"/.rate-limits.json.tmp.*; do + [[ -e "$candidate" ]] || continue + find "$dir" -maxdepth 1 -type f -name '.rate-limits.json.tmp.*' \ + -mmin +1 -exec rm -f {} + 2>/dev/null || true + return 0 + done + return 0 +} + # Write one contract snapshot. Every failure path returns 0: the tee must # never propagate into the statusline pipeline. tee_snapshot() { @@ -86,25 +138,46 @@ tee_snapshot() { ' 2>/dev/null) || return 0 [[ -n "$payload" ]] || return 0 + # A session with no windows — API-key or enterprise auth — must not overwrite + # a snapshot that HAS them. The reader contract routes a snapshot missing + # rate_limits to whole-guard reactive-only, and this write would carry a FRESH + # captured_at, so consumers would never see "stale" and would instead see a + # current snapshot with no data: up to the contract's full 10-minute staleness + # budget of usable proactive data destroyed on a mixed-auth machine, silently. + # Both tests are spawn-free, so this costs nothing on the hot path. + if [[ "$payload" != *'"rate_limits"'* && -f "$target" ]]; then + local existing="" + existing=$(<"$target") 2>/dev/null || existing="" + if [[ "$existing" == *'"rate_limits"'* ]]; then + return 0 + fi + fi + + sweep_stale_tee_temps "$dir" + local tmp="$dir/.rate-limits.json.tmp.$$.$RANDOM" + TEE_TMP="$tmp" # Subshell umask so the snapshot lands owner-only without altering the # umask the wrapped statusline command inherits. ( umask 077 printf '%s\n' "$payload" >"$tmp" ) 2>/dev/null || { - rm -f "$tmp" 2>/dev/null + reclaim_tee_tmp return 0 } local _try # shellcheck disable=SC2034 # bounded-retry counter; the value itself is unused for _try in 1 2 3; do if mv -f "$tmp" "$target" 2>/dev/null; then + # The temp path is the target now; clear it so the EXIT trap cannot + # reclaim a name that no longer refers to this refresh's file. + TEE_TMP="" return 0 fi sleep 0.1 2>/dev/null || true done - rm -f "$tmp" 2>/dev/null + reclaim_tee_tmp return 0 } diff --git a/plugins/rate-limit-guard/scripts/statusline-tee.test.sh b/plugins/rate-limit-guard/scripts/statusline-tee.test.sh index 6973e99d9c..5088049f81 100755 --- a/plugins/rate-limit-guard/scripts/statusline-tee.test.sh +++ b/plugins/rate-limit-guard/scripts/statusline-tee.test.sh @@ -236,6 +236,82 @@ printf '{"session_id":"sess-later","rate_limits":{"five_hour":{"used_percentage" HOME="$HOME1" bash "$TEE" cat >/dev/null if [[ "$(jq -r '.session_id' <"$TEEFILE")" == "sess-later" ]]; then ok "snapshot is last-writer-wins"; else fail "stale snapshot retained: $(jq -c . <"$TEEFILE")"; fi +# --- Case 14: cancellation mid-window leaves no temp behind ------------------ +# Claude Code "cancels the in-flight script" when a new update arrives while +# this one is still running, so a kill between the write and the rename is +# routine rather than exceptional. Driven by an `mv` shim that parks, so the +# signal lands inside the window deterministically. +HOME14="$WORK/home14" +mkdir -p "$HOME14" +SHIM14="$WORK/shim14" +mkdir -p "$SHIM14" +printf '#!/usr/bin/env bash\nsleep 10\n' >"$SHIM14/mv" +chmod +x "$SHIM14/mv" +printf '%s' "$(build_input)" | HOME="$HOME14" PATH="$SHIM14:$PATH" bash "$TEE" >/dev/null 2>&1 & +TEE_PID=$! +sleep 2 +kill -TERM "$TEE_PID" 2>/dev/null +wait "$TEE_PID" 2>/dev/null +sleep 0.5 +LEFT14=$(find "$HOME14/.claude/rate-limit-guard" -name '.rate-limits.json.tmp.*' 2>/dev/null | wc -l | tr -d ' \r') +if [[ "$LEFT14" == "0" ]]; then + ok "cancelled mid-window → temp reclaimed by trap" +else + fail "$LEFT14 temp file(s) leaked on cancellation" +fi + +# --- Case 15: the sweep reclaims an aged orphan and spares a live sibling ----- +# A trap cannot survive SIGKILL, a crash, or power loss, so the sweep is the +# mechanism that actually bounds the litter. The age floor is what keeps it +# from racing a concurrent session's in-flight temp. +HOME15="$WORK/home15" +DIR15="$HOME15/.claude/rate-limit-guard" +mkdir -p "$DIR15" +OLD15="$DIR15/.rate-limits.json.tmp.111.222" +FRESH15="$DIR15/.rate-limits.json.tmp.333.444" +printf 'orphan\n' >"$OLD15" +printf 'live\n' >"$FRESH15" +# POSIX `touch -t [[CC]YY]MMDDhhmm` — no GNU `-d 'N minutes ago'`. +touch -t 200001010000 "$OLD15" +run "$HOME15" "$(build_input)" cat >/dev/null +if [[ ! -e "$OLD15" ]]; then ok "sweep reclaims an aged temp orphan"; else fail "aged orphan survived the sweep"; fi +if [[ -e "$FRESH15" ]]; then ok "sweep spares a live sibling temp"; else fail "sweep deleted a live sibling temp"; fi +if [[ -e "$HOME15/$TEE_REL" ]]; then ok "sweep does not disturb the snapshot write"; else fail "snapshot missing after sweep"; fi + +# --- Case 16: a windowless session never clobbers a snapshot with windows ---- +# The reader contract routes a snapshot missing rate_limits to whole-guard +# reactive-only. Such a write carries a FRESH captured_at, so consumers never +# see "stale" — they see a current snapshot with no data, and lose up to the +# full staleness budget of usable proactive data on a mixed-auth machine. +HOME16="$WORK/home16" +mkdir -p "$HOME16" +run "$HOME16" "$(build_input)" cat >/dev/null +BEFORE16="$(jq -r '.captured_at' <"$HOME16/$TEE_REL")" +printf '{"session_id":"sess-no-windows","model":{"display_name":"Opus"}}' | + HOME="$HOME16" bash "$TEE" cat >/dev/null +if jq -e '.rate_limits' <"$HOME16/$TEE_REL" >/dev/null 2>&1; then + ok "windowless session does not clobber windows" +else + fail "windowless write destroyed the window-bearing snapshot" +fi +if [[ "$(jq -r '.captured_at' <"$HOME16/$TEE_REL")" == "$BEFORE16" ]]; then + ok "windowless session does not refresh captured_at over real data" +else + fail "windowless write refreshed captured_at, hiding staleness" +fi + +# A windowless session still writes when there is nothing to preserve, so a +# machine with no window-bearing session keeps its staleness signal honest. +HOME17="$WORK/home17" +mkdir -p "$HOME17" +printf '{"session_id":"sess-no-windows","model":{"display_name":"Opus"}}' | + HOME="$HOME17" bash "$TEE" cat >/dev/null +if [[ -e "$HOME17/$TEE_REL" ]]; then + ok "windowless session still writes when the target has no windows" +else + fail "windowless session wrote nothing on a fresh machine" +fi + echo echo "PASS=$PASS FAIL=$FAIL" [[ $FAIL -eq 0 ]] From cd2fa56fd330f454181bf72528d99e2bc7dac0ad Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:43:26 -0400 Subject: [PATCH 2/2] fix(rate-limit-guard): structural window detection, writer lock, sweep before skip Three review findings on the windowless-clobber guard. Window-bearing is now decided with jq has(), not a substring a forwarded value like "session_name":"rate_limits" defeats. The preservation check and the rename are serialized through a mkdir writer lock with a stale-holder age floor, closing the check-then-write race between a windowless and a window-bearing writer. The orphan sweep runs before the preservation early-return so a machine with only windowless sessions still reclaims a killed session's temp. Regression tests for all three. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01STASxNiqEm7doy2Z65un4H --- plugins/rate-limit-guard/CHANGELOG.md | 14 +++- .../scripts/statusline-tee.sh | 75 ++++++++++++++++-- .../scripts/statusline-tee.test.sh | 77 +++++++++++++++++++ 3 files changed, 156 insertions(+), 10 deletions(-) diff --git a/plugins/rate-limit-guard/CHANGELOG.md b/plugins/rate-limit-guard/CHANGELOG.md index 82fbf98f54..1dde833692 100644 --- a/plugins/rate-limit-guard/CHANGELOG.md +++ b/plugins/rate-limit-guard/CHANGELOG.md @@ -36,8 +36,18 @@ All notable changes to the `rate-limit-guard` plugin are documented here. Format ten-minute staleness budget of usable proactive data. The tee now skips the write when this session has no `rate_limits` and the target already has - them. Both tests are substring checks — one on the buffered stdin, one on the target read with - `$(<…)` — so the guard adds no process to the hot path. A windowless session still writes when the + them. Window-bearing is decided structurally (jq `has("rate_limits")`, on the payload and on the + target) — never by substring, which a forwarded value merely containing the string + `"rate_limits"` (e.g. a session name) would defeat and clobber real windows. The preservation + decision is serialized with the rename through a `mkdir`-based writer lock (atomic everywhere + this runs, including Git Bash where `flock` is unavailable; a lock left by a killed writer is + stolen past the same one-minute age floor the temp sweep uses), because an unserialized + check-then-write let a windowless writer pass its check, lose the CPU to a window-bearing + writer's rename, and clobber the fresh windows anyway. On lock-acquisition failure the + windowless writer skips its write and the window-bearing writer proceeds unlocked — + last-writer-wins between window-bearing snapshots is the pre-existing contract. The orphan sweep + runs before the preservation early-return, so a machine where only windowless sessions remain + active still reclaims a killed session's temp file. A windowless session still writes when the target has no windows either, so a machine with no window-bearing session keeps an honest staleness signal rather than an empty directory. diff --git a/plugins/rate-limit-guard/scripts/statusline-tee.sh b/plugins/rate-limit-guard/scripts/statusline-tee.sh index c61a9e0559..18c812c357 100755 --- a/plugins/rate-limit-guard/scripts/statusline-tee.sh +++ b/plugins/rate-limit-guard/scripts/statusline-tee.sh @@ -86,11 +86,22 @@ reclaim_tee_tmp() { return 0 } +# Lock directory currently held (see acquire_tee_lock below); a global for +# the same trap-lifetime reason as TEE_TMP, and defined before the trap is +# installed so an early exit never references an undefined function. +TEE_LOCK="" + +release_tee_lock() { + [[ -n "$TEE_LOCK" ]] && rmdir "$TEE_LOCK" 2>/dev/null + TEE_LOCK="" + return 0 +} + # The signal traps exit rather than reclaiming directly, so the EXIT trap stays # the single reclaim path. Exiting is also the right response to a cancelling # signal: this refresh's snapshot is already superseded by the update that # cancelled it. -trap 'reclaim_tee_tmp' EXIT +trap 'reclaim_tee_tmp; release_tee_lock' EXIT trap 'exit 143' TERM trap 'exit 130' INT trap 'exit 129' HUP @@ -114,6 +125,31 @@ sweep_stale_tee_temps() { return 0 } +# Serialize the preservation decision with the rename. Check-then-write +# without mutual exclusion lets a windowless writer pass its check, lose the +# CPU to a window-bearing writer's rename, and then clobber the fresh windows +# anyway — concurrent sessions are the normal operating model here. The lock +# is a directory (mkdir-as-lock is atomic on every platform this runs on, +# including Git Bash on Windows, where flock is unavailable). A holder killed +# between mkdir and rmdir would leave the lock forever, so a contender steals +# any lock older than the same one-minute age floor the temp sweep uses — +# far above the sub-second hold time of a live writer. The release side lives +# with the traps above. +acquire_tee_lock() { + local dir="$1" lock="$1/.rate-limits.json.lock" _try + # shellcheck disable=SC2034 # bounded-retry counter; the value itself is unused + for _try in 1 2 3; do + if mkdir "$lock" 2>/dev/null; then + TEE_LOCK="$lock" + return 0 + fi + find "$dir" -maxdepth 1 -type d -name '.rate-limits.json.lock' \ + -mmin +1 -exec rmdir {} + 2>/dev/null || true + sleep 0.1 2>/dev/null || true + done + return 1 +} + # Write one contract snapshot. Every failure path returns 0: the tee must # never propagate into the statusline pipeline. tee_snapshot() { @@ -138,23 +174,43 @@ tee_snapshot() { ' 2>/dev/null) || return 0 [[ -n "$payload" ]] || return 0 + # Window-bearing is a structural property — jq's has(), never a substring + # test: a forwarded value that merely contains the string "rate_limits" + # (e.g. "session_name":"rate_limits") must not count as window-bearing and + # overwrite a snapshot holding real windows. + local has_windows=false + jq -e 'has("rate_limits")' >/dev/null 2>&1 <<<"$payload" && has_windows=true + + # Sweep before any early return: a machine where only windowless sessions + # remain active would otherwise skip below on every refresh and never + # reclaim the orphan a killed window-bearing session left behind. + sweep_stale_tee_temps "$dir" + + # All writers take the lock around [check+]rename — serialization needs + # both parties. On acquisition failure the windowless writer skips its + # write (nothing precious is lost; the reader treats absence reactively), + # while the window-bearing writer proceeds unlocked: its payload carries + # data, and last-writer-wins between two window-bearing snapshots is the + # pre-existing contract. + if ! acquire_tee_lock "$dir"; then + [[ "$has_windows" == true ]] || return 0 + fi + # A session with no windows — API-key or enterprise auth — must not overwrite # a snapshot that HAS them. The reader contract routes a snapshot missing # rate_limits to whole-guard reactive-only, and this write would carry a FRESH # captured_at, so consumers would never see "stale" and would instead see a # current snapshot with no data: up to the contract's full 10-minute staleness # budget of usable proactive data destroyed on a mixed-auth machine, silently. - # Both tests are spawn-free, so this costs nothing on the hot path. - if [[ "$payload" != *'"rate_limits"'* && -f "$target" ]]; then - local existing="" - existing=$(<"$target") 2>/dev/null || existing="" - if [[ "$existing" == *'"rate_limits"'* ]]; then + # A target jq cannot parse counts as windowless — torn or corrupt content is + # exactly what an atomic overwrite should replace. + if [[ "$has_windows" != true && -f "$target" ]]; then + if jq -e 'has("rate_limits")' "$target" >/dev/null 2>&1; then + release_tee_lock return 0 fi fi - sweep_stale_tee_temps "$dir" - local tmp="$dir/.rate-limits.json.tmp.$$.$RANDOM" TEE_TMP="$tmp" # Subshell umask so the snapshot lands owner-only without altering the @@ -164,6 +220,7 @@ tee_snapshot() { printf '%s\n' "$payload" >"$tmp" ) 2>/dev/null || { reclaim_tee_tmp + release_tee_lock return 0 } local _try @@ -173,11 +230,13 @@ tee_snapshot() { # The temp path is the target now; clear it so the EXIT trap cannot # reclaim a name that no longer refers to this refresh's file. TEE_TMP="" + release_tee_lock return 0 fi sleep 0.1 2>/dev/null || true done reclaim_tee_tmp + release_tee_lock return 0 } diff --git a/plugins/rate-limit-guard/scripts/statusline-tee.test.sh b/plugins/rate-limit-guard/scripts/statusline-tee.test.sh index 5088049f81..4a5b1e64ec 100755 --- a/plugins/rate-limit-guard/scripts/statusline-tee.test.sh +++ b/plugins/rate-limit-guard/scripts/statusline-tee.test.sh @@ -312,6 +312,83 @@ else fail "windowless session wrote nothing on a fresh machine" fi +# --- Case 18: window-bearing is a structural property, not a substring -------- +# A windowless payload whose forwarded value merely CONTAINS the string +# "rate_limits" (session_name here) must still be classified windowless, or it +# skips the preservation check and clobbers real windows. +HOME18="$WORK/home18" +mkdir -p "$HOME18" +run "$HOME18" "$(build_input)" cat >/dev/null +printf '{"session_id":"sess-imposter","session_name":"rate_limits"}' | + HOME="$HOME18" bash "$TEE" cat >/dev/null +if jq -e '.rate_limits' <"$HOME18/$TEE_REL" >/dev/null 2>&1; then + ok "substring imposter payload stays windowless — windows preserved" +else + fail "payload containing the literal string rate_limits clobbered real windows" +fi + +# --- Case 19: the sweep runs even on a refresh that skips its write ----------- +# A machine where only windowless sessions remain active takes the preserve +# early-return on every refresh; the orphan a killed window-bearing session +# left must still be reclaimed there, or it survives indefinitely. +HOME19="$WORK/home19" +DIR19="$HOME19/.claude/rate-limit-guard" +mkdir -p "$DIR19" +run "$HOME19" "$(build_input)" cat >/dev/null +OLD19="$DIR19/.rate-limits.json.tmp.555.666" +printf 'orphan\n' >"$OLD19" +touch -t 200001010000 "$OLD19" +printf '{"session_id":"sess-no-windows"}' | HOME="$HOME19" bash "$TEE" cat >/dev/null +if [[ ! -e "$OLD19" ]]; then + ok "sweep reclaims the orphan on a skipped windowless refresh" +else + fail "orphan survived — sweep does not run before the windowless early return" +fi +if jq -e '.rate_limits' <"$HOME19/$TEE_REL" >/dev/null 2>&1; then + ok "skipped windowless refresh still preserved the windows" +else + fail "windowless refresh clobbered windows in the sweep-order case" +fi + +# --- Case 20: the writer lock serializes the preservation decision ------------ +# A fresh (unstealable) lock held by another writer: the windowless writer +# must skip its write, the window-bearing writer must still write (its +# payload carries data; last-writer-wins between window-bearing snapshots is +# the pre-existing contract). A stale lock left by a killed writer is stolen. +HOME20="$WORK/home20" +DIR20="$HOME20/.claude/rate-limit-guard" +LOCK20="$DIR20/.rate-limits.json.lock" +mkdir -p "$LOCK20" +printf '{"session_id":"sess-no-windows"}' | HOME="$HOME20" bash "$TEE" cat >/dev/null +if [[ ! -e "$HOME20/$TEE_REL" ]]; then + ok "held lock → windowless writer skips its write" +else + fail "windowless writer wrote through a held lock" +fi +run "$HOME20" "$(build_input)" cat >/dev/null +if jq -e '.rate_limits' <"$HOME20/$TEE_REL" >/dev/null 2>&1; then + ok "held lock → window-bearing writer still writes" +else + fail "window-bearing writer lost its snapshot to a held lock" +fi +rmdir "$LOCK20" 2>/dev/null || true +HOME21="$WORK/home21" +DIR21="$HOME21/.claude/rate-limit-guard" +LOCK21="$DIR21/.rate-limits.json.lock" +mkdir -p "$LOCK21" +touch -t 200001010000 "$LOCK21" +printf '{"session_id":"sess-no-windows"}' | HOME="$HOME21" bash "$TEE" cat >/dev/null +if [[ -e "$HOME21/$TEE_REL" ]]; then + ok "stale lock is stolen — windowless writer proceeds on a fresh machine" +else + fail "stale lock permanently blocked the writer" +fi +if [[ ! -e "$LOCK21" ]]; then + ok "stolen stale lock directory removed" +else + fail "stale lock directory survived" +fi + echo echo "PASS=$PASS FAIL=$FAIL" [[ $FAIL -eq 0 ]]