diff --git a/packages/spill/spill-policy/README.md b/packages/spill/spill-policy/README.md index dcc2fffb30..fe128de9a0 100644 --- a/packages/spill/spill-policy/README.md +++ b/packages/spill/spill-policy/README.md @@ -24,7 +24,7 @@ This plugin registers **no service** and owns no storage or preview mechanics: p (Omitted N bytes. Full formatted result saved to: /…/session-…/…-web_fetch.txt. Use read with offset/limit to inspect it.) ``` - When the notice alone fills the budget (a tiny cap or a long path) the preview is empty and only the notice is returned. If even that notice-only replacement is not smaller than the original result, the policy keeps the inline result — spilling would only add bytes. + When the notice alone fills the budget (a tiny cap or a long path) the preview is empty and only the notice is returned. If even that notice-only replacement would exceed `maxInlineBytes`, the policy keeps the inline result — it never emits a replacement over the cap (and a within-cap replacement is always smaller than the original, so this also means spilling never adds bytes). **Best-effort:** no session owner, no `ctx.spillFiles` backend, or a `saveText` rejection ⇒ the policy logs a warning and returns the original result. A spill failure never turns a successful call into an `isError` or hides the inline result. diff --git a/packages/spill/spill-policy/src/index.ts b/packages/spill/spill-policy/src/index.ts index 9fa55fcdf4..0472bd9a8a 100644 --- a/packages/spill/spill-policy/src/index.ts +++ b/packages/spill/spill-policy/src/index.ts @@ -157,12 +157,15 @@ export function apply(ctx: Context, config: Config): void { const { text: previewText, omitted } = preview(text, previewBudget) const notice = spillNotice(omitted, path) const replacedText = previewText.length > 0 ? `${previewText}\n\n${notice}` : notice - // Guard against a pathological tiny cap + long path where even the - // notice-only replacement is not smaller than the original: spilling then - // gains nothing and would only add bytes, so keep the inline result. (The - // spill file already written is a harmless orphan; cleanup is deferred.) - if (Buffer.byteLength(replacedText, 'utf8') >= totalBytes) { - ctx.logger.warn(`spill-policy: spill notice for ${exec.name} is not smaller than the result; keeping the inline result`) + // Invariant: the policy NEVER emits a replacement larger than the cap. When + // the notice alone exceeds maxInlineBytes (a tiny cap or a long spill root), + // there is no within-cap replacement, so keep the inline result — spilling + // would break the advertised context cap. (A within-cap replacement is + // always smaller than the original, which is > cap by the entry condition, + // so this one check subsumes "not smaller than the original" too. The spill + // file already written is a harmless orphan; cleanup is deferred.) + if (Buffer.byteLength(replacedText, 'utf8') > maxInlineBytes) { + ctx.logger.warn(`spill-policy: spill notice for ${exec.name} exceeds maxInlineBytes; keeping the inline result`) return decision } const replaced: ContentBlock[] = [{ type: 'text', text: replacedText }] diff --git a/packages/spill/spill-policy/tests/spill-policy.spec.ts b/packages/spill/spill-policy/tests/spill-policy.spec.ts index dd6235a9a3..b137a73144 100644 --- a/packages/spill/spill-policy/tests/spill-policy.spec.ts +++ b/packages/spill/spill-policy/tests/spill-policy.spec.ts @@ -53,7 +53,7 @@ function exec(name: string, session = 's1'): ToolExecution { * Build a context with tools + the policy, and optionally a spill backend. * Returns the context and the backend handle (undefined when `withSpill` false). */ -async function setup(config: SpillPolicy.Config, withSpill = true): Promise<{ ctx: Context; spill?: StubSpill }> { +async function setup(config: SpillPolicy.Config, withSpill = true): Promise<{ ctx: Context; spill?: StubSpill; fiber: Awaited> }> { const ctx = new Context() await ctx.plugin(SystemPrompt) await ctx.plugin(ToolRegistry) @@ -62,8 +62,8 @@ async function setup(config: SpillPolicy.Config, withSpill = true): Promise<{ ct await ctx.plugin(StubSpill) spill = ctx.spillFiles as StubSpill } - await ctx.plugin(SpillPolicy, config) - return { ctx, ...spill ? { spill } : {} } + const fiber = await ctx.plugin(SpillPolicy, config) + return { ctx, fiber, ...spill ? { spill } : {} } } /** Flatten a result's text blocks. */ @@ -118,9 +118,9 @@ describe('oversized plain-text replacement', () => { expect(Buffer.byteLength(text, 'utf8')).toBeLessThan(body.length) }) - it('keeps the inline result when even the notice-only replacement is not smaller', async () => { - // A body just over a tiny cap: the notice alone is larger than the result, - // so spilling would only add bytes — the policy keeps the inline result. + it('keeps the inline result when the notice-only replacement would exceed the cap', async () => { + // A body just over a tiny cap: the notice alone is larger than the cap, so + // there is no within-cap replacement — the policy keeps the inline result. const { ctx } = await setup({ maxInlineBytes: 4 }) const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {}) const body = 'xxxxx' // 5 bytes > 4, but far shorter than the notice @@ -198,7 +198,7 @@ describe('best-effort fallback', () => { describe('composition', () => { it('bounds content a downstream post-execute listener replaced', async () => { - const { ctx, spill } = await setup({ maxInlineBytes: 10 }) + const { ctx, spill } = await setup({ maxInlineBytes: 200 }) // A later-registered listener replaces the (small) tool result with a big one; // the policy delegated via next(), so it bounds the replacement. ctx.on('tools/post-execute', async (_e, _r, _next) => @@ -210,7 +210,7 @@ describe('composition', () => { }) it('preserves a downstream accept decision additionalContext when spilling', async () => { - const { ctx } = await setup({ maxInlineBytes: 10 }) + const { ctx } = await setup({ maxInlineBytes: 200 }) const context = { content: [{ type: 'text' as const, text: 'note' }], source: { kind: 'plugin' as const, plugin: 'test' } } ctx.on('tools/post-execute', async (_e, _r, _next) => ({ kind: 'accept', additionalContext: context })) @@ -220,3 +220,38 @@ describe('composition', () => { expect(result.additionalContext).toEqual(context) }) }) + +describe('cap invariant', () => { + it('keeps the inline result when the notice alone exceeds the cap, even for a large original', async () => { + // A large body (so it is well over the cap) but a cap smaller than the + // notice itself: there is no within-cap replacement, so the policy must keep + // the inline result rather than emit content over maxInlineBytes. + const { ctx } = await setup({ maxInlineBytes: 8 }) + const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {}) + const body = 'x'.repeat(5000) + ctx.tools.register(textTool('big', body)) + const result = await ctx.tools.execute(exec('big')) + expect(textOf(result.content)).toBe(body) + expect(warn).toHaveBeenCalled() + }) +}) + +describe('disposal (HMR safety)', () => { + it('stops transforming oversized results after the plugin fiber is disposed', async () => { + const { ctx, spill, fiber } = await setup({ maxInlineBytes: 200 }) + const body = 'HEAD'.repeat(200) + 'TAIL'.repeat(200) + ctx.tools.register(textTool('big', body)) + + // Live: the listener spills and replaces. + const before = await ctx.tools.execute(exec('big')) + expect(textOf(before.content)).toContain('Full formatted result saved to') + expect(spill?.saves).toHaveLength(1) + + // After disposal the listener is gone — the result passes through untouched + // and nothing more is spilled (no leaked registration across reload). + await fiber.dispose() + const after = await ctx.tools.execute(exec('big')) + expect(textOf(after.content)).toBe(body) + expect(spill?.saves).toHaveLength(1) + }) +})