fix(copilot): let a read placeholder carry its own kind instead of sniffing its text

Three rounds of review found the same defect in three places: a gate matching a
prefix no producer emitted so it never fired, a real file misread as a refusal, a
filename newline hiding a refusal. Each fix tightened the text matching and the
next round found another way the text was the wrong thing to trust.

So stop trusting it. `readPlaceholder.*` now returns the whole read result —
content, totalLines, and a `placeholder` kind — and the two classifiers read the
kind. A producer cannot emit a placeholder without tagging it, and no amount of
user content can imitate one, so both failure directions are gone by construction
rather than by a better pattern.

`oversized` (the four size refusals) is reported as a tool error; `unreadable`
(undecodable, binary, unparseable) is returned as content. The type flows through
FileReadResult and ReadResult, so the compiler checks the tag survives to the
gates rather than leaving a silent drop to be discovered later.

Also removes the totalLines-of-1 heuristic, which was only ever a proxy for this.

Author: waleedlatif1

apps/sim/lib/copilot/tools/handlers/vfs.test.ts

@@ -119,10 +119,9 @@ describe('vfs handlers oversize policy', () => {
 
   it('fails file-backed oversized read placeholders with original message', async () => {
     const vfs = makeVfs()
-    vfs.readFileContent.mockResolvedValue({
-      content: '[File too large to display inline: big.txt (6000000 bytes, limit 5242880)]',
-      totalLines: 1,
-    })
+    vfs.readFileContent.mockResolvedValue(
+      readPlaceholder.fileTooLarge('big.txt', 6_000_000, 5_242_880)
+    )
     getOrMaterializeVFS.mockResolvedValue(vfs)
 
     const result = await executeVfsRead(
@@ -182,17 +181,17 @@ describe('vfs handlers oversize policy', () => {
 
   /**
    * Every size refusal is a failed read, whichever path produced it. Built from the
-   * producers so a prefix leaving `OVERSIZED_PREFIXES` fails here rather than
+   * producers so one that stops tagging itself `oversized` fails here rather than
    * silently downgrading a refusal to a one-line "successful" read.
    */
   it.each([
     ['image', readPlaceholder.imageTooLarge('huge.png', 99, 5)],
     ['file', readPlaceholder.fileTooLarge('huge.txt', 99, 5)],
     ['document', readPlaceholder.documentTooLarge('huge.pdf', 99, 5)],
     ['compiled artifact', readPlaceholder.compiledArtifactTooLarge('app.js', 99, 5)],
-  ])('fails the read when a %s exceeds its size limit', async (_kind, content) => {
+  ])('fails the read when a %s exceeds its size limit', async (_kind, placeholder) => {
     const vfs = makeVfs()
-    vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 })
+    vfs.readFileContent.mockResolvedValue(placeholder)
     getOrMaterializeVFS.mockResolvedValue(vfs)
 
     const result = await executeVfsRead(
@@ -202,14 +201,31 @@ describe('vfs handlers oversize policy', () => {
 
     expect(result.success).toBe(false)
     // The placeholder verbatim, not the generic "grep this instead" fallback.
-    expect(result.error).toBe(content)
+    expect(result.error).toBe(placeholder.content)
+  })
+
+  it('still fails the read when the stored name contains a newline', async () => {
+    // Nothing about the message text decides this, so a name that would break a
+    // text-shape match cannot hide a refusal.
+    const vfs = makeVfs()
+    const placeholder = readPlaceholder.fileTooLarge('we\nird.txt', 99, 5)
+    vfs.readFileContent.mockResolvedValue(placeholder)
+    getOrMaterializeVFS.mockResolvedValue(vfs)
+
+    const result = await executeVfsRead(
+      { path: 'files/weird/content' },
+      { userId: 'user-1', workflowId: 'wf-1', workspaceId: 'ws-1' }
+    )
+
+    expect(result.success).toBe(false)
+    expect(result.error).toBe(placeholder.content)
   })
 
-  it('returns a real file that merely opens like a size refusal', async () => {
-    // Prefix-only matching would turn this user's file into a tool error. The
-    // refusal is recognised by its whole shape, which real prose does not have.
+  it('returns a real file whose content is exactly a size-refusal message', async () => {
+    // Untagged, so it is content. Recognising refusals by their text would turn this
+    // user's file into a tool error instead of returning it.
     const vfs = makeVfs()
-    const content = '[Document too large to parse inline: is the message we emit here]'
+    const { content } = readPlaceholder.documentTooLarge('huge.pdf', 99, 5)
     vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 })
     getOrMaterializeVFS.mockResolvedValue(vfs)
 
@@ -226,12 +242,13 @@ describe('vfs handlers oversize policy', () => {
     const vfs = makeVfs()
     // Not a size problem — the bytes were read fine and the reason is already in the
     // message, so the model should see it rather than a "too large, use grep" error.
-    const content = readPlaceholder.imageUnavailable(
+    const placeholder = readPlaceholder.imageUnavailable(
       'bomb.png',
       90,
       'It is too large to decode safely.'
     )
-    vfs.readFileContent.mockResolvedValue({ content, totalLines: 1 })
+    const content = placeholder.content
+    vfs.readFileContent.mockResolvedValue(placeholder)
     getOrMaterializeVFS.mockResolvedValue(vfs)
 
     const result = await executeVfsRead(

apps/sim/lib/copilot/tools/handlers/vfs.ts

@@ -330,7 +330,7 @@ export async function executeVfsRead(
         const isAttachment = hasModelAttachment(uploadResult)
         if (
           !isAttachment &&
-          (isOversizedReadPlaceholder(uploadResult.content) ||
+          (isOversizedReadPlaceholder(uploadResult) ||
             serializedResultSize(uploadResult) > TOOL_RESULT_MAX_INLINE_CHARS)
         ) {
           logger.warn('Upload read result too large', {
@@ -341,7 +341,7 @@ export async function executeVfsRead(
           })
           return {
             success: false,
-            error: isOversizedReadPlaceholder(uploadResult.content)
+            error: isOversizedReadPlaceholder(uploadResult)
               ? uploadResult.content
               : // Same as the workspace-file branch below: this size gate runs on
                 // the whole upload before any window, so "retry with offset/limit"
@@ -400,7 +400,7 @@ export async function executeVfsRead(
       const isAttachment = hasModelAttachment(fileContent)
       if (
         !isAttachment &&
-        (isOversizedReadPlaceholder(fileContent.content) ||
+        (isOversizedReadPlaceholder(fileContent) ||
           serializedResultSize(fileContent) > TOOL_RESULT_MAX_INLINE_CHARS)
       ) {
         logger.warn('File read result too large', {
@@ -411,7 +411,7 @@ export async function executeVfsRead(
         })
         return {
           success: false,
-          error: isOversizedReadPlaceholder(fileContent.content)
+          error: isOversizedReadPlaceholder(fileContent)
             ? fileContent.content
             : 'Read result too large to return inline. Use grep with a more specific pattern or narrower path to locate the relevant section, then retry read with offset/limit. Avoid catch-all greps or full-file reads because they waste context window.',
         }
@@ -459,7 +459,7 @@ export async function executeVfsRead(
     }
     if (
       !hasModelAttachment(result) &&
-      (isOversizedReadPlaceholder(result.content) ||
+      (isOversizedReadPlaceholder(result) ||
         serializedResultSize(result) > TOOL_RESULT_MAX_INLINE_CHARS)
     ) {
       return {

apps/sim/lib/copilot/vfs/file-reader.ts

@@ -12,7 +12,7 @@ import { TraceEvent } from '@/lib/copilot/generated/trace-events-v1'
 import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
 import { recordFileRead } from '@/lib/copilot/request/metrics'
 import { markSpanForError } from '@/lib/copilot/request/otel'
-import { readPlaceholder } from '@/lib/copilot/vfs/read-placeholders'
+import { type PlaceholderKind, readPlaceholder } from '@/lib/copilot/vfs/read-placeholders'
 import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
 import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
 import { fetchWorkspaceFileBuffer } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
@@ -436,6 +436,8 @@ async function prepareImageForVision(
 export interface FileReadResult {
   content: string
   totalLines: number
+  /** Set when `content` stands in for the file rather than being it — see `readPlaceholder`. */
+  placeholder?: PlaceholderKind
   attachment?: {
     type: string
     name?: string
@@ -478,10 +480,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise<FileR
           span.setAttribute(TraceAttr.CopilotVfsReadPath, CopilotVfsReadPath.Image)
           const imageTooLarge = (bytes: number) => {
             span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge)
-            return {
-              content: readPlaceholder.imageTooLarge(record.name, bytes, MAX_IMAGE_SOURCE_BYTES),
-              totalLines: 1,
-            }
+            return readPlaceholder.imageTooLarge(record.name, bytes, MAX_IMAGE_SOURCE_BYTES)
           }
           // The recorded size only skips a doomed download; the cap on the download
           // itself is what bounds the bytes actually read.
@@ -492,16 +491,13 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise<FileR
           const prepared = await prepareImageForVision(fetched.buffer, record.type)
           if (!prepared.ok) {
             span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ImageTooLarge)
-            return {
-              // The fetched buffer, not `record.size`: the bytes are in hand by now,
-              // so there is no reason to quote the client-declared figure back.
-              content: readPlaceholder.imageUnavailable(
-                record.name,
-                fetched.buffer.length,
-                prepared.reason
-              ),
-              totalLines: 1,
-            }
+            // The fetched buffer, not `record.size`: the bytes are in hand by now, so
+            // there is no reason to quote the client-declared figure back.
+            return readPlaceholder.imageUnavailable(
+              record.name,
+              fetched.buffer.length,
+              prepared.reason
+            )
           }
           const { buffer, mediaType, resized } = prepared.image
           const sizeKb = (buffer.length / 1024).toFixed(1)
@@ -531,10 +527,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise<FileR
           span.setAttribute(TraceAttr.CopilotVfsReadPath, CopilotVfsReadPath.Text)
           const textTooLarge = (bytes: number) => {
             span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.TextTooLarge)
-            return {
-              content: readPlaceholder.fileTooLarge(record.name, bytes, MAX_TEXT_READ_BYTES),
-              totalLines: 1,
-            }
+            return readPlaceholder.fileTooLarge(record.name, bytes, MAX_TEXT_READ_BYTES)
           }
           if (record.size > MAX_TEXT_READ_BYTES) return textTooLarge(record.size)
 
@@ -559,14 +552,7 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise<FileR
               TraceAttr.CopilotVfsReadOutcome,
               CopilotVfsReadOutcome.DocumentTooLarge
             )
-            return {
-              content: readPlaceholder.documentTooLarge(
-                record.name,
-                bytes,
-                MAX_PARSEABLE_READ_BYTES
-              ),
-              totalLines: 1,
-            }
+            return readPlaceholder.documentTooLarge(record.name, bytes, MAX_PARSEABLE_READ_BYTES)
           }
           if (record.size > MAX_PARSEABLE_READ_BYTES) return documentTooLarge(record.size)
           const fetched = await fetchWithinLimit(record, MAX_PARSEABLE_READ_BYTES)
@@ -594,21 +580,15 @@ export async function readFileRecord(record: WorkspaceFileRecord): Promise<FileR
               [TraceAttr.ErrorMessage]: toError(parseErr).message.slice(0, 500),
             })
             span.setAttribute(TraceAttr.CopilotVfsReadOutcome, CopilotVfsReadOutcome.ParseFailed)
-            return {
-              content: readPlaceholder.couldNotParse(record.name, record.type, record.size),
-              totalLines: 1,
-            }
+            return readPlaceholder.couldNotParse(record.name, record.type, record.size)
           }
         }
 
         span.setAttributes({
           [TraceAttr.CopilotVfsReadPath]: CopilotVfsReadPath.Binary,
           [TraceAttr.CopilotVfsReadOutcome]: CopilotVfsReadOutcome.BinaryPlaceholder,
         })
-        return {
-          content: readPlaceholder.binaryFile(record.name, record.type, record.size),
-          totalLines: 1,
-        }
+        return readPlaceholder.binaryFile(record.name, record.type, record.size)
       } catch (err) {
         logger.warn('Failed to read workspace file', {
           fileName: record.name,

apps/sim/lib/copilot/vfs/operations.test.ts

@@ -197,14 +197,15 @@ describe('grep regex safety', () => {
 })
 
 describe('grepReadResult placeholders', () => {
-  const grepPlaceholder = (content: string) =>
-    grepReadResult('files/x.png/content', { content, totalLines: 1 }, 'x', 'files/x.png/content')
+  const grepResult = (result: {
+    content: string
+    totalLines: number
+    placeholder?: 'oversized' | 'unreadable'
+  }) => grepReadResult('files/x.png/content', result, 'x', 'files/x.png/content')
 
   /**
-   * Built from the producers rather than hand-copied: a literal here would only
-   * prove the matcher agrees with this file, which is exactly the drift that let a
-   * gate test for a prefix no producer emitted. Covers every builder, so dropping
-   * one from the shared table fails here.
+   * Built from the producers rather than hand-assembled: covers every builder, so
+   * one that stops tagging itself fails here.
    */
   const everyPlaceholder = Object.entries({
     fileTooLarge: readPlaceholder.fileTooLarge('big.txt', 99, 5),
@@ -218,25 +219,19 @@ describe('grepReadResult placeholders', () => {
 
   it.each(everyPlaceholder)(
     'reports the %s placeholder instead of grepping it',
-    (_name, content) => {
-      expect(() => grepPlaceholder(content)).toThrow(WorkspaceFileGrepError)
-      expect(() => grepPlaceholder(content)).toThrow(content)
+    (_name, result) => {
+      expect(() => grepResult(result)).toThrow(WorkspaceFileGrepError)
+      expect(() => grepResult(result)).toThrow(result.content)
     }
   )
 
   it('still greps ordinary single-line content', () => {
-    expect(grepPlaceholder('x marks the spot')).toHaveLength(1)
-  })
-
-  it('greps a real multi-line file that merely opens like a placeholder', () => {
-    // The single-line guard is what keeps this file searchable rather than swallowed.
-    const content = `${readPlaceholder.binaryFile('app.bin', 'text/plain', 10)}\nx marks the spot`
-    const matches = grepReadResult(
-      'files/notes.txt/content',
-      { content, totalLines: 2 },
-      'x',
-      'files/notes.txt/content'
-    )
-    expect(matches.length).toBeGreaterThan(0)
+    expect(grepResult({ content: 'x marks the spot', totalLines: 1 })).toHaveLength(1)
+  })
+
+  it('greps a real file whose content is exactly a placeholder message', () => {
+    // Untagged, so it is content — text alone never makes something a placeholder.
+    const { content } = readPlaceholder.binaryFile('app.bin', 'text/plain', 10)
+    expect(grepResult({ content, totalLines: 1 })).toHaveLength(1)
   })
 })

apps/sim/lib/copilot/vfs/operations.ts

@@ -1,7 +1,10 @@
 import { createLogger } from '@sim/logger'
 import { truncate } from '@sim/utils/string'
 import micromatch from 'micromatch'
-import { isNonGreppablePlaceholder } from '@/lib/copilot/vfs/read-placeholders'
+import {
+  isNonGreppablePlaceholder,
+  type PlaceholderKind,
+} from '@/lib/copilot/vfs/read-placeholders'
 import {
   compileLinearRegex,
   isPlainText,
@@ -75,7 +78,12 @@ export class WorkspaceFileGrepError extends Error {
  */
 export function grepReadResult(
   path: string,
-  result: { content: string; totalLines: number; attachment?: unknown },
+  result: {
+    content: string
+    totalLines: number
+    placeholder?: PlaceholderKind
+    attachment?: unknown
+  },
   pattern: string,
   readHint: string,
   options?: GrepOptions
@@ -85,7 +93,7 @@ export function grepReadResult(
       `Cannot grep "${path}" — it has no searchable text (image/binary). Use read("${readHint}") to view it.`
     )
   }
-  if (isNonGreppablePlaceholder(result.content, result.totalLines)) {
+  if (isNonGreppablePlaceholder(result)) {
     throw new WorkspaceFileGrepError(result.content)
   }
   return grep(new Map([[path, result.content]]), pattern, undefined, options)
@@ -94,6 +102,7 @@ export function grepReadResult(
 export interface ReadResult {
   content: string
   totalLines: number
+  placeholder?: PlaceholderKind
 }
 
 /**