diff --git a/src/daemon/context-math.ts b/src/daemon/context-math.ts index f23f750..946f2b9 100644 --- a/src/daemon/context-math.ts +++ b/src/daemon/context-math.ts @@ -96,12 +96,26 @@ export function decideRotation(input: RotationDecisionInput): RotationDecision { return { shouldRotate: false, reason: "no_signal", occupancy: 0 }; } const occupancy = ctx / W; - if (input.numTurns < input.minTurnsBeforeRotate) { - return { shouldRotate: false, reason: "below_min_turns", occupancy }; - } + // The hard ceiling is checked FIRST, ahead of the min-turns guard. + // + // It is the last line of defence before the backend rejects the prompt + // outright, and it is documented to fire regardless of the `enabled` + // toggle — so letting a *different* guard preempt it made it conditional + // in practice. The min-turns window exists to stop churn right after a + // rotation (the seed prompt needs a few turns to earn its keep); it was + // never meant to license running past the context ceiling. A session that + // legitimately reaches 97% inside its first few turns — one big paste, a + // wide repo scan — needs the net MORE than a long-running one, not less. + // + // Ordering it this way also means a stuck/never-advancing turn counter can + // no longer disable the net silently, which is exactly how this failed + // before (see the counter hoist in Session#recordUsageFromTurn). if (occupancy >= input.hardRotatePct) { return { shouldRotate: true, reason: "hard_threshold", occupancy }; } + if (input.numTurns < input.minTurnsBeforeRotate) { + return { shouldRotate: false, reason: "below_min_turns", occupancy }; + } if (!input.enabled) { return { shouldRotate: false, reason: "disabled_below_hard", occupancy }; } diff --git a/src/daemon/session.ts b/src/daemon/session.ts index 809eb7c..17e3b50 100644 --- a/src/daemon/session.ts +++ b/src/daemon/session.ts @@ -2691,6 +2691,19 @@ export class Session { primaryCacheReadThisTurn = cacheRead; // best we can do on fallback } + // Turns-since-rotation is the min-turns guard's only input, and it is + // process-local (never read back from the store), so it MUST advance on + // both paths. It used to live inside the `!store` branch below, which + // meant that with memory enabled — the common configuration — it stayed + // pinned at 0 for the life of the session. `decideRotation` then returned + // `below_min_turns` on every check, and because that guard is evaluated + // BEFORE the hard-rotate ceiling, it silently disabled auto-rotation + // entirely: soft threshold and the "fires even when disabled" safety net + // alike. Observed on a 1M session that sat at 999,627 tokens across 52 + // primary calls, rotated zero times, and then failed the turn outright + // with `prompt is too long`. + this.#turnsSinceLastRotation += 1; + // If the memory engine isn't enabled we have no durable store — fall // back to pure in-memory accumulation so StatusBar still gets usage. const store = this.#memory?.store; @@ -2702,7 +2715,6 @@ export class Session { this.#usage.totalCostUsd += result.totalCostUsd; this.#usage.durationMs += result.durationMs; this.#usage.numTurns += 1; - this.#turnsSinceLastRotation += 1; // PEAK tracks primary-only — ignore subagent contributions so a // subagent-heavy turn doesn't poison the bloat canary. this.#usage.peakInputTokens = Math.max( diff --git a/src/tests/context-math.test.ts b/src/tests/context-math.test.ts index 5007144..010cdf4 100644 --- a/src/tests/context-math.test.ts +++ b/src/tests/context-math.test.ts @@ -119,10 +119,13 @@ describe("decideRotation", () => { expect(out.reason).toBe("no_signal"); }); - it("below min-turns guard → no rotation even at high occupancy", () => { + it("below min-turns guard → no rotation at elevated (but sub-ceiling) occupancy", () => { + // 91%: past the soft threshold, still under the 97% hard ceiling. This is + // the guard's actual job — suppress churn in the first few turns after a + // rotation, while the seed prompt is still earning its keep. const out = decideRotation({ ...baseInput(), - primaryLastTurnContext: 980_000, + primaryLastTurnContext: 910_000, numTurns: 2, }); expect(out.shouldRotate).toBe(false); @@ -168,14 +171,36 @@ describe("decideRotation", () => { expect(out.reason).toBe("hard_threshold"); }); - it("hard threshold still respects min-turns (don't rotate fresh sessions)", () => { + it("hard threshold PREEMPTS min-turns — the net is unconditional", () => { + // Deliberate inversion of the previous behaviour. The ceiling is the last + // defence before the backend rejects the prompt outright, so no other + // guard may gate it. A session that reaches 97% inside its first turns — + // one large paste, a wide repo scan — needs the net more than a + // long-running one, not less. + // + // This ordering also means a stuck turn counter can no longer disable the + // net silently, which is precisely how it failed: with memory enabled the + // counter never advanced past 0, so `below_min_turns` was returned on + // every check and a 999,627-token session rotated zero times. const out = decideRotation({ ...baseInput(), numTurns: 1, primaryLastTurnContext: 999_999, }); - expect(out.shouldRotate).toBe(false); - expect(out.reason).toBe("below_min_turns"); + expect(out.shouldRotate).toBe(true); + expect(out.reason).toBe("hard_threshold"); + }); + + it("a never-advancing turn counter cannot suppress the ceiling", () => { + // The exact shape of the bug: numTurns pinned at 0 forever. + const out = decideRotation({ + ...baseInput(), + enabled: false, + numTurns: 0, + primaryLastTurnContext: 999_627, // the observed live value + }); + expect(out.shouldRotate).toBe(true); + expect(out.reason).toBe("hard_threshold"); }); it("the user's 24-turn 214k avg case: NO rotation (healthy)", () => { diff --git a/src/tests/rotation-counter-guard.test.ts b/src/tests/rotation-counter-guard.test.ts new file mode 100644 index 0000000..ddfa77d --- /dev/null +++ b/src/tests/rotation-counter-guard.test.ts @@ -0,0 +1,51 @@ +import { describe, test, expect } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +/** + * Guard: the rotation turn-counter must advance on BOTH usage paths. + * + * `#turnsSinceLastRotation` is the only input to `decideRotation`'s min-turns + * guard, and it is process-local — never read back from the store. It used to + * be incremented inside the `if (!store)` branch of + * `Session#recordUsageFromTurn`, i.e. only when the memory engine was OFF. + * With memory enabled (the common configuration) it stayed pinned at 0, so + * every rotation check returned `below_min_turns` and auto-rotation was + * silently dead — soft threshold and hard ceiling alike. + * + * That shipped because nothing asserted it. A behavioural test would need a + * full daemon + memory store + provider harness for a single integer, so this + * checks the structure instead: the increment must appear BEFORE the + * `if (!store)` branch it used to hide in. Crude, but it fails loudly on the + * one edit that would reintroduce the bug. + * + * The behavioural half of this fix — that the hard ceiling can no longer be + * suppressed by a stuck counter — is covered in `context-math.test.ts`. + */ +describe("rotation turn counter", () => { + const src = readFileSync( + join(import.meta.dir, "..", "daemon", "session.ts"), + "utf8", + ); + + test("is incremented exactly once, outside any store branch", () => { + const increments = [...src.matchAll(/#turnsSinceLastRotation\s*\+=\s*1/g)]; + expect(increments).toHaveLength(1); + }); + + test("the increment precedes the `if (!store)` fallback branch", () => { + const inc = src.indexOf("#turnsSinceLastRotation += 1"); + const storeBranch = src.indexOf("if (!store) {"); + expect(inc).toBeGreaterThan(-1); + expect(storeBranch).toBeGreaterThan(-1); + // Strictly before — inside or after the branch means memory-enabled + // sessions stop counting turns and lose auto-rotation entirely. + expect(inc).toBeLessThan(storeBranch); + }); + + test("is reset only by an actual rotation", () => { + const resets = [...src.matchAll(/#turnsSinceLastRotation\s*=\s*0/g)]; + // One field initialiser + one reset inside #rotate(). + expect(resets).toHaveLength(2); + }); +});