Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

## [Unreleased]

### Added

- **session-health early-warning extension (#158, #160).** A new read-only observation extension (`proxy/extensions/session-health.mjs`, order 590) that flags long-running Opus 4.7 `[1m]` sessions approaching the thinking-desync wedge (upstream `anthropics/claude-code#63147`) before they die. It never mutates the request/response body and never attempts to repair the desync — it only warns so the operator can retire the session deliberately. This is the *warn-before* half of the thinking-desync response; mitigation (#162) and offline heal are tracked separately.

**New per-session JSON fields (additive).** `~/.claude/quota-status/sessions/<id>.json` now also carries `context_tokens` (latest live context = `input + cache_read + cache_creation`), `thinking_block_count` (`thinking`/`redacted_thinking` blocks in the latest request), `thinking_block_max` (session high-water, carried across proxy restarts), `first_seen`, `request_count`, and `thinking_desync_risk` (`ok`/`warn`/`high`). Fields are written by the existing single per-session writer (`cache-telemetry`); existing consumers are unaffected (all use optional reads). Counts only — no thinking text or signatures are ever recorded.

**Token-gated warning.** `thinking_desync_risk` is computed from `context_tokens` against `CACHE_FIX_THINKING_RISK_HIGH_TOKENS` (default `340000`, just under the observed ~382K trip) and `CACHE_FIX_THINKING_RISK_WARN_TOKENS` (default `250000`). On first crossing into `high`, a one-time content-free stderr line is emitted. Block-count is recorded but does not yet gate the warning (calibrated fast-follow). `CACHE_FIX_THINKING_RISK=off` suppresses the warning signal (stderr line + risk field) while raw count telemetry keeps recording.

### Fixed

- **`ttl-management`: never inject a TTL into `thinking` / `redacted_thinking` blocks (#157).** `injectTtl` iterated every block in the request; if a `cache_control: {type: "ephemeral"}` breakpoint landed on a thinking block (possible on Opus 4.7 interleaved-thinking turns), it rewrote the block to add `ttl`, which mutates a signed thinking block — the API rejects that with `400 ... thinking blocks ... cannot be modified`. The injector now skips `thinking`/`redacted_thinking` blocks entirely (the chokepoint covers both the system-block and message-block paths). Defensive hardening: this was not the cause of the 2026-05-28 interleaved-thinking incident (that was CC-side, `anthropics/claude-code#63172`), but it's a real latent mutation path with zero upside to keeping. Regression tests pin the skip and the still-inject-on-non-thinking happy path.
Expand Down
25 changes: 24 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ That's it. The proxy applies all 7 cache-fix extensions automatically. No wrappe

### What the proxy does

On every `/v1/messages` request, 7 extensions run in order:
On every `/v1/messages` request, 8 extensions run in order:

| Extension | What it fixes |
|-----------|--------------|
Expand All @@ -40,6 +40,7 @@ On every `/v1/messages` request, 7 extensions run in order:
| `fresh-session-sort` | Fixes non-deterministic ordering on first turn |
| `cache-control-normalize` | Normalizes cache_control markers across messages |
| `cache-telemetry` | Extracts cache stats from response headers → `~/.claude/quota-status/{account.json,sessions/<id>.json}` |
| `session-health` | Observes per-session thinking-desync risk (context size + thinking-block count) and warns before a session reaches the danger zone. Read-only |

Extensions are hot-reloadable — add, remove, or modify `.mjs` files in `proxy/extensions/` and changes apply to the next request without restarting. Configuration in `proxy/extensions.json`.

Expand Down Expand Up @@ -723,6 +724,28 @@ Scoping rules baked into the extension:
|---------|---------|---------|
| `CACHE_FIX_THINKING_DISPLAY` | `summarized` (built-in) | One of `summarized` / `omitted` / `disabled`. `summarized` restores thinking summaries (default). `omitted` force-suppresses thinking blocks. `disabled` opts the extension out entirely. |

## Session-health early-warning (proxy mode, thinking-desync risk)

Long-running Opus 4.7 `[1m]` sessions accumulate interleaved thinking blocks and grow their live context until Claude Code's own history reconstruction desyncs a thinking-block signature, producing a permanent `400 … thinking blocks … cannot be modified` on every subsequent turn (upstream root cause: [anthropics/claude-code#63147](https://github.com/anthropics/claude-code/issues/63147)). The session dies abruptly with no prior signal.

The `session-health` extension watches the conditions that correlate with the trip and warns **before** a session reaches the danger zone, so the operator can retire it deliberately (write a session-state handoff, `/clear`) instead of being surprised by a dead session. It is **read-only** — it never mutates the request/response body and never attempts to repair the desync (that is CC-side, #63147). It records numeric telemetry into the per-session file (`~/.claude/quota-status/sessions/<id>.json`) on each request and, when a session first crosses into `high` risk, emits a one-time stderr line. Counts only — no thinking text or signatures are ever logged.

Fields added to the per-session JSON:

- `context_tokens` — latest request's live context (`input + cache_read + cache_creation`)
- `thinking_block_count` — `thinking`/`redacted_thinking` blocks in the latest request
- `thinking_block_max` — session high-water mark (carried across proxy restarts)
- `first_seen`, `request_count` — session age + request tally
- `thinking_desync_risk` — `ok` / `warn` / `high` (omitted when the signal is disabled)

Token thresholds are anchored to the observed ~382K-token trip with margin; the warning is conservative by design — a premature "retire soon" is far cheaper than a dead session. Block-count is recorded but does not yet gate the warning (it activates in a calibrated fast-follow once the failure distribution is known).

| Env var | Default | Purpose |
|---------|---------|---------|
| `CACHE_FIX_THINKING_RISK_WARN_TOKENS` | `250000` | Context-token level at which `thinking_desync_risk` becomes `warn`. |
| `CACHE_FIX_THINKING_RISK_HIGH_TOKENS` | `340000` | Context-token level at which risk becomes `high` and the one-time stderr warn fires. |
| `CACHE_FIX_THINKING_RISK` | unset (on) | Set to `off` to suppress the warning signal (stderr line + `thinking_desync_risk` field). Raw count telemetry keeps recording. |

## System prompt rewrite (preload mode, optional)

The interceptor can rewrite Claude Code's `# Output efficiency` system-prompt section. Disabled by default. Enable with `CACHE_FIX_OUTPUT_EFFICIENCY_REPLACEMENT`. See [docs/output-efficiency-prompts.md](docs/output-efficiency-prompts.md) for the three known prompt variants and usage instructions.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Review: session-health early-warning directive

Date: 2026-05-28
Reviewed: `docs/directives/proxy-session-health-warning.md`
Label applied: `changes-requested`

## What Is Correct

- The directive now includes a real `## Non-Functional Requirements` section, and the size, threat-model, maintainability, and performance lines are concrete and non-empty rather than boilerplate (`docs/directives/proxy-session-health-warning.md:70-76`).
- The design remains disciplined. It keeps the feature on the warn-only side of the boundary, explicitly forbids any thinking-block mutation or repair behavior, and leaves auto-retire/auto-clear and cross-host aggregation out of scope (`docs/directives/proxy-session-health-warning.md:20,78-82`).
- The split-by-dimension release plan is still the right shape. Token-gated warning now and block-gated warning only after calibration matches the evidence quality we actually have: a live-context failure point around `~382K`, but no in-context block-count failure distribution yet (`docs/directives/proxy-session-health-warning.md:31,45,55,61,65-68`).
- The implementation surfaces the directive relies on are present in the current codebase. Request bodies are available and rewritten from the post-pipeline `reqCtx.body` before forwarding (`proxy/server.mjs:43-58`), SSE usage is already extracted from `message_start` / `message_delta` (`proxy/stream.mjs:15-29`), and `cache-telemetry` already persists per-session JSON keyed from request-side session headers (`proxy/extensions/cache-telemetry.mjs:158-167,182-238`).
- The load-bearing classification itself is directionally correct: this feature is read-only on request/response bodies, so it does not carry the request-path mutation risk of the sibling sanitize directive, but it does extend a consumed JSON contract and therefore qualifies as load-bearing on the schema-contract dimension (`docs/directives/proxy-session-health-warning.md:73-76`, `CLAUDE.md:86-94`).

## Blockers

- `docs/directives/proxy-session-health-warning.md:76` understates the required review burden for a load-bearing change. The repo standard is explicit that anything touching a wire/schema contract is load-bearing and therefore requires Chris review before merge (`CLAUDE.md:92-94`). The current text says "Recommend ... a brief human (Chris) confirmation," which is weaker than the rule it is trying to satisfy. This should be tightened from a recommendation to an explicit requirement before the directive is treated as approved under the revised workflow.

## What Needs Attention

- `docs/directives/proxy-session-health-warning.md:54` should be made unambiguous before implementation: `CACHE_FIX_THINKING_RISK=off` currently says "disable the warning (telemetry still recorded)," but that still leaves room for disagreement about whether `thinking_desync_risk` continues to be written in JSON or whether only the raw numeric telemetry remains.
- `docs/directives/proxy-session-health-warning.md:26` slightly overstates the current state of the session file. The per-session file already exists, but `first_seen` is new persisted state, not an already-present field; current writes are still limited to `cache`, `timestamp`, and `session_id` (`proxy/extensions/cache-telemetry.mjs:213-227`).
- Implementation should count `thinking_block_count` from the post-pipeline request body that is actually forwarded upstream, not from a raw pre-pipeline snapshot. The current server architecture makes that straightforward and keeps the metric aligned with the true live request shape (`proxy/server.mjs:44-58`).

## Bloat / Non-Functional

- No bloat finding. The stated `~100-200 LOC + tests` budget is reasonable for additive fields on the existing per-session writer, a small threshold computation, and one transition-gated stderr warning. The directive explicitly avoids a new subsystem and stays consistent with the repo's existing telemetry-first patterns (`docs/directives/proxy-session-health-warning.md:72-75`, `preload.mjs:1752-1760,2711-2789`).
- The schema-contract-only load-bearing call is the right strength. It should not be weakened to "non-load-bearing" just because the fields are additive, because the repo rule explicitly treats wire/schema-contract changes as load-bearing. It also does not need to be strengthened to request-path/security-risk parity with `#162`, because this directive is explicitly read-only on request/response bodies (`docs/directives/proxy-session-health-warning.md:73-76`, `CLAUDE.md:92-94`).

## Size Baseline

- `docs/directives/proxy-session-health-warning.md` — 86 LOC — directive is focused, with clear scope boundaries and one new NFR gate to tighten.
- `proxy/extensions/cache-telemetry.mjs` — 248 LOC — natural implementation home for additive session JSON fields and usage-derived risk data.
- `proxy/server.mjs` — 355 LOC — already exposes the post-pipeline request body needed for accurate block counting without new plumbing.
- `proxy/stream.mjs` — 110 LOC — already extracts the token telemetry the warning logic depends on.
- `tools/quota-statusline.sh` — 204 LOC — existing consumer of the per-session JSON; relevant to schema compatibility, intentionally untouched by this release.

## Recommendations

- Update the `Load-bearing?` line so it explicitly says Chris review is required before merge for this directive's schema-contract change, then keep the `schema-change` label on the PR.
- Keep the directive approved on design once that wording is fixed; the core plan is still sound and scoped appropriately for a minor release.
- When implementation begins, lock the `CACHE_FIX_THINKING_RISK=off` behavior in code and README at the same time so stderr behavior and JSON contract stay aligned.

## Bottom Line

The design still looks good, and the new NFR section is materially better than the earlier version. The remaining problem is process correctness, not architecture: the directive correctly identifies itself as load-bearing on the schema-contract dimension, but it phrases the Chris review requirement as optional. Under the revised repo standard, that needs to be made explicit before this directive should carry an approval state again.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Review: session-health early-warning directive

Date: 2026-05-28
Reviewed: `docs/directives/proxy-session-health-warning.md`
Label applied: `reviewed-by-codex-agent`

## What Is Correct

- The prior blocker is resolved at `92f8192`: the `Load-bearing?` line now states that this schema-contract change requires Chris review before merge, which is the right gate for additive per-session JSON fields consumed by downstream status surfaces (`docs/directives/proxy-session-health-warning.md:76`).
- The `CACHE_FIX_THINKING_RISK=off` contract is now explicit. It disables both built-in warning surfaces that this directive introduces: the one-time stderr warn line and the computed `thinking_desync_risk` field, while keeping raw numeric telemetry recording in place for calibration (`docs/directives/proxy-session-health-warning.md:54`).
- The broader directive remains disciplined and implementation-ready: warn-only scope, token-gated warning in this release, block telemetry recorded now but held out of the risk computation until the fast-follow calibration, and no statusline/community-code coupling in v3.8.0 (`docs/directives/proxy-session-health-warning.md:45-68,78-82`).

## Blockers

None.

## What Needs Attention

None at the directive level. Implementation review should still verify that `thinking_block_count` is derived from the forwarded post-pipeline body and that the additive per-session JSON fields remain backward-safe for existing consumers.

## Recommendations

- Keep the `schema-change` label on the PR.
- Treat Chris review as a required merge gate for this directive's additive per-session JSON fields, as the directive now says explicitly.
- Carry the clarified `CACHE_FIX_THINKING_RISK=off` behavior through implementation and documentation without drifting from the approved contract.

## Bottom Line

Approve. The only blocking process mismatch from the prior re-review is fixed at `92f8192`, the kill-switch scope is now unambiguous, and I do not see a new directive-level issue that should keep this PR in `changes-requested`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Review: session-health early-warning directive

Date: 2026-05-28
Reviewed: `docs/directives/proxy-session-health-warning.md`
Label applied: `reviewed-by-codex-agent`

## What Is Correct

- The warn-only boundary is explicit and appropriately narrow. The directive states twice that cache-fix must not try to repair or mutate thinking blocks and keeps auto-retire, auto-clear, and cross-host aggregation out of scope, which is the right safety cut for an upstream Claude Code failure mode rather than a proxy-owned one (`docs/directives/proxy-session-health-warning.md:16-18,68-72`).
- The split-by-dimension release shape is sound. Shipping token-gated warning now while holding block-gated warning for a calibrated fast-follow matches the evidence quality we actually have: one live-context trip point at ~382K tokens, but no in-context block-count distribution yet (`docs/directives/proxy-session-health-warning.md:27-30,43,50-53,57-59`).
- The measurability claims are real against the current proxy surfaces. Request bodies are fully parsed and passed through `runOnRequest()` before forwarding (`proxy/server.mjs:26-61`), SSE `usage` is already captured from `message_start` / `message_delta` (`proxy/stream.mjs:15-29`), and `cache-telemetry` already resolves session id on the request side and writes the per-session file on stream completion (`proxy/extensions/cache-telemetry.mjs:158-239`).
- The telemetry extension point is backward-safe as described, provided the existing cache fields stay intact. Current shipped consumers read `cache.ttl_tier`, `cache.hit_rate`, and the top-level `timestamp`; additive top-level risk fields will not break that contract (`proxy/extensions/cache-telemetry.mjs:213-229`, `tools/quota-statusline.sh:85-99,185-194`).
- Keeping statusline changes out of v3.8.0 is the right coordination boundary. Per-session JSON plus a stderr signal is enough to ship the warning without pulling community-owned UI code into the same change set (`docs/directives/proxy-session-health-warning.md:44-46,61-65`).

## Blockers

None.

## What Needs Attention

- `docs/directives/proxy-session-health-warning.md:52` should be tightened before implementation/docs fan-out so `CACHE_FIX_THINKING_RISK=off` has one unambiguous meaning. As written, "telemetry still recorded" leaves room for two materially different behaviors: raw counts only, or raw counts plus a still-populated `thinking_desync_risk` field. The implementation should not have to guess whether "off" suppresses only stderr or every built-in warning surface.
- `docs/directives/proxy-session-health-warning.md:24` slightly overstates today's surface. The current per-session file does not already carry `first_seen`; it only writes `cache`, `timestamp`, and `session_id` today (`proxy/extensions/cache-telemetry.mjs:214-227`). That does not block the design, but the wording should reflect that `first_seen` is part of the new persisted state, not an already-available field.
- The eventual implementation should count `thinking_block_count` from the post-pipeline request body that is actually forwarded upstream, not a raw pre-pipeline snapshot. The server runs the full request pipeline before serializing `reqCtx.body` back to `forwardBody` (`proxy/server.mjs:43-58`), and using that final shape gives the most accurate "live risk driver" measurement if any earlier extension has normalized the body.

## Recommendations

- Approve the directive and move to implementation, but lock the `CACHE_FIX_THINKING_RISK=off` behavior in the implementation PR and README so JSON consumers and stderr behavior stay consistent.
- In the implementation review checklist, require the one-time stderr signal to be keyed on a per-session transition into `high` rather than "every request while high." The directive already implies that outcome; making it explicit will keep tests and implementation aligned.
- Preserve the current per-session JSON keys exactly and append the new risk fields additively at the top level. That keeps `tools/quota-statusline.sh` and other current readers compatible while still exposing the new telemetry.

## Bottom Line

Ship this directive. The scope boundary is disciplined, the token/block split matches the current evidence, and the underlying proxy already has the measurement surfaces the design relies on. I do not see a directive-level blocker to implementation; only a few wording clarifications should be tightened so the env toggle and persisted-state contract are interpreted the same way by every implementer and consumer.
Loading
Loading