fix: address codex review round 2

- spill-policy reserves the spill notice's byte cost inside maxInlineBytes, so
  the replacement (preview + notice) never exceeds the documented model-facing
  cap. When the notice alone fills the budget the preview is empty; when even a
  notice-only replacement is not smaller than the original, the inline result is
  kept (spilling would only add bytes).
- retention TextRetainer trims an oversized single suffix chunk to the last
  suffixCap bytes on push, so tail/headTail retention stays bounded by suffixCap
  instead of retaining and re-copying the whole chunk in finish() — this is the
  spill preview path, which pushes the whole result as one chunk.

packages/spill/spill-policy/README.md

@@ -16,7 +16,7 @@ This plugin registers **no service** and owns no storage or preview mechanics: p
 2. Skip `read` (avoids a `read → spill file → read again` loop) and any non-`accept` decision (a `block`'s corrective feedback passes through).
 3. Flatten the accepted content only when it is **plain text** (all `text` blocks); a result with any non-text block is left untouched.
 4. If its UTF-8 size is `≤ maxInlineBytes`, leave it unchanged.
-5. Otherwise save the full text and replace the result with a preview + this notice:
+5. Otherwise save the full text and replace the result with a preview + this notice, sized so the whole replacement (preview + blank line + notice) stays within `maxInlineBytes` — the notice's byte cost is reserved out of the budget, so the preview shrinks to fit and the model-facing result never exceeds the cap:
 
    ```text
    <retained head/tail preview>
@@ -24,6 +24,8 @@ 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.
+
 **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.
 
 ## Scope

packages/spill/spill-policy/src/index.ts

@@ -76,25 +76,20 @@ function ownerSessionId(exec: ToolExecution): SessionId | undefined {
   return (exec as SpillPolicyExec).agent?.session.header.id
 }
 
-/** Build the bounded head/tail preview for `text`, splitting `maxInlineBytes` across the two ends. */
-function preview(text: string, maxInlineBytes: number): { text: string; omitted: Omitted } {
-  const headBytes = Math.ceil(maxInlineBytes / 2)
-  const tailBytes = Math.floor(maxInlineBytes / 2)
+/** Build the bounded head/tail preview for `text`, splitting `budget` bytes across the two ends. */
+function preview(text: string, budget: number): { text: string; omitted: Omitted } {
+  const headBytes = Math.ceil(budget / 2)
+  const tailBytes = Math.floor(budget / 2)
   const retainer = new TextRetainer({ kind: 'headTail', headBytes, tailBytes })
   retainer.push(text)
   const kept = retainer.finish()
   return { text: kept.text, omitted: kept.omittedBytes }
 }
 
-/**
- * Compose the replacement text: the bounded preview, a blank line, then the
- * spill notice. The omission clause comes from the retention library
- * (`describeOmitted`); the recovery sentence names the concrete spill path.
- */
-function replacementText(previewText: string, omitted: Omitted, spillPath: string): string {
+/** The spill-notice line for a given omission + path (no preview, no leading blank line). */
+function spillNotice(omitted: Omitted, spillPath: string): string {
   const omission = describeOmitted(omitted, 'bytes')
-  const notice = `(${omission} Full formatted result saved to: ${spillPath}. Use read with offset/limit to inspect it.)`
-  return `${previewText}\n\n${notice}`
+  return `(${omission} Full formatted result saved to: ${spillPath}. Use read with offset/limit to inspect it.)`
 }
 
 export function apply(ctx: Context, config: Config): void {
@@ -119,7 +114,8 @@ export function apply(ctx: Context, config: Config): void {
     const content = decision.content ?? result.content
     const text = flattenPlainText(content)
     if (text === undefined) return decision
-    if (Buffer.byteLength(text, 'utf8') <= maxInlineBytes) return decision
+    const totalBytes = Buffer.byteLength(text, 'utf8')
+    if (totalBytes <= maxInlineBytes) return decision
 
     const sessionId = ownerSessionId(exec)
     if (sessionId === undefined) {
@@ -148,8 +144,28 @@ export function apply(ctx: Context, config: Config): void {
       return decision
     }
 
-    const { text: previewText, omitted } = preview(text, maxInlineBytes)
-    const replaced: ContentBlock[] = [{ type: 'text', text: replacementText(previewText, omitted, path) }]
+    // Reserve the notice's byte cost INSIDE maxInlineBytes so the replacement
+    // (preview + blank line + notice) never exceeds the documented cap — a naive
+    // preview that spent the whole budget then appended the notice could be
+    // larger than the cap, and for a marginally-over result even larger than the
+    // original. The reservation uses a notice priced at the worst-case omission
+    // count (the full byte total): its digit count bounds the real count's, so
+    // the reserved size is a safe upper bound and the final notice is never
+    // longer than what we reserved. `\n\n` is the 2-byte join.
+    const reserve = Buffer.byteLength(spillNotice({ kind: 'exact', count: totalBytes }, path), 'utf8') + 2
+    const previewBudget = Math.max(0, maxInlineBytes - reserve)
+    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`)
+      return decision
+    }
+    const replaced: ContentBlock[] = [{ type: 'text', text: replacedText }]
     return { kind: 'accept', content: replaced, ...decision.additionalContext ? { additionalContext: decision.additionalContext } : {} }
   })
 }

packages/spill/spill-policy/tests/spill-policy.spec.ts

@@ -93,9 +93,9 @@ describe('config validation', () => {
 })
 
 describe('oversized plain-text replacement', () => {
-  it('spills the full text and replaces the result with a preview + path', async () => {
-    const { ctx, spill } = await setup({ maxInlineBytes: 20 })
-    const body = 'HEAD'.repeat(20) + 'TAIL'.repeat(20) // 160 bytes > 20
+  it('spills the full text and replaces the result with a preview + path within the cap', async () => {
+    const { ctx, spill } = await setup({ maxInlineBytes: 200 })
+    const body = 'HEAD'.repeat(200) + 'TAIL'.repeat(200) // 1600 bytes > 200
     ctx.tools.register(textTool('big', body))
     const result = await ctx.tools.execute(exec('big'))
 
@@ -112,6 +112,22 @@ describe('oversized plain-text replacement', () => {
     expect(text).toContain('Full formatted result saved to: /spill/big.txt')
     expect(text).toContain('Use read with offset/limit')
     expect(text).toContain('Omitted')
+    // The replacement (preview + blank line + notice) stays within the cap and
+    // is smaller than the original — the whole point of spilling.
+    expect(Buffer.byteLength(text, 'utf8')).toBeLessThanOrEqual(200)
+    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.
+    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
+    ctx.tools.register(textTool('big', body))
+    const result = await ctx.tools.execute(exec('big'))
+    expect(textOf(result.content)).toBe(body)
+    expect(warn).toHaveBeenCalled()
   })
 
   it('leaves a small plain-text result unchanged', async () => {

packages/util/retention/src/index.ts

@@ -355,6 +355,19 @@ export class TextRetainer {
         this.suffixHeld -= head.length
         head = this.suffixChunks[0]
       }
+      // The head chunk can still hold leading bytes beyond the last `suffixCap`
+      // — a single chunk LARGER than the window is retained whole by the loop
+      // above (dropping the only chunk would leave < cap). Trim those leading
+      // bytes so the accumulator (and finish()'s concat) stays bounded by
+      // `suffixCap` instead of allocating/copying the full chunk again;
+      // finish() only ever reads the last `suffixLen ≤ suffixCap` bytes, so this
+      // drops nothing it would return. (head.length > excess by the loop
+      // invariant `suffixHeld - head.length < suffixCap`, so the slice is non-empty.)
+      if (head !== undefined && this.suffixHeld > this.suffixCap) {
+        const excess = this.suffixHeld - this.suffixCap
+        this.suffixChunks[0] = head.subarray(excess)
+        this.suffixHeld -= excess
+      }
     }
 
     // Dropped = bytes that no side can keep. Compute cumulative omission the

packages/web/tool-web/tests/spill.spec.ts

@@ -33,7 +33,8 @@ let handler: Handler
 let spillRoot: string
 let ctx: Context
 
-const BODY = 'X'.repeat(4000) // formatted result is well over the 200-byte policy cap
+const BODY = 'X'.repeat(4000) // formatted result is well over the policy cap
+const MAX_INLINE_BYTES = 1000 // leaves room for a head/tail preview beside the notice
 
 beforeEach(async () => {
   handler = (_req, res) => { res.writeHead(200, { 'content-type': 'text/plain' }); res.end(BODY) }
@@ -50,7 +51,7 @@ beforeEach(async () => {
   // policy cap is what triggers the spill (the RFC's separation of concerns).
   await ctx.plugin(WebFetchLocal, { maxBodyChars: 500_000 })
   await ctx.plugin(LocalSpillFiles, { root: spillRoot })
-  await ctx.plugin(SpillPolicy, { maxInlineBytes: 200 })
+  await ctx.plugin(SpillPolicy, { maxInlineBytes: MAX_INLINE_BYTES })
   await ctx.plugin(ToolWeb)
 })
 
@@ -72,8 +73,9 @@ describe('web_fetch spill showcase', () => {
     expect(out.isError).toBe(false)
     const text = out.content.map(b => b.text).join('')
 
-    // Model-facing text is a preview + notice, NOT the full body.
+    // Model-facing text is a preview + notice within the cap, NOT the full body.
     expect(text.length).toBeLessThan(BODY.length)
+    expect(Buffer.byteLength(text, 'utf8')).toBeLessThanOrEqual(MAX_INLINE_BYTES)
     expect(text).toContain(`Fetched ${base}`) // the head of the formatted result survives
     expect(text).toContain('Full formatted result saved to:')
     expect(text).toContain('Use read with offset/limit')