From 18ee93bf6bd4379ca65e9f9cac875febb19e8d3b Mon Sep 17 00:00:00 2001 From: iceteaSA <171169159+iceteaSA@users.noreply.github.com> Date: Sat, 19 Sep 2026 01:06:54 +0200 Subject: [PATCH 1/2] fix(opencode): reach the effort-marker trim tolerance in both loss shapes The trimmed-prefix tolerance at effort-history.ts:658-688 was unreachable in the two shapes that actually occur, so a legitimate host prefix trim became a 400 and manufactured the contentless assistant that widens the next merge. Shape A (expected N, found 0): the !hasCandidate path threw unconditionally on markerCount !== 0, ~90 lines before the tolerance. A full trim with a resolvable plan is the trimmedPrefix === expectedTransitions.length case, so fold every consumed transition into the baseline and proceed. The throw stays for the untrusted case (markerCount > 0 with no resolvable plan). Shape B (expected N, found 1): the request carried a valid plan header but the tracker could not resolve it back to a plan, so expectedTransitions was null and the exact path made any loss fatal. The tracker held one slot per (sessionId, messageId); when the host trims the history prefix between provider calls the same current message is re-marked with a shorter timeline, and record() overwrote the slot while a header generated from the previous plan was still in flight. Keep a bounded identity-keyed history so resolveHeader can still find the plan a header references. Tests: full-trim fold asserts the folded baseline is the last consumed transition's effort; full trim without a resolvable plan still throws; a re-recorded trimmed timeline no longer loses the in-flight plan; non-prefix loss still throws. --- packages/opencode/src/effort-history.ts | 32 +++++-- .../opencode/src/tests/effort-history.test.ts | 88 ++++++++++++++++++- packages/opencode/src/tests/index.test.ts | 12 +-- 3 files changed, 116 insertions(+), 16 deletions(-) diff --git a/packages/opencode/src/effort-history.ts b/packages/opencode/src/effort-history.ts index 2e1c0bf1..e0d32807 100644 --- a/packages/opencode/src/effort-history.ts +++ b/packages/opencode/src/effort-history.ts @@ -7,6 +7,7 @@ import { const MAX_EFFORT_MARKERS = 512 const MAX_TRACKED_EFFORT_PLANS = 1024 +const MAX_TRACKED_EFFORT_PLAN_HISTORY = 4096 const MARKER_CHECK_HEX_LENGTH = 32 const SCOPE_HEX_LENGTH = 32 const MESSAGE_ID_PATTERN = '[A-Za-z0-9_-]{1,128}' @@ -569,7 +570,11 @@ export function applyOpenCodeEffortMarkers( if (!hasCandidate) { if (!requestPlan) return { found: 0, inserted: 0 } - if (requestPlan.markerCount !== 0) { + // A full trim is the `trimmedPrefix === expectedTransitions.length` case: + // every transition was consumed by the host's prefix trim, so fold them all + // into the baseline. Without a resolvable plan the request is untrusted and + // stays fail-closed. + if (requestPlan.markerCount !== 0 && !expectedTransitions) { throw new EffortMarkerCorrelationError( `Fable 5.1 effort marker correlation failed: expected ${requestPlan.markerCount}, found 0`, ) @@ -743,19 +748,33 @@ export function applyOpenCodeEffortMarkers( export class OpenCodeEffortPlanTracker { private readonly plans = new Map() + // A message's plan is re-recorded whenever the host trims the history prefix + // between provider calls, so the single per-message slot can be overwritten + // while a header generated from the previous plan is still in flight. Keep a + // bounded identity-keyed history so resolveHeader can still find that plan. + private readonly history = new Map() record(plan: OpenCodeEffortMarkerPlan): void { const key = this.key(plan.sessionId, plan.messageId) - this.plans.delete(key) - this.plans.set(key, { + const stored = { ...plan, transitionTokens: [...plan.transitionTokens], - }) + } + this.plans.delete(key) + this.plans.set(key, stored) + const encoded = encodeOpenCodeEffortPlan(stored) + this.history.delete(encoded) + this.history.set(encoded, stored) while (this.plans.size > MAX_TRACKED_EFFORT_PLANS) { const oldest = this.plans.keys().next().value if (typeof oldest !== 'string') break this.plans.delete(oldest) } + while (this.history.size > MAX_TRACKED_EFFORT_PLAN_HISTORY) { + const oldest = this.history.keys().next().value + if (typeof oldest !== 'string') break + this.history.delete(oldest) + } } clear(sessionId: string, messageId: string): void { @@ -784,10 +803,7 @@ export class OpenCodeEffortPlanTracker { value: string | undefined, ): OpenCodeEffortMarkerPlan | undefined { if (!value) return undefined - for (const plan of this.plans.values()) { - if (encodeOpenCodeEffortPlan(plan) === value) return plan - } - return undefined + return this.history.get(value) } private key(sessionId: string, messageId: string): string { diff --git a/packages/opencode/src/tests/effort-history.test.ts b/packages/opencode/src/tests/effort-history.test.ts index c0e4a55e..6d4c9c0b 100644 --- a/packages/opencode/src/tests/effort-history.test.ts +++ b/packages/opencode/src/tests/effort-history.test.ts @@ -282,14 +282,15 @@ describe('OpenCode Fable 5.1 effort markers', () => { }, ], } - expect(() => + expect( applyOpenCodeEffortMarkers( missingAnchorBody, true, encodeOpenCodeEffortPlan(plan as NonNullable), plan as NonNullable, ), - ).toThrow('Fable 5.1 effort marker correlation failed: expected 2, found 0') + ).toEqual({ found: 0, inserted: 0 }) + expect(missingAnchorBody.output_config).toEqual({ effort: 'high' }) const body = { model: 'claude-fable-5-1', @@ -323,6 +324,89 @@ describe('OpenCode Fable 5.1 effort markers', () => { ]) }) + test('folds a full trim into the last consumed transition effort when the plan resolves', () => { + const messages = [ + user('msg_low', 'ses_full_trim_fold', 'claude-fable-5-1', 'low'), + user('msg_medium', 'ses_full_trim_fold', 'claude-fable-5-1', 'medium'), + user('msg_high', 'ses_full_trim_fold', 'claude-fable-5-1', 'high'), + user('msg_current', 'ses_full_trim_fold', 'claude-fable-5-1', 'high'), + ] + const plan = markOpenCodeEffortTransitions(messages) + expect(plan).not.toBeNull() + expect(plan?.markerCount).toBe(2) + const body = { + model: 'claude-fable-5-1', + output_config: { effort: 'low' }, + messages: [ + { role: 'user', content: [{ type: 'text', text: 'msg_current' }] }, + ], + } + + expect( + applyOpenCodeEffortMarkers( + body, + true, + encodeOpenCodeEffortPlan(plan as NonNullable), + plan as NonNullable, + ), + ).toEqual({ found: 0, inserted: 0 }) + // The folded baseline is the last consumed transition's effort ('high'), + // not the plan's original baseline ('low'). + expect(body.output_config).toEqual({ effort: 'high' }) + }) + + test('rejects a full trim without a resolvable plan', () => { + const messages = [ + user('msg_low', 'ses_full_trim_untrusted', 'claude-fable-5-1', 'low'), + user('msg_high', 'ses_full_trim_untrusted', 'claude-fable-5-1', 'high'), + ] + const plan = markOpenCodeEffortTransitions(messages) + expect(plan).not.toBeNull() + const body = { + model: 'claude-fable-5-1', + output_config: { effort: 'high' }, + messages: [ + { role: 'user', content: [{ type: 'text', text: 'msg_high' }] }, + ], + } + + expect(() => + applyOpenCodeEffortMarkers( + body, + true, + encodeOpenCodeEffortPlan(plan as NonNullable), + ), + ).toThrow('Fable 5.1 effort marker correlation failed: expected 1, found 0') + }) + + test('resolves a plan header after the same message is re-recorded with a trimmed timeline', () => { + const full = [ + user('msg_low', 'ses_overwrite', 'claude-fable-5-1', 'low'), + user('msg_medium', 'ses_overwrite', 'claude-fable-5-1', 'medium'), + user('msg_high', 'ses_overwrite', 'claude-fable-5-1', 'high'), + user('msg_current', 'ses_overwrite', 'claude-fable-5-1', 'high'), + ] + const fullPlan = markOpenCodeEffortTransitions(full) + expect(fullPlan?.markerCount).toBe(2) + const tracker = new OpenCodeEffortPlanTracker() + tracker.record(fullPlan as NonNullable) + const header = encodeOpenCodeEffortPlan( + fullPlan as NonNullable, + ) + + // The host trims the history prefix between provider calls; the same current + // message is re-marked with a shorter timeline and overwrites the slot. + const trimmedPlan = markOpenCodeEffortTransitions(full.slice(2)) + expect(trimmedPlan?.messageId).toBe('msg_current') + expect(trimmedPlan?.markerCount).toBe(0) + tracker.record(trimmedPlan as NonNullable) + + // The header generated from the full plan must still resolve. + expect(tracker.resolveHeader(header)).toEqual( + fullPlan as NonNullable, + ) + }) + test('rejects non-prefix transition loss even with the resolved plan', () => { const messages = [ user('msg_low', 'ses_non_prefix', 'claude-fable-5-1', 'low'), diff --git a/packages/opencode/src/tests/index.test.ts b/packages/opencode/src/tests/index.test.ts index 54de6cda..56e51c38 100644 --- a/packages/opencode/src/tests/index.test.ts +++ b/packages/opencode/src/tests/index.test.ts @@ -10464,7 +10464,7 @@ describe('Fable 5.1 request-scoped effort history', () => { ]) }) - test('fails locally when request-correlated effort markers cannot be validated', async () => { + test('folds a resolvable full trim and fails locally when effort markers cannot be validated', async () => { await useTempAccountFile( createFallbackStorage({ accounts: [], @@ -10603,11 +10603,11 @@ describe('Fable 5.1 request-scoped effort history', () => { 'Missing or invalid internal Fable 5.1 effort request plan', ) + // A full trim with a resolvable plan is the trimmedPrefix === length case: + // every transition was consumed, so the request proceeds with the folded + // baseline instead of failing closed. const missingAllMarkers = await send('ses_effort_missing_all', []) - expect(missingAllMarkers.status).toBe(400) - expect((await missingAllMarkers.json()).error.message).toBe( - 'Fable 5.1 effort marker correlation failed: expected 1, found 0', - ) + expect(missingAllMarkers.status).toBe(200) const duplicateTransition = await send('ses_effort_duplicate_transition', [ transitionMarker, @@ -10617,7 +10617,7 @@ describe('Fable 5.1 request-scoped effort history', () => { expect((await duplicateTransition.json()).error.message).toBe( 'Multiple internal Fable 5.1 effort markers on one user boundary', ) - expect(messagesCalled).toBe(false) + expect(messagesCalled).toBe(true) }) }) From c3930f8337ee91667caac58fbd80dd1ebf406339 Mon Sep 17 00:00:00 2001 From: iceteaSA <171169159+iceteaSA@users.noreply.github.com> Date: Sat, 19 Sep 2026 01:12:57 +0200 Subject: [PATCH 2/2] test(opencode): pin the effort plan history eviction bound The identity-keyed history map is bounded but was unasserted, so a later refactor could delete the eviction silently. Record past the cap and assert both halves: the size stays at the cap, and insertion-order eviction drops the oldest plan while the newest still resolves (the newest is the one an in-flight header needs). --- .../opencode/src/tests/effort-history.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/opencode/src/tests/effort-history.test.ts b/packages/opencode/src/tests/effort-history.test.ts index 6d4c9c0b..c189af2d 100644 --- a/packages/opencode/src/tests/effort-history.test.ts +++ b/packages/opencode/src/tests/effort-history.test.ts @@ -5,6 +5,7 @@ import { EFFORT_MARKER_PREFIX, encodeOpenCodeEffortPlan, markOpenCodeEffortTransitions, + type OpenCodeEffortMarkerPlan, OpenCodeEffortPlanTracker, } from '../effort-history.ts' @@ -407,6 +408,37 @@ describe('OpenCode Fable 5.1 effort markers', () => { ) }) + test('evicts the oldest plan history entry once the cap is exceeded', () => { + const tracker = new OpenCodeEffortPlanTracker() + const historySize = () => + (tracker as unknown as { history: Map }).history.size + const plan = (index: number): OpenCodeEffortMarkerPlan => ({ + scope: 'a'.repeat(32), + baseline: 'high', + markerCount: 0, + digest: index.toString(16).padStart(64, '0'), + transitionTokens: [], + sessionId: `ses_history_${index}`, + messageId: `msg_history_${index}`, + }) + const cap = 4096 + const oldest = plan(0) + const newest = plan(cap) + + for (let index = 0; index <= cap; index += 1) { + tracker.record(plan(index)) + } + + expect(historySize()).toBe(cap) + // Insertion-order eviction: the oldest plan is gone, the newest survives. + expect( + tracker.resolveHeader(encodeOpenCodeEffortPlan(oldest)), + ).toBeUndefined() + expect(tracker.resolveHeader(encodeOpenCodeEffortPlan(newest))).toEqual( + newest, + ) + }) + test('rejects non-prefix transition loss even with the resolved plan', () => { const messages = [ user('msg_low', 'ses_non_prefix', 'claude-fable-5-1', 'low'),