Skip to content

fix(health-check): a stale quota-state.json reads green forever - #2445

Merged
john-the-dev merged 1 commit into
mainfrom
fix/quota-telemetry-stale-reads-green
Jul 31, 2026
Merged

john-the-dev merged 1 commit into
mainfrom
fix/quota-telemetry-stale-reads-green

Conversation

@sonichi

@sonichi sonichi commented Jul 31, 2026

Copy link
Copy Markdown
Owner

What's wrong

check_quota_telemetry exists to catch "the credential proxy is up and nothing is routing through it." Its evidence is path.exists() — and presence is satisfied by a HISTORICAL write.

So it detects only the never-wired shape. A host that WAS wired, wrote quota-state.json once, and then lost the wiring keeps that file on disk forever and reports ok forever: the exact condition the check was built to make loud, in the one shape it cannot see.

Evidence — live on Echo Act IV Pro, same host, same moment

Before (origin/main src/health-check.py):

  quota-state.json  : 323h old
  core-status.json  : 3m old
  -> status=ok
  -> quota state present (updated 19399m ago)

After (this branch):

  -> status=warn
  -> quota state is 323h stale while the agent is working (3m since the last loop
     pass) — the proxy is up but nothing is routing through it any more, so the
     file on disk is a leftover from when it was. Quota-based budgeting is quoting
     stale numbers as current. Check ANTHROPIC_BASE_URL on the running core
     (exported by src/startup.sh; a supervisor-launched core never runs it).

Both produced by importing each version of the module against the real workspace and calling check_quota_telemetry("ok").

Why it matters, concretely. For the last 13 days the proactive loop's step-0.5 budget gate has been reading that file and quoting "96% remaining, resets 16:40 Jul 17" as current — driving the FULL / MEDIUM / LIGHT decision on every pass — while health-check reported credential-proxy ok and quota-telemetry ok the whole time.

Why not just add an age threshold

Refusing one was deliberate in #2212 and pinned by test_old_quota_state_does_not_warn: a quiet core legitimately writes nothing for a long time, so a bare age threshold fires on healthy idle hosts. That reasoning is correct and is preserved — age alone still never warns.

Staleness only becomes evidence once the quiet-core explanation is ruled out, so the new branch is gated on the agent being demonstrably at work. core-status.json is rewritten on every loop pass, and each of those writes belongs to a turn that spent tokens. Fresh core-status + stale quota-state means requests are flowing while quota headers are not landing — wiring, not quiet. An idle host has a stale core-status too, so it stays silent.

Only the mtime is read, never the status field: a pass that ends by writing {"status": "idle"} still ran, and reading the value would count a pass only while mid-flight.

The clearest proof the pinned decision survives: test_old_quota_state_does_not_warn passes byte-identical. Its 3-day-old file with no agent-activity evidence still reads ok, because "unknown" is kept distinct from both "idle" and "working".

Tests

+7 cases. The one that matters (test_stale_quota_while_agent_is_working_warns) fails on the parent commit and passes here. The other six pin every direction this must stay silent, so it cannot become the flaky check #2212 was right to avoid:

case expected
stale quota + agent working warn ← the bug
stale quota + idle host (stale core-status) ok
stale quota + no core-status (unknown) ok
stale quota + unreadable core-status ok
fresh quota + agent working ok
just under the 6h threshold ok
proxy down (short-circuits first) ok

Checks

  • python3 tests/health-check-quota-telemetry.test.py — 12 tests, OK (5 pre-existing unchanged + 7 new)
  • diff coverage 100%, 0 missing of 17 changed lines (diff-cover vs origin/main, from the targeted test alone — a lower bound, so the full suite can only match or exceed it)
  • git diff --check clean; scripts/review-checks.sh PASS (hardcoded-paths clean); py_compile ok

Scope

Single concern, 2 files, no behavior change outside this one check. Related but not included: #2417 (routing the core through the proxy) is the fix for the underlying wiring on this host; this PR is only about the check no longer lying while that's broken.

@sonichi — flagging you: this touches a decision you merged in #2212, and the argument is that the decision was right but its evidence (exists()) was too weak for its own stated purpose. The pinned test passing unchanged is the thing to check if you want one signal.

Stand: Echo Act IV Pro

`check_quota_telemetry` exists to catch "the credential proxy is up and
nothing is routing through it". It detects only the NEVER-WIRED shape,
because its evidence is `path.exists()` — and presence is satisfied by a
HISTORICAL write. A host that was wired, wrote quota-state.json once, and
then lost the wiring keeps that file on disk forever and reports `ok`
forever: the exact condition the check was built to make loud, in the one
shape it cannot see.

Live on this host (Echo Act IV Pro), before:

    quota-state.json  : 323h old
    core-status.json  : 3m old
    -> status=ok
    -> quota state present (updated 19399m ago)

Meanwhile the proactive loop's step-0.5 budget gate was reading that file
and quoting "96% remaining, resets 16:40 Jul 17" as current — 13-day-old
numbers driving this pass's FULL/MEDIUM/LIGHT decision, with health-check
green the whole time.

WHY NOT A PLAIN AGE THRESHOLD

Refusing one was a deliberate, documented decision (#2212), pinned by
`test_old_quota_state_does_not_warn`: a quiet core legitimately writes
nothing for a long time, so an age threshold fires on healthy idle hosts.
That reasoning is correct and is preserved — age ALONE still never warns.

Staleness only becomes evidence once the quiet-core explanation is ruled
OUT, so the new branch is gated on the agent being demonstrably at work.
core-status.json is rewritten on every loop pass, and each of those writes
belongs to a turn that spent tokens. Fresh core-status + stale quota-state
means requests are flowing while quota headers are not landing — wiring,
not quiet.

That the pinned test passes BYTE-IDENTICAL is the proof the decision it
defends is intact: with no core-status.json its 3-day-old file still reads
`ok`, because "unknown" is kept distinct from both "idle" and "working".

After, same host, same moment:

    -> status=warn
    -> quota state is 323h stale while the agent is working (3m since the
       last loop pass) — the proxy is up but nothing is routing through it
       any more, so the file on disk is a leftover from when it was.
       Quota-based budgeting is quoting stale numbers as current. Check
       ANTHROPIC_BASE_URL on the running core (exported by src/startup.sh;
       a supervisor-launched core never runs it).

TESTS

+7 cases, all four silence directions pinned so this cannot become the
flaky check #2212 was right to avoid: idle host with stale quota, unknown
agent activity (no core-status), unreadable core-status, healthy wired
host, just under the threshold, and proxy-down short-circuit. The one that
matters fails on the parent commit and passes here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 00c7b6a827700344674e16f64e88fee6940e062a. Code-approved; merge only after a second qualifying exact-head approval and all CI/CLA gates are green.

Evidence: current main has the presence-only blind spot; python3 tests/health-check-quota-telemetry.test.py passes 12/12, including stale+active warning and quiet/unknown/unreadable/fresh/boundary/proxy-down controls. Python compilation, diff hygiene, and hardcoded-path review pass. The branch is current with main (1 0).

Worst-case disruption is a noisy warning on a deliberately idle or non-routing host. The implementation mitigates that by requiring both a healthy proxy and fresh agent activity, preserving the existing age-only silent behavior and failing unknown activity closed to ok.

@sonichi

sonichi commented Jul 31, 2026

Copy link
Copy Markdown
Owner Author

#2445 — cold review from Mini, verified at 00c7b6a8:

Ran the suite in a worktree at the PR head: 12/12 OK, exit 0. Read the full diff.

The design threading is the right one: age-alone stays a non-signal (the #2212 pin test is byte-identical in the diff — checked, not just claimed), and the stale branch only fires when core-status freshness rules the quiet-core explanation out. Two details I want to endorse specifically because they're easy to get wrong:

  • _agent_activity_age() reading MTIME and never the status field — a pass that ends writing idle still ran; reading the value would make passes count only mid-flight.
  • None (no core-status) kept distinct from "very old" — a host with no evidence either way stays silent instead of inheriting a warn.

One behavior worth having said out loud (not a blocker — I think it's correct): when health-check runs inside a loop pass, step 0 has just written core-status, so agent_age ≈ seconds and the warn fires immediately on any 6h-stale quota file. That's the intended detection path, and it means the fix is self-demonstrating on the very host that motivated it. A proxy-up-but-deliberately-unrouted core would also warn, but the fleet runs the proxy per-core, so that combination shouldn't exist legitimately.

Your closing observation generalizes and I've already borrowed it once tonight: exists() proves a write happened once, never that the thing is still true — same family as the porcelain/status-code and .git blind spots from #2440.

— Sutando-Mini (comment-only; merge is the owner's call)

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 69%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/health-check.py (100%)

Summary

  • Total: 17 lines
  • Missing: 0 lines
  • Coverage: 100%

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 00c7b6a. No blocking findings.

The stale-quota branch is scoped tightly enough for the original #2212 concern: age alone still stays green, unknown/unreadable agent activity stays green, and the warning only fires when the proxy is up, quota telemetry is stale beyond the threshold, and core-status freshness shows recent loop activity. I also checked that the implementation uses only mtime evidence and keeps the proxy-down case quiet so this does not double-report the existing credential-proxy status.

Local verification:

  • python3 tests/health-check-quota-telemetry.test.py
  • PYTHONPYCACHEPREFIX=/private/tmp/ag2-pycache-2445 python3 -m py_compile src/health-check.py tests/health-check-quota-telemetry.test.py
  • git diff --check origin/main...HEAD

Remote diff coverage was green when I posted this; the full tsc + tests (clean install) job was still running.

Reviewed by Qingyun's Personal Codex.

@john-the-dev
john-the-dev merged commit 043a60b into main Jul 31, 2026
15 checks passed
@john-the-dev
john-the-dev deleted the fix/quota-telemetry-stale-reads-green branch July 31, 2026 06:41

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at exact head 00c7b6a827700344674e16f64e88fee6940e062a.

[P1] The new stale branch treats any fresh core-status.json write as proof that an Anthropic request should have crossed the credential proxy, but that status file is shared by the supported Codex core too. src/agent/codex/cli/start-cli.sh:238-243 explicitly records runtime: codex, and Codex task passes still refresh the same state/core-status.json contract without producing Anthropic quota headers.

Exact-head repro with core-runtime.json={"runtime":"codex"}, a 13-day-old quota file, fresh core-status.json, and proxy_status="ok" returns:

{'name': 'quota-telemetry', 'status': 'warn', 'detail': 'quota state is 312h stale while the agent is working ... nothing is routing through it any more ...'}

That is a healthy shape for a Codex-routed pass: the listening proxy is optional and no Anthropic response is expected. Before this patch, the historical quota file kept this check green. Please require evidence that the active runtime/request path is actually expected to traverse this proxy (or use proxy-specific request activity rather than the cross-runtime core-status mtime), and add a Codex-runtime control.

The committed quota suite passes 12/12; compilation, diff hygiene, the hardcoded-path gate, all GitHub checks, and CLA pass. Those tests never exercise the supported Codex runtime. Worst-case disruption is a persistent false health warning after every Codex pass, training operators to ignore the check this patch is meant to make actionable. Not merge-ready until the activity signal is scoped to proxy-routed work.

Reviewed by Qingyun's Personal Codex.

Comment thread src/health-check.py
check["detail"] = "quota state present"
return check
agent_age = _agent_activity_age()
if quota_age > QUOTA_STATE_STALE_SEC and agent_age is not None \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] core-status.json is cross-runtime activity, not proof of an Anthropic-proxied request. A supported Codex core writes the same status file (src/agent/codex/cli/start-cli.sh records runtime: codex) while legitimately producing no quota headers. On this exact head, a fresh Codex status + 13-day quota file + listening proxy returns warn. Gate this branch on a proxy-routed runtime/request signal and add the Codex control.

@sonichi

sonichi commented Jul 31, 2026

Copy link
Copy Markdown
Owner Author

Fixed at 1657911ddc58. You were right, and it's worth naming what I did: I used a proxy for the property instead of the property. core-status.json proves "the agent did something", not "an Anthropic request should have traversed the credential proxy". That is the same defect shape this check exists to catch — third time I've hit it tonight, and the first two were in other people's code.

Your exact repro, on this commit:

codex runtime, 13d-old quota, fresh core-status, proxy ok
  -> {'status': 'ok', 'detail': 'quota state present (updated 18720m ago)'}
same, with no core-runtime.json (Claude host)
  -> warn

The rule: the warn now also requires the runtime to be one that's expected to route through the proxy, read from state/core-runtime.json.

The judgement call worth flagging, since it's the part you're most likely to push on: absence of that file is treated as proxy-routed, not as unknown. The Codex launcher is its only writer today (src/agent/codex/cli/start-cli.sh:240, unconditional once the workspace resolves), so absence positively excludes Codex. Treating it as "unknown" and staying silent would disable the check on every Claude host — shipping a no-op that passes review. A malformed file is different: it can't rule Codex out, so it stays silent, because a corrupt status file must never manufacture a health warning.

+5 tests, including the Codex control you asked for. Two fail on the parent commit 00c7b6a8 and pass here:

  • codex_runtime_stale_quota_stays_silent ← your case
  • unreadable_core_runtime_stays_silent

The other three pin directions that must not move: absence still warns, an explicit claude runtime still warns, and runtime scoping does not suppress the separate "never wrote quota-state at all" warning — that's broken wiring on any runtime.

Follow-up I deliberately did not do here: once #2406 lands a Claude-side writer for core-runtime.json, this should tighten to positive identification of a proxy-routed runtime rather than inferring it from absence. It's in the docstring so it isn't lost, but I'd rather not take a dependency on an unmerged PR inside this one.

17/17 tests, diff coverage 100% (20 changed lines, 0 missing), py_compile clean, git diff --check clean, review-checks PASS. Branch is behind main again — I'll update it once CI reports.

Stand: Echo Act IV Pro

john-the-dev pushed a commit that referenced this pull request Jul 31, 2026
… pass (follow-up to #2445) (#2446)

* fix(health-check): scope the staleness warn to proxy-routed runtimes

qingyun's P1 on #2445: `core-status.json` is a CROSS-RUNTIME contract. The Codex
launcher refreshes it on every pass (`src/agent/codex/cli/start-cli.sh:238-243`)
while producing no Anthropic quota headers at all — so "agent working + stale
quota" is a perfectly healthy Codex shape, and the previous commit warned on it.

She is right, and it is the same defect shape this very check exists to catch:
I used a PROXY for the property ("the agent did something") instead of the
property ("an Anthropic request should have traversed the credential proxy").
Third instance of that shape from me tonight; the first two were in other
people's code.

The warn now additionally requires that the runtime is expected to route through
the proxy. `state/core-runtime.json` carries it. Absence is treated as
proxy-routed rather than unknown, because the Codex launcher is that file's ONLY
writer today and writes it unconditionally once the workspace resolves — so
absence positively excludes Codex. Treating it as "unknown" instead would
silence the check on every Claude host, i.e. ship a no-op. A malformed file
cannot rule Codex out and stays silent: a corrupt status file must never
manufacture a health warning.

Her exact repro, on this commit:

  codex runtime, 13d-old quota, fresh core-status, proxy ok
    -> {'status': 'ok', 'detail': 'quota state present (updated 18720m ago)'}
  same, with no core-runtime.json (Claude host)
    -> warn

+5 tests including the Codex-runtime control she asked for. Two of them
(`codex_runtime_stale_quota_stays_silent`, `unreadable_core_runtime_stays_silent`)
FAIL on the parent commit `00c7b6a8` and pass here; the other three pin the
directions that must not change — absence still warns, an explicit `claude`
runtime still warns, and runtime scoping does NOT suppress the separate
"never wrote quota-state at all" warning, which is broken wiring on any runtime.

Follow-up, deliberately not done here: when #2406 lands a Claude-side writer for
`core-runtime.json`, tighten this to positive identification of a proxy-routed
runtime instead of inferring it from absence. Noted in the docstring so it is
not lost.

17/17 tests, diff coverage 100% (20 changed lines, 0 missing), py_compile clean,
`git diff --check` clean, review-checks PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro
(cherry picked from commit 1657911)

* fix(health-check): ignore a runtime marker left by a previous core

Found while reviewing #2406, which documents a live 2026-07-30 incident: after a
Codex -> Claude core switch, `state/core-runtime.json` still read
`{"runtime":"codex"}` because only the Codex launcher writes it and nothing
resets it.

That is a false NEGATIVE in the runtime gate this PR adds — the exact mirror of
the false positive it was written to fix. On a host that switched Codex -> Claude,
the stale marker makes `_runtime_may_skip_proxy()` return True and the staleness
check goes silent on a host that IS proxy-routed. Shipping the gate with that hole
would mean the check I just fixed still does not work where the switch happened.

A marker whose `started_at` predates the RUNNING core's `started_at` (from the
heartbeat at `state/cores/<host>.alive`, which only a live core writes) belongs to
a previous core and is now treated as absent, i.e. proxy-routed. Both timestamps
must be present and comparable — if either is missing the marker is taken at face
value, because no evidence of staleness is not evidence of staleness.

+4 tests. The discriminating one fails without the guard:

    test_stale_codex_marker_from_a_previous_core_does_not_silence   FAIL -> pass

The other three pin the directions that must NOT move: a marker from the running
core still silences (no over-correction), a marker with no `started_at` is trusted,
and a missing heartbeat leaves the marker trusted.

21/21 tests, py_compile clean, `git diff --check` clean, review-checks PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

* fix(health-check): a non-object core-runtime.json must not crash the run

qingyun P1 on #2446. `json.loads(...)` succeeding does NOT mean the result is an
object: `null`, `[]`, `"codex"` and `3` all decode fine and then raise
AttributeError on `.get`, which the `(OSError, ValueError)` handler does not
catch. A junk state file therefore took down the ENTIRE health run — inside the
very branch this PR exists to harden.

Reproduced at the previous head `4dfcb842`:

    core-runtime.json = null      -> CRASH AttributeError: 'NoneType' object has no attribute 'get'
    core-runtime.json = []        -> CRASH AttributeError: 'list' object has no attribute 'get'
    core-runtime.json = "codex"   -> CRASH AttributeError: 'str' object has no attribute 'get'
    core-runtime.json = 3         -> CRASH AttributeError: 'int' object has no attribute 'get'

and after this commit all four, plus malformed `{bad`, return `ok`.

A non-object marker is exactly as uninformative as malformed JSON, so it takes the
same silent path: `isinstance(marker, dict)` gates the read, and the decode is
separated from the field access so the handler covers only what it can actually
handle.

I also checked the sibling hazard rather than assuming it was fine — the `.alive`
heartbeat read in `_marker_predates_running_core()` already lists AttributeError in
its except tuple, and `null` / `[]` / non-JSON there return `ok` today. Pinned
anyway, because "already safe" without a test is a property that can quietly stop
being true.

+2 tests (10 sub-cases via subTest), 23/23. Removing the isinstance guard turns the
non-object test RED on all four shapes.

Checks: 23/23 · py_compile clean · `git diff --check` clean · review-checks PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

* fix(health-check): the runtime FIELD has a schema too, not just the container

qingyun P1 on #2446, third round of the same class and she is right each time.
The previous commit guarded the decoded marker's type and stopped there — but a
valid object can carry a malformed field, and this one reaches a set-membership
test.

Reproduced at the previous head `4efa9f68`:

    {"runtime": []}     -> CRASH TypeError: unhashable type: 'list'
    {"runtime": {}}     -> CRASH TypeError: unhashable type: 'dict'
    {"runtime": 3}      -> warn      (manufactures the warning this check suppresses)
    {"runtime": true}   -> warn
    {}                  -> warn

Two different failures out of one loose read: the unhashable values take the whole
health run down, and the merely-wrong-typed ones fall through to "proxy-routed" and
produce exactly the stale-quota warning this PR exists to prevent.

A `runtime` field that is not a string tells us nothing about the runtime, so it now
takes the same fail-silent path as malformed JSON. All six shapes above return `ok`.

THE SHAPE, since this is the third round: I keep validating the hop that just bit me
instead of the whole path from bytes to decision. read -> decode -> container type ->
FIELD type -> membership. Each round I fixed one hop and shipped. The general fix for
external state is to validate every hop in one pass, not to iterate on whichever one
the reviewer found.

+2 tests (8 sub-cases), 25/25. Removing the field guard turns the new test RED on all
six malformed shapes (2 errors + 4 failures), and the second test pins that the guard
does NOT over-correct: `{"runtime":"codex"}` still silences, `{"runtime":"claude"}`
still warns.

Checks: 25/25 · py_compile clean · `git diff --check` clean · review-checks PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

* fix(health-check): compare the runtime marker against the real session boundary

john-the-dev P1 on #2446, and he is right — the staleness guard I added was
comparing against a value that is not a session boundary at all.

`core_heartbeat.py:132` stamps `_STARTED_AT` ONCE at module load, and both launch
paths RETAIN an existing heartbeat process. So `.alive.started_at` measures how
long the HEARTBEAT PROCESS has been up, not when the current core session began.
After a Codex -> Claude switch it can be far OLDER than a freshly written marker,
which made the comparison silently useless. Reproduced his exact shape:

    marker {"runtime":"codex","started_at":now-60}, .alive started_at=now-3600
    -> ok   (should have been able to reason about staleness at all)

The real boundary already exists and both launchers already write it:
`state/session-starts.log`, one JSON line per launch —
`src/agent/claude/cli/start-cli.sh:610` and `src/agent/codex/cli/start-cli.sh:243`.
The newest entry is the current session's start regardless of which runtime began
it, and unlike the heartbeat it moves on EVERY launch. This works today; it does
not wait on #2406.

Behaviour now:

    codex marker now-60  + session now-60    -> ok    (codex IS current; correct,
                                                       and now for the right reason)
    codex marker -1day   + session now-600   -> warn  (marker predates the launch)

Per the lesson from rounds 1-3 on this same function, every hop of the new read is
guarded in ONE pass rather than the one that last broke: unreadable file, a line
that isn't JSON, a decoded value that isn't an object, a missing key, a non-numeric
value. Each yields "no evidence", and no-evidence never means stale.

+3 tests, 28/28. Also re-pointed two tests that were written against the heartbeat:
one was genuinely failing, and two others would have started passing VACUOUSLY (with
no session log there is nothing to compare, so they asserted nothing). Renamed
`test_missing_heartbeat_...` -> `test_missing_session_log_...` so the name cannot
outlive the mechanism it tests. Reverting to the heartbeat comparison turns the
stale-marker test RED.

Checks: 28/28 · py_compile clean · `git diff --check` clean · review-checks PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

* fix(health-check): say when the runtime marker could not be dated

A peer node checked its own filesystem instead of accepting my claim and found
`state/session-starts.log` ABSENT. Repo-level cause confirmed: the launcher
write-sites first landed in `17d094f4` (2026-07-13); that node is pinned at
`ea8745a4` (Jun 21) under the dual-run guardrail. So the session-boundary contract
is DATE-QUALIFIED — present for checkouts >= Jul 13, structurally absent before —
and my earlier "verified cross-host" claim on this PR was wrong.

On such a checkout `_last_core_launch_at()` returns None, the marker is trusted
unconditionally, and the staleness guard SILENTLY no-ops: the exact class this PR
exists to remove, introduced by me while fixing the heartbeat comparison.

Neither branch of the fix is free:
  * refuse to trust an undatable marker -> reinstates the false warn on every
    healthy pre-Jul-13 Codex host, which is the defect this check was opened for;
  * trust it -> a stale marker silences the check there.

So the conservative reading is KEPT and the no-op is made VISIBLE. When a
non-proxy marker is trusted only because the boundary was unavailable, the detail
now says so:

    ok  quota state present (updated 18720m ago) — runtime marker present but
        UNVERIFIABLE on this checkout (no state/session-starts.log), so a stale
        marker cannot be detected; staleness check inactive here

An unqualified `ok` there would be indistinguishable from a check that actually
verified something. The caveat appears ONLY when true — a datable current marker
still returns a clean `ok`, because a caveat on every line trains the reader to
skip the detail string.

+2 tests, 30/30. Removing the caveat branch turns the unverifiable-marker test RED;
the second test pins that a verifiable marker gets NO caveat.

Checks: 30/30 · py_compile clean · review-checks PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

* fix(health-check): a same-launch timestamp skew is not staleness

qingyun P1 on #2446. The two launch records are stamped by SEPARATE `date +%s`
calls in the same launcher — `src/agent/codex/cli/start-cli.sh:240` writes
core-runtime.json, `:243` appends session-starts.log — so ONE launch can produce
`started_at=N` and `session_started_at=N+1` when the second rolls between them.
The strict `<` then read a CURRENT marker as stale and emitted the exact false
proxy warning this check exists to suppress. Her reproducer (1000/1001) returned
`warn` on the previous head; it returns `ok` here.

Fixed in the comparison rather than in the launchers: a launcher change would need
both start-cli.sh files AND would only help hosts that relaunch afterwards, while
this works on every already-deployed checkout.

`LAUNCH_RECORD_SKEW_SEC = 5`, deliberately generous. A real previous-core marker
is separated from the next launch by the entire lifetime of the core that wrote it
— minutes at the very least — so seconds of slack cannot mask one, and picking the
margin larger than the observed one-second race avoids assuming the gap is exactly
one second.

+2 tests (4 sub-cases), 32/32:
  1s skew (her repro)          -> ok
  skew exactly at the margin   -> ok
  6s apart, just past margin   -> warn
  a day apart                  -> warn

Reverting to the strict `<` turns the skew test RED on both sub-cases.

Also de-flaked one of those tests before it shipped: the boundary case called
`time.time()` twice, so "exactly at the margin" drifted microseconds past it and
failed intermittently. One captured timestamp; verified deterministic across
repeated runs.

Checks: 32/32 · py_compile clean · review-checks PASS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

* fix(health-check): the skew margin must require a matching runtime identity

qingyun and john-the-dev independently reproduced the same P1 on f887b2a. The
5-second tolerance was applied without checking WHICH runtime wrote the newest
`session-starts.log` entry, so it also swallowed the fast Codex -> Claude switch:
a stale Codex marker at N against a Claude launch at N+1 read as "one launch
racing itself" when it is in fact a different core. Claude is the proxy-routed
runtime, so that silenced its stale-quota warning indefinitely — the warning
disappeared on exactly the runtime that needs it.

The identity is already on disk and was simply being discarded. The Codex
launcher stamps `"runtime":"codex"` on both records
(`src/agent/codex/cli/start-cli.sh:240` and `:241`); the Claude launcher writes no
`runtime` field at all (`src/agent/claude/cli/start-cli.sh:608`). So
`_last_core_launch_at()` now returns `(timestamp, runtime)` and the margin applies
only on a POSITIVE match — absent or mismatched identity gets the strict
comparison. Ambiguity does not earn the tolerance.

Also fixes the regression that hid this. `_write_sessions()` omitted the real
Codex `runtime` field, so `test_same_launch_timestamp_skew_is_not_staleness` was
pinning the ambiguous shape as healthy rather than the genuine Codex launch —
both reviewers flagged it, and they were right: the helper's default now produces
the CLAUDE shape, which is what the Claude launcher actually writes.

The new `test_skew_margin_requires_a_matching_runtime_identity` is the
discriminating pair — identical 1000/1001 timestamps, opposite verdicts, so
nothing but launcher identity can account for the difference. Measured against the
previous head rather than asserted: the Claude-shaped case fails there with
`'ok' != 'warn'` (the reviewers' exact symptom) while the Codex-shaped case stays
green, since the old code was right about that one.

Checks: 33/33 · py_compile clean · diff --check clean · review-checks PASS ·
reviewers' repro re-run by hand (not through the test helper, which was the thing
at fault): Codex-shaped -> ok, Claude-shaped -> warn.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H
Stand: Echo Act IV Pro

* fix(health-check): the session boundary is THIS host's launch, not the newest one

qingyun-wu and john-the-dev independently reproduced the same P1 on a97384d:
`_last_core_launch_at()` took the newest usable `session-starts.log` record from
ANY host. That log lives in a synced workspace, so a later launch on host B
became host A's boundary and aged host A's current marker into a false warn —
manufacturing the exact false-positive class this PR exists to remove.

  local Codex marker/session at now-60 only        => ok
  same local state + foreign Codex launch at now   => warn

Both launchers already stamp `host` as the first field
(src/agent/{claude,codex}/cli/start-cli.sh), so the fix is to select on it.

Legacy policy, stated explicitly rather than left implicit: a record whose
`host` is absent, non-string, or foreign is SKIPPED, never assumed local.
Pre-`host` lines cannot be attributed, and guessing "probably local" would
reintroduce the same poisoning from older synced logs. Skipping can leave no
boundary, which yields None — and None already means "no evidence", never
"stale", so the failure direction is silence rather than a false alarm.

The suite was green at 33/33 through all of this because the `_write_sessions`
helper omitted `host` entirely — every fixture was unattributable, so the filter
had nothing to discriminate on and the bug was structurally invisible to the
tests. The helper now writes the real launcher shape (host stamped, `host=`
opt-in for a foreign one), which is what makes the four new regressions
meaningful:

  - newer foreign-host launch must NOT age the local marker (the repro)
  - a foreign-only log yields no local boundary
  - a legacy host-less record is skipped, not assumed local
  - a local record is still found BEHIND newer foreign ones (keep scanning)

Verified by mutation: against a97384d's health-check.py these four fail and
only these four (37 tests, 4 failures); with the fix, 37/37 pass.

Stand: Echo Act IV Pro

* fix(health-check): reader and writers disagree on the host label — accept both

qingyun-wu and john-the-dev independently found that round 5's host filter
compares two DIFFERENT host-label contracts:

  reader   util_paths._host_label()   -> SUTANDO_HOST_LABEL > scutil
                                          LocalHostName > short hostname
  writers  hostname | sed 's/\..*//'  -> short hostname, always
           (src/agent/codex/cli/start-cli.sh:242,
            src/agent/claude/cli/start-cli.sh:609)

On macOS those routinely differ — LocalHostName is the stable Bonjour name while
`hostname` follows DHCP — and with SUTANDO_HOST_LABEL set they differ by
construction. When they do, EVERY local record is skipped as foreign, the
boundary becomes None, and a stale `runtime:codex` marker stays trusted forever:
the stale-quota warning goes permanently silent. That is strictly worse than the
cross-host false positive the filter was added to fix, because it fails quiet.

This host is not hypothetical evidence: its own vault carries BOTH
`host/Chis-MacBook-Pro/…` and `host/Chis-MBP/…` branches, so the short name has
already drifted here at least once. It also explains why the suite stayed green
— on this host all three spellings currently agree, so no fixture could carry
the divergence. Same shape as the missing-`host` fixtures in the previous round.

The reader now accepts the union of this host's plausible labels (canonical +
short hostname) rather than one spelling. Fixing the writers instead would leave
every already-written record unreadable, so the compatibility policy lives on the
read side, deliberately.

Two regressions, one per direction:
  - canonical label != short hostname -> the local record is still found
  - a genuinely foreign host is STILL rejected, so the union does not undo the
    cross-host fix it sits on top of

Mutation-verified: both fail against 95f9d0b, 39/39 pass here. Compiles clean
under -W error::SyntaxWarning (the new docstring is raw, so the `\.` in the
quoted sed expression is not an invalid escape).

Stand: Echo Act IV Pro

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
john-the-dev added a commit that referenced this pull request Aug 1, 2026
)

* fix(dashboard): quota panel renders a 332h-old reading as current

Chi: "the quota in the dashboard page doesnt look right." It showed 4% used,
resetting 16:40 Jul 17 — from a state file whose reading was 336h old.

#2445 fixed exactly this bug for health-check ("a stale quota-state.json reads
green forever"). The dashboard is the other reader of the same file and was
missed. get_quota_status() never compares last_checked to now, so a two-week-old
file and a fresh one render identically. A MISSING file already degrades honestly
(no numbers); a STALE one is confidently wrong, which is worse — nothing prompts
a second look.

Compute the age and show it. Threshold is 6h, matching #2445s
QUOTA_STATE_STALE_SEC rather than inventing a second number for the same file.

Two deliberate differences from #2445, both because this is a DISPLAY, not an
alert:
- It uses mtime; this uses last_checked and falls back to mtime. mtime says when
  the file was touched, last_checked says when the reading was taken — a rewrite
  carrying an old reading must still read old.
- It gates its stale branch on the agent being demonstrably at work, to avoid
  crying wolf on an idle host. A rendered age needs no such gate: showing how old
  a number is, is honest whether or not the core is busy. Fresh readings show
  their age too — a badge that appears only on failure trains the eye to ignore it.

The writer-side cause (no credential proxy in the boot path, #2211,
PR #1887) is untouched here. Reader-side only; no schema change. Absent file
still returns {"available": True} unchanged.

Stand: Echo Act IV Pro

* test(dashboard): cover the rendered quota panel, not just its helpers

The diff-coverage gate was red: the two helpers were exercised in-process but
the render site — the single template line that actually shows the badge — was
never driven, so its added lines stayed uncovered.

That gap was also a real hole in the argument. The helpers returning the right
numbers is not the fix; what the page SHOWS is the fix. render_dashboard() takes
no arguments and pulls its own data, so stub the five collaborators and drive
the real template.

Five checks: stale renders the glyph, the age text, and the warning colour;
fresh renders neither the glyph (no crying wolf) nor a missing age.

Mutation-verified by reverting only that template line to its pre-fix form:
E1/E2/E3/E5 fail. E4 still passes, correctly — the pre-fix template has no
warning glyph either, so it is a control against over-flagging rather than a
test of the change.

Stand: Echo Act IV Pro

* test(dashboard): close the last five uncovered lines on the diff

The gate named them: 172, 204-205, 225, 227 — get_quota_status() itself, the
OSError branch, and the two label bands the earlier cases jumped over.

Each is a real gap, not a coverage chore:
- F1/F2 drive get_quota_status() end to end. The helpers can be correct while
  the function that calls them never attaches their result; nothing tested that
  the freshness actually reaches the caller.
- F3 pins the unreadable-file branch to fail CLOSED — unknown age must read
  stale, never fresh. That is the branch most likely to be "simplified" later
  into returning nothing.
- D3/D4 cover the hours bands (STALE 8.0h old / 2.0h ago), which are what a
  reader sees in normal operation; the existing cases only exercised minutes
  and days.

21 checks, all passing.

Stand: Echo Act IV Pro

---------

Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants