fix(#3709): clear the context-monitor warn sentinel on PreCompact (#3808)

* fix(#3709): clear the context-monitor warn sentinel on PreCompact

The monitor's per-session warn sentinel survived a compaction, so once the
first CRITICAL of a session had fired, `lastLevel` stayed pinned at 'critical'
for the rest of the run. The hook was already wired to PreCompact (#772), but
read the event only at the very END, and solely to pick an output envelope.

Two documented behaviours died as a result:

  - "First warning always fires immediately" — the first warning of the
    post-compaction cycle was debounced instead.
  - "Severity escalation (WARNING -> CRITICAL) bypasses debounce" — computed as
    `lastLevel === 'warning'`, which can never be true again, so every later
    CRITICAL waited out the full five-tool-use debounce, exactly when an
    immediate warning matters most.

`criticalRecorded` was equally sticky: a session that compacted and later truly
ran out kept a /gsd:resume-work breadcrumb (#1974) describing the earlier
near-miss rather than the exhaustion that ended the run.

Reproduced first, with the issue's own literal repro, including the detail that
the compaction consumed a debounce slot (callsSinceWarn 0 -> 1).

The reset runs BEFORE the metrics read, deliberately: a post-compaction reading
is healthy again, so the ENOENT / stale / above-threshold branches would all
exit first and never reach it. Returning early also stops the compaction from
eating a slot of the cycle it was meant to restart. The event name is now read
once through a shared `readEventName()` helper, so this reset and the #2289
output allowlist cannot drift on what counts as "no event name".

Seven rows against a real sequence (the defect is state carried ACROSS calls, so
they need their own driver — the existing helpers delete the sentinel after each
invocation). Reverting the reset turns SIX of them red; the seventh is the
non-vacuity row asserting a NON-compaction event must not clear the sentinel,
which correctly passes either way.

AC4 initially passed with and without the fix — asserting `criticalRecorded ===
true` is vacuous when the seeded stale sentinel already carries it. It now seeds
a `staleProbe` marker that can only survive if the sentinel survives, so its
absence is what proves the state was rebuilt.

hooks/dist/ is gitignored and regenerated by build:hooks, so no committed dist
copy needs syncing.

Verified: `npm run lint:ci` exit 0; acceptance criteria 1-6 driven end-to-end
against the real hook.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3709): reset ahead of the config gate, and pin the placement itself

Codex review of the #3709 fix, before opening the PR. Three findings, all in
this change's own new code.

1. `context_warnings: false` prevented the reset. The config early-exit sits
   ABOVE where the reset was placed, so a session that disabled warnings,
   compacted, then re-enabled them mid-session resurrected the stale sentinel
   and the original bug with it. Config is re-read per invocation, so that
   sequence is supported rather than hypothetical. The reset now runs ahead of
   the config gate: clearing the sentinel is CLEANUP, not a warning — state that
   must not outlive a compaction should not outlive it merely because warnings
   are switched off right now. It cannot emit anything from there, so the
   disabled contract is untouched.

2. Nothing pinned the "before the metrics read" placement. Every row wrote a
   fresh metrics file, so the reset could have been moved below the metrics
   read, the stale check, or the healthy-threshold exit with all seven rows
   still green — while a REAL PreCompact, which carries no fresh metrics and
   follows a recovery to healthy usage, silently kept its sentinel. Three rows
   now pin it: no metrics file at all, usage recovered to healthy, and warnings
   disabled. Each catches a distinct wrong placement — moving the reset below
   the config check reds the third; below the metrics read reds all three.

3. The absent-sentinel row proved nothing. `assert.doesNotThrow` was vacuous
   because the driver caught every child exit, so a hook that exited 1 on the
   ENOENT unlink would still have passed. The driver now returns the exit code
   and the row asserts it is 0.

Also corrected the `readEventName` comment: it said the event is "read once",
which is not literally true — there are two call sites. The point is one
DEFINITION of what counts as an event name, so the reset and the #2289
allowlist cannot drift; the comment now says that.

Verified: 60 rows in tests/perf-317-context-monitor-fs.test.cjs, 0 fail, with
both placement mutations driven to red and reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#3709): backfill changeset pr number

The fragment shipped with the documented `pr: 0` placeholder, which the
changeset lint treats as always-silent, because the PR number does not exist
until the PR is opened. Backfilled to 3808 now that it does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3709): a compaction clears the stale reading too, not just the state

Review round 1. Major 1 was right and it mattered: clearing only the sentinel
traded a warning that never fires for one that fires when it must not.

The statusline bridge still holds the PRE-compaction reading, and STALE_SECONDS
is 60, so for up to a minute it still reads fresh and still says the context is
exhausted. With the sentinel gone, firstWarn is true, so the next PostToolUse
emitted a spurious CONTEXT CRITICAL immediately after the compaction that FREED
the context — and flipped criticalRecorded, spawning a false context-exhaustion
breadcrumb. That is the same breadcrumb inaccuracy #3709 exists to fix, re-entered
from the other side. Reproduced before fixing, exactly as the review described.

A compaction now invalidates the warning state AND the reading that produced it.
Removing the bridge loses nothing: the statusline owns that file and rewrites it
on every render, and its absence is already the "no reading yet" state a fresh
session starts in, which exits silently.

Two things my own verification caught while fixing it:

  - The first attempt did NOTHING. metricsPath was declared below the PreCompact
    block, so referencing it hit the temporal dead zone, threw, and the outer
    catch swallowed it into a silent exit 0. The probe still printed "silent",
    which looked like success but was the old debounce. metricsPath is now
    hoisted beside warnPath.

  - The new Major 1 row was VACUOUS. The driver's `metrics: false` DELETES the
    bridge, but the defect is a bridge that is still there and still reads fresh,
    so the row passed on the ENOENT early-exit rather than on the fix. Only the
    sentinel-only mutation exposed it. The driver grew a `metrics: 'keep'` mode
    that leaves the stale file in place; both Major 1 rows now red under that
    mutation.

Also from the review:
  - Minor 1 — the compaction-abort path is now stated in the source rather than
    left silent, including why a conditional reset (SessionStart source "compact")
    is out of scope for this fix.
  - Minor 2 — docs/context-monitor.md completed: PreCompact wiring and the early
    return under How It Works, a table of all three things the reset clears, the
    breadcrumb guard, the warnings-disabled interaction, and the never-block
    property under Safety.
  - Minor 3 — changeset trimmed from ~1,400 chars of implementation narration to
    the user-visible change.
  - Nit 1 — a failed unlink (Windows EPERM/EBUSY) no longer leaves the bug
    silently intact: the file is neutralised in place instead, with a shape safe
    for each (an empty sentinel, a timestamp-0 bridge).
  - Nit 2 — reviewer-process narration removed from shipped test source. The
    remaining "Codex" mentions are pre-existing and name the RUNTIME.
  - Nit 3 — the debounce-slot row now asserts the observable consequence (the
    first post-compaction warning fires) rather than repeating AC1's assertion.
  - Nit 4 — the file docblock now lists the folded-in blocks and asks the next
    contributor to extend it.

Verified: `npm run lint:ci` exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3709): the unlink-failure fallback truncates to empty, matching deletion

The fallback wrote well-formed neutral values, and neither was equivalent
to the deletion it stood in for: '{}' parses, so firstWarn was false and
the first post-compaction warning was debounced — AC2 undone on exactly
the path the fallback exists for — and '{"timestamp":0}' was never stale
(the guard is `metrics.timestamp && ...`), so the flow reached emit with
remaining === undefined and injected a literal 'Usage at undefined%'.
Truncating to '' makes JSON.parse throw on both reads: the sentinel read
keeps firstWarn true, the bridge read falls to the outer catch and exits
0 silently (review of #3808, Blocker 1).

The branch is now executed for real: an EPERM is injected into the
child's fs.unlinkSync via --require preload — method monkeypatching,
never chmod 0o000, which root bypasses under Docker/CI (Blocker 2). Both
rows proved failing-first against the neutral-value fallback. The
boundary trios at WARNING=35 / CRITICAL=25 are completed on the emit
path with 34, 26, and 24 (Major 3); 36/35/25 were already pinned.

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

* fix(#3709): the truncation fallback refuses to follow a planted symlink

The per-session files live in a shared sticky tmpdir, where an unlink
failing EPERM is exactly what another user's planted file produces — and
a planted SYMLINK would make the fallback's plain truncating write empty
out its TARGET, weaponising the hook against any file its own user can
write. Open with O_WRONLY|O_TRUNC|O_NOFOLLOW instead: a symlink fails
ELOOP into the same give-up arm. On Windows the constant is absent and
'|| 0' keeps the fallback alive there, where the held-handle case it
exists for occurs and temp dirs are per-user. Found by Codex review;
the new row proved failing-first against the writeFileSync fallback
(victim file truncated to zero bytes).

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

* fix(#3709): refuse non-regular files everywhere, not only where O_NOFOLLOW exists

Codex round 2: '|| 0' removed the no-follow protection exactly where it
cannot be expressed as an open flag — Windows, whose tmpdir is NOT
guaranteed per-user (TEMP/TMP overrides, system-temp fallback). An
lstat isFile() guard now rejects symlinks and every other non-regular
shape on all platforms before the truncating open; O_NOFOLLOW stays, as
the lstat->open substitution-race backstop where the platform has it.
The symlink row additionally asserts the planted link SURVIVES the call,
so a preload match that stops engaging can no longer pass the row
vacuously off a successful unlink.

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

* test(#3709): tolerate the Windows give-up, still outlaw neutral values

Both windows-latest CI lanes fail the two EPERM rows deterministically:
the runners hold freshly written files with a share mode that allows
DELETE (every real-unlink row passes) but refuses a truncating
write-open, so the fallback's give-up arm engages — which is the
fallback working as designed, not the defect the rows exist to catch.
The rows are now platform-aware: POSIX still requires exact truncation
and the behavioural follow-ons; Windows accepts truncated-or-untouched
but still rejects the Blocker-1 regression class (a parseable neutral
value is never legal anywhere), with the follow-ons gated on the
truncation actually landing. Also corrects the hook comment: libuv
defines O_NOFOLLOW as 0 on Windows — a no-op, not an absent constant.

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

* fix(#3709): a compaction watermark closes the window bridge deletion only narrowed

Round-3 Major 1: the statusline is an uncoordinated process that
re-writes the bridge on every render, so a render landing between the
PreCompact clear and the compaction's completion re-created the
PRE-compaction reading under a CURRENT timestamp — past STALE_SECONDS,
into a spurious post-compaction CRITICAL and a false exhaustion
breadcrumb: the exact failure the deletion was added to prevent.
PreCompact now also writes claude-ctx-<id>-compacted.json ({at}) and the
metrics read drops any reading not STRICTLY newer than it — which also
covers unstamped/zero timestamps once a compaction happened. Written
unlink-then-O_EXCL so a planted file or symlink is never followed;
failure degrades to the old narrowing. Docs and changeset now describe
the watermark instead of overclaiming for the deletion.

Round-3 Major 2: DEBOUNCE_CALLS and STALE_SECONDS get their trios — the
gate increments BEFORE comparing, so seeds 3/4/5 pin 4-debounced,
5-emits, 6-emits; ages 59/60/61 pin the strict >. The child's clock is
pinned via a --require preload (a wall-clock boundary row would flip on
one second of startup delay). timestamp-0's falsy bypass is pinned
directly as characterized behaviour. Mutation-proven: dropping
O_NOFOLLOW, <= for <, and >= for > each red exactly one row.

Minors: the symlink row's comment now names the lstat guard it actually
pins, and a preload-blinded-lstat row drives the O_NOFOLLOW substitution
-race backstop for real (3); absence assertions use warnRaw so a
corrupt leftover cannot pass as deleted (4); the Windows give-up is an
explicit t.skip, never a silent if (5); readEventName is total via
String(), keeping #2289's side-effects-always-run contract for
malformed event names, with a row (6); the PreCompact rationale lives
once in docs/context-monitor.md with the code keeping only line-level
constraints (9); the changeset is release-note-sized (10).

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

* fix(#3709): the grace window covers the compaction's duration, not just its start

Codex on the first watermark cut: the watermark stamps the compaction's
START, so a statusline render one second later — still mid-compaction,
still the old reading — passed 'strictly newer' and re-fired the false
CRITICAL. Readings inside COMPACT_GRACE_SECONDS (60) past the watermark
are now dropped: the window covers the compaction's own duration, a
healthy reading dropped there behaves identically to an accepted one
(it exits above-threshold anyway), and a genuine exhaustion warning is
delayed at most one window after a compact. A watermark stamped ahead
of the reader's clock is ignored — a clock step backwards or a stray
file must degrade to plain staleness, never mute the monitor
indefinitely. Both proven failing-first.

readEventName is strict about TYPE, not coerced: String() rendered
['PreCompact'] as 'PreCompact' and would run the reset off a malformed
payload. typeof: every non-string is 'no event' — silent, side effects
intact — with rows for the number, hostile-object, and array-wrapped
cases. The lstat-claim preload arm now writes an engagement marker the
substitution-race row asserts on, so a match string that silently stops
matching can no longer let the row pass off the real lstat guard.

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

* chore: retrigger CI — the previous wave was cancelled by an Actions outage, zero job failures

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

* fix(#3709): drive the compaction rows on the clock, not on a future stamp

Round-4 review raised three majors, all in the test scaffolding around the
fix rather than in the fix itself.

Major 2 (taken first — it is the cheapest and it unblocks Minor 6): call()
passed process.env to the child unmodified, so two rows depended on ambient
GEMINI_API_KEY. The preserved Gemini fallback is `eventName === "" &&
!!process.env.GEMINI_API_KEY`, and readEventName returns "" for every
malformed name, so with the key set the malformed-event row's `stdout === ''`
assertion failed outright — reproduced by running it under GEMINI_API_KEY=x.
call() now takes an explicit env, the way the sibling runMonitor helper in
this file always has, and both rows pin the variable unset. (The array row
survived an ambient key only because its reading was debounced — incidental,
not independence, so it is pinned too.)

Major 1: the AC2/AC3 rows drove the hook with a bridge stamped 62 seconds in
the FUTURE — a shape hooks/gsd-statusline.js cannot produce, since it always
stamps Math.floor(Date.now()/1000) on the same clock. They proved "the
sentinel was cleared" while their assertion messages claimed the documented
immediate-warning behaviour, which is gated behind the grace window and went
unexercised. Both rows now run the real sequence on the clock-pinning preload
this PR already added for the STALE trio: PreCompact at a fixed instant, then
a normally-stamped render one second past the window. Verified non-vacuous —
stubbing the sentinel unlink reds both.

Major 3: COMPACT_GRACE_SECONDS, the one constant this PR introduces, was the
only threshold without a limit-1/limit/limit+1 trio, in a PR that adds full
trios for four pre-existing ones. The seeded offsets were +0, +1 and +61; the
boundary itself (+60) and limit-1 (+59) were untested. Added, driven by
advancing the reader's clock rather than post-dating the reading, so the
reading is never ahead of the reader and only the grace gate can drop it.
Verified against three mutations — `>` to `>=`, the constant to 59, and the
constant to 61 — each of which reds exactly one row of the trio.

No production code changed. Verified: 85/85 in this file, lint:ci exit 0,
and the two Minor-6 rows now pass under GEMINI_API_KEY=x as well as unset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAbQy7n4mLMB7h3TnZ8GdG

* fix(#3709): harden the watermark read and pin the thresholds it introduces

Codex review of the full PR found four majors. All reproduced here against
the real hook before fixing.

MAJOR — the watermark was write-hardened but read-untrusted. PreCompact
already refuses to follow or overwrite a planted object (unlink-then-O_EXCL),
but the read was a bare readFileSync, so anything the write side gave up on
was followed by every later invocation. In a shared sticky os.tmpdir() that
is a mute primitive — a planted recent watermark suppresses monitoring — and
a symlink to a FIFO stalls a synchronous read. Measured against the
pre-hardening file: a symlink to a planted watermark WAS honored and muted
the monitor. The read now uses the same lstat + O_NOFOLLOW pair the sentinel
path uses, plus a size bound; symlink, directory and oversized cases are all
refused, with a plain-file control proving watermarks still work.

MAJOR — the `now + 5` skew tolerance was an unnamed, untested threshold. It
is now WATERMARK_SKEW_SECONDS with a +4/+5/+6 trio, verified against two
mutations (`<=` to `<`, and the constant to 6), each of which reds one row.
This is the same class as round 4's Major 3, one layer up.

MAJOR — the malformed-event row shared one session across both subcases, so
the hostile-object iteration's `assert.ok(s.warn())` passed off the sentinel
the `42` iteration left behind. A regression throwing before the bookkeeping
would have kept it green — vacuous for exactly the subcase it exists for.
Fresh session per subcase, with an explicit no-sentinel precondition.

MAJOR — the stale-reading row's non-vacuity is an artifact of call()'s future
stamp: with a production stamp the watermark suppresses the same reading, so
the row cannot isolate bridge deletion. The two guards genuinely overlap
inside the window, so no end-to-end row can separate them; the comment now
says so and points at the direct pin (s.metrics() === null) instead of
claiming an isolation it does not have.

Docs corrected where measurement contradicted them: the window NARROWS the
race rather than covering the compaction's duration, and the delay is not
bounded by the window alone — first recovery is watermark+61s with no skew
but watermark+66s at the accepted +5s skew. Aborted compactions are muted
the same way. The truncation fallback is documented as best-effort, which is
what the code and the Windows rows already do.

Verified: 89/89 in this file, lint:ci exit 0, symlink/directory/oversize all
refused where the pre-hardening file honored them, both new trios
mutation-checked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAbQy7n4mLMB7h3TnZ8GdG

* fix(#3709): move the PR's two new exits onto the declared-policy vocabulary

#3911 / ADR-3889 migrated this hook off raw process.exit() while this PR was
in review, replacing every exit with hooks/lib/hook-exit.js's allow(), which
forces each call site to name its crash policy. The PreCompact reset and the
watermark gate are added by THIS PR, so they did not exist to be migrated and
came through the merge as the only two raw exits left in the file — caught by
the new local/require-registered-exit rule. Both are ALLOW: a compaction is
never blocked by this hook, which is the policy the rest of the file declares.

Caught only in CI, not locally: `npm run lint` runs eslint with --cache, and
the cached entry for this file predated the new rule, so a warm local cache
reported clean. Re-verified with the cache cleared.

allow() terminates rather than throwing, which matters for the watermark call
site because it sits inside a try/catch — a throwing helper would unwind into
that catch and silently drop the grace-window mute. Verified behaviourally,
not by reading: the grace trio, the skew trio and the non-regular-file rows
all still pass.

Verified: lint:ci exit 0 with a cold eslint cache, full suite exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAbQy7n4mLMB7h3TnZ8GdG

* fix(#3709): harden the routine sentinel writes and the read beside them

Round 7 ruled that the three routine debounce-accounting writes to the warn
sentinel must match the three writes this PR already hardened: leaving the
fourth unhardened beside them is the asymmetry that invites the defect back.
They now go through one writeSentinel() helper using the compaction
watermark's own unlink-then-O_EXCL shape, rather than a second policy — the
unlink removes any existing object, and O_EXCL then refuses to create through
one, so a write can only land on a fresh regular file this process made.

The routine READ beside them was the last bare readFileSync on warnPath, and
the same rationale applies to it verbatim; the watermark's read was hardened in
round 4 for exactly this reason. Same lstat + O_NOFOLLOW + size bound. Its
scope is stated in the test rather than overclaimed: lstat establishes that the
sentinel is a plain regular file, not that it is trustworthy, so a cross-owner
regular file at the predictable path is still read and is left as a disclosed
pre-existing residual.

Also fixes an accept-direction regression this PR introduced and six rounds of
review missed. readEventName collapsed an ABSENT event name and a MALFORMED one
onto the same '', and the preserved Gemini fallback keys off eventName === "",
so with GEMINI_API_KEY set a malformed payload began emitting an AfterTool
envelope. At the merge-base, data.hook_event_name.trim() threw on a truthy
non-string after the side effects and nothing was ever emitted. Measured
base-vs-head with a fresh sentinel per run: 42, ['PreCompact'] and {} all went
silent -> EMITS, while an absent name and 'PostToolUse' were unchanged.
readEventName now returns '' only for an absent name and null for a
present-but-non-string one; both call sites compare for equality only, so every
well-formed payload behaves identically.

Five new rows, each proven fail-first with the mutations attributed separately:
reverting the writes reds the write-through and non-regular rows, reverting the
read reds the mute and non-regular rows, and reverting the absent/malformed
split reds the Gemini row. The changeset's "behaves like a fresh session" is
narrowed to name the 60-second suppression window and the best-effort reset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018FUAVz49BghqxoJgwt7EW9

* test(#3709): pin both 4096-byte read bounds at their boundaries

Round 8 asked for limit-1/limit/limit+1 coverage on the size bound the
round-7 sentinel read-hardening introduced (gsd-context-monitor.js:335).
The existing refusal row pads to 8192 -- a full 4096 bytes clear of the
fence -- so `>` vs `>=`, or an off-by-one in the constant itself, was
invisible to it.

Covers the sibling bound too. The identical check guards the round-4
WATERMARK read at :278 and its refusal row pads to 8192 in exactly the
same way; the review's own rationale (this file already holds
WATERMARK_SKEW_SECONDS to a boundary trio, so an uncovered bound is the
odd one out) applies to it unchanged. That half is a class sweep of a
pre-existing bound and is test-only -- say the word and it comes out
without touching the rest.

Both trios assert on observable hook output rather than an internal
error. Sentinel: an honored {callsSinceWarn:1,lastLevel:'warning'} keeps
the debounce arm taken at remaining=30, so nothing is emitted, while a
refused one falls back to first-warn defaults and emits. Watermark:
honored mutes (stdout empty), refused leaves the warning. The 4097 row is
the non-vacuity control for the two accept rows. Payloads are sized by
measurement, with Buffer.byteLength asserted to equal the target, not by
arithmetic on an assumed prefix width.

Proven fail-first in both directions, with the hook restored after:
`> 4096` -> `>= 4096` reds both trios (94/96); `> 4096` -> `> 4097` reds
both trios (94/96); restored, 96/96. Under both mutations only the two
new rows fail -- the pre-existing 8192-padded rows stay green, which is
the review's fencepost claim demonstrated rather than assumed.

* fix(#3709): correct the changeset's mute-window claim and a superseded comment

Both from the pre-push Codex pass on the full PR.

The changeset said readings are "suppressed for up to 60 seconds after a
compaction starts". That is false at the accepted skew boundary, and this
repo's own docs/context-monitor.md already carried the accurate figure:
first recovery is watermark+61s with no skew and watermark+66s for a
watermark at the +5s skew limit. Measured independently at +64 silent,
+65 silent, +66 warning. The changeset now states the window plus the
accepted skew, matching the doc rather than contradicting it.

A comment in the malformed-event row still described readEventName as
returning "" for every malformed name. Round 7 superseded that: a
present-but-non-string name returns null and only an ABSENT one returns
"", so a malformed payload can no longer reach the Gemini fallback at
all. Marked as historical and corrected. The GEMINI_API_KEY pin stays --
the row is about readEventName's typing, not the fallback, and an ambient
key would still change what it measures.

Codex's three Major findings are not taken, on attribution rather than
logic; the reasoning is in the PR reply. In short: the watermark does not
exist at the merge-base at all (0 occurrences), so "base emits, HEAD
mutes" compares a new feature against its absence rather than showing a
regression; and the base sentinel read is a bare readFileSync, which
blocks on a planted FIFO exactly as the hardened read would, so the
TOCTOU stall is not introduced here. The underlying limits -- watermark
provenance, and lstat->open races on a non-symlink substitution -- are
real, pre-existing, and already offered to the maintainer as follow-ups.

* fix(#3709): read both sentinels through one hardened helper; state the two limits precisely

Round 9's Major, with a correction to its premise, and both Minors.

The review names "watermark read/write helpers this PR adds" that a call
site at :238-250 duplicates inline. There are no such helpers: this PR
adds readEventName and writeSentinel, the latter a write-side primitive a
read cannot call, and :238-248 is base code the diff never touched. What
IS duplicated is the hardened READ. The watermark read (round 4) and the
warnPath read (round 7) are the same ten lines twice -- lstat, isFile and
a 4096-byte bound, O_RDONLY|O_NOFOLLOW, readSync, close -- differing only
in the path variable and the error string, and that is two copies to keep
in step by hand. Now one function, readSentinel(target), beside
writeSentinel. Refusal throws; both callers already wrapped the read in a
try/catch that degrades to "no file", so behaviour is unchanged by
construction.

Proven rather than assumed: with the helper replaced by a bare
readFileSync in a complete scratch tree, exactly the five hardened-read
rows in tests/perf-317-context-monitor-fs.test.cjs go red -- round 7's
symlinked and non-regular sentinel and its size bound, round 4's
non-regular watermark, round 8's watermark size bound -- so the helper
carries both call sites' guarantees and the rows pin it. 96/96 with the
helper in place.

Minor, drop vs delay: the grace-window comment said "dropped" on one line
and "delayed" three lines later, and docs/context-monitor.md said
"delayed". A genuine exhaustion reading inside the window is skipped, not
queued: its warning and its #1974 breadcrumb both fire on the next reading
after the window, so both are delayed when a later reading comes and lost
when none does -- a session ending inside the window records neither.
Comment and docs now say exactly that, and that the loss is accepted over
trusting a reading that may be the pre-compaction value under a fresh
timestamp.

Minor, ordering: the PreCompact unlink and the debounce
writeSentinel(warnPath) are two writers with nothing serialising them; a
debounce invocation that read pre-compaction state and lands its write
after the unlink would resurrect the sentinel the reset removes. The hook
relies on the host dispatching a session's hooks one at a time, which
Claude Code does and the other runtimes are assumed to. Stated at the
reset as an assumption, with the lock-file alternative named and not
taken.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* fix(#3709): write the compaction watermark through writeSentinel

Review of #3808, round 10. The PreCompact watermark write was the block
writeSentinel was lifted from in round 7, and it kept its own inline copy
of unlink-then-O_EXCL a few lines below the helper. Round 9 flagged that
write-side duplication; the round-9 reply misread it as the read side and
unified only the reads. The write now calls the helper too, so the hook
holds one copy of the hardened write, not two.

Behaviour is unchanged: same unlink-then-O_EXCL sequence, same flags,
same best-effort outer catch. The one difference is that writeSentinel
closes the descriptor in a finally, where the inline copy leaked it if
writeSync threw before closeSync.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7bQLXAKubb4EFLtCPiLiL

* fix(#3709): read the statusline bridge through the same hardening as the sentinels

Review of #3808, round 11. `metricsPath` is built one line from `warnPath` and
`watermarkPath` — same tmpdir, same predictable `claude-ctx-{sessionId}` shape,
same threat model this PR documents at length for its siblings — and it is the
only one of the three read on EVERY invocation. It was also the only one still
reached by a bare `readFileSync`, so the symlink-follow and the symlink-to-FIFO
stall that rounds 4 and 7 closed on the other two stayed reachable here, on the
file's highest-traffic path. It now goes through `readSentinel` like the rest.

The 4096-byte bound is ample for it: the statusline writes four fixed fields
(`gsd-statusline.js`), about 140 bytes with a UUID session id, so no legitimate
bridge approaches it. A refusal lands in the same rethrow an unreadable or
malformed bridge already did.

The comment introducing `readSentinel` claimed the warn sentinel was "the one
bare readFileSync". Read as scoped to `warnPath` that was true, but it reads as
a claim about the file and it is not one — the bridge kept its own until this
round. Corrected rather than left to mislead the next reader.

Round 11 Minor: `readSentinel` discarded `fs.readSync`'s return value and
assumed the buffer was full, so a file truncated between the `lstat` and the
read left a zero-filled tail. It now refuses a short read. Stated plainly
because it was measured: this guard has NO observable behavioural delta —
deleting it leaves the new row green, because the NUL tail makes `JSON.parse`
throw one line later and both paths degrade to "no sentinel". It is a
consistency fix in a function whose purpose is refusing to trust what it read,
and the test comment says exactly that rather than implying coverage it lacks.

Five rows added: the bridge refusing a planted symlink (with an attacker-chosen
reading that WOULD warn if followed, so silence is proof), a non-regular bridge,
an oversized bridge, the shrink path end to end, and the direction that matters
most — a healthy bridge still warns, so the hardening is not a mute. Proven by
mutation: reverting the bridge to `readFileSync` reddens two rows. The shrink
injection carries an engagement marker for the same reason the lstat-claim one
does, learned the same way: the hook rewrites the sentinel later in the
invocation, so a size check afterwards passes whether the truncation landed or
not.

An independent full-PR pass on this round added two more, both taken:

`writeSentinel` discarded `fs.writeSync`'s return value, and a short write is
permitted by the syscall — so a truncated sentinel could reach disk and every
later read would reject it, silently losing the debounce accounting or the
watermark this write exists to record. It now loops until the payload is
written, as Node's own `writeFileSync` does, with an explicit no-progress guard.
Pinned by a row that injects a one-byte first write; reverting the loop reddens
it.

The directory row's comment claimed it pinned the `lstat` isFile() check. It
does not — measured: deleting that condition leaves the row green, because
reading a directory fails on its own a line later. The comment now says the row
pins the outcome, and names the symlink row as the one that pins isFile().

DISCLOSED, NOT FIXED HERE — a session id long enough to push the derived
filenames past NAME_MAX. The bridge is `claude-ctx-{id}.json`; the sentinel and
watermark add longer suffixes, so on a 255-byte limit the watermark stops fitting
at a 230-character id and the sentinel at 233. Measured base-vs-HEAD at 233+:
base is SILENT, HEAD emits the warning, because the bare `writeFileSync` base
used threw ENAMETOOLONG out of the warning path while `writeSentinel` degrades
best-effort and lets the warning through. That is an accept-direction delta and
it is in the delivering direction — base swallowed a warning the user should
have seen, which is this issue's own failure class. The underlying limit is a
property of the per-session filename scheme, shared by two files that predate
this PR, and bounding session ids belongs to whatever writes them
(`gsd-statusline.js`), not to the sentinel logic. Happy to fold a length guard
in here if you would rather have it in this PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CRMEuzNMWn3gs5uUW2ghcF

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
Behruz Nassre Esfahani
2026-09-05 03:29:06 -07:00
committed by GitHub
parent f9bb489363
commit 5d804dd287
6 changed files with 1809 additions and 11 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3808
---
**A context compaction no longer permanently disables context-warning escalation** — the monitor's per-session warn state survived `PreCompact`, so after a session's first CRITICAL the immediate-first-warning and WARNING→CRITICAL escalation rules were dead for the rest of the run, and the #1974 resume breadcrumb kept describing the wrong near-miss. A compaction now clears that state, deletes the statusline reading that produced it, and writes a compaction watermark so a reading the statusline re-creates mid-compaction — the old value under a fresh timestamp — is dropped instead of trusted. Escalation and the immediate-first-warning rule are live again on the next cycle. Two bounds on that, both deliberate: readings are suppressed for the 60-second window after a compaction starts plus any accepted clock skew, so first recovery is the watermark plus 61 seconds with no skew and plus 66 seconds for a watermark at the +5s skew limit, because a mid-compaction statusline render is indistinguishable from a genuine post-compaction reading, so a compaction outlasting that window can still surface one stale reading; and the reset is best-effort, degrading to the previous narrowing rather than failing the compaction if the filesystem refuses it. All three of the monitor's per-session files in the temp directory — the statusline bridge, the warn sentinel and the compaction watermark — are now read through one hardened path that refuses anything that is not a plain, bounded regular file, closing a symlink-follow and stall exposure on the bridge read that runs for every tool call. The watermark is read for its shape and sanity, not its writer: a plain regular file planted at the path is honored for at most one window plus the skew, the same bounded residual the warn sentinel already carries. (#3709)

View File

@@ -13,6 +13,13 @@ The statusline shows context usage to the **user**, but the **agent** has no awa
3. When remaining context drops below thresholds, it injects a warning as `additionalContext`
4. The agent receives the warning in its conversation and can act accordingly
The hook is also registered for other lifecycle events on some hosts — including
`PreCompact` (#772). Those events never emit a warning, because only the
injection-capable events accept the `additionalContext` envelope. `PreCompact` is
handled specially: it resets the per-session state described under
[Debounce](#debounce) and returns immediately, without running the debounce or
breadcrumb bookkeeping.
## Thresholds
| Level | Remaining | Agent Behavior |
@@ -27,6 +34,39 @@ To avoid spamming the agent with repeated warnings:
- First warning always fires immediately
- Subsequent warnings require 5 tool uses between them
- Severity escalation (WARNING -> CRITICAL) bypasses debounce
- A context compaction (`PreCompact`) resets this state, so the cycle after a
compact behaves like a fresh session: its first warning fires immediately and
its WARNING -> CRITICAL escalation bypasses debounce again. Without the reset
both rules above would be dead for the rest of the session once a CRITICAL had
fired, since the escalation test is "the previous level was WARNING" (#3709).
### PreCompact reset
The compaction reset does four things together:
| what | why |
|---|---|
| clears the debounce counter and last-seen severity | a compact restarts the context lifecycle, so the next climb is a fresh cycle |
| clears the one-time critical-session guard | otherwise the resume breadcrumb keeps describing the earlier near-miss rather than the exhaustion that actually ended the run (#1974) |
| deletes the statusline metrics file | it holds the pre-compaction reading, and metrics stay "fresh" for 60s — a warning fired off it right after the compaction would be exactly backwards |
| writes a compaction **watermark** (`claude-ctx-{session_id}-compacted.json`) | deleting the bridge only narrows the stale-reading window: the statusline re-writes the bridge on every render, so a render landing mid-compaction re-creates the pre-compaction reading with a current timestamp. The watermark records the compaction's *start*, and the monitor drops every reading inside a grace window (60s) past it; an unstamped reading (no/zero timestamp) is dropped too. The window **narrows** the race rather than closing it: 60s is a heuristic bound, not a measured maximum, so a compaction running longer than the window can still be followed by a render that passes both the watermark and staleness gates. The cost is bounded but not by the window alone: a healthy reading dropped in the window behaves identically to an accepted one. A genuine exhaustion reading inside the window is *skipped, not queued*: its warning and its #1974 resume breadcrumb both fire on the next reading after the window, so they are delayed by up to the window plus the accepted clock skew when a later reading comes (measured: first recovery is watermark+61s with no skew, watermark+66s for a watermark at the +5s skew limit), and lost when none does — a session that ends inside the window records neither. That loss is accepted over trusting a reading that may be the pre-compaction value under a fresh timestamp. An aborted compaction is muted for the same period — nothing in the event distinguishes abort from success. A watermark more than 5s ahead of the reader's clock is discarded as insane (a stray or clock-stepped file must not mute the monitor); one within that skew is honored, which is why it can extend the delay. A watermark that is not a plain regular file — a symlink, a directory, an oversized file — is never followed, and neither is the statusline bridge or the warn sentinel: all three per-session files in that directory are read through the same hardened path (round 11). A plain regular file planted at the predictable path *is* honored for its window: the reader checks the object's shape and sanity, not who wrote it, so a same-user (or, in a shared sticky tmpdir, cross-owner) planted watermark mutes the monitor for at most the window plus the accepted skew (65s) per planting. That is the same residual the warn sentinel at the sibling path already carries, bounded here by the window; refusing it needs an ownership check, which is a different policy than this file's |
Properties of the reset worth knowing:
- It runs even when `hooks.context_warnings` is `false`. Clearing this state is
cleanup, not a warning, and it emits nothing — but config is re-read on every
invocation, so a session that disables warnings, compacts, then re-enables them
would otherwise resurrect the stale state.
- `PreCompact` fires *before* the compaction. If a compaction is aborted, the
state has already been reset. The effect is mild: one extra immediate warning,
and the breadcrumb guard re-armed so a later, more current breadcrumb can
replace the old one.
- The reset covers **compaction only**. No other context-shrinking path (a
`/clear`, a session restart that reuses the id) fires `PreCompact`, so state
keyed to a surviving `session_id` outlives those; wiring `SessionStart` is
separate work.
- Everything here is best-effort: the reset, the fallback truncation, and the
watermark write all degrade silently rather than ever failing a compaction.
## Architecture
@@ -70,6 +110,14 @@ As a brief reference: the statusline hook registers as `statusLine` in `settings
- It never blocks tool execution — a broken monitor should not break the agent's workflow
- Stale metrics (older than 60s) are ignored
- Missing bridge files are handled gracefully (subagents, fresh sessions)
- A compaction is never blocked by this hook: if the per-session state cannot be
removed (a held file handle on Windows, for instance) the hook *attempts* to
truncate the file to empty in place — which later reads treat exactly like an
absent file — and any remaining error is swallowed. The truncation is
best-effort, not a guarantee: if that open is refused too (or the path is not a
plain regular file, which is never followed) the original file survives and the
stale state persists for that session. Exiting cleanly always wins over
clearing state
---

View File

@@ -34,6 +34,134 @@ const WARNING_THRESHOLD = 35; // remaining_percentage <= 35%
const CRITICAL_THRESHOLD = 25; // remaining_percentage <= 25%
const STALE_SECONDS = 60; // ignore metrics older than 60s
const DEBOUNCE_CALLS = 5; // min tool uses between warnings
// How long after a PreCompact readings stay suspect. The watermark records the
// compaction's START; the compaction keeps running after it, and a statusline
// render during it stamps the PRE-compaction reading with a CURRENT timestamp
// (Codex review of #3808, round 3) — so "newer than the watermark" alone still
// admits it. Everything inside this window is dropped instead. The cost is
// bounded: a healthy reading dropped here behaves identically to an accepted
// one (it would exit above-threshold anyway). A genuine exhaustion reading
// inside the window is SKIPPED, not queued — its warning and its #1974
// breadcrumb both fire on the next reading after the window, so they are
// delayed by at most this window plus the accepted skew below when a later
// reading comes, and lost when
// none does, i.e. when the session ends inside the window (review of #3808,
// round 9). That loss is accepted over the alternative, which is trusting a
// reading that may be the pre-compaction value under a fresh timestamp.
const COMPACT_GRACE_SECONDS = 60;
// How far AHEAD of this process's clock a watermark may be and still be
// honored. PreCompact stamps it from the same clock as the reader, so the
// legitimate skew is 0; this tolerance only absorbs a clock step. It is a
// THRESHOLD, so it is named rather than inlined and carries its own boundary
// trio (Codex review of #3808, round 4). Note it also extends the mute: a
// watermark this far ahead pushes first recovery from +61 to +66 (measured).
const WATERMARK_SKEW_SECONDS = 5;
// One DEFINITION of what counts as a lifecycle event name, shared by the #3709
// PreCompact reset and the #2289 output-envelope allowlist. Two call sites, one
// rule — so the two cannot drift into disagreeing about what "no event name" is.
// TOTAL, and STRICT about type: only an actual string is an event name. The old
// inline expression threw on a truthy non-string, and hoisting it ahead of the
// pipeline would have moved that throw ahead of the side effects #2289
// documents as always running; a String() coercion is no better — it renders
// ['PreCompact'] as 'PreCompact' and would run the reset off a malformed
// payload, and a hostile toString still throws (Codex review of #3808,
// round 3). typeof does neither: any non-string reads as "no event" — silent,
// side effects intact — on both call sites.
function readEventName(data) {
const name = data && data.hook_event_name;
if (typeof name === 'string') return name.trim();
// ABSENT vs MALFORMED are not the same event (Codex review of #3808, round 7,
// measured base-vs-head). A MISSING name is the documented pre-#2289 Gemini
// fallback: under GEMINI_API_KEY it means AfterTool and still emits. A name
// that is PRESENT but not a string is a malformed payload and must not
// inherit that fallback — at the merge-base it threw on `.trim()` after the
// side effects, so no envelope was ever produced, and collapsing both onto ''
// silently turned `42`, `{}` and `['PreCompact']` into emitting AfterTool
// events. Measured: base silent, head emitted, for both `42` and
// `['PreCompact']`. null keeps them distinguishable while staying unequal to
// every event name, so the PreCompact reset and the allowlist below are
// byte-for-byte unchanged for every well-formed payload.
return (name === undefined || name === null) ? '' : null;
}
// SENTINEL WRITE HARDENING (review of #3808, round 7). `warnPath` lives in
// os.tmpdir(), which may resolve to a shared sticky directory — not guaranteed
// per-user, and the file persists across invocations — so an object already
// sitting there may be a planted symlink. The three routine debounce-accounting writes were bare
// writeFileSync, which follows one and writes through to its target, while the
// PreCompact clear and the compaction watermark in this same file already
// refuse to. Unlink-then-O_EXCL is the watermark's own shape (the watermark
// write itself now calls this helper — review round 10): the unlink
// removes any existing object (regular file or link) and O_EXCL then refuses
// to create through one, so the write can only ever land on a fresh regular
// file this process made. Best effort by design — a lost sentinel write costs
// only debounce accounting, which is never worth breaking the hook over, so
// every failure is swallowed exactly as the watermark write's is.
// NOT an atomic read-modify-write, and not claimed to be (Codex review of
// #3808, round 7): two concurrent invocations can read the same state and race
// through unlink/create, so one invocation's accounting can be lost — the same
// lost-update race the bare writeFileSync already had, not a class this change
// introduces. What a lost write leaves behind is whatever the competing writer
// wrote, which may be a perfectly valid sentinel; it does not reliably mean
// "defaults on the next call". Advisory debounce bookkeeping is the right place
// to accept that.
// The read-side twin of writeSentinel (review of #3808, round 9). Both
// sentinel files this hook reads — the compaction watermark and the warn
// state — must be read the same way: lstat first so a planted link, FIFO or
// directory is refused before any open; O_NOFOLLOW so a link raced in between
// is refused by the kernel too (0 on Windows, where lstat already carries the
// check); a 4096-byte bound so a planted large file cannot stall a
// synchronous read. Rounds 4 and 7 each wrote that sequence inline at their
// own call site, which left two copies to keep in step by hand. One place
// now. Refusal THROWS; every caller already wraps the read in a try/catch and
// degrades to "no file", which is the same behaviour the inline copies had.
function readSentinel(target) {
const st = fs.lstatSync(target);
if (!st.isFile() || st.size > 4096) throw new Error('not a plain sentinel');
const fd = fs.openSync(target, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0));
try {
const buf = Buffer.alloc(st.size);
// The RETURN VALUE, not just the call (review of #3808, round 11). A file that shrinks
// between the lstat above and this read — a concurrent legitimate writer truncating
// mid-write, not the planted-object case the rest of this function guards — leaves the tail
// of `buf` zero-filled, and those NULs reach JSON.parse as garbage. Every caller already
// treats a throw here as "no file", so refusing a short read is both safer and the same
// outcome the caller would reach one line later, stated on purpose rather than by accident.
const bytesRead = fs.readSync(fd, buf, 0, st.size, 0);
if (bytesRead !== st.size) throw new Error('sentinel shrank under the read');
return buf.toString('utf8');
} finally { fs.closeSync(fd); }
}
function writeSentinel(target, payload) {
try {
try {
fs.unlinkSync(target);
} catch (e) {
if (!e || e.code !== 'ENOENT') throw e;
}
const fd = fs.openSync(
target,
fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL
);
try {
// LOOP, and check progress (Codex review of round 11). A single `fs.writeSync` is
// permitted to write fewer bytes than it was given, and the return value was discarded —
// a short write left a truncated sentinel that JSON.parse rejects, silently defeating the
// debounce accounting or the compaction watermark this write exists to record. Node's own
// `writeFileSync` loops for exactly this reason; the explicit no-progress guard keeps a
// pathological fd from spinning. Symmetric with the bytesRead check in readSentinel.
const buf = Buffer.from(payload, 'utf8');
let written = 0;
while (written < buf.length) {
const n = fs.writeSync(fd, buf, written, buf.length - written);
if (!(n > 0)) throw new Error('sentinel write made no progress');
written += n;
}
} finally { fs.closeSync(fd); }
} catch (e) { /* best effort — see above */ }
}
let input = '';
// Timeout guard: if stdin doesn't close within 10s (e.g. pipe issues on
@@ -60,6 +188,84 @@ process.stdin.on('end', () => {
allow(undefined);
}
const tmpDir = os.tmpdir();
const warnPath = path.join(tmpDir, `claude-ctx-${sessionId}-warned.json`);
const metricsPath = path.join(tmpDir, `claude-ctx-${sessionId}.json`);
const watermarkPath = path.join(tmpDir, `claude-ctx-${sessionId}-compacted.json`);
// #3709: a compaction RESTARTS the context lifecycle, so neither the warn
// sentinel nor the pre-compaction statusline reading may survive it. Full
// rationale — what dies when the sentinel outlives a compaction, why the
// reset sits ahead of the config gate and the metrics read, and why an
// aborted compaction deliberately stays cleared — lives in ONE place:
// docs/context-monitor.md, "PreCompact reset". Constraints the code itself
// must keep are stated at their lines below.
if (readEventName(data) === 'PreCompact') {
// ORDERING ASSUMPTION, stated rather than enforced (review of #3808,
// round 9): this reset and the debounce writeSentinel(warnPath) further
// down are two writers to the same file, and nothing here serialises
// them. A debounce invocation that read the pre-compaction state and
// lands its write AFTER this unlink would resurrect exactly the stale
// sentinel this block removes. The hook relies on the host dispatching a
// session's hooks one at a time, which Claude Code does; the other
// runtimes this hook is installed for are assumed to, and that is not
// tested. A lock file would close it at the cost of a second file to
// harden on every platform; not taken here.
// BOTH files: with the sentinel gone but the bridge still holding the
// pre-compaction reading (fresh for STALE_SECONDS), the next PostToolUse
// would fire a spurious CRITICAL off a context the compaction just freed
// (review of #3709).
for (const stale of [warnPath, metricsPath]) {
try {
fs.unlinkSync(stale);
} catch (e) {
if (e && e.code === 'ENOENT') continue; // already absent — that IS the reset
// Best-effort fallback for a held handle (Windows EPERM/EBUSY):
// truncate to EMPTY — the one state both readers treat exactly like
// deletion, because JSON.parse('') throws. A well-formed "neutral"
// value is NOT equivalent: '{}' debounces the first post-compaction
// warning, '{"timestamp":0}' is never stale (falsy guard) and emits
// "undefined%" (review of #3808). Never through a LINK: lstat
// rejects non-regular files on every platform (Windows has no
// effective O_NOFOLLOW — libuv defines it as 0 — and TEMP/TMP means
// its tmpdir is not guaranteed per-user); O_NOFOLLOW additionally
// closes the lstat→open substitution race where honored. Every
// refusal lands in this give-up arm — including a Windows runner
// refusing the write-open of a freshly written file outright —
// which is why the fallback is best-effort, never asserted-on.
try {
if (fs.lstatSync(stale).isFile()) {
fs.closeSync(fs.openSync(
stale,
fs.constants.O_WRONLY | fs.constants.O_TRUNC | (fs.constants.O_NOFOLLOW || 0)
));
}
} catch (e2) { /* give up, never throw */ }
}
}
// COMPACTION WATERMARK (review of #3808, round 3). Deleting the bridge
// only NARROWS the stale-reading window: the statusline is an
// uncoordinated process that re-writes the bridge on every render, so a
// render landing between this clear and the compaction's completion
// re-creates the PRE-compaction reading with a CURRENT timestamp — and
// it would sail past STALE_SECONDS as freshly valid. The watermark makes
// the pre-compaction reading identifiable rather than merely absent: the
// metrics read drops any reading not strictly newer than it. Written
// through writeSentinel (review of #3808, round 10 — this block was the
// shape writeSentinel was lifted from in round 7 and kept its own copy):
// unlink-then-O_EXCL so an existing file — or a planted symlink — is
// never followed or overwritten in place; failure to write degrades to
// the old narrowing, never throws.
writeSentinel(watermarkPath, JSON.stringify({ at: Math.floor(Date.now() / 1000) }));
// allow(), not raw process.exit: #3911/ADR-3889 moved this hook onto the
// declared-policy exit vocabulary while this PR was in review, and the
// PreCompact branch is new here, so it needs the same conversion.
// A compaction is never blocked by this hook — ALLOW is the policy the
// rest of the file already declares.
allow(undefined);
}
// Check if context warnings are disabled via config.
// Collapsed existsSync+readFileSync into a single read guarded by try/catch
// (ENOENT or parse error → use defaults, same as old "planningDir absent" branch).
@@ -74,15 +280,22 @@ process.stdin.on('end', () => {
// Missing or unparseable config → proceed with defaults (context warnings enabled)
}
const tmpDir = os.tmpdir();
const metricsPath = path.join(tmpDir, `claude-ctx-${sessionId}.json`);
// If no metrics file, this is a subagent or fresh session -- exit silently.
// Collapsed existsSync+readFileSync: ENOENT → exit 0 (identical to old !existsSync branch),
// other errors rethrow to the outer catch (swallowed → exit 0, as before).
//
// Through readSentinel, like the other two (review of #3808, round 11). This read was the
// asymmetry left in this file: `metricsPath` is built one line away from `warnPath` and
// `watermarkPath` (same tmpdir, same predictable `claude-ctx-{sessionId}` shape), it is the
// only one of the three read on EVERY invocation, and it was the only one still reached by a
// bare readFileSync — so the symlink-to-FIFO stall the other two are hardened against was
// still reachable here, on the highest-traffic path in the file. The 4096-byte bound is
// ample: the statusline writes four fixed fields (`gsd-statusline.js`, ~140 bytes with a
// UUID session id), so no legitimate bridge approaches it. A refusal throws and lands in the
// rethrow below exactly as an unreadable or malformed bridge already did.
let metricsRaw;
try {
metricsRaw = fs.readFileSync(metricsPath, 'utf8');
metricsRaw = readSentinel(metricsPath);
} catch (e) {
if (e && e.code === 'ENOENT') allow(undefined);
throw e;
@@ -90,6 +303,46 @@ process.stdin.on('end', () => {
const metrics = JSON.parse(metricsRaw);
const now = Math.floor(Date.now() / 1000);
// #3709 (round 3): a reading not clearly PAST the compaction is suspect,
// whatever its timestamp says — the statusline re-writes the bridge on
// every render, and a render during the compaction stamps the OLD
// remaining_percentage with a current time. The watermark records the
// compaction's START, so "newer than the watermark" alone still admits a
// mid-compaction render (Codex review of #3808, round 3): the grace
// window covers the compaction's own duration. `!(>)` rather than `<=` so
// a missing/zero/garbage timestamp is also dropped once a compaction has
// happened — an unstamped reading cannot prove it is post-compaction.
//
// The watermark itself must be SANE to count: one stamped in the future
// (a clock step backwards, a stray file) would otherwise drop every
// reading indefinitely and silently self-disable monitoring — so it is
// honored only when its own timestamp is not ahead of this process's
// clock (small skew allowed). No watermark, an unreadable one, or an
// insane one all degrade to the plain STALE_SECONDS behaviour below.
//
// READ HARDENING (Codex review of #3808, round 4). The WRITE side already
// refuses to follow or overwrite a planted object (unlink-then-O_EXCL
// above), but this read was a bare readFileSync — so on any write-side
// give-up the planted object survived and every later invocation followed
// it. In a shared sticky os.tmpdir() that is a mute primitive (a planted
// recent watermark suppresses monitoring) and a stall primitive (a symlink
// to a FIFO blocks this synchronous read indefinitely; measured: such a
// read is still running after 300ms). The same lstat + O_NOFOLLOW pair the
// sentinel path uses, plus a size bound, applied to the file this PR adds.
// Every refusal degrades to "no watermark", never throws.
try {
const watermark = JSON.parse(readSentinel(watermarkPath));
if (
watermark && typeof watermark.at === 'number'
&& watermark.at <= now + WATERMARK_SKEW_SECONDS
&& !(metrics.timestamp > watermark.at + COMPACT_GRACE_SECONDS)
) {
// Same #3911/ADR-3889 conversion as the PreCompact branch above: this
// gate is new in this PR, so it did not exist to be migrated.
allow(undefined);
}
} catch (e) { /* no watermark — nothing to compare against */ }
// Ignore stale metrics
if (metrics.timestamp && (now - metrics.timestamp) > STALE_SECONDS) {
allow(undefined);
@@ -103,15 +356,32 @@ process.stdin.on('end', () => {
allow(undefined);
}
// Debounce: check if we warned recently
const warnPath = path.join(tmpDir, `claude-ctx-${sessionId}-warned.json`);
// Debounce: check if we warned recently. `warnPath` is resolved above, next to
// metricsPath, because the PreCompact reset needs it before this point.
let warnData = { callsSinceWarn: 0, lastLevel: null };
let firstWarn = true;
// Collapsed existsSync+readFileSync: ENOENT or parse error → keep default warnData
// (same as old "file absent" branch). firstWarn tracks whether we read a valid sentinel.
//
// READ HARDENING (self-found while addressing round 7; same class, same
// file). Hardening the writes above leaves this read as a bare
// readFileSync on warnPath, which is the exact asymmetry round 7 asks be
// removed from the write side — and the watermark's read was hardened in
// round 4 for this same reason, so leaving this one recreates it. It was
// not the LAST bare read in the file: the statusline bridge kept its own
// until round 11 found it. All three go through readSentinel now. The
// exposure is real but bounded: the writes now unlink any planted object,
// so only a read reaching this line BEFORE the first write of an
// invocation can follow one, and re-planting reopens it every invocation.
// Following it is a mute primitive — attacker-chosen callsSinceWarn keeps
// the debounce arm below taken so no warning is ever emitted — and a
// symlink to a FIFO stalls this synchronous read, the same two primitives
// measured on the watermark. Same lstat + O_NOFOLLOW + size bound; every
// refusal degrades to the default warnData this catch already produces,
// so a normal regular file behaves exactly as before.
try {
warnData = JSON.parse(fs.readFileSync(warnPath, 'utf8'));
warnData = JSON.parse(readSentinel(warnPath));
firstWarn = false;
} catch (e) {
// Missing or corrupted sentinel → firstWarn stays true, warnData stays at defaults
@@ -127,14 +397,14 @@ process.stdin.on('end', () => {
const severityEscalated = currentLevel === 'critical' && warnData.lastLevel === 'warning';
if (!firstWarn && warnData.callsSinceWarn < DEBOUNCE_CALLS && !severityEscalated) {
// Update counter and exit without warning
fs.writeFileSync(warnPath, JSON.stringify(warnData));
writeSentinel(warnPath, JSON.stringify(warnData));
allow(undefined);
}
// Reset debounce counter
warnData.callsSinceWarn = 0;
warnData.lastLevel = currentLevel;
fs.writeFileSync(warnPath, JSON.stringify(warnData));
writeSentinel(warnPath, JSON.stringify(warnData));
// Detect if GSD is active (has .planning/STATE.md in working directory)
const isGsdActive = fs.existsSync(path.join(cwd, '.planning', 'STATE.md'));
@@ -161,7 +431,7 @@ process.stdin.on('end', () => {
).unref();
warnData.criticalRecorded = true;
// Persist the sentinel so subsequent debounce cycles don't re-fire
fs.writeFileSync(warnPath, JSON.stringify(warnData));
writeSentinel(warnPath, JSON.stringify(warnData));
} catch { /* non-critical — don't let state recording break the hook */ }
}
@@ -198,7 +468,7 @@ process.stdin.on('end', () => {
// not enough — a missing name would still fall through to the injection path.
// All side effects above (debounce counter, one-time critical-session
// recording) have already run regardless of whether output is emitted.
const eventName = (data.hook_event_name && data.hook_event_name.trim()) || "";
const eventName = readEventName(data);
// Preserve the pre-#2289 Gemini fallback: a missing event name under a
// Gemini-dialect runtime (GEMINI_API_KEY set) still means AfterTool, so its
// advisory output is unchanged. A missing name on any other host is silent.

View File

@@ -0,0 +1,24 @@
'use strict';
/**
* Preload fixture for #3808 — pin the SPAWNED context-monitor hook's
* `Date.now()` to `GSD_TEST_NOW_MS`, so any wall-clock boundary can be driven
* at exactly limit-1 / limit / limit+1. Added for STALE_SECONDS (review round 3,
* Major 2); now also drives AC2, AC3, the COMPACT_GRACE_SECONDS trio and the
* WATERMARK_SKEW_SECONDS trio, which is what lets those rows use the real
* writer's timestamp shape instead of a future stamp (round 4, Major 1).
*
* Without this, the age of a bridge written by the parent drifts by however
* long the child takes to start: a fixture built at age 60 can be read at 61
* and flip the row's verdict — the boundary is REAL wall-clock. Pinning the
* child's clock makes the arithmetic exact. Same `--require` seam as the
* EPERM preload; one-shot subprocess, no restoration needed. Only Date.now is
* patched — `new Date()` stays real, which is fine: the hook uses it only to
* render a date string inside the breadcrumb text, never for comparisons.
*/
const fixed = Number(process.env.GSD_TEST_NOW_MS || '');
if (Number.isFinite(fixed) && fixed > 0) {
Date.now = () => fixed;
}

View File

@@ -0,0 +1,114 @@
'use strict';
/**
* Preload fixture for #3709 review Blocker 2 — force `fs.unlinkSync` to throw
* EPERM inside the SPAWNED gsd-context-monitor.js hook, so the PreCompact
* unlink-failure fallback branch is actually executed.
*
* `GSD_TEST_UNLINK_EPERM_MATCH` selects WHICH unlink fails: any path containing
* the substring throws EPERM; every other path unlinks for real. That lets one
* row fail only the warn sentinel (`-warned.json`) and another only the metrics
* bridge, so each half of the fallback is pinned separately.
*
* Loaded via `node --require <this file> hooks/gsd-context-monitor.js` — the
* same seam as `shadow-report-throws-preload.cjs`, and for the same reasons:
* this is NOT a chmod/mode-bit trick (CONTRIBUTING.md: those no-op under root,
* so Docker/CI would pass the test with zero coverage), and NOT the in-process
* `withFaultyFs` seam (in-process-only — a spawned subprocess offers no shared
* memory to monkeypatch into). One-shot subprocess: no restoration needed.
*/
const fs = require('fs');
const realUnlinkSync = fs.unlinkSync;
const match = process.env.GSD_TEST_UNLINK_EPERM_MATCH || '';
if (match) {
fs.unlinkSync = function unlinkSyncWithInjectedEperm(p) {
if (String(p).includes(match)) {
const err = new Error(`EPERM: operation not permitted, unlink '${p}'`);
err.code = 'EPERM';
throw err;
}
return realUnlinkSync.apply(fs, arguments);
};
}
// `GSD_TEST_LSTAT_CLAIMS_FILE_MATCH`: make lstat CLAIM a regular file for
// matching paths — the lstat→open substitution-race shape (a symlink swapped in
// after the lstat), so the O_NOFOLLOW backstop is the guard actually under test
// (review of #3808, round 3, Minor 3). The real stat object is returned with
// only isFile overridden; everything else stays truthful.
const lstatMatch = process.env.GSD_TEST_LSTAT_CLAIMS_FILE_MATCH || '';
if (lstatMatch) {
const realLstatSync = fs.lstatSync;
fs.lstatSync = function lstatSyncClaimingFile(p) {
const st = realLstatSync.apply(fs, arguments);
if (String(p).includes(lstatMatch)) {
st.isFile = () => true;
// Engagement marker: without it, a match string that silently stops
// matching lets the REAL lstat refuse the symlink and every assertion
// in the substitution-race row still passes — without O_NOFOLLOW ever
// being the guard under test (Codex review of #3808, round 3). The row
// asserts this file exists.
try {
fs.writeFileSync(`${p}.gsd-test-lstat-claimed`, '1');
} catch { /* marker is best-effort; the row will fail loudly without it */ }
}
return st;
};
}
// `GSD_TEST_SHRINK_AFTER_LSTAT_MATCH`: TRUNCATE a matching file immediately after
// `lstatSync` has measured it, so the subsequent `readSync` returns FEWER bytes than
// the size the guard allocated for. That is the shape round 11's Minor names — a
// concurrent legitimate writer truncating mid-write, independent of the planted-object
// case the rest of readSentinel guards — and it cannot be produced by ordinary file
// setup, because the shrink has to land inside the window between the two calls.
//
// Layered over the lstat wrapper above rather than folded into it: the two injections
// are selected by different env vars and a row may want either alone.
const shrinkMatch = process.env.GSD_TEST_SHRINK_AFTER_LSTAT_MATCH || '';
if (shrinkMatch) {
const beforeShrink = fs.lstatSync;
fs.lstatSync = function lstatSyncThenShrink(p) {
const st = beforeShrink.apply(fs, arguments);
if (String(p).includes(shrinkMatch) && st.isFile() && st.size > 1) {
// Truncate to a single byte: the read still succeeds, it just returns 1 instead
// of `st.size`, which is exactly the short read the guard must refuse.
//
// Engagement marker, same reason as the lstat-claim one above and learned the same way:
// the hook REWRITES this sentinel later in the same invocation, so a row that checked the
// file's size afterwards would see it back at full length and pass whether the truncation
// landed or not. The marker is the only durable evidence that this injection fired.
try {
fs.truncateSync(p, 1);
fs.writeFileSync(`${p}.gsd-test-shrunk`, '1');
} catch { /* best effort — the row asserts the marker and fails loudly without it */ }
}
return st;
};
}
// `GSD_TEST_SHORT_WRITE_MATCH`: make `fs.writeSync` write only the FIRST BYTE for a
// matching fd's payload, once. A short write is permitted by the syscall and was
// previously discarded, so a truncated sentinel reached disk and every later read
// rejected it — the debounce accounting and the compaction watermark silently lost.
// The injection fires once so the retry loop under test can complete normally.
const shortWriteMatch = process.env.GSD_TEST_SHORT_WRITE_MATCH || '';
if (shortWriteMatch) {
const realWriteSync = fs.writeSync;
let fired = false;
fs.writeSync = function writeSyncShort(fd, buf, off, len) {
if (!fired && Buffer.isBuffer(buf) && len > 1) {
fired = true;
try { fs.writeFileSync(`${shortWriteMatch}.gsd-test-short-write`, '1'); } catch { /* marker best effort */ }
return realWriteSync.call(fs, fd, buf, off, 1);
}
return realWriteSync.apply(fs, arguments);
};
}

File diff suppressed because it is too large Load Diff