diff --git a/packages/core/src/config/config.constants.ts b/packages/core/src/config/config.constants.ts index 119b554e..2e8dfd5a 100644 --- a/packages/core/src/config/config.constants.ts +++ b/packages/core/src/config/config.constants.ts @@ -12,4 +12,5 @@ export const ENV_FLAG = { expertRescue: "TSFORGE_EXPERT_RESCUE", noNearGreenCheckpoint: "TSFORGE_NO_NEAR_GREEN_CHECKPOINT", noE2eAcceptance: "TSFORGE_NO_E2E_ACCEPTANCE", + noNearGreenRotation: "TSFORGE_NO_NEAR_GREEN_ROTATION", } as const; diff --git a/packages/core/src/config/flags.ts b/packages/core/src/config/flags.ts index a768f6ec..eac07e6c 100644 --- a/packages/core/src/config/flags.ts +++ b/packages/core/src/config/flags.ts @@ -54,4 +54,13 @@ export const flags = { * without knowing a flag exists. Kill-switch TSFORGE_NO_NEAR_GREEN_CHECKPOINT=1 disables * it (A/B control / escape hatch). Thresholds N=2, M=3 from Phase 0a. */ nearGreenCheckpoint: (): boolean => !isOn(ENV_FLAG.noNearGreenCheckpoint), + /** WS-B near-green ROTATION steer (#77): when the build sits at a near-green count but the + * SPECIFIC error set rotates for several cycles (the model fixes one error and the fix spawns + * another — e.g. extracts a component → its siblings/tests are now missing), inject a + * completion-only steer ("finish the files that already have errors; don't create new + * files/modules unless the same edit adds their siblings + tests"). This is the last-mile gap + * that made green non-deterministic (build17 parked on it; build16 crossed by luck). Count-only + * WS-B can't see it (the count never sprays). DEFAULT ON, deterministic (no network); kill with + * TSFORGE_NO_NEAR_GREEN_ROTATION=1. Generic — keyed only on rule/file, no stack knowledge. */ + nearGreenRotation: (): boolean => !isOn(ENV_FLAG.noNearGreenRotation), }; diff --git a/packages/core/src/loop/near-green-checkpoint.ts b/packages/core/src/loop/near-green-checkpoint.ts index 6ffab7cd..8c600c14 100644 --- a/packages/core/src/loop/near-green-checkpoint.ts +++ b/packages/core/src/loop/near-green-checkpoint.ts @@ -139,3 +139,124 @@ export function shouldRollback( return checkpoint.errorCount <= n && curr > checkpoint.errorCount + m; } + +/** #77: near-green ROTATING-error oscillation. At near green the model can clear its single + * remaining error, but the FIX spawns a new one — e.g. it extracts a component, so that + * component's required sibling set + colocated tests are now missing; fixing those creates + * more, etc. The error COUNT stays low while the error SET rotates, so the build never reaches + * 0 (build17 parked on this; build16 crossed the same tail by luck). Count-only WS-B can't see + * it (count never sprays past the checkpoint). The window of consecutive near-green cycles over + * which a CHANGING error-set signature counts as rotation rather than a one-off wobble. */ +export const ROTATION_WINDOW = 3; + +/** Max CONSECUTIVE spikes (cycles above the near-green ceiling) tolerated between two near-green + * samples before the rotation window is considered STALE and cleared. The rotating pattern is a + * tight fix-one → spray → revert → near-green loop, so real near-green visits sit a handful of + * cycles apart; without a bound, two near-green samples could survive an arbitrarily long + * regression and combine with a much later, unrelated near-green result to fire the steer + * falsely (a fresh near-green episode, not the same rotation). */ +export const MAX_NEAR_GREEN_SPIKE_GAP = 3; + +/** A stable, order- and line-independent per-error identity token for rotation detection. When the + * error has BOTH a `rule` and a `file` (the eslint/type-aware shape), use the `rule|file` FAMILY: + * the key carries the line (`file:line:rule`), so a re-emission at a shifted line must still read + * as the SAME error — the family form gives that. Otherwise (`rule` and/or `file` absent — both are + * OPTIONAL on IErrorItem; custom gates emit key-only errors) fall back to the required `key`: a + * PARTIAL family (`rule|` or `|file`) would collapse DISTINCT errors that merely share the one + * present field (their keys differ), hiding rotations among them — the key keeps them distinct. */ +function errorToken(e: IErrorItem): string { + return e.rule !== undefined && e.file !== undefined + ? `${e.rule}|${e.file}` + : e.key; +} + +/** A stable, order- and line-independent signature of an error SET: the sorted unique per-error + * identity tokens (see errorToken). Re-emitting the SAME errors at shifted lines reads as + * unchanged; a genuinely different set reads as changed — which is what makes rotation detectable. + * Generic: keyed only on the stack-agnostic rule/file/key fields. + * DELIBERATE trade-off: the token DEDUPES by `rule|file` family and drops multiplicity, so a + * plateau rotating between two DISTINCT instances of the SAME rule in the SAME file (e.g. one + * `no-unsafe-call` in x.ts swapped for another `no-unsafe-call` in x.ts at count 1) reads as + * unchanged and is NOT detected. Distinguishing those needs the line (it's the only differing + * field), and the line is exactly what `autofixApps` (prettier/eslint --fix, run every cycle) + * reflows — so a line-sensitive signature would fire FALSE rotations on every autofix. Line- + * independence is the more important property: the observed failure (build17) rotates across + * DIFFERENT rules, which the family form detects. Same-rule-same-file instance rotation is + * knowingly left to the count-based WS-B checkpoint + escalation ladder. */ +export function errorSetSignature(errors: readonly IErrorItem[]): string { + return [...new Set(errors.map(errorToken))].sort().join(";"); +} + +/** One recorded near-green cycle: its error COUNT, the gate FRONTIER phase it sits at, the + * `errorSetSignature` of its error set, and `rev` — an INDEPENDENT worktree revision (a content + * hash of the scope files at gate time; see scopeRevision in turn.ts). Rotation is judged over a + * trailing window of these — the count + phase pin the PLATEAU (a stuck frontier at a stable + * count), the signature pins the changing IDENTITY, and `rev` proves a GENUINE per-cycle EDIT + * happened between two samples: without it, a flaky/stateful gate — or a re-evaluation of the SAME + * unedited files (e.g. the check-tool + settleGate double-run) — could emit A→B→C error signatures + * with no fix-one→spawn-another cycle at all and falsely stand the WS-B rollback net down. */ +export interface INearGreenSample { + readonly count: number; + readonly phase: number; + readonly sig: string; + readonly rev: string; +} + +/** Whether the recent near-green cycles are ROTATING. `samples` is the trailing list captured on + * cycles whose count was near green (the caller only records them there). Rotation requires BOTH: + * • a PLATEAU — the full window of `k` cycles sits at ONE error count AND ONE gate-frontier phase. + * build17's shape (and the 4/4 diagnosis) is "the count stays at best-ever (1) while the + * fingerprint rotates"; a window whose count MOVES (e.g. 2→1→1, a descent, or 1→2, a + * regression) is progress/regress, NOT rotation. A window whose PHASE moves is also progress: + * a short-circuiting composed gate reveals the next phase's errors only after the current + * phase clears, so A@phase1 → B@phase2 at the same count is genuine frontier advancement (the + * convergence logic counts it as progress), NOT rotation — firing the "don't open new + * routes/modules" steer there would fight exactly the downstream work that just became + * reachable. Requiring a constant phase excludes it. (When the gate sets no phase, all samples + * are phase 0 — constant — so this is a no-op there.) + * • GENUINE per-cycle rotation — on EVERY cycle of the window the signature CHANGES *and* the + * worktree `rev` CHANGES (no two CONSECUTIVE samples share a signature OR a rev). Requiring the + * rev to move is what makes the rotation genuine rather than merely apparent: a changing + * signature ALONE can come from a flaky/stateful gate or a re-evaluation of the SAME unedited + * files (A,B,C with rev held constant) — there was no fix-one→spawn-another edit, so it must NOT + * fire. Only a signature that rotates BECAUSE the model edited every cycle (rev moves in + * lock-step) is the build17 pattern. This is stricter than "≥2 distinct signatures in the + * window": A,A,B (one error merely replaced once, then stable) is NOT rotation, but A,B,C and + * the 2-cycle ring A,B,A both are (provided each carries a fresh rev). The strictness is + * deliberate and load-bearing: setting `nearGreenRotation` DISABLES the WS-B rollback safety net + * (nearGreenRollbackStep stands down so the steered atomic-completion spike isn't reverted). + * Firing on weak evidence (a single error-swap, or gate flake on unchanged files) would disable + * that net for a build that is merely progressing — so the detector must be CONFIDENT the + * frontier is actually cycling under real edits before it fires. A full window of ONE signature + * is a stuck single error (the ladder/expert own it), also not rotation. + * Only the last `k` matter, so a rotation that has since stabilized reads as not-rotating. */ +export function isNearGreenRotation( + samples: readonly INearGreenSample[], + k: number = ROTATION_WINDOW +): boolean { + if (samples.length < k) { + return false; + } + + const window = samples.slice(-k); + const countPlateau = new Set(window.map((s) => s.count)).size === 1; + const phasePlateau = new Set(window.map((s) => s.phase)).size === 1; + + if (!countPlateau || !phasePlateau) { + return false; + } + + // Genuine rotation: on every cycle BOTH the identity and the worktree rev must change (no two + // consecutive equal). The sig-change is the rotating frontier; the rev-change proves a real + // per-cycle EDIT drove it — so a flaky gate re-emitting different signatures over UNCHANGED files + // (rev held constant) reads as not-rotating and leaves the WS-B rollback net engaged. + return window.every((s, i) => { + if (i === 0) { + return true; + } + + const prev = window[i - 1]; + + return prev !== undefined && s.sig !== prev.sig && s.rev !== prev.rev; + }); +} diff --git a/packages/core/src/loop/session.ts b/packages/core/src/loop/session.ts index 5f9fa1aa..07d9798e 100644 --- a/packages/core/src/loop/session.ts +++ b/packages/core/src/loop/session.ts @@ -425,6 +425,12 @@ export function resetDriveConvergence(state: ILoopState): void { delete state.nearGreenBest; delete state.nearGreenRollbacks; delete state.completionPhase; + // #77: the near-green ROTATION window + flag are per-drive too — else a send that ends mid- + // rotation leaks the flag into the next send, injecting the completion-only steer with no + // evidence from the new drive. + delete state.nearGreenSamples; + delete state.nearGreenSpikeGap; + delete state.nearGreenRotation; } /** How many times a send recovers from a repetition loop before giving up. */ @@ -2451,6 +2457,11 @@ export class Session { this.state.nearGreenCheckpoint = undefined; this.state.nearGreenBest = undefined; this.state.nearGreenRollbacks = 0; + // #77: the rotation window + flag are per-drive (a stale flag would inject the completion-only + // steer on a fresh drive with no evidence). + this.state.nearGreenSamples = undefined; + this.state.nearGreenSpikeGap = undefined; + this.state.nearGreenRotation = undefined; for (let turn = 1; turn <= maxTurns; turn += 1) { const turnStart = performance.now(); diff --git a/packages/core/src/loop/turn.ts b/packages/core/src/loop/turn.ts index ebffa93f..26482484 100644 --- a/packages/core/src/loop/turn.ts +++ b/packages/core/src/loop/turn.ts @@ -30,8 +30,13 @@ import { shouldCheckpoint, shouldRollback, nextCompletionPhase, + errorSetSignature, + isNearGreenRotation, + NEAR_GREEN_N, MAX_NEAR_GREEN_ROLLBACKS, + MAX_NEAR_GREEN_SPIKE_GAP, type INearGreenCheckpoint, + type INearGreenSample, } from "./near-green-checkpoint"; // The SHARED rollback substrate (aliased — turn.ts has its own polish-only `snapshotFiles` // returning a plain Map). `snapshotFilesForRollback` captures an IFileSnapshot and @@ -441,6 +446,21 @@ export interface ILoopState { * error remains; cleared when none remain (completion done → WS-B re-engages) or on green / * at the drive boundary. While set, WS-B does not roll back and the banner says "build it". */ completionPhase?: boolean; + /** WS-B rotation (#77): the trailing {count, error-set signature} samples of the near-green + * cycles the drive has visited (curr in 1..N). Spikes above N are ignored (not pushed, not + * cleared) so a transient regression between two near-green visits doesn't reset the window; + * completion-phase cycles are NOT recorded (their churn is legitimate, not rotation); cleared on + * green / at the drive boundary. `isNearGreenRotation` reads the last ROTATION_WINDOW of these — + * it needs the COUNT (to require a plateau) as well as the signature (the changing identity). */ + nearGreenSamples?: INearGreenSample[]; + /** WS-B rotation (#77): count of CONSECUTIVE spikes (cycles above N) since the last near-green + * sample. Reset to 0 on each near-green sample; past MAX_NEAR_GREEN_SPIKE_GAP the window is + * stale and cleared so far-apart near-green visits can't combine into a false rotation. */ + nearGreenSpikeGap?: number; + /** WS-B rotation (#77): true when the near-green error set has been ROTATING (fixing one error + * spawns another at the same low count). Set from `isNearGreenRotation(nearGreenSamples)`; while + * set, injectFeedback prepends the completion-only steer. Cleared on green. Flag-gated. */ + nearGreenRotation?: boolean; /** Guard-specific identity of the current stuck block (canonical, not the raw error * set). Derived from the guard that fired: samePersist → single error key, * gateStuckRepeats → sorted-join of current keys, plateau → normalized count|keys @@ -2004,7 +2024,9 @@ const NEAR_GREEN_LOCKDOWN = 3; /** A lockdown/regression banner for the top of the feedback, or "" when far from * green and not regressing. `total` = open errors this cycle; `best` = the all-time - * low (watermark). Regression = this cycle is WORSE than the best already reached. */ + * low (watermark). Regression = this cycle is WORSE than the best already reached. + * (The #77 rotation caller suppresses this banner entirely while rotation is active — the steer + * owns the finishing discipline there — so this function need not special-case rotation.) */ export function nearGreenBanner( total: number, best: number, @@ -2058,6 +2080,43 @@ export function nearGreenBanner( * the conversation as the next user message, so the model fixes in-context. * Exported for unit testing (like checkStuck/autoFixStep) — the convention PUSH * delivery is a seam we need to assert actually reaches the model's messages. */ +/** #77 near-green ROTATION emit decision, extracted from injectFeedback to keep that function's + * branching under the cognitive-complexity ceiling. Returns the two leading text blocks: + * • `rotation` — the completion-only steer, emitted only when rotation is ACTIVE and the current + * cycle is actually near green (≤ N). Rotation is active when ALL of: the kill-switch flag is on + * (read HERE too so it's authoritative at the emit point regardless of tracker ordering); the + * detector fired (`state.nearGreenRotation`); and it is NOT the completion phase (that state + * legitimately wants the model to ADD the missing UI, which the "do NOT create new files" steer + * would contradict). The steer's "one or two errors from done" wording only holds at a near-green + * count, so above N (the model executing the steered atomic completion) it is withheld. + * • `banner` — the near-green lockdown / regression callout that otherwise leads (the finishing + * discipline that stops "spray after best"). SUPPRESSED entirely while rotation is active (near + * green OR spike): its "don't create new files" lockdown and its "UNDO that collateral" callout + * BOTH contradict the steer's "create the required siblings/tests together, complete atomically" + * — stacking them is the spray→revert loop build17 parked on. WS-B has already stood down, so + * there is nothing to "undo". When not rotating, the banner flips to "build the missing UI" in + * the completion phase instead of "don't create files" (else it fights the completeness guard). */ +function rotationEmit( + state: ILoopState, + errorCount: number +): { rotation: string; banner: string } { + const rotationActive = + flags.nearGreenRotation() && + state.nearGreenRotation === true && + state.completionPhase !== true; + const rotating = rotationActive && errorCount <= NEAR_GREEN_N; + const rotation = rotating ? `${NEAR_GREEN_ROTATION_STEER}\n\n` : ""; + const banner = rotationActive + ? "" + : nearGreenBanner( + errorCount, + state.bestErrorCount, + state.completionPhase === true + ); + + return { rotation, banner }; +} + export async function injectFeedback( ctx: ILoopCtx, state: ILoopState, @@ -2131,19 +2190,15 @@ export async function injectFeedback( }); } - // NEAR-GREEN lockdown / regression callout leads everything — the finishing - // discipline that stops "spray after best" (the dominant late-run failure). When the - // remaining errors are all completion-class, the banner flips to "build the missing UI" - // instead of "don't create files" (else it contradicts the completeness guard). - const banner = nearGreenBanner( - gateErrors.length, - state.bestErrorCount, - state.completionPhase === true - ); + // #77 + near-green finishing discipline: the completion-only rotation steer that leads when the + // frontier is rotating, and whether the lockdown/regression banner must be suppressed (both + // contradict the steer). Extracted into rotationEmit so injectFeedback stays under the + // cognitive-complexity ceiling — see that helper for the full reasoning. + const { rotation, banner } = rotationEmit(state, gateErrors.length); ctx.messages.push({ role: "user", - content: `${banner}${steer}${notice}${feedback}${how}`, + content: `${rotation}${banner}${steer}${notice}${feedback}${how}`, }); } @@ -2323,6 +2378,21 @@ async function nearGreenRollbackStep( return false; } + // #77: while the near-green error set is ROTATING, injectFeedback is steering the model to + // COMPLETE a file atomically — create its required companion/sibling files + colocated test in + // ONE change. That coordinated edit transiently spikes the count past the checkpoint, exactly + // like a completion-phase add. Rolling it back would undo the completion the steer just asked + // for — the very spray→revert loop build17 parked on, with the steer and WS-B fighting. So WS-B + // stands its rollback down here too. It is not defenceless: if the spike does NOT resolve back + // to near green within MAX_NEAR_GREEN_SPIKE_GAP cycles, trackNearGreenRotation clears the + // rotation flag (the streak is stale), WS-B re-arms, and the held near-green checkpoint reverts + // the persistent spray on the next cycle. Re-check the kill-switch HERE too (not only in the + // tracker) so disabling it is authoritative at this decision point regardless of call ordering — + // symmetric with injectFeedback's emit-path check. + if (flags.nearGreenRotation() && state.nearGreenRotation === true) { + return false; + } + if ( shouldRollback( state.nearGreenCheckpoint, @@ -2394,6 +2464,117 @@ async function nearGreenCheckpointStep( } } +/** #77 completion-only steer text — GENERIC (no stack knowledge). Injected while the near-green + * error set is ROTATING: the model is ~1 error from done but each fix spawns another at the same + * low count (e.g. it extracts a component, so that component's required siblings/tests are now + * missing). The fix is to stop opening new surface and COMPLETE existing files atomically. */ +export const NEAR_GREEN_ROTATION_STEER = + "You are only one or two errors from done, but the errors keep ROTATING — each fix spawns a " + + "different error at the same low count, so the build never reaches zero. This happens when a fix " + + "OPENS NEW SURFACE that carries its own requirements. Two rules until the count reaches zero:\n" + + "1. Do NOT open new surface: do not extract components, split existing code into new modules/" + + "units, or add new features/routes/schemas that no current error requires. That is what keeps " + + "spawning the next error.\n" + + "2. But DO finish what the current errors demand, ATOMICALLY: when you fix a file that has an " + + "error, create ALL the companion/sibling files and the colocated test that satisfying it " + + "requires in the SAME change — even though those files are not themselves in the error list yet " + + "— so the fix can't leave a fresh gap that becomes the next error.\n" + + "Resolve exactly the listed errors and the siblings/tests they require — nothing else."; + +/** #77: a cheap, INDEPENDENT worktree revision of the scope files — a content hash over the + * glob-resolved scope-file set (each path + its text, plus the existed-set so a created/deleted + * file moves the rev even when surviving files are byte-identical). trackNearGreenRotation stamps + * it on each near-green sample so isNearGreenRotation can require a GENUINE per-cycle EDIT (the rev + * must move) before it treats a rotating error signature as real: a flaky/stateful gate re-run over + * UNCHANGED files leaves the rev fixed and therefore cannot fake a fix-one→spawn-another rotation + * (and cannot wrongly stand the WS-B rollback net down). Reuses the SAME snapshot substrate WS-B + * rolls back with, so "what counts as the scope" can never drift between detection and revert. */ +async function scopeRevision( + cwd: string, + files: readonly string[] +): Promise { + const snap = await snapshotFilesForRollback(cwd, files); + const parts: string[] = []; + + for (const path of [...snap.existed].sort()) { + parts.push(`${path}${snap.contents.get(path) ?? ""}`); + } + + return String(Bun.hash(parts.join(""))); +} + +/** #77: update the near-green ROTATION signal for this cycle. Records this cycle's {count, error-set + * signature} on the drive's near-green window ONLY when `curr` is near green (1..N) — spikes above + * N are IGNORED (not pushed, not cleared) so a transient regression between two near-green visits + * doesn't reset the window (build17's real pattern is 1↔3↔6: only the 1s are near green, and they + * rotate). COMPLETION-PHASE cycles are also skipped: a hollow state legitimately churns its error + * identity while the model ADDS the missing UI, and recording that churn would pre-fill the window + * and mis-fire the "don't create new files" steer the moment completion ends. Green clears it. Sets + * `state.nearGreenRotation` from the window (needs the count → plateau AND the signature → changing + * identity; see isNearGreenRotation). When the flag is OFF this CLEARS the sticky state (so a + * mid-run kill-switch flip is authoritative — the steer stops), not just early-returns. GENERIC: + * keyed only on the stack-agnostic rule/file via errorSetSignature. Exported for tests. */ +export function trackNearGreenRotation( + state: ILoopState, + curr: number, + gateErrors: readonly IErrorItem[], + rev: string +): void { + // Flag off OR a green cycle OR the completion phase: clear the window + flag. Clearing (not just + // returning) makes the kill-switch authoritative mid-run and prevents completion-phase churn from + // filling the window and firing on the first post-completion near-green cycle. + if ( + !flags.nearGreenRotation() || + curr === 0 || + state.completionPhase === true + ) { + state.nearGreenSamples = []; + state.nearGreenRotation = false; + state.nearGreenSpikeGap = 0; + + return; + } + + // Spikes above the near-green ceiling are transient — tolerate a FEW between near-green visits so + // the rotating near-green states on either side of a spray→revert still accumulate. But a LONG + // run of them means the earlier near-green streak is stale (a later near-green state is a fresh + // episode, not the same rotation), so past the gap bound clear the window rather than let far- + // apart samples combine into a false rotation. + if (curr > NEAR_GREEN_N) { + const gap = (state.nearGreenSpikeGap ?? 0) + 1; + + state.nearGreenSpikeGap = gap; + + if (gap > MAX_NEAR_GREEN_SPIKE_GAP) { + state.nearGreenSamples = []; + state.nearGreenRotation = false; + } + + return; + } + + // A near-green sample resets the spike gap (the streak is contiguous again). + state.nearGreenSpikeGap = 0; + + const samples = state.nearGreenSamples ?? []; + // The gate FRONTIER phase this cycle sits at (a short-circuiting composed gate emits only the + // current phase's errors). A phase move across the window is frontier PROGRESS, not rotation — + // isNearGreenRotation requires a constant phase. 0 when the gate tags no phase (→ no-op). + const phase = Math.max(0, ...gateErrors.map((err) => err.phase ?? 0)); + + samples.push({ count: curr, phase, sig: errorSetSignature(gateErrors), rev }); + + // Bound the window — only the last few matter to isNearGreenRotation. + const MAX_SAMPLES = 8; + + if (samples.length > MAX_SAMPLES) { + samples.splice(0, samples.length - MAX_SAMPLES); + } + + state.nearGreenSamples = samples; + state.nearGreenRotation = isNearGreenRotation(samples); +} + export async function settleGate( ctx: ILoopCtx, state: ILoopState, @@ -2420,6 +2601,18 @@ export async function settleGate( gateErrors ); + // WS-B rotation (#77): update the near-green rotation signal BEFORE injectFeedback reads it, + // and before the green branch (curr===0 clears it). Ignores spikes above N, so the rotating + // near-green states accumulate across the transient regressions between them. The worktree + // rev is only consumed on the near-green record branch (1..N), so compute it there only — a + // deep-red or green cycle skips the scope read entirely. + const rev = + curr >= 1 && curr <= NEAR_GREEN_N + ? await scopeRevision(ctx.cwd, ctx.task.files) + : ""; + + trackNearGreenRotation(state, curr, gateErrors, rev); + if (state.lastGateCount >= 0 && curr > state.lastGateCount) { state.regressions += 1; } diff --git a/packages/core/tests/flags.test.ts b/packages/core/tests/flags.test.ts index 063feeac..5e49400a 100644 --- a/packages/core/tests/flags.test.ts +++ b/packages/core/tests/flags.test.ts @@ -7,6 +7,7 @@ import { flags } from "../src/config"; afterEach(() => { delete process.env.TSFORGE_NO_NEAR_GREEN_CHECKPOINT; + delete process.env.TSFORGE_NO_NEAR_GREEN_ROTATION; }); test("nearGreenCheckpoint is ON by default; the kill-switch disables it", () => { @@ -20,3 +21,17 @@ test("nearGreenCheckpoint is ON by default; the kill-switch disables it", () => process.env.TSFORGE_NO_NEAR_GREEN_CHECKPOINT = "0"; expect(flags.nearGreenCheckpoint()).toBe(true); }); + +// #77: the near-green ROTATION steer is DEFAULT ON (the last-mile fix for the rotating-error +// oscillation that parked build17) with the same TSFORGE_NO_* kill-switch convention. +test("nearGreenRotation is ON by default; the kill-switch disables it", () => { + delete process.env.TSFORGE_NO_NEAR_GREEN_ROTATION; + expect(flags.nearGreenRotation()).toBe(true); + + process.env.TSFORGE_NO_NEAR_GREEN_ROTATION = "1"; + expect(flags.nearGreenRotation()).toBe(false); + + // Any non-"1" value leaves it ON (the FLAG_ON contract). + process.env.TSFORGE_NO_NEAR_GREEN_ROTATION = "0"; + expect(flags.nearGreenRotation()).toBe(true); +}); diff --git a/packages/core/tests/near-green-banner.test.ts b/packages/core/tests/near-green-banner.test.ts index 99e63d11..6aa84ae5 100644 --- a/packages/core/tests/near-green-banner.test.ts +++ b/packages/core/tests/near-green-banner.test.ts @@ -59,4 +59,9 @@ describe("nearGreenBanner (finishing discipline near green)", () => { expect(b).not.toContain("REGRESSION"); expect(b).not.toContain("UNDO"); }); + + // #77: nearGreenBanner has no rotation-specific mode — the injectFeedback caller SUPPRESSES this + // banner entirely while rotation is active (the steer owns the finishing discipline), so there is + // nothing to test on the banner itself for rotation. That suppression is exercised by the + // injectFeedback rotation tests in near-green-rotation.test.ts. }); diff --git a/packages/core/tests/near-green-checkpoint-loop.test.ts b/packages/core/tests/near-green-checkpoint-loop.test.ts index 7a49f56f..169dd4a6 100644 --- a/packages/core/tests/near-green-checkpoint-loop.test.ts +++ b/packages/core/tests/near-green-checkpoint-loop.test.ts @@ -9,11 +9,15 @@ import type { IGate } from "../src/gate/gate-runner"; import type { ILoopCtx, ILoopState } from "../src/loop/turn"; import { captureNearGreenCheckpoint, + NEAR_GREEN_ROTATION_STEER, rollbackNearGreen, settleGate, } from "../src/loop/turn"; import type { IValidateResult } from "../src/validate"; -import { MAX_NEAR_GREEN_ROLLBACKS } from "../src/loop/near-green-checkpoint"; +import { + MAX_NEAR_GREEN_ROLLBACKS, + MAX_NEAR_GREEN_SPIKE_GAP, +} from "../src/loop/near-green-checkpoint"; // WS-B end-to-end: with the flag ON, a build that reaches near-green (1 error) then SPRAYS // (8 errors) must REVERT the scope files to the near-green best; with the flag OFF the path @@ -211,6 +215,247 @@ test("#61: settleGate does NOT checkpoint a HOLLOW near-green state (all-complet } }, 30_000); +test("#77 INTEGRATION: settleGate wires the rotation detector → 3 rotating near-green cycles set the flag AND inject the steer", async () => { + const dir = await mkdtemp(join(tmpdir(), "tsforge-nearg-")); + + try { + await Bun.write(join(dir, "feature.ts"), "export const GOOD = 1;\n"); + + // A DIFFERENT single near-green error each cycle (count stays 1, the identity rotates) — the + // build17 shape. Driving settleGate (not the pieces) proves trackNearGreenRotation is wired in + // BEFORE injectFeedback: removing/reordering that call would leave this red. + const rotating = [ + { + key: "feature.ts:1:jsx-computation", + rule: "jsx-computation", + file: "feature.ts", + message: "a", + }, + { + key: "feature.ts:1:component-folder-structure", + rule: "component-folder-structure", + file: "feature.ts", + message: "b", + }, + { + key: "feature.ts:1:no-unsafe-call", + rule: "no-unsafe-call", + file: "feature.ts", + message: "c", + }, + ]; + let cycle = 0; + const ctx: ILoopCtx = { + task: { + id: "t", + intent: "test", + accept: "", + files: ["**/*"], + context: [], + }, + cwd: dir, + tsService: null, + report: () => undefined, + messages: [], + tool: { touched: new Set(["feature.ts"]) }, + gate: { + parse: undefined, + runner: { + run: async (): Promise => { + const err = rotating[Math.min(cycle, rotating.length - 1)]; + + cycle += 1; + + return { passed: false, errors: err ? [err] : [], output: "" }; + }, + }, + }, + }; + const state: ILoopState = { + prevGateErrors: [], + gateNoProgress: 0, + bestErrorCount: Number.POSITIVE_INFINITY, + noNewLow: 0, + errorAge: new Map(), + lastGateCount: -1, + edits: 0, + regressions: 0, + ttsrInterrupts: 0, + steerLevel: 0, + conventionsEnabled: false, + }; + + // A genuine per-cycle edit moves the worktree rev — the #77 precondition for a REAL rotation + // (fix-one → spawn-another), distinguishing it from a flaky gate re-run over unchanged files. + await Bun.write(join(dir, "feature.ts"), "export const A = 1;\n"); + await settleGate(ctx, state, 1); + await Bun.write(join(dir, "feature.ts"), "export const B = 2;\n"); + await settleGate(ctx, state, 2); + await Bun.write(join(dir, "feature.ts"), "export const C = 3;\n"); + await settleGate(ctx, state, 3); + + // The detector fired through the real harness path … + expect(state.nearGreenRotation).toBe(true); + // … and the completion-only steer reached the model in the injected feedback. + const injected = ctx.messages + .filter((m) => m.role === "user") + .map((m) => m.content) + .join("\n"); + + expect(injected.includes(NEAR_GREEN_ROTATION_STEER)).toBe(true); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}, 30_000); + +test("#77: while ROTATING, WS-B rollback STANDS DOWN — the steered atomic-completion spike is NOT reverted", async () => { + const dir = await mkdtemp(join(tmpdir(), "tsforge-nearg-")); + + try { + await Bun.write(join(dir, "feature.ts"), "export const GOOD = 1;\n"); + + // The rotation steer told the model to complete a file atomically (create its siblings + test + // in one edit); that coordinated edit spikes the count. Without the stand-down, WS-B reverts + // it — the exact spray→revert loop build17 parked on, steer vs WS-B. With rotation set, the + // rollback must NOT fire on this expected spike. + const ctx: ILoopCtx = { + task: { + id: "t", + intent: "test", + accept: "", + files: ["**/*"], + context: [], + }, + cwd: dir, + tsService: null, + report: () => undefined, + messages: [], + tool: { touched: new Set(["feature.ts"]) }, + gate: { + parse: undefined, + runner: { + run: async (): Promise => ({ + passed: false, + // 8 errors — a spray well past N+M that WOULD roll back if rotation weren't set. + errors: Array.from({ length: 8 }, (_, i) => ({ + key: `feature.ts:${String(i)}:r`, + rule: "r", + file: "feature.ts", + message: `e${String(i)}`, + })), + output: "", + }), + }, + }, + }; + const checkpoint = await captureNearGreenCheckpoint(ctx, 1, [ + { key: "k", rule: "r", file: "feature.ts", message: "near-green" }, + ]); + const state: ILoopState = { + prevGateErrors: [], + gateNoProgress: 0, + bestErrorCount: 1, + noNewLow: 0, + errorAge: new Map(), + lastGateCount: 1, + edits: 3, + regressions: 0, + ttsrInterrupts: 0, + steerLevel: 0, + conventionsEnabled: false, + nearGreenCheckpoint: checkpoint, + nearGreenBest: 1, + nearGreenRollbacks: 0, + nearGreenRotation: true, + }; + + await settleGate(ctx, state, 5); + + // No rollback fired (the spike was the steered completion, not a spray to revert). + expect(state.nearGreenRollbacks).toBe(0); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}, 30_000); + +test("#77 BACKSTOP: a spike that persists past MAX_NEAR_GREEN_SPIKE_GAP clears rotation and WS-B re-arms to roll back", async () => { + const dir = await mkdtemp(join(tmpdir(), "tsforge-nearg-")); + + try { + await Bun.write(join(dir, "feature.ts"), "export const GOOD = 1;\n"); + + // A persistent 8-error spray. While rotation is set WS-B stands down (the atomic-completion + // grace), but if the spike does NOT resolve back to near green within the gap bound, the + // streak is stale: trackNearGreenRotation clears the flag and WS-B rolls the spray back. + const ctx: ILoopCtx = { + task: { + id: "t", + intent: "test", + accept: "", + files: ["**/*"], + context: [], + }, + cwd: dir, + tsService: null, + report: () => undefined, + messages: [], + tool: { touched: new Set(["feature.ts"]) }, + gate: { + parse: undefined, + runner: { + run: async (): Promise => ({ + passed: false, + errors: Array.from({ length: 8 }, (_, i) => ({ + key: `feature.ts:${String(i)}:r`, + rule: "r", + file: "feature.ts", + message: `e${String(i)}`, + })), + output: "", + }), + }, + }, + }; + const checkpoint = await captureNearGreenCheckpoint(ctx, 1, [ + { key: "k", rule: "r", file: "feature.ts", message: "near-green" }, + ]); + const state: ILoopState = { + prevGateErrors: [], + gateNoProgress: 0, + bestErrorCount: 1, + noNewLow: 0, + errorAge: new Map(), + lastGateCount: 8, + edits: 3, + regressions: 0, + ttsrInterrupts: 0, + steerLevel: 0, + conventionsEnabled: false, + nearGreenCheckpoint: checkpoint, + nearGreenBest: 1, + nearGreenRollbacks: 0, + nearGreenRotation: true, + nearGreenSamples: [ + { count: 1, phase: 0, sig: "a", rev: "r0" }, + { count: 1, phase: 0, sig: "b", rev: "r1" }, + { count: 1, phase: 0, sig: "c", rev: "r2" }, + ], + nearGreenSpikeGap: 0, + }; + + // Drive one more spike cycle than the gap tolerates → rotation clears, rollback re-arms. + for (let i = 0; i <= MAX_NEAR_GREEN_SPIKE_GAP; i += 1) { + await settleGate(ctx, state, i + 1); + } + + // The stale rotation was dropped and WS-B reverted the persistent spray. + expect(state.nearGreenRotation).toBe(false); + expect(state.nearGreenRollbacks).toBeGreaterThanOrEqual(1); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}, 30_000); + test("#61: during the completion phase, a mixed-error SPIKE is NOT rolled back (banner+rollback stand down through the spike)", async () => { const dir = await mkdtemp(join(tmpdir(), "tsforge-nearg-")); diff --git a/packages/core/tests/near-green-checkpoint.test.ts b/packages/core/tests/near-green-checkpoint.test.ts index cf28ebc7..124aeb67 100644 --- a/packages/core/tests/near-green-checkpoint.test.ts +++ b/packages/core/tests/near-green-checkpoint.test.ts @@ -5,6 +5,9 @@ import { isCompletionClass, allCompletionClass, nextCompletionPhase, + errorSetSignature, + isNearGreenRotation, + ROTATION_WINDOW, NEAR_GREEN_N, NEAR_GREEN_M, MAX_NEAR_GREEN_ROLLBACKS, @@ -139,3 +142,126 @@ test("thresholds are overridable (for config / A/B)", () => { expect(shouldRollback(cp(3), 7, 0, 4, 3)).toBe(true); // N=4,M=3 → 3→7 (+4) expect(shouldRollback(cp(1), 3, 0, 2, 1)).toBe(true); // M=1 → 1→3 (+2) }); + +// #77: near-green ROTATING-error oscillation — the count stays ≤N but the SPECIFIC error set +// rotates (jsx-computation → missing-sibling → no-unsafe-call → …), so each single-error fix +// swaps for a new one and the build never reaches 0 (build17 parked here; build16 crossed by +// luck). The detector must tell rotation apart from a genuinely stuck single error. +const rerr = (rule: string, file: string): IErrorItem => ({ + key: `${file}:1:${rule}`, + rule, + file, + message: rule, +}); + +test("errorSetSignature: order- and line-independent, deduped, keyed by rule|file", () => { + // Order-independent. + expect( + errorSetSignature([rerr("no-unsafe-call", "x.ts"), rerr("jsx", "y.tsx")]) + ).toBe( + errorSetSignature([rerr("jsx", "y.tsx"), rerr("no-unsafe-call", "x.ts")]) + ); + // Line-independent (the key's line moves as code shifts; the signature must not). + expect( + errorSetSignature([ + { key: "x.ts:9:r", rule: "r", file: "x.ts", message: "m" }, + ]) + ).toBe( + errorSetSignature([ + { key: "x.ts:1:r", rule: "r", file: "x.ts", message: "m" }, + ]) + ); + // Deduped: two errors sharing rule|file collapse to one. + expect( + errorSetSignature([ + rerr("no-unsafe-call", "x.ts"), + rerr("no-unsafe-call", "x.ts"), + ]) + ).toBe(errorSetSignature([rerr("no-unsafe-call", "x.ts")])); + // A DIFFERENT set has a different signature (so rotation is detectable). + expect(errorSetSignature([rerr("jsx", "x.ts")])).not.toBe( + errorSetSignature([rerr("no-unsafe-call", "x.ts")]) + ); + // rule AND file both absent (custom gates emit key-only errors): fall back to the required key + // so distinct errors don't all collapse to "|" and hide rotation among them. + expect(errorSetSignature([{ key: "a", message: "m" }])).not.toBe( + errorSetSignature([{ key: "b", message: "m" }]) + ); + // PARTIAL family — file present, rule absent: two DISTINCT errors in the same file must not + // collapse to one "|file" token (they'd hide a rotation between them). The key keeps them apart. + expect( + errorSetSignature([ + { key: "x.ts:1", file: "x.ts", message: "parse error" }, + { key: "x.ts:9", file: "x.ts", message: "type error" }, + ]) + ).not.toBe( + errorSetSignature([{ key: "x.ts:1", file: "x.ts", message: "parse error" }]) + ); + // PARTIAL family — rule present, file absent: two distinct errors sharing a rule stay distinct. + expect(errorSetSignature([{ key: "k1", rule: "r", message: "a" }])).not.toBe( + errorSetSignature([{ key: "k2", rule: "r", message: "b" }]) + ); +}); + +test("isNearGreenRotation: full window + count PLATEAU + changing signature", () => { + // A near-green sample: fixed count 1 and phase 0 unless the test varies them. Each sample gets a + // FRESH worktree rev by default (a real edit happened that cycle) — the #77 genuine-rotation + // precondition; pass an explicit rev to model an unedited re-run. + let revSeq = 0; + const s = ( + sig: string, + count = 1, + phase = 0, + rev = `rev-${String(revSeq++)}` + ): { count: number; phase: number; sig: string; rev: string } => ({ + count, + phase, + sig, + rev, + }); + + expect(ROTATION_WINDOW).toBe(3); + // Fewer than a full window → can't conclude rotation yet. + expect(isNearGreenRotation([s("a"), s("a")])).toBe(false); + // A full window of the SAME signature = a genuinely stuck single error, NOT rotation + // (the escalation ladder/expert handle that; rotation would mis-fire on it). + expect(isNearGreenRotation([s("a"), s("a"), s("a")])).toBe(false); + // GENUINE per-cycle rotation (identity changes EVERY cycle, under fresh edits) = rotation. + expect(isNearGreenRotation([s("a"), s("b"), s("c")])).toBe(true); + // #77 finding: distinct signatures but a CONSTANT rev (no edit between cycles — a flaky/stateful + // gate or the check+settleGate double-run) is NOT rotation. Without the rev guard this fired and + // wrongly stood the WS-B rollback net down. + expect( + isNearGreenRotation([ + s("a", 1, 0, "r"), + s("b", 1, 0, "r"), + s("c", 1, 0, "r"), + ]) + ).toBe(false); + // A 2-cycle RING (A→B→A) is genuine rotation too (no two consecutive equal). + expect(isNearGreenRotation([s("a"), s("b"), s("a")])).toBe(true); + // A single swap then stable (A,A,B — one identity change) is NOT rotation: it's progress toward + // green, not a cycling frontier. Firing here would wrongly disable the WS-B rollback safety net. + expect(isNearGreenRotation([s("a"), s("a"), s("b")])).toBe(false); + expect(isNearGreenRotation([s("a"), s("b"), s("b")])).toBe(false); + // A MOVING count (2→1→1) is progress/regress, NOT rotation — even though the signatures differ, + // the window is not a plateau. This is the panel's count-blind false positive. + expect(isNearGreenRotation([s("a", 2), s("b", 1), s("c", 1)])).toBe(false); + // Same identity, count descends 2→1: not rotation (a plateau requires one count). + expect(isNearGreenRotation([s("a", 2), s("a", 1), s("a", 1)])).toBe(false); + // A plateau at count 2 that rotates IS rotation (near green isn't only count 1). + expect(isNearGreenRotation([s("a", 2), s("b", 2), s("c", 2)])).toBe(true); + // A MOVING gate-frontier PHASE is progress, NOT rotation: A@phase1 → B@phase2 → B@phase2 at a + // stable count is the short-circuit gate revealing the next phase's work, not a rotating error. + expect(isNearGreenRotation([s("a", 1, 1), s("b", 1, 2), s("b", 1, 2)])).toBe( + false + ); + // A rotation WITHIN one phase (constant phase, changing sig) still fires. + expect(isNearGreenRotation([s("a", 1, 2), s("b", 1, 2), s("c", 1, 2)])).toBe( + true + ); + // Only the LAST window matters: an earlier rotation that has since stabilized is not rotation. + expect( + isNearGreenRotation([s("a"), s("b"), s("c"), s("d"), s("d"), s("d")]) + ).toBe(false); +}); diff --git a/packages/core/tests/near-green-rotation.test.ts b/packages/core/tests/near-green-rotation.test.ts new file mode 100644 index 00000000..6cc83054 --- /dev/null +++ b/packages/core/tests/near-green-rotation.test.ts @@ -0,0 +1,380 @@ +import { test, expect, afterEach } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + injectFeedback, + NEAR_GREEN_ROTATION_STEER, + trackNearGreenRotation, +} from "../src/loop/turn"; +import type { ILoopCtx, ILoopState } from "../src/loop/turn"; +import { MAX_NEAR_GREEN_SPIKE_GAP } from "../src/loop/near-green-checkpoint"; +import type { IErrorItem } from "../src/validate/validate.types"; +import type { IValidateResult } from "../src/validate"; +import type { IChatMessage } from "../src/inference"; + +// #77: the settleGate-level tracking that drives the near-green ROTATION steer. It accumulates +// the error-set signatures of near-green (1..N) cycles, IGNORES transient spikes above N (so the +// rotating near-green states accumulate across the regressions between them), and clears on green. + +function freshState(): ILoopState { + return { + prevGateErrors: [], + gateNoProgress: 0, + bestErrorCount: Number.POSITIVE_INFINITY, + noNewLow: 0, + errorAge: new Map(), + lastGateCount: -1, + edits: 0, + regressions: 0, + ttsrInterrupts: 0, + steerLevel: 0, + }; +} + +const e = (rule: string, file: string): IErrorItem => ({ + key: `${file}:1:${rule}`, + rule, + file, + message: rule, +}); + +// trackNearGreenRotation now takes a worktree `rev` (see #77 finding: a rotation is only genuine +// when a real EDIT moved the scope between cycles). These tests model a normal build where the +// model edits every cycle, so unless a test pins `rev` explicitly, each call gets a FRESH rev — +// exactly the "an edit happened this cycle" precondition the detector requires. +let revSeq = 0; + +function track( + s: ILoopState, + count: number, + errors: IErrorItem[], + rev?: string +): void { + trackNearGreenRotation(s, count, errors, rev ?? `rev-${String(revSeq++)}`); +} + +test("rotating near-green error sets flip nearGreenRotation on after a full window", () => { + const s = freshState(); + + track(s, 1, [e("jsx-computation", "Page.tsx")]); + expect(s.nearGreenRotation).toBe(false); // 1 sig — not enough + track(s, 1, [e("component-folder-structure", "Row.tsx")]); + expect(s.nearGreenRotation).toBe(false); // 2 sigs — still short of the window + track(s, 1, [e("no-unsafe-call", "queries.ts")]); + expect(s.nearGreenRotation).toBe(true); // 3 DISTINCT near-green sigs → rotation +}); + +test("a flaky/re-run gate over UNCHANGED files (constant rev) is NOT rotation, even with rotating sigs", () => { + const s = freshState(); + + // The #77 finding: three DISTINCT near-green signatures at the same count/phase — but the + // worktree rev is HELD CONSTANT, i.e. no edit happened between cycles (a flaky/stateful gate, or + // the check-tool + settleGate double-run re-evaluating the same files). Without the rev guard this + // fired rotation and stood the WS-B rollback net down despite no fix-one→spawn-another cycle. + track(s, 1, [e("jsx-computation", "Page.tsx")], "same"); + track(s, 1, [e("component-folder-structure", "Row.tsx")], "same"); + track(s, 1, [e("no-unsafe-call", "queries.ts")], "same"); + + expect(s.nearGreenRotation).toBe(false); +}); + +test("a genuinely stuck single error (same set repeated) is NOT rotation", () => { + const s = freshState(); + + for (let i = 0; i < 5; i++) { + track(s, 1, [e("no-jsx-computation", "Page.tsx")]); + } + + expect(s.nearGreenRotation).toBe(false); +}); + +test("transient spikes above N are IGNORED — they don't reset the rotation window", () => { + const s = freshState(); + + // build17's real shape: near-green states interleaved with spikes to 3/6. + track(s, 1, [e("jsx-computation", "Page.tsx")]); + track(s, 6, [e("x", "a.ts"), e("y", "b.ts")]); // spike — ignored + track(s, 1, [e("missing-sibling", "Row.tsx")]); + track(s, 3, [e("z", "c.ts")]); // spike — ignored + track(s, 1, [e("no-unsafe-call", "queries.ts")]); + + // Three DISTINCT near-green sigs (all at the same count=1 plateau) accumulated across the + // spikes → rotation. + expect(s.nearGreenRotation).toBe(true); + expect(s.nearGreenSamples?.length).toBe(3); // spikes were not pushed +}); + +test("a descent (2→1→1) is NOT rotation — a moving count is progress, not a plateau", () => { + const s = freshState(); + + // Same file/rule identity, but the COUNT changes 2→1→1. The count-blind version treated the + // count-driven signature change as rotation (the panel's false positive); the count plateau + // requirement rejects it. + track(s, 2, [e("a", "x.ts"), e("b", "y.ts")]); + track(s, 1, [e("a", "x.ts")]); + track(s, 1, [e("a", "x.ts")]); + + expect(s.nearGreenRotation).toBe(false); +}); + +test("a moving gate-frontier PHASE is progress, NOT rotation (short-circuit gate reveals the next phase)", () => { + const s = freshState(); + + const phased = (rule: string, file: string, phase: number): IErrorItem => ({ + key: `${file}:1:${rule}`, + rule, + file, + message: rule, + phase, + }); + + // Count stays at 1, but the frontier advances phase1 → phase2 (a downstream phase's error only + // became visible after phase1 cleared). That is progress, so it must NOT read as rotation. + track(s, 1, [phased("a", "x.ts", 1)]); + track(s, 1, [phased("b", "y.ts", 2)]); + track(s, 1, [phased("b", "y.ts", 2)]); + + expect(s.nearGreenRotation).toBe(false); +}); + +test("completion-phase cycles are NOT recorded — the window can't pre-fill then fire post-completion", () => { + const s = freshState(); + + // While adding the missing UI (completion phase) the error identity legitimately churns. None of + // it may accumulate toward rotation, else the flag would be set the instant completion ends. + s.completionPhase = true; + track(s, 1, [e("reachability", "Page.tsx")]); + track(s, 1, [e("i18n-locale-keys-used", "Page.tsx")]); + track(s, 1, [e("no-unsafe-call", "queries.ts")]); + + expect(s.nearGreenRotation).toBe(false); + expect(s.nearGreenSamples).toEqual([]); +}); + +test("kill-switch flipped ON mid-run CLEARS sticky rotation state (authoritative, not just early-return)", () => { + const s = freshState(); + + track(s, 1, [e("a", "x.ts")]); + track(s, 1, [e("b", "y.ts")]); + track(s, 1, [e("c", "z.ts")]); + expect(s.nearGreenRotation).toBe(true); + + process.env.TSFORGE_NO_NEAR_GREEN_ROTATION = "1"; + + try { + track(s, 1, [e("d", "w.ts")]); + // Disabled mid-run: the sticky flag is cleared, not left set — so injectFeedback stops. + expect(s.nearGreenRotation).toBe(false); + expect(s.nearGreenSamples).toEqual([]); + } finally { + delete process.env.TSFORGE_NO_NEAR_GREEN_ROTATION; + } +}); + +test("green (curr 0) clears the window and the flag", () => { + const s = freshState(); + + track(s, 1, [e("a", "x.ts")]); + track(s, 1, [e("b", "y.ts")]); + track(s, 1, [e("c", "z.ts")]); + expect(s.nearGreenRotation).toBe(true); + + track(s, 0, []); + expect(s.nearGreenRotation).toBe(false); + expect(s.nearGreenSamples).toEqual([]); + expect(s.nearGreenSpikeGap).toBe(0); +}); + +// #77 — the USER-VISIBLE output: injectFeedback must prepend NEAR_GREEN_ROTATION_STEER only +// when the detector fired AND the current cycle is genuinely near green AND it's not the +// completion phase (which legitimately wants the model to ADD the missing UI, contradicting a +// "do not create new files" steer). These pin the three-condition gate the panel flagged as +// untested — a regression in the concatenation or the conditional would otherwise pass silently. + +const dirs: string[] = []; + +afterEach(async () => { + await Promise.all( + dirs.splice(0).map((d) => rm(d, { recursive: true, force: true })) + ); +}); + +async function makeCtx(): Promise<{ + ctx: ILoopCtx; + messages: IChatMessage[]; +}> { + const messages: IChatMessage[] = []; + const cwd = await mkdtemp(join(tmpdir(), "tsforge-rotation-inj-")); + + dirs.push(cwd); + + const ctx: ILoopCtx = { + task: { id: "t", intent: "test", accept: "", files: ["**/*"], context: [] }, + cwd, + tsService: null, + report: () => undefined, + messages, + tool: { touched: new Set() }, + gate: { + parse: undefined, + runner: { + run: async (): Promise => ({ + passed: true, + errors: [], + output: "", + }), + }, + }, + }; + + return { ctx, messages }; +} + +test("injectFeedback PREPENDS the rotation steer when detector fired + near green + not completion", async () => { + const { ctx, messages } = await makeCtx(); + const s = freshState(); + + s.nearGreenRotation = true; + + await injectFeedback(ctx, s, [e("jsx-computation", "Page.tsx")], [], []); + + expect(messages).toHaveLength(1); + expect(messages[0]?.content.startsWith(NEAR_GREEN_ROTATION_STEER)).toBe(true); + // The near-green banner ("Do NOT create new files") is SUPPRESSED when the rotation steer fires + // — else the model gets that flat prohibition stacked against the steer's "create the required + // sibling/test files together in one edit", i.e. contradictory last-mile instructions. + expect(messages[0]?.content.includes("NEAR-GREEN — only")).toBe(false); +}); + +test("injectFeedback SUPPRESSES the rotation steer on a spike above N (flag stays set, count too high)", async () => { + const { ctx, messages } = await makeCtx(); + const s = freshState(); + + s.nearGreenRotation = true; + + // 3 errors > NEAR_GREEN_N (2): the sticky flag is still true, but the "one or two errors from + // done" wording doesn't hold at a spike, so it must NOT be injected. + await injectFeedback( + ctx, + s, + [e("a", "a.ts"), e("b", "b.ts"), e("c", "c.ts")], + [], + [] + ); + + expect(messages).toHaveLength(1); + expect(messages[0]?.content.includes(NEAR_GREEN_ROTATION_STEER)).toBe(false); +}); + +test("injectFeedback SUPPRESSES the rotation steer during the completion phase", async () => { + const { ctx, messages } = await makeCtx(); + const s = freshState(); + + s.nearGreenRotation = true; + s.completionPhase = true; + + await injectFeedback(ctx, s, [e("reachability", "Page.tsx")], [], []); + + expect(messages).toHaveLength(1); + expect(messages[0]?.content.includes(NEAR_GREEN_ROTATION_STEER)).toBe(false); +}); + +test("injectFeedback on a ROTATION SPIKE suppresses the undo/lockdown banner (no contradiction with the steer)", async () => { + const { ctx, messages } = await makeCtx(); + const s = freshState(); + + // Sticky rotation through the atomic-completion spike; best was 1 so a spike to 8 is a + // regression from best. WS-B stands the rollback down for this spike — the banner must match + // and NOT tell the model to undo it / not create files (that's build17's model-driven loop). + s.nearGreenRotation = true; + s.bestErrorCount = 1; + + await injectFeedback( + ctx, + s, + Array.from({ length: 8 }, (_, i) => e(`r${String(i)}`, `f${String(i)}.ts`)), + [], + [] + ); + + const content = messages[0]?.content ?? ""; + + expect(content.includes("REGRESSION")).toBe(false); + expect(content.includes("UNDO")).toBe(false); + expect(content.includes("Do NOT create new files")).toBe(false); + // The steer is near-green-only, so it's absent on the spike — the model just gets the errors. + expect(content.includes(NEAR_GREEN_ROTATION_STEER)).toBe(false); +}); + +test("injectFeedback does NOT inject the rotation steer when the detector never fired", async () => { + const { ctx, messages } = await makeCtx(); + const s = freshState(); + + // nearGreenRotation unset — a normal near-green cycle with no rotation. + await injectFeedback(ctx, s, [e("jsx-computation", "Page.tsx")], [], []); + + expect(messages).toHaveLength(1); + expect(messages[0]?.content.includes(NEAR_GREEN_ROTATION_STEER)).toBe(false); +}); + +test("injectFeedback emit-path checks the flag: kill-switch OFF suppresses the steer even with sticky state set", async () => { + const { ctx, messages } = await makeCtx(); + const s = freshState(); + + // Sticky state left true (e.g. the detector fired before the kill-switch was flipped). The + // emit-path flag check — not only the tracker's clear-on-disable — must stop the injection. + s.nearGreenRotation = true; + process.env.TSFORGE_NO_NEAR_GREEN_ROTATION = "1"; + + try { + await injectFeedback(ctx, s, [e("jsx-computation", "Page.tsx")], [], []); + expect(messages[0]?.content.includes(NEAR_GREEN_ROTATION_STEER)).toBe( + false + ); + } finally { + delete process.env.TSFORGE_NO_NEAR_GREEN_ROTATION; + } +}); + +test("rotating at a near-green count above best: the steer leads and the banner is FULLY suppressed (no REGRESSION/UNDO/lockdown)", async () => { + const { ctx, messages } = await makeCtx(); + const s = freshState(); + + // Plateau at 2 (near green, ≤ N) above a best of 1 → rotation window AND a regression from best. + // During rotation the model is in atomic-completion mode and WS-B has stood down, so the banner + // is suppressed WHOLE — its "UNDO that collateral to get back to best" would tell the model to + // undo the very completion the steer is asking for (build17's loop). The steer alone leads. + s.nearGreenRotation = true; + s.bestErrorCount = 1; + + await injectFeedback(ctx, s, [e("a", "x.ts"), e("b", "y.ts")], [], []); + + const content = messages[0]?.content ?? ""; + + expect(content.startsWith(NEAR_GREEN_ROTATION_STEER)).toBe(true); + expect(content.includes("REGRESSION")).toBe(false); + expect(content.includes("UNDO")).toBe(false); + expect(content.includes("NEAR-GREEN — only")).toBe(false); +}); + +test("near-green samples separated by more than the spike-gap bound do NOT combine into a false rotation", () => { + const s = freshState(); + + // Two near-green visits, then a LONG regression (more than MAX_NEAR_GREEN_SPIKE_GAP consecutive + // spikes), then another near-green visit. The stale early samples must be dropped so a fresh, + // unrelated near-green episode can't combine with them into a false rotation. + track(s, 1, [e("a", "x.ts")]); + track(s, 1, [e("b", "y.ts")]); + + for (let i = 0; i <= MAX_NEAR_GREEN_SPIKE_GAP; i++) { + track(s, 9, [e("spray", `s${String(i)}.ts`)]); + } + + track(s, 1, [e("c", "z.ts")]); + + // The window was cleared by the over-long spike run, so only the last sample survives → not a + // full window → not rotation. + expect(s.nearGreenRotation).toBe(false); + expect(s.nearGreenSamples?.length).toBe(1); +}); diff --git a/packages/core/tests/session-gate.test.ts b/packages/core/tests/session-gate.test.ts index 2f71d71a..09a31af3 100644 --- a/packages/core/tests/session-gate.test.ts +++ b/packages/core/tests/session-gate.test.ts @@ -56,6 +56,11 @@ test("resetDriveConvergence gives a revisit a fresh ladder while preserving cumu // WS-B is per-drive — the send-boundary reset must clear its watermark + budget too. nearGreenBest: 1, nearGreenRollbacks: 3, + // #77: the rotation window + flag are per-drive too — a stale flag would inject the + // completion-only steer on a fresh drive with no evidence. + nearGreenSamples: [{ count: 1, phase: 0, sig: "a", rev: "r0" }], + nearGreenSpikeGap: 2, + nearGreenRotation: true, }); resetDriveConvergence(state); @@ -71,6 +76,10 @@ test("resetDriveConvergence gives a revisit a fresh ladder while preserving cumu expect(state.nearGreenBest).toBeUndefined(); expect(state.nearGreenRollbacks).toBeUndefined(); expect(state.nearGreenCheckpoint).toBeUndefined(); + // #77: the rotation window + flag + spike-gap are per-drive — cleared so they can't leak. + expect(state.nearGreenSamples).toBeUndefined(); + expect(state.nearGreenSpikeGap).toBeUndefined(); + expect(state.nearGreenRotation).toBeUndefined(); }); test("Session.setGate flips hasGate on and routes the gate through the loop", async () => {