From 6142c35bc096109d46e454739962e0957d906a95 Mon Sep 17 00:00:00 2001 From: Franklin Moormann Date: Fri, 7 Aug 2026 22:54:37 -0400 Subject: [PATCH] fix(keepwarm,lessons): make the tripwire reachable, the gaps per-session, the anchors real Three defects that #293 named in its description and did not fix. It merged before this landed, so they are still on master. Listing a defect is not fixing it, which is the whole reason this exists as its own change. THE TRIPWIRE COULD NEVER REACH ITS OWN THRESHOLD. It read outcomes through readMetrics, whose window is 5000 events and 2 MB, and there is one keepwarm outcome per refresh in a log dominated by reads and captures. On any busy project the ten TRIPWIRE_MIN demands aged out before the tenth was written, so it returned "only N/10 refreshes observed" for the life of the project and shouldKeepWarm could never be vetoed. A guard that cannot reach its own threshold is not a guard. 'keepwarm' joins BALANCE_KINDS, so outcomes are mirrored into balance.jsonl and read back unwindowed. That also required splitting shouldKeepWarm's event sources, which passed ONE array to both consumers. They want opposite things: gapDistribution wants the firehose, because every event is a timestamp and the recent ones describe the current rhythm; the tripwire wants the unwindowed log, because its outcomes are exactly what the window evicts. Whichever reader was chosen was wrong for one. A GAP BETWEEN TWO SESSIONS IS NOT A GAP BETWEEN TURNS. Every timestamp was sorted and differenced, so the interval between the last event of one session and the first of the next -- routinely sixteen hours -- was counted as a turn gap. It dominated p90 and p99 and diluted probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on projects where it would have paid. Silent, because that bias only ever declines to act. Long gaps INSIDE a session are deliberately kept. They are real evidence that caching does not pay there, and dropping them would bias the answer the other way -- making keep-warm look better than it is, which is the direction this project cares about most. Events with no sessionId are pooled rather than discarded, since most kinds carry one and discarding the rest would throw away whole projects' history for a technicality. LESSON ANCHORS ARE HELD TO THE FILES THE SESSION TOUCHED. The finding path in the same worker already passes knownFiles: filesIn(digest) and explains why in its own comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while the lesson path accepted any non-empty string, so the identical hallucination walked straight through. A lesson anchored to a file nobody opened is then injected on every future touch of that file: a permanent instruction attached to code it was never about. Anchors are also checked with isFsSafePath, because these strings reach the filesystem later through the graph and a path holding U+10FFFF aborts the process rather than throwing. Rebased onto current master, which has since taken #291 and #293. The only conflict was BALANCE_KINDS, where master's set had gained the two forecast kinds; resolved by keeping master's and adding 'keepwarm' to it. Verified: tests/hooks 831 passed, 3 skipped, 0 failed across 54 suites, with 7 new tests. tsc --noEmit and prettier --check clean. Co-Authored-By: Claude Opus 5 --- hooks-core/keepwarm.mjs | 59 +++++++++++++++--- hooks-core/lessons.mjs | 16 ++++- hooks-core/metrics.mjs | 5 ++ integrations/codex/hooks/lib/keepwarm.mjs | 59 +++++++++++++++--- integrations/codex/hooks/lib/lessons.mjs | 16 ++++- integrations/codex/hooks/lib/metrics.mjs | 5 ++ .../codex/plugin/hooks/lib/keepwarm.mjs | 59 +++++++++++++++--- .../codex/plugin/hooks/lib/lessons.mjs | 16 ++++- .../codex/plugin/hooks/lib/metrics.mjs | 5 ++ integrations/gemini/hooks/lib/keepwarm.mjs | 59 +++++++++++++++--- integrations/gemini/hooks/lib/lessons.mjs | 16 ++++- integrations/gemini/hooks/lib/metrics.mjs | 5 ++ integrations/opencode/hooks/lib/keepwarm.mjs | 59 +++++++++++++++--- integrations/opencode/hooks/lib/lessons.mjs | 16 ++++- integrations/opencode/hooks/lib/metrics.mjs | 5 ++ integrations/qwen/hooks/lib/keepwarm.mjs | 59 +++++++++++++++--- integrations/qwen/hooks/lib/lessons.mjs | 16 ++++- integrations/qwen/hooks/lib/metrics.mjs | 5 ++ plugin/hooks/harvest-worker.mjs | 7 ++- plugin/hooks/lib/keepwarm.mjs | 59 +++++++++++++++--- plugin/hooks/lib/lessons.mjs | 16 ++++- plugin/hooks/lib/metrics.mjs | 5 ++ tests/hooks/cache.test.mjs | 62 +++++++++++++++++++ tests/hooks/lessons.test.mjs | 35 +++++++++++ 24 files changed, 586 insertions(+), 78 deletions(-) diff --git a/hooks-core/keepwarm.mjs b/hooks-core/keepwarm.mjs index aa0fa7b00..68942ee36 100644 --- a/hooks-core/keepwarm.mjs +++ b/hooks-core/keepwarm.mjs @@ -24,7 +24,7 @@ * whole thing stops and says so, which is the same rule the forecast follows. */ -import { record, readMetrics } from './metrics.mjs'; +import { record, readMetrics, readBalance } from './metrics.mjs'; import { WRITE_MULTIPLIER, READ_MULTIPLIER } from './cache.mjs'; /** @@ -54,14 +54,37 @@ export const TRIPWIRE_MIN = 10; * was called. */ export function gapDistribution(dir, { events = readMetrics(dir) } = {}) { - const stamps = events.map((e) => e.at).filter(Number.isFinite).sort((a, b) => a - b); + // PARTITIONED BY SESSION, because the interval between the last event of one session and the + // first of the next is not a gap between turns at all. The log spans days, so an overnight + // sixteen-hour boundary was counted as one -- dominating p90 and p99 and diluting + // probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on + // projects where it would have paid. Silent, because that bias only ever declines to act. + // + // A long gap WITHIN a session is kept. It is real evidence that caching does not pay there, and + // dropping it would bias the answer the other way -- making keep-warm look better than it is, + // which is the direction this project cares about most. + // + // Events carrying no sessionId are pooled into one group rather than discarded: most kinds do + // carry one, and discarding the rest would throw away whole projects' history for a technicality. + const bySession = new Map(); + for (const event of events) { + if (!Number.isFinite(event?.at)) continue; + const key = event.sessionId || ''; + if (!bySession.has(key)) bySession.set(key, []); + bySession.get(key).push(event.at); + } + + const stamps = [...bySession.values()].flat(); if (stamps.length < 8) return null; const gaps = []; - for (let i = 1; i < stamps.length; i++) { - const gap = stamps[i] - stamps[i - 1]; - // A burst of events inside one turn is not a gap between turns. - if (gap > 250) gaps.push(gap); + for (const session of bySession.values()) { + session.sort((a, b) => a - b); + for (let i = 1; i < session.length; i++) { + const gap = session[i] - session[i - 1]; + // A burst of events inside one turn is not a gap between turns. + if (gap > 250) gaps.push(gap); + } } if (gaps.length < 6) return null; @@ -207,7 +230,16 @@ export function recordRefreshOutcome(dir, { tier, prefixTokens, hit }) { * refreshes, keep-warm stops -- and says which tier and by how much, so the * stop is a finding rather than a silent behaviour change. */ -export function tripwire(dir, { events = readMetrics(dir) } = {}) { +/** + * The backstop, read from the log that is NOT windowed. + * + * readBalance rather than readMetrics, and that distinction is the difference between a backstop + * that works and one that cannot. There is one outcome per refresh, in a log dominated by reads + * and captures, so through the 5000-event window the ten TRIPWIRE_MIN demands aged out before the + * tenth was written -- and this returned "only N/10 refreshes observed" for the life of the + * project. A guard that can never reach its own threshold is not a guard. + */ +export function tripwire(dir, { events = readBalance(dir) } = {}) { const outcomes = events.filter((e) => e.kind === 'keepwarm' && e.action === 'outcome'); if (outcomes.length < TRIPWIRE_MIN) { return { tripped: false, observed: outcomes.length, reason: `only ${outcomes.length}/${TRIPWIRE_MIN} refreshes observed` }; @@ -232,8 +264,17 @@ export function tripwire(dir, { events = readMetrics(dir) } = {}) { * The decision as it should actually be taken: expected value, with the * tripwire able to veto it. */ -export function shouldKeepWarm(dir, { prefixTokens, events = readMetrics(dir) } = {}) { - const trip = tripwire(dir, { events }); +export function shouldKeepWarm(dir, { + prefixTokens, + events = readMetrics(dir), + // TWO SOURCES, because the two consumers want different things. gapDistribution wants the + // firehose -- every event is a timestamp and the recent ones describe the current rhythm. The + // tripwire wants the unwindowed balance log, because its outcomes are rare and are exactly what + // the window evicts. Passing one shared array to both, as this used to, meant whichever reader + // was chosen was wrong for one of them. + outcomes = readBalance(dir), +} = {}) { + const trip = tripwire(dir, { events: outcomes }); if (trip.tripped) return { action: 'skip', reason: trip.reason, trippedWire: true }; const gaps = gapDistribution(dir, { events }); diff --git a/hooks-core/lessons.mjs b/hooks-core/lessons.mjs index 9b707e377..88214424c 100644 --- a/hooks-core/lessons.mjs +++ b/hooks-core/lessons.mjs @@ -29,6 +29,7 @@ import { ORIGIN_HARVESTED, ORIGIN_HUMAN } from './curate.mjs'; import { safeTrigger } from './inject.mjs'; +import { isFsSafePath } from './paths.mjs'; /** Lessons longer than this are prose, not instructions. */ const MAX_CLAIM = 400; @@ -170,7 +171,7 @@ export function isImperative(claim) { * Returns { lessons, rejected } so a caller can report what was dropped rather * than silently storing less than it was given. */ -export function validateLessons(raw, turns) { +export function validateLessons(raw, turns, { knownFiles = null } = {}) { let parsed = raw; if (typeof raw === 'string') { try { @@ -236,8 +237,19 @@ export function validateLessons(raw, turns) { // A verified human correction is as close to ground truth as this system // gets. An unverified paraphrase is a model's reading of one. confidence: verified ? 0.95 : 0.6, + // HELD TO THE FILES THE SESSION ACTUALLY TOUCHED, exactly as the finding path already is. + // harvest-worker calls validate() with `knownFiles: filesIn(digest)` and says why in its own + // comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while + // this path accepted any non-empty string. The identical hallucination therefore walked + // straight through, and a lesson anchored to a file nobody opened is then injected on every + // future touch of it: a permanent instruction attached to code it was never about. + // + // isFsSafePath as well, because these strings reach the filesystem later through the graph. + // A path holding U+10FFFF ABORTS the process inside libuv rather than throwing, so no + // try/catch downstream can contain it. anchors: Array.isArray(item?.anchors) - ? item.anchors.filter((a) => typeof a === 'string' && a.trim()) + ? item.anchors.filter((a) => typeof a === 'string' && a.trim() && isFsSafePath(a) + && (!knownFiles || knownFiles.includes(a))) : [], }); } diff --git a/hooks-core/metrics.mjs b/hooks-core/metrics.mjs index ae62a1166..eb611652a 100644 --- a/hooks-core/metrics.mjs +++ b/hooks-core/metrics.mjs @@ -76,6 +76,11 @@ export const BALANCE_KINDS = new Set([ // and an outcome whose forecast had scrolled away was dropped with no record at all. 'forecast', 'forecast-outcome', + // The keep-warm tripwire's evidence. It needs TRIPWIRE_MIN outcomes before it may have an + // opinion, and there is one outcome per refresh -- so through the windowed reader the tenth + // aged out before it was written and the backstop was structurally unable to fire, reporting + // "only N/10 refreshes observed" for the life of the project. + 'keepwarm', ]); /** diff --git a/integrations/codex/hooks/lib/keepwarm.mjs b/integrations/codex/hooks/lib/keepwarm.mjs index 0c12e62f9..142f838d1 100644 --- a/integrations/codex/hooks/lib/keepwarm.mjs +++ b/integrations/codex/hooks/lib/keepwarm.mjs @@ -26,7 +26,7 @@ * whole thing stops and says so, which is the same rule the forecast follows. */ -import { record, readMetrics } from './metrics.mjs'; +import { record, readMetrics, readBalance } from './metrics.mjs'; import { WRITE_MULTIPLIER, READ_MULTIPLIER } from './cache.mjs'; /** @@ -56,14 +56,37 @@ export const TRIPWIRE_MIN = 10; * was called. */ export function gapDistribution(dir, { events = readMetrics(dir) } = {}) { - const stamps = events.map((e) => e.at).filter(Number.isFinite).sort((a, b) => a - b); + // PARTITIONED BY SESSION, because the interval between the last event of one session and the + // first of the next is not a gap between turns at all. The log spans days, so an overnight + // sixteen-hour boundary was counted as one -- dominating p90 and p99 and diluting + // probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on + // projects where it would have paid. Silent, because that bias only ever declines to act. + // + // A long gap WITHIN a session is kept. It is real evidence that caching does not pay there, and + // dropping it would bias the answer the other way -- making keep-warm look better than it is, + // which is the direction this project cares about most. + // + // Events carrying no sessionId are pooled into one group rather than discarded: most kinds do + // carry one, and discarding the rest would throw away whole projects' history for a technicality. + const bySession = new Map(); + for (const event of events) { + if (!Number.isFinite(event?.at)) continue; + const key = event.sessionId || ''; + if (!bySession.has(key)) bySession.set(key, []); + bySession.get(key).push(event.at); + } + + const stamps = [...bySession.values()].flat(); if (stamps.length < 8) return null; const gaps = []; - for (let i = 1; i < stamps.length; i++) { - const gap = stamps[i] - stamps[i - 1]; - // A burst of events inside one turn is not a gap between turns. - if (gap > 250) gaps.push(gap); + for (const session of bySession.values()) { + session.sort((a, b) => a - b); + for (let i = 1; i < session.length; i++) { + const gap = session[i] - session[i - 1]; + // A burst of events inside one turn is not a gap between turns. + if (gap > 250) gaps.push(gap); + } } if (gaps.length < 6) return null; @@ -209,7 +232,16 @@ export function recordRefreshOutcome(dir, { tier, prefixTokens, hit }) { * refreshes, keep-warm stops -- and says which tier and by how much, so the * stop is a finding rather than a silent behaviour change. */ -export function tripwire(dir, { events = readMetrics(dir) } = {}) { +/** + * The backstop, read from the log that is NOT windowed. + * + * readBalance rather than readMetrics, and that distinction is the difference between a backstop + * that works and one that cannot. There is one outcome per refresh, in a log dominated by reads + * and captures, so through the 5000-event window the ten TRIPWIRE_MIN demands aged out before the + * tenth was written -- and this returned "only N/10 refreshes observed" for the life of the + * project. A guard that can never reach its own threshold is not a guard. + */ +export function tripwire(dir, { events = readBalance(dir) } = {}) { const outcomes = events.filter((e) => e.kind === 'keepwarm' && e.action === 'outcome'); if (outcomes.length < TRIPWIRE_MIN) { return { tripped: false, observed: outcomes.length, reason: `only ${outcomes.length}/${TRIPWIRE_MIN} refreshes observed` }; @@ -234,8 +266,17 @@ export function tripwire(dir, { events = readMetrics(dir) } = {}) { * The decision as it should actually be taken: expected value, with the * tripwire able to veto it. */ -export function shouldKeepWarm(dir, { prefixTokens, events = readMetrics(dir) } = {}) { - const trip = tripwire(dir, { events }); +export function shouldKeepWarm(dir, { + prefixTokens, + events = readMetrics(dir), + // TWO SOURCES, because the two consumers want different things. gapDistribution wants the + // firehose -- every event is a timestamp and the recent ones describe the current rhythm. The + // tripwire wants the unwindowed balance log, because its outcomes are rare and are exactly what + // the window evicts. Passing one shared array to both, as this used to, meant whichever reader + // was chosen was wrong for one of them. + outcomes = readBalance(dir), +} = {}) { + const trip = tripwire(dir, { events: outcomes }); if (trip.tripped) return { action: 'skip', reason: trip.reason, trippedWire: true }; const gaps = gapDistribution(dir, { events }); diff --git a/integrations/codex/hooks/lib/lessons.mjs b/integrations/codex/hooks/lib/lessons.mjs index da932db24..89f3d408e 100644 --- a/integrations/codex/hooks/lib/lessons.mjs +++ b/integrations/codex/hooks/lib/lessons.mjs @@ -31,6 +31,7 @@ import { ORIGIN_HARVESTED, ORIGIN_HUMAN } from './curate.mjs'; import { safeTrigger } from './inject.mjs'; +import { isFsSafePath } from './paths.mjs'; /** Lessons longer than this are prose, not instructions. */ const MAX_CLAIM = 400; @@ -172,7 +173,7 @@ export function isImperative(claim) { * Returns { lessons, rejected } so a caller can report what was dropped rather * than silently storing less than it was given. */ -export function validateLessons(raw, turns) { +export function validateLessons(raw, turns, { knownFiles = null } = {}) { let parsed = raw; if (typeof raw === 'string') { try { @@ -238,8 +239,19 @@ export function validateLessons(raw, turns) { // A verified human correction is as close to ground truth as this system // gets. An unverified paraphrase is a model's reading of one. confidence: verified ? 0.95 : 0.6, + // HELD TO THE FILES THE SESSION ACTUALLY TOUCHED, exactly as the finding path already is. + // harvest-worker calls validate() with `knownFiles: filesIn(digest)` and says why in its own + // comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while + // this path accepted any non-empty string. The identical hallucination therefore walked + // straight through, and a lesson anchored to a file nobody opened is then injected on every + // future touch of it: a permanent instruction attached to code it was never about. + // + // isFsSafePath as well, because these strings reach the filesystem later through the graph. + // A path holding U+10FFFF ABORTS the process inside libuv rather than throwing, so no + // try/catch downstream can contain it. anchors: Array.isArray(item?.anchors) - ? item.anchors.filter((a) => typeof a === 'string' && a.trim()) + ? item.anchors.filter((a) => typeof a === 'string' && a.trim() && isFsSafePath(a) + && (!knownFiles || knownFiles.includes(a))) : [], }); } diff --git a/integrations/codex/hooks/lib/metrics.mjs b/integrations/codex/hooks/lib/metrics.mjs index 4b0eb2712..734d36d04 100644 --- a/integrations/codex/hooks/lib/metrics.mjs +++ b/integrations/codex/hooks/lib/metrics.mjs @@ -78,6 +78,11 @@ export const BALANCE_KINDS = new Set([ // and an outcome whose forecast had scrolled away was dropped with no record at all. 'forecast', 'forecast-outcome', + // The keep-warm tripwire's evidence. It needs TRIPWIRE_MIN outcomes before it may have an + // opinion, and there is one outcome per refresh -- so through the windowed reader the tenth + // aged out before it was written and the backstop was structurally unable to fire, reporting + // "only N/10 refreshes observed" for the life of the project. + 'keepwarm', ]); /** diff --git a/integrations/codex/plugin/hooks/lib/keepwarm.mjs b/integrations/codex/plugin/hooks/lib/keepwarm.mjs index 0c12e62f9..142f838d1 100644 --- a/integrations/codex/plugin/hooks/lib/keepwarm.mjs +++ b/integrations/codex/plugin/hooks/lib/keepwarm.mjs @@ -26,7 +26,7 @@ * whole thing stops and says so, which is the same rule the forecast follows. */ -import { record, readMetrics } from './metrics.mjs'; +import { record, readMetrics, readBalance } from './metrics.mjs'; import { WRITE_MULTIPLIER, READ_MULTIPLIER } from './cache.mjs'; /** @@ -56,14 +56,37 @@ export const TRIPWIRE_MIN = 10; * was called. */ export function gapDistribution(dir, { events = readMetrics(dir) } = {}) { - const stamps = events.map((e) => e.at).filter(Number.isFinite).sort((a, b) => a - b); + // PARTITIONED BY SESSION, because the interval between the last event of one session and the + // first of the next is not a gap between turns at all. The log spans days, so an overnight + // sixteen-hour boundary was counted as one -- dominating p90 and p99 and diluting + // probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on + // projects where it would have paid. Silent, because that bias only ever declines to act. + // + // A long gap WITHIN a session is kept. It is real evidence that caching does not pay there, and + // dropping it would bias the answer the other way -- making keep-warm look better than it is, + // which is the direction this project cares about most. + // + // Events carrying no sessionId are pooled into one group rather than discarded: most kinds do + // carry one, and discarding the rest would throw away whole projects' history for a technicality. + const bySession = new Map(); + for (const event of events) { + if (!Number.isFinite(event?.at)) continue; + const key = event.sessionId || ''; + if (!bySession.has(key)) bySession.set(key, []); + bySession.get(key).push(event.at); + } + + const stamps = [...bySession.values()].flat(); if (stamps.length < 8) return null; const gaps = []; - for (let i = 1; i < stamps.length; i++) { - const gap = stamps[i] - stamps[i - 1]; - // A burst of events inside one turn is not a gap between turns. - if (gap > 250) gaps.push(gap); + for (const session of bySession.values()) { + session.sort((a, b) => a - b); + for (let i = 1; i < session.length; i++) { + const gap = session[i] - session[i - 1]; + // A burst of events inside one turn is not a gap between turns. + if (gap > 250) gaps.push(gap); + } } if (gaps.length < 6) return null; @@ -209,7 +232,16 @@ export function recordRefreshOutcome(dir, { tier, prefixTokens, hit }) { * refreshes, keep-warm stops -- and says which tier and by how much, so the * stop is a finding rather than a silent behaviour change. */ -export function tripwire(dir, { events = readMetrics(dir) } = {}) { +/** + * The backstop, read from the log that is NOT windowed. + * + * readBalance rather than readMetrics, and that distinction is the difference between a backstop + * that works and one that cannot. There is one outcome per refresh, in a log dominated by reads + * and captures, so through the 5000-event window the ten TRIPWIRE_MIN demands aged out before the + * tenth was written -- and this returned "only N/10 refreshes observed" for the life of the + * project. A guard that can never reach its own threshold is not a guard. + */ +export function tripwire(dir, { events = readBalance(dir) } = {}) { const outcomes = events.filter((e) => e.kind === 'keepwarm' && e.action === 'outcome'); if (outcomes.length < TRIPWIRE_MIN) { return { tripped: false, observed: outcomes.length, reason: `only ${outcomes.length}/${TRIPWIRE_MIN} refreshes observed` }; @@ -234,8 +266,17 @@ export function tripwire(dir, { events = readMetrics(dir) } = {}) { * The decision as it should actually be taken: expected value, with the * tripwire able to veto it. */ -export function shouldKeepWarm(dir, { prefixTokens, events = readMetrics(dir) } = {}) { - const trip = tripwire(dir, { events }); +export function shouldKeepWarm(dir, { + prefixTokens, + events = readMetrics(dir), + // TWO SOURCES, because the two consumers want different things. gapDistribution wants the + // firehose -- every event is a timestamp and the recent ones describe the current rhythm. The + // tripwire wants the unwindowed balance log, because its outcomes are rare and are exactly what + // the window evicts. Passing one shared array to both, as this used to, meant whichever reader + // was chosen was wrong for one of them. + outcomes = readBalance(dir), +} = {}) { + const trip = tripwire(dir, { events: outcomes }); if (trip.tripped) return { action: 'skip', reason: trip.reason, trippedWire: true }; const gaps = gapDistribution(dir, { events }); diff --git a/integrations/codex/plugin/hooks/lib/lessons.mjs b/integrations/codex/plugin/hooks/lib/lessons.mjs index da932db24..89f3d408e 100644 --- a/integrations/codex/plugin/hooks/lib/lessons.mjs +++ b/integrations/codex/plugin/hooks/lib/lessons.mjs @@ -31,6 +31,7 @@ import { ORIGIN_HARVESTED, ORIGIN_HUMAN } from './curate.mjs'; import { safeTrigger } from './inject.mjs'; +import { isFsSafePath } from './paths.mjs'; /** Lessons longer than this are prose, not instructions. */ const MAX_CLAIM = 400; @@ -172,7 +173,7 @@ export function isImperative(claim) { * Returns { lessons, rejected } so a caller can report what was dropped rather * than silently storing less than it was given. */ -export function validateLessons(raw, turns) { +export function validateLessons(raw, turns, { knownFiles = null } = {}) { let parsed = raw; if (typeof raw === 'string') { try { @@ -238,8 +239,19 @@ export function validateLessons(raw, turns) { // A verified human correction is as close to ground truth as this system // gets. An unverified paraphrase is a model's reading of one. confidence: verified ? 0.95 : 0.6, + // HELD TO THE FILES THE SESSION ACTUALLY TOUCHED, exactly as the finding path already is. + // harvest-worker calls validate() with `knownFiles: filesIn(digest)` and says why in its own + // comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while + // this path accepted any non-empty string. The identical hallucination therefore walked + // straight through, and a lesson anchored to a file nobody opened is then injected on every + // future touch of it: a permanent instruction attached to code it was never about. + // + // isFsSafePath as well, because these strings reach the filesystem later through the graph. + // A path holding U+10FFFF ABORTS the process inside libuv rather than throwing, so no + // try/catch downstream can contain it. anchors: Array.isArray(item?.anchors) - ? item.anchors.filter((a) => typeof a === 'string' && a.trim()) + ? item.anchors.filter((a) => typeof a === 'string' && a.trim() && isFsSafePath(a) + && (!knownFiles || knownFiles.includes(a))) : [], }); } diff --git a/integrations/codex/plugin/hooks/lib/metrics.mjs b/integrations/codex/plugin/hooks/lib/metrics.mjs index 4b0eb2712..734d36d04 100644 --- a/integrations/codex/plugin/hooks/lib/metrics.mjs +++ b/integrations/codex/plugin/hooks/lib/metrics.mjs @@ -78,6 +78,11 @@ export const BALANCE_KINDS = new Set([ // and an outcome whose forecast had scrolled away was dropped with no record at all. 'forecast', 'forecast-outcome', + // The keep-warm tripwire's evidence. It needs TRIPWIRE_MIN outcomes before it may have an + // opinion, and there is one outcome per refresh -- so through the windowed reader the tenth + // aged out before it was written and the backstop was structurally unable to fire, reporting + // "only N/10 refreshes observed" for the life of the project. + 'keepwarm', ]); /** diff --git a/integrations/gemini/hooks/lib/keepwarm.mjs b/integrations/gemini/hooks/lib/keepwarm.mjs index 0c12e62f9..142f838d1 100644 --- a/integrations/gemini/hooks/lib/keepwarm.mjs +++ b/integrations/gemini/hooks/lib/keepwarm.mjs @@ -26,7 +26,7 @@ * whole thing stops and says so, which is the same rule the forecast follows. */ -import { record, readMetrics } from './metrics.mjs'; +import { record, readMetrics, readBalance } from './metrics.mjs'; import { WRITE_MULTIPLIER, READ_MULTIPLIER } from './cache.mjs'; /** @@ -56,14 +56,37 @@ export const TRIPWIRE_MIN = 10; * was called. */ export function gapDistribution(dir, { events = readMetrics(dir) } = {}) { - const stamps = events.map((e) => e.at).filter(Number.isFinite).sort((a, b) => a - b); + // PARTITIONED BY SESSION, because the interval between the last event of one session and the + // first of the next is not a gap between turns at all. The log spans days, so an overnight + // sixteen-hour boundary was counted as one -- dominating p90 and p99 and diluting + // probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on + // projects where it would have paid. Silent, because that bias only ever declines to act. + // + // A long gap WITHIN a session is kept. It is real evidence that caching does not pay there, and + // dropping it would bias the answer the other way -- making keep-warm look better than it is, + // which is the direction this project cares about most. + // + // Events carrying no sessionId are pooled into one group rather than discarded: most kinds do + // carry one, and discarding the rest would throw away whole projects' history for a technicality. + const bySession = new Map(); + for (const event of events) { + if (!Number.isFinite(event?.at)) continue; + const key = event.sessionId || ''; + if (!bySession.has(key)) bySession.set(key, []); + bySession.get(key).push(event.at); + } + + const stamps = [...bySession.values()].flat(); if (stamps.length < 8) return null; const gaps = []; - for (let i = 1; i < stamps.length; i++) { - const gap = stamps[i] - stamps[i - 1]; - // A burst of events inside one turn is not a gap between turns. - if (gap > 250) gaps.push(gap); + for (const session of bySession.values()) { + session.sort((a, b) => a - b); + for (let i = 1; i < session.length; i++) { + const gap = session[i] - session[i - 1]; + // A burst of events inside one turn is not a gap between turns. + if (gap > 250) gaps.push(gap); + } } if (gaps.length < 6) return null; @@ -209,7 +232,16 @@ export function recordRefreshOutcome(dir, { tier, prefixTokens, hit }) { * refreshes, keep-warm stops -- and says which tier and by how much, so the * stop is a finding rather than a silent behaviour change. */ -export function tripwire(dir, { events = readMetrics(dir) } = {}) { +/** + * The backstop, read from the log that is NOT windowed. + * + * readBalance rather than readMetrics, and that distinction is the difference between a backstop + * that works and one that cannot. There is one outcome per refresh, in a log dominated by reads + * and captures, so through the 5000-event window the ten TRIPWIRE_MIN demands aged out before the + * tenth was written -- and this returned "only N/10 refreshes observed" for the life of the + * project. A guard that can never reach its own threshold is not a guard. + */ +export function tripwire(dir, { events = readBalance(dir) } = {}) { const outcomes = events.filter((e) => e.kind === 'keepwarm' && e.action === 'outcome'); if (outcomes.length < TRIPWIRE_MIN) { return { tripped: false, observed: outcomes.length, reason: `only ${outcomes.length}/${TRIPWIRE_MIN} refreshes observed` }; @@ -234,8 +266,17 @@ export function tripwire(dir, { events = readMetrics(dir) } = {}) { * The decision as it should actually be taken: expected value, with the * tripwire able to veto it. */ -export function shouldKeepWarm(dir, { prefixTokens, events = readMetrics(dir) } = {}) { - const trip = tripwire(dir, { events }); +export function shouldKeepWarm(dir, { + prefixTokens, + events = readMetrics(dir), + // TWO SOURCES, because the two consumers want different things. gapDistribution wants the + // firehose -- every event is a timestamp and the recent ones describe the current rhythm. The + // tripwire wants the unwindowed balance log, because its outcomes are rare and are exactly what + // the window evicts. Passing one shared array to both, as this used to, meant whichever reader + // was chosen was wrong for one of them. + outcomes = readBalance(dir), +} = {}) { + const trip = tripwire(dir, { events: outcomes }); if (trip.tripped) return { action: 'skip', reason: trip.reason, trippedWire: true }; const gaps = gapDistribution(dir, { events }); diff --git a/integrations/gemini/hooks/lib/lessons.mjs b/integrations/gemini/hooks/lib/lessons.mjs index da932db24..89f3d408e 100644 --- a/integrations/gemini/hooks/lib/lessons.mjs +++ b/integrations/gemini/hooks/lib/lessons.mjs @@ -31,6 +31,7 @@ import { ORIGIN_HARVESTED, ORIGIN_HUMAN } from './curate.mjs'; import { safeTrigger } from './inject.mjs'; +import { isFsSafePath } from './paths.mjs'; /** Lessons longer than this are prose, not instructions. */ const MAX_CLAIM = 400; @@ -172,7 +173,7 @@ export function isImperative(claim) { * Returns { lessons, rejected } so a caller can report what was dropped rather * than silently storing less than it was given. */ -export function validateLessons(raw, turns) { +export function validateLessons(raw, turns, { knownFiles = null } = {}) { let parsed = raw; if (typeof raw === 'string') { try { @@ -238,8 +239,19 @@ export function validateLessons(raw, turns) { // A verified human correction is as close to ground truth as this system // gets. An unverified paraphrase is a model's reading of one. confidence: verified ? 0.95 : 0.6, + // HELD TO THE FILES THE SESSION ACTUALLY TOUCHED, exactly as the finding path already is. + // harvest-worker calls validate() with `knownFiles: filesIn(digest)` and says why in its own + // comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while + // this path accepted any non-empty string. The identical hallucination therefore walked + // straight through, and a lesson anchored to a file nobody opened is then injected on every + // future touch of it: a permanent instruction attached to code it was never about. + // + // isFsSafePath as well, because these strings reach the filesystem later through the graph. + // A path holding U+10FFFF ABORTS the process inside libuv rather than throwing, so no + // try/catch downstream can contain it. anchors: Array.isArray(item?.anchors) - ? item.anchors.filter((a) => typeof a === 'string' && a.trim()) + ? item.anchors.filter((a) => typeof a === 'string' && a.trim() && isFsSafePath(a) + && (!knownFiles || knownFiles.includes(a))) : [], }); } diff --git a/integrations/gemini/hooks/lib/metrics.mjs b/integrations/gemini/hooks/lib/metrics.mjs index 4b0eb2712..734d36d04 100644 --- a/integrations/gemini/hooks/lib/metrics.mjs +++ b/integrations/gemini/hooks/lib/metrics.mjs @@ -78,6 +78,11 @@ export const BALANCE_KINDS = new Set([ // and an outcome whose forecast had scrolled away was dropped with no record at all. 'forecast', 'forecast-outcome', + // The keep-warm tripwire's evidence. It needs TRIPWIRE_MIN outcomes before it may have an + // opinion, and there is one outcome per refresh -- so through the windowed reader the tenth + // aged out before it was written and the backstop was structurally unable to fire, reporting + // "only N/10 refreshes observed" for the life of the project. + 'keepwarm', ]); /** diff --git a/integrations/opencode/hooks/lib/keepwarm.mjs b/integrations/opencode/hooks/lib/keepwarm.mjs index 0c12e62f9..142f838d1 100644 --- a/integrations/opencode/hooks/lib/keepwarm.mjs +++ b/integrations/opencode/hooks/lib/keepwarm.mjs @@ -26,7 +26,7 @@ * whole thing stops and says so, which is the same rule the forecast follows. */ -import { record, readMetrics } from './metrics.mjs'; +import { record, readMetrics, readBalance } from './metrics.mjs'; import { WRITE_MULTIPLIER, READ_MULTIPLIER } from './cache.mjs'; /** @@ -56,14 +56,37 @@ export const TRIPWIRE_MIN = 10; * was called. */ export function gapDistribution(dir, { events = readMetrics(dir) } = {}) { - const stamps = events.map((e) => e.at).filter(Number.isFinite).sort((a, b) => a - b); + // PARTITIONED BY SESSION, because the interval between the last event of one session and the + // first of the next is not a gap between turns at all. The log spans days, so an overnight + // sixteen-hour boundary was counted as one -- dominating p90 and p99 and diluting + // probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on + // projects where it would have paid. Silent, because that bias only ever declines to act. + // + // A long gap WITHIN a session is kept. It is real evidence that caching does not pay there, and + // dropping it would bias the answer the other way -- making keep-warm look better than it is, + // which is the direction this project cares about most. + // + // Events carrying no sessionId are pooled into one group rather than discarded: most kinds do + // carry one, and discarding the rest would throw away whole projects' history for a technicality. + const bySession = new Map(); + for (const event of events) { + if (!Number.isFinite(event?.at)) continue; + const key = event.sessionId || ''; + if (!bySession.has(key)) bySession.set(key, []); + bySession.get(key).push(event.at); + } + + const stamps = [...bySession.values()].flat(); if (stamps.length < 8) return null; const gaps = []; - for (let i = 1; i < stamps.length; i++) { - const gap = stamps[i] - stamps[i - 1]; - // A burst of events inside one turn is not a gap between turns. - if (gap > 250) gaps.push(gap); + for (const session of bySession.values()) { + session.sort((a, b) => a - b); + for (let i = 1; i < session.length; i++) { + const gap = session[i] - session[i - 1]; + // A burst of events inside one turn is not a gap between turns. + if (gap > 250) gaps.push(gap); + } } if (gaps.length < 6) return null; @@ -209,7 +232,16 @@ export function recordRefreshOutcome(dir, { tier, prefixTokens, hit }) { * refreshes, keep-warm stops -- and says which tier and by how much, so the * stop is a finding rather than a silent behaviour change. */ -export function tripwire(dir, { events = readMetrics(dir) } = {}) { +/** + * The backstop, read from the log that is NOT windowed. + * + * readBalance rather than readMetrics, and that distinction is the difference between a backstop + * that works and one that cannot. There is one outcome per refresh, in a log dominated by reads + * and captures, so through the 5000-event window the ten TRIPWIRE_MIN demands aged out before the + * tenth was written -- and this returned "only N/10 refreshes observed" for the life of the + * project. A guard that can never reach its own threshold is not a guard. + */ +export function tripwire(dir, { events = readBalance(dir) } = {}) { const outcomes = events.filter((e) => e.kind === 'keepwarm' && e.action === 'outcome'); if (outcomes.length < TRIPWIRE_MIN) { return { tripped: false, observed: outcomes.length, reason: `only ${outcomes.length}/${TRIPWIRE_MIN} refreshes observed` }; @@ -234,8 +266,17 @@ export function tripwire(dir, { events = readMetrics(dir) } = {}) { * The decision as it should actually be taken: expected value, with the * tripwire able to veto it. */ -export function shouldKeepWarm(dir, { prefixTokens, events = readMetrics(dir) } = {}) { - const trip = tripwire(dir, { events }); +export function shouldKeepWarm(dir, { + prefixTokens, + events = readMetrics(dir), + // TWO SOURCES, because the two consumers want different things. gapDistribution wants the + // firehose -- every event is a timestamp and the recent ones describe the current rhythm. The + // tripwire wants the unwindowed balance log, because its outcomes are rare and are exactly what + // the window evicts. Passing one shared array to both, as this used to, meant whichever reader + // was chosen was wrong for one of them. + outcomes = readBalance(dir), +} = {}) { + const trip = tripwire(dir, { events: outcomes }); if (trip.tripped) return { action: 'skip', reason: trip.reason, trippedWire: true }; const gaps = gapDistribution(dir, { events }); diff --git a/integrations/opencode/hooks/lib/lessons.mjs b/integrations/opencode/hooks/lib/lessons.mjs index da932db24..89f3d408e 100644 --- a/integrations/opencode/hooks/lib/lessons.mjs +++ b/integrations/opencode/hooks/lib/lessons.mjs @@ -31,6 +31,7 @@ import { ORIGIN_HARVESTED, ORIGIN_HUMAN } from './curate.mjs'; import { safeTrigger } from './inject.mjs'; +import { isFsSafePath } from './paths.mjs'; /** Lessons longer than this are prose, not instructions. */ const MAX_CLAIM = 400; @@ -172,7 +173,7 @@ export function isImperative(claim) { * Returns { lessons, rejected } so a caller can report what was dropped rather * than silently storing less than it was given. */ -export function validateLessons(raw, turns) { +export function validateLessons(raw, turns, { knownFiles = null } = {}) { let parsed = raw; if (typeof raw === 'string') { try { @@ -238,8 +239,19 @@ export function validateLessons(raw, turns) { // A verified human correction is as close to ground truth as this system // gets. An unverified paraphrase is a model's reading of one. confidence: verified ? 0.95 : 0.6, + // HELD TO THE FILES THE SESSION ACTUALLY TOUCHED, exactly as the finding path already is. + // harvest-worker calls validate() with `knownFiles: filesIn(digest)` and says why in its own + // comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while + // this path accepted any non-empty string. The identical hallucination therefore walked + // straight through, and a lesson anchored to a file nobody opened is then injected on every + // future touch of it: a permanent instruction attached to code it was never about. + // + // isFsSafePath as well, because these strings reach the filesystem later through the graph. + // A path holding U+10FFFF ABORTS the process inside libuv rather than throwing, so no + // try/catch downstream can contain it. anchors: Array.isArray(item?.anchors) - ? item.anchors.filter((a) => typeof a === 'string' && a.trim()) + ? item.anchors.filter((a) => typeof a === 'string' && a.trim() && isFsSafePath(a) + && (!knownFiles || knownFiles.includes(a))) : [], }); } diff --git a/integrations/opencode/hooks/lib/metrics.mjs b/integrations/opencode/hooks/lib/metrics.mjs index 4b0eb2712..734d36d04 100644 --- a/integrations/opencode/hooks/lib/metrics.mjs +++ b/integrations/opencode/hooks/lib/metrics.mjs @@ -78,6 +78,11 @@ export const BALANCE_KINDS = new Set([ // and an outcome whose forecast had scrolled away was dropped with no record at all. 'forecast', 'forecast-outcome', + // The keep-warm tripwire's evidence. It needs TRIPWIRE_MIN outcomes before it may have an + // opinion, and there is one outcome per refresh -- so through the windowed reader the tenth + // aged out before it was written and the backstop was structurally unable to fire, reporting + // "only N/10 refreshes observed" for the life of the project. + 'keepwarm', ]); /** diff --git a/integrations/qwen/hooks/lib/keepwarm.mjs b/integrations/qwen/hooks/lib/keepwarm.mjs index 0c12e62f9..142f838d1 100644 --- a/integrations/qwen/hooks/lib/keepwarm.mjs +++ b/integrations/qwen/hooks/lib/keepwarm.mjs @@ -26,7 +26,7 @@ * whole thing stops and says so, which is the same rule the forecast follows. */ -import { record, readMetrics } from './metrics.mjs'; +import { record, readMetrics, readBalance } from './metrics.mjs'; import { WRITE_MULTIPLIER, READ_MULTIPLIER } from './cache.mjs'; /** @@ -56,14 +56,37 @@ export const TRIPWIRE_MIN = 10; * was called. */ export function gapDistribution(dir, { events = readMetrics(dir) } = {}) { - const stamps = events.map((e) => e.at).filter(Number.isFinite).sort((a, b) => a - b); + // PARTITIONED BY SESSION, because the interval between the last event of one session and the + // first of the next is not a gap between turns at all. The log spans days, so an overnight + // sixteen-hour boundary was counted as one -- dominating p90 and p99 and diluting + // probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on + // projects where it would have paid. Silent, because that bias only ever declines to act. + // + // A long gap WITHIN a session is kept. It is real evidence that caching does not pay there, and + // dropping it would bias the answer the other way -- making keep-warm look better than it is, + // which is the direction this project cares about most. + // + // Events carrying no sessionId are pooled into one group rather than discarded: most kinds do + // carry one, and discarding the rest would throw away whole projects' history for a technicality. + const bySession = new Map(); + for (const event of events) { + if (!Number.isFinite(event?.at)) continue; + const key = event.sessionId || ''; + if (!bySession.has(key)) bySession.set(key, []); + bySession.get(key).push(event.at); + } + + const stamps = [...bySession.values()].flat(); if (stamps.length < 8) return null; const gaps = []; - for (let i = 1; i < stamps.length; i++) { - const gap = stamps[i] - stamps[i - 1]; - // A burst of events inside one turn is not a gap between turns. - if (gap > 250) gaps.push(gap); + for (const session of bySession.values()) { + session.sort((a, b) => a - b); + for (let i = 1; i < session.length; i++) { + const gap = session[i] - session[i - 1]; + // A burst of events inside one turn is not a gap between turns. + if (gap > 250) gaps.push(gap); + } } if (gaps.length < 6) return null; @@ -209,7 +232,16 @@ export function recordRefreshOutcome(dir, { tier, prefixTokens, hit }) { * refreshes, keep-warm stops -- and says which tier and by how much, so the * stop is a finding rather than a silent behaviour change. */ -export function tripwire(dir, { events = readMetrics(dir) } = {}) { +/** + * The backstop, read from the log that is NOT windowed. + * + * readBalance rather than readMetrics, and that distinction is the difference between a backstop + * that works and one that cannot. There is one outcome per refresh, in a log dominated by reads + * and captures, so through the 5000-event window the ten TRIPWIRE_MIN demands aged out before the + * tenth was written -- and this returned "only N/10 refreshes observed" for the life of the + * project. A guard that can never reach its own threshold is not a guard. + */ +export function tripwire(dir, { events = readBalance(dir) } = {}) { const outcomes = events.filter((e) => e.kind === 'keepwarm' && e.action === 'outcome'); if (outcomes.length < TRIPWIRE_MIN) { return { tripped: false, observed: outcomes.length, reason: `only ${outcomes.length}/${TRIPWIRE_MIN} refreshes observed` }; @@ -234,8 +266,17 @@ export function tripwire(dir, { events = readMetrics(dir) } = {}) { * The decision as it should actually be taken: expected value, with the * tripwire able to veto it. */ -export function shouldKeepWarm(dir, { prefixTokens, events = readMetrics(dir) } = {}) { - const trip = tripwire(dir, { events }); +export function shouldKeepWarm(dir, { + prefixTokens, + events = readMetrics(dir), + // TWO SOURCES, because the two consumers want different things. gapDistribution wants the + // firehose -- every event is a timestamp and the recent ones describe the current rhythm. The + // tripwire wants the unwindowed balance log, because its outcomes are rare and are exactly what + // the window evicts. Passing one shared array to both, as this used to, meant whichever reader + // was chosen was wrong for one of them. + outcomes = readBalance(dir), +} = {}) { + const trip = tripwire(dir, { events: outcomes }); if (trip.tripped) return { action: 'skip', reason: trip.reason, trippedWire: true }; const gaps = gapDistribution(dir, { events }); diff --git a/integrations/qwen/hooks/lib/lessons.mjs b/integrations/qwen/hooks/lib/lessons.mjs index da932db24..89f3d408e 100644 --- a/integrations/qwen/hooks/lib/lessons.mjs +++ b/integrations/qwen/hooks/lib/lessons.mjs @@ -31,6 +31,7 @@ import { ORIGIN_HARVESTED, ORIGIN_HUMAN } from './curate.mjs'; import { safeTrigger } from './inject.mjs'; +import { isFsSafePath } from './paths.mjs'; /** Lessons longer than this are prose, not instructions. */ const MAX_CLAIM = 400; @@ -172,7 +173,7 @@ export function isImperative(claim) { * Returns { lessons, rejected } so a caller can report what was dropped rather * than silently storing less than it was given. */ -export function validateLessons(raw, turns) { +export function validateLessons(raw, turns, { knownFiles = null } = {}) { let parsed = raw; if (typeof raw === 'string') { try { @@ -238,8 +239,19 @@ export function validateLessons(raw, turns) { // A verified human correction is as close to ground truth as this system // gets. An unverified paraphrase is a model's reading of one. confidence: verified ? 0.95 : 0.6, + // HELD TO THE FILES THE SESSION ACTUALLY TOUCHED, exactly as the finding path already is. + // harvest-worker calls validate() with `knownFiles: filesIn(digest)` and says why in its own + // comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while + // this path accepted any non-empty string. The identical hallucination therefore walked + // straight through, and a lesson anchored to a file nobody opened is then injected on every + // future touch of it: a permanent instruction attached to code it was never about. + // + // isFsSafePath as well, because these strings reach the filesystem later through the graph. + // A path holding U+10FFFF ABORTS the process inside libuv rather than throwing, so no + // try/catch downstream can contain it. anchors: Array.isArray(item?.anchors) - ? item.anchors.filter((a) => typeof a === 'string' && a.trim()) + ? item.anchors.filter((a) => typeof a === 'string' && a.trim() && isFsSafePath(a) + && (!knownFiles || knownFiles.includes(a))) : [], }); } diff --git a/integrations/qwen/hooks/lib/metrics.mjs b/integrations/qwen/hooks/lib/metrics.mjs index 4b0eb2712..734d36d04 100644 --- a/integrations/qwen/hooks/lib/metrics.mjs +++ b/integrations/qwen/hooks/lib/metrics.mjs @@ -78,6 +78,11 @@ export const BALANCE_KINDS = new Set([ // and an outcome whose forecast had scrolled away was dropped with no record at all. 'forecast', 'forecast-outcome', + // The keep-warm tripwire's evidence. It needs TRIPWIRE_MIN outcomes before it may have an + // opinion, and there is one outcome per refresh -- so through the windowed reader the tenth + // aged out before it was written and the backstop was structurally unable to fire, reporting + // "only N/10 refreshes observed" for the life of the project. + 'keepwarm', ]); /** diff --git a/plugin/hooks/harvest-worker.mjs b/plugin/hooks/harvest-worker.mjs index 8181bb8f4..d7796e1f8 100644 --- a/plugin/hooks/harvest-worker.mjs +++ b/plugin/hooks/harvest-worker.mjs @@ -94,7 +94,12 @@ async function main() { const feedback = buildFeedbackDigest(turns); if (feedback) { const rawLessons = await extract(feedback, { prompt: LESSON_PROMPT }); - const { lessons, rejected } = validateLessons(rawLessons, turns); + // The same restriction the finding path above applies, for the same reason: a model that + // invents a plausible path must not be able to anchor a lesson to it. `turns` is the + // archived transcript for this session, so its rendered digest is the honest file list. + const { lessons, rejected } = validateLessons(rawLessons, turns, { + knownFiles: filesIn(feedback), + }); // Anchors are optional on a lesson -- "always run npm test" is about no // file -- so each is anchored to the project root when it names nothing, diff --git a/plugin/hooks/lib/keepwarm.mjs b/plugin/hooks/lib/keepwarm.mjs index 0c12e62f9..142f838d1 100644 --- a/plugin/hooks/lib/keepwarm.mjs +++ b/plugin/hooks/lib/keepwarm.mjs @@ -26,7 +26,7 @@ * whole thing stops and says so, which is the same rule the forecast follows. */ -import { record, readMetrics } from './metrics.mjs'; +import { record, readMetrics, readBalance } from './metrics.mjs'; import { WRITE_MULTIPLIER, READ_MULTIPLIER } from './cache.mjs'; /** @@ -56,14 +56,37 @@ export const TRIPWIRE_MIN = 10; * was called. */ export function gapDistribution(dir, { events = readMetrics(dir) } = {}) { - const stamps = events.map((e) => e.at).filter(Number.isFinite).sort((a, b) => a - b); + // PARTITIONED BY SESSION, because the interval between the last event of one session and the + // first of the next is not a gap between turns at all. The log spans days, so an overnight + // sixteen-hour boundary was counted as one -- dominating p90 and p99 and diluting + // probabilityWithin in the conservative direction, so ttlTier answered "neither tier pays" on + // projects where it would have paid. Silent, because that bias only ever declines to act. + // + // A long gap WITHIN a session is kept. It is real evidence that caching does not pay there, and + // dropping it would bias the answer the other way -- making keep-warm look better than it is, + // which is the direction this project cares about most. + // + // Events carrying no sessionId are pooled into one group rather than discarded: most kinds do + // carry one, and discarding the rest would throw away whole projects' history for a technicality. + const bySession = new Map(); + for (const event of events) { + if (!Number.isFinite(event?.at)) continue; + const key = event.sessionId || ''; + if (!bySession.has(key)) bySession.set(key, []); + bySession.get(key).push(event.at); + } + + const stamps = [...bySession.values()].flat(); if (stamps.length < 8) return null; const gaps = []; - for (let i = 1; i < stamps.length; i++) { - const gap = stamps[i] - stamps[i - 1]; - // A burst of events inside one turn is not a gap between turns. - if (gap > 250) gaps.push(gap); + for (const session of bySession.values()) { + session.sort((a, b) => a - b); + for (let i = 1; i < session.length; i++) { + const gap = session[i] - session[i - 1]; + // A burst of events inside one turn is not a gap between turns. + if (gap > 250) gaps.push(gap); + } } if (gaps.length < 6) return null; @@ -209,7 +232,16 @@ export function recordRefreshOutcome(dir, { tier, prefixTokens, hit }) { * refreshes, keep-warm stops -- and says which tier and by how much, so the * stop is a finding rather than a silent behaviour change. */ -export function tripwire(dir, { events = readMetrics(dir) } = {}) { +/** + * The backstop, read from the log that is NOT windowed. + * + * readBalance rather than readMetrics, and that distinction is the difference between a backstop + * that works and one that cannot. There is one outcome per refresh, in a log dominated by reads + * and captures, so through the 5000-event window the ten TRIPWIRE_MIN demands aged out before the + * tenth was written -- and this returned "only N/10 refreshes observed" for the life of the + * project. A guard that can never reach its own threshold is not a guard. + */ +export function tripwire(dir, { events = readBalance(dir) } = {}) { const outcomes = events.filter((e) => e.kind === 'keepwarm' && e.action === 'outcome'); if (outcomes.length < TRIPWIRE_MIN) { return { tripped: false, observed: outcomes.length, reason: `only ${outcomes.length}/${TRIPWIRE_MIN} refreshes observed` }; @@ -234,8 +266,17 @@ export function tripwire(dir, { events = readMetrics(dir) } = {}) { * The decision as it should actually be taken: expected value, with the * tripwire able to veto it. */ -export function shouldKeepWarm(dir, { prefixTokens, events = readMetrics(dir) } = {}) { - const trip = tripwire(dir, { events }); +export function shouldKeepWarm(dir, { + prefixTokens, + events = readMetrics(dir), + // TWO SOURCES, because the two consumers want different things. gapDistribution wants the + // firehose -- every event is a timestamp and the recent ones describe the current rhythm. The + // tripwire wants the unwindowed balance log, because its outcomes are rare and are exactly what + // the window evicts. Passing one shared array to both, as this used to, meant whichever reader + // was chosen was wrong for one of them. + outcomes = readBalance(dir), +} = {}) { + const trip = tripwire(dir, { events: outcomes }); if (trip.tripped) return { action: 'skip', reason: trip.reason, trippedWire: true }; const gaps = gapDistribution(dir, { events }); diff --git a/plugin/hooks/lib/lessons.mjs b/plugin/hooks/lib/lessons.mjs index da932db24..89f3d408e 100644 --- a/plugin/hooks/lib/lessons.mjs +++ b/plugin/hooks/lib/lessons.mjs @@ -31,6 +31,7 @@ import { ORIGIN_HARVESTED, ORIGIN_HUMAN } from './curate.mjs'; import { safeTrigger } from './inject.mjs'; +import { isFsSafePath } from './paths.mjs'; /** Lessons longer than this are prose, not instructions. */ const MAX_CLAIM = 400; @@ -172,7 +173,7 @@ export function isImperative(claim) { * Returns { lessons, rejected } so a caller can report what was dropped rather * than silently storing less than it was given. */ -export function validateLessons(raw, turns) { +export function validateLessons(raw, turns, { knownFiles = null } = {}) { let parsed = raw; if (typeof raw === 'string') { try { @@ -238,8 +239,19 @@ export function validateLessons(raw, turns) { // A verified human correction is as close to ground truth as this system // gets. An unverified paraphrase is a model's reading of one. confidence: verified ? 0.95 : 0.6, + // HELD TO THE FILES THE SESSION ACTUALLY TOUCHED, exactly as the finding path already is. + // harvest-worker calls validate() with `knownFiles: filesIn(digest)` and says why in its own + // comment -- "a model that invents a plausible path cannot anchor a finding to it" -- while + // this path accepted any non-empty string. The identical hallucination therefore walked + // straight through, and a lesson anchored to a file nobody opened is then injected on every + // future touch of it: a permanent instruction attached to code it was never about. + // + // isFsSafePath as well, because these strings reach the filesystem later through the graph. + // A path holding U+10FFFF ABORTS the process inside libuv rather than throwing, so no + // try/catch downstream can contain it. anchors: Array.isArray(item?.anchors) - ? item.anchors.filter((a) => typeof a === 'string' && a.trim()) + ? item.anchors.filter((a) => typeof a === 'string' && a.trim() && isFsSafePath(a) + && (!knownFiles || knownFiles.includes(a))) : [], }); } diff --git a/plugin/hooks/lib/metrics.mjs b/plugin/hooks/lib/metrics.mjs index 4b0eb2712..734d36d04 100644 --- a/plugin/hooks/lib/metrics.mjs +++ b/plugin/hooks/lib/metrics.mjs @@ -78,6 +78,11 @@ export const BALANCE_KINDS = new Set([ // and an outcome whose forecast had scrolled away was dropped with no record at all. 'forecast', 'forecast-outcome', + // The keep-warm tripwire's evidence. It needs TRIPWIRE_MIN outcomes before it may have an + // opinion, and there is one outcome per refresh -- so through the windowed reader the tenth + // aged out before it was written and the backstop was structurally unable to fire, reporting + // "only N/10 refreshes observed" for the life of the project. + 'keepwarm', ]); /** diff --git a/tests/hooks/cache.test.mjs b/tests/hooks/cache.test.mjs index 1c1ab1cc1..5c22585af 100644 --- a/tests/hooks/cache.test.mjs +++ b/tests/hooks/cache.test.mjs @@ -398,3 +398,65 @@ describe('keep-warm never returns a verdict that contradicts its own reason', () } }); }); + +describe('the backstop can actually reach its own threshold', () => { + test('outcomes survive a firehose long enough to have evicted them', () => { + // THE DEFECT: tripwire read through readMetrics, whose window is 5000 events and 2 MB. There + // is one keepwarm outcome per refresh, in a log dominated by reads and captures, so the ten + // TRIPWIRE_MIN demands aged out before the tenth was written. It returned + // "only N/10 refreshes observed" for the life of the project, and shouldKeepWarm could never + // be vetoed -- a guard that cannot reach its own threshold is not a guard. + for (let i = 0; i < TRIPWIRE_MIN + 2; i++) { + recordRefreshOutcome(dir, { tier: TIERS[0].name, prefixTokens: 10_000, hit: false }); + } + // Bury them under more events than the window will hold. + for (let i = 0; i < 5_200; i++) record(dir, { kind: 'read', anchor: `/n${i}.ts`, tokens: 10 }); + + const windowed = readMetrics(dir).filter((e) => e.kind === 'keepwarm'); + expect(windowed.length).toBeLessThan(TRIPWIRE_MIN); // the window really did evict them + + const trip = tripwire(dir); + expect(trip.observed).toBe(TRIPWIRE_MIN + 2); + expect(trip.tripped).toBe(true); + }); +}); + +describe('a gap between two sessions is not a gap between turns', () => { + test('an overnight boundary does not enter the distribution', () => { + // THE DEFECT: every timestamp in the log was sorted and differenced, so the interval between + // the last event of one session and the first of the next -- routinely sixteen hours -- was + // counted as a turn gap. That dominated p90/p99 and diluted probabilityWithin in the + // conservative direction, so ttlTier returned "neither tier pays" on projects where it would + // have paid. Silent, because the bias only ever declines to act. + const base = Date.now() - 48 * 60 * 60 * 1000; + const events = []; + for (let s = 0; s < 2; s++) { + for (let i = 0; i < 6; i++) { + events.push({ kind: 'read', sessionId: `s${s}`, at: base + s * 24 * 3600_000 + i * 60_000 }); + } + } + record(dir, { kind: 'seed' }); + writeFileSync(join(dir, 'metrics.jsonl'), `${events.map((e) => JSON.stringify(e)).join('\n')}\n`); + + const gaps = gapDistribution(dir); + expect(gaps).not.toBeNull(); + // Ten one-minute gaps within the two sessions, and no 24-hour one between them. + expect(gaps.count).toBe(10); + expect(gaps.p99).toBeLessThan(2 * 60_000); + }); + + test('a long gap INSIDE a session is kept, because it is real evidence', () => { + // Dropping these would bias the answer the other way -- making keep-warm look better than it + // is, which is the direction this project cares about most. + const base = Date.now() - 10 * 60 * 60 * 1000; + const events = Array.from({ length: 8 }, (_, i) => ({ + kind: 'read', sessionId: 'one', at: base + i * 90 * 60_000, + })); + record(dir, { kind: 'seed' }); + writeFileSync(join(dir, 'metrics.jsonl'), `${events.map((e) => JSON.stringify(e)).join('\n')}\n`); + + const gaps = gapDistribution(dir); + expect(gaps.median).toBeGreaterThan(60 * 60_000); + expect(ttlTier({ prefixTokens: 47_000, gaps })).toBeNull(); // and it correctly declines + }); +}); diff --git a/tests/hooks/lessons.test.mjs b/tests/hooks/lessons.test.mjs index dcf8aa151..74154e9f7 100644 --- a/tests/hooks/lessons.test.mjs +++ b/tests/hooks/lessons.test.mjs @@ -369,3 +369,38 @@ describe('a lesson is only stored if its trigger can ever fire', () => { expect(out.rejected.filter((r) => r.reason === 'bad-trigger-regex')).toHaveLength(0); }); }); + +describe('a lesson cannot be anchored to a file the session never opened', () => { + const lesson = (anchors) => JSON.stringify([{ + claim: 'Use npm test rather than npx jest for this repository.', + trigger: String.raw`\bnpx\s+jest\b`, + anchors, + }]); + + it('an invented path is dropped when the touched-file list is known', () => { + // THE DEFECT: the finding path calls validate() with `knownFiles: filesIn(digest)` and says + // why in its own comment -- "a model that invents a plausible path cannot anchor a finding to + // it" -- while this path accepted any non-empty string. A lesson anchored to a file nobody + // opened is then injected on every future touch of that file: a permanent instruction attached + // to code it was never about. + const { lessons } = validateLessons( + lesson(['/repo/src/real.ts', '/repo/src/invented-by-the-model.ts']), + TURNS, + { knownFiles: ['/repo/src/real.ts'] } + ); + expect(lessons[0].anchors).toEqual(['/repo/src/real.ts']); + }); + + it('without a known-file list every anchor is still accepted', () => { + // The full-delta path has no such list, so the restriction must not become mandatory. + const { lessons } = validateLessons(lesson(['/repo/a.ts', '/repo/b.ts']), TURNS); + expect(lessons[0].anchors).toHaveLength(2); + }); + + it('a path that would abort the process is refused either way', () => { + // U+10FFFF trips an assert inside libuv rather than throwing, so no downstream try/catch helps. + const unsafe = `/repo/${String.fromCodePoint(0x10ffff)}.ts`; + const { lessons } = validateLessons(lesson(['/repo/ok.ts', unsafe]), TURNS); + expect(lessons[0].anchors).toEqual(['/repo/ok.ts']); + }); +});