refactor: clarify binary file attachment handling for AI agents

Add explicit instructions across all agent prompts explaining that binary
files (PDFs, images, documents) return content as attachments in the
message following the tool result, not as text in the result itself.

Update tool response messages to clearly state the attachment delivery
mechanism and prevent agents from re-reading the same file expecting
different output.

Improve build safety by neutralizing dynamic eval constructs and
hardening the officeparser plugin against bundle shape changes.
This commit is contained in:
Andrew Beal 2026-05-31 17:58:23 +01:00
parent 68a918338e
commit 72bf43a8ea
9 changed files with 111 additions and 21 deletions

View file

@ -13,6 +13,8 @@ You are a task execution agent. You execute assigned tasks and report outcomes.
When you encounter genuine ambiguity about *what* to do, ask the user before proceeding.
**Reading binary files (PDFs, images, documents):** When you read one of these, its content does NOT come back as text in the tool result. The result is a brief confirmation, and the actual content arrives as an **attachment in the message immediately after**. That attachment IS the file you read and is already in your context use it directly. Do NOT re-read the file to "get the real content" (re-reading returns the same attachment), and do NOT report failure because the result wasn't text the read succeeded.
## Boundaries
### You MUST:

View file

@ -141,6 +141,8 @@ You can search and read vault files at any point before making a decision. This
- Read files when you need to understand structure or content, not just confirm existence
- 13 searches is typical; if you need significantly more, consider whether the plan itself needs revision
**Reading binary files (PDFs, images, documents):** Their content does NOT come back as text in the tool result the result is a brief confirmation, and the content arrives as an **attachment in the message immediately after**. That attachment IS the file content and is already in your context. Do NOT re-read the same file expecting text; re-reading just returns the same attachment.
## Obsidian Bases
**Bases** is a core plugin (available since Obsidian 1.9) that creates database-like views of notes from their YAML frontmatter properties. Bases are stored as \`.base\` files — treat them like notes: reference with \`[[MyBase.base]]\`, embed with \`![[MyBase.base]]\` or \`![[MyBase.base#ViewName]]\`.

View file

@ -143,6 +143,8 @@ Start broad, then narrow. Short queries return more results than long, specific
When searches return images or PDFs, read them don't just note their existence. Extract relevant information to inform your plan. Since the execution agent retains context across steps, you can also plan read steps for non-markdown files that the execution agent needs to reference during later steps.
**How binary reads return content:** When you read a PDF, image, or Office/ODF document, its content does NOT come back as text in the tool result. The result is a brief confirmation, and the actual content arrives as an **attachment in the message immediately after**. That attachment IS the file you read it is already in your context. Do NOT re-read the same file to "get the real content"; re-reading just returns the same attachment.
## Plan Structure
### 1. Simple Tasks (1-3 steps)

View file

@ -1,5 +1,3 @@
import { Copy } from "Enums/Copy";
export const SystemInstruction: string = `
# Obsidian AI Assistant
@ -358,12 +356,19 @@ Example: If the user says "create a note in \`folder:"Projects/2025"\`", the fol
### File Attachments
**Users can attach files directly to their messages.** When a file is attached, its content is provided inline in the conversation. Attached files are likely NOT present in the vault so use the attached content directly.
**File content reaches you as an attachment in two distinct situations treat both the same way once the content is present:**
**How to recognize attached files:**
- A text block states: ${Copy.AttachedFile}
1. **User uploads** the user attaches a file directly to their message.
2. **Tool reads of binary vault files** when you call \`read_vault_files\` on a PDF, image, or Office/ODF document, the file's content is NOT returned as text in the function result. Instead, the function result is a short confirmation message, and the actual file content is delivered as an **attachment in the message immediately following the result**. This attachment IS the vault file you asked to read.
**Critical:** When you read a binary file (PDF/image/document), do not be misled by the brief "files retrieved" confirmation the real content is the attachment that follows it. The content is already in your context. **Do NOT call \`read_vault_files\` again for the same file** in an attempt to "get the real content" — re-reading returns the same attachment and accomplishes nothing.
**How to recognize an attachment:**
- A text block introduces it (e.g. stating the file name and that its contents follow)
- The file content or file ID immediately follows that text block
- Attachments for document filetypes (.docx, .odt, .xlsx, etc.,) are included as plain text
- Attachments for document filetypes (.docx, .odt, .xlsx, etc.) are included as plain text; PDFs and images are provided as native attachments
Whether the attachment came from a user upload or your own tool read, the attached content is authoritative read and use it directly to answer.
---

View file

@ -123,8 +123,7 @@ export enum Copy {
ButtonAttachFiles = "Attach Files",
// Agent file message
AttachedFile = `The user has attached the file {fileName}. The contents of the file are included below.
**Note that this is an attachment to the chat and the file is likely NOT present in the vault**`,
AttachedFile = `The file {fileName} is attached and its full contents follow below. This is the actual content of the file — read it directly to answer the user. This attachment may be a file the user uploaded to the chat, or a vault file you retrieved with a tool; either way, the content below is authoritative and you do NOT need to read or fetch this file again.`,
// Execution Plan Messages
PlanningFailedError = `Failed to generate plan. You should attempt to recover from this.

View file

@ -6,7 +6,7 @@ import { parseOffice } from 'officeparser';
// Handles PDF format
export async function readPDF(arrayBuffer: ArrayBuffer): Promise<IPageText[]> {
try {
const pdf = await getDocumentProxy(new Uint8Array(arrayBuffer));
const pdf = await getDocumentProxy(new Uint8Array(arrayBuffer), { isEvalSupported: false });
const pages = (await extractText(pdf, { mergePages: false })).text;
const pageTexts: IPageText[] = pages.map((pageText, index) => ({

View file

@ -322,13 +322,17 @@ export class AIToolService {
return new Attachment(fileName, mimeType, file.contents);
});
const binaryMessage = "The requested file(s) have been attached to the conversation immediately after this result. " +
"This IS the content of the file(s) you read from the vault — PDFs, images, and documents are provided as attachments, " +
"not as text in this response. Read the attached content directly to answer the user. You do NOT need to read or fetch this file again.";
const response = textResults.length > 0 || errorResults.length > 0
? {
results: [...textResults, ...errorResults],
...(binaryResults.length > 0 && { message: "The contents of the files are included below." })
...(binaryResults.length > 0 && { message: binaryMessage })
}
: {
message: "Files retrieved successfully. The contents of the files are included below.",
message: binaryMessage,
count: binaryResults.length
};

View file

@ -262,7 +262,8 @@ describe('AIToolService - Integration Tests', () => {
toolId: 'tool_9'
} as any);
expect(result.payload.response).toEqual({ message: 'Files retrieved successfully. The contents of the files are included below.', count: 0 });
expect((result.payload.response as any).count).toBe(0);
expect((result.payload.response as any).message).toContain('attached to the conversation');
});
it('should handle single file read', async () => {

View file

@ -39,6 +39,68 @@ function copyDir(src, dest) {
}
}
// Neutralises `new Function(...)` literals in the bundled output.
//
// All such literals come from dependencies, never from our own source:
// - unpdf/PDF.js: 3x feature-detection probes `new Function("")` wrapped in
// try/catch (a throw makes them return `false` = "eval not supported", the
// safe answer), plus 1x PostScript JIT compiler gated behind
// `isEvalSupported` (we pass `false`, so it is never entered).
// - diff2html/@profoundlogic/hogan: 2x template compilers reachable only via
// Hogan.compile(), which diff2html invokes only for runtime `rawTemplates`.
// We use diff2html's precompiled defaults, so these never run.
//
// Every occurrence is therefore dead code in our usage. We rewrite them to a
// throwing stub so the bundle contains no `new Function(` literal, which clears
// the Obsidian scanner's "Dynamic Code Execution" warning. The build FAILS if
// the count drifts from EXPECTED_NEW_FUNCTION_COUNT, so a dependency change can
// never silently reintroduce a live (or un-neutralised) dynamic-eval path.
const EXPECTED_NEW_FUNCTION_COUNT = 6;
const NEW_FUNCTION_STUB = "VKBlockedDynamicFn";
function neutraliseDynamicEval(outfile) {
if (!existsSync(outfile)) return;
let contents = readFileSync(outfile, "utf-8");
const matches = contents.match(/new Function\s*\(/g) || [];
if (matches.length !== EXPECTED_NEW_FUNCTION_COUNT) {
throw new Error(
`neutraliseDynamicEval: expected ${EXPECTED_NEW_FUNCTION_COUNT} ` +
`\`new Function(\` literals in ${outfile} but found ${matches.length}. ` +
`A dependency changed — re-audit which library introduced/removed the ` +
`call and confirm it is unreachable before updating EXPECTED_NEW_FUNCTION_COUNT.`,
);
}
// Throwing stub: callable as `new VKBlockedDynamicFn(...)` with any args.
const stubDecl =
`function ${NEW_FUNCTION_STUB}(){throw new Error("Dynamic code execution is disabled in this build.")}`;
contents = contents.replace(/new Function\s*\(/g, `new ${NEW_FUNCTION_STUB}(`);
// Insert the stub right after the generated banner comment (a hoisted function
// declaration is in scope for the whole module regardless of position).
const bannerEnd = contents.indexOf("*/");
if (bannerEnd !== -1) {
const insertAt = contents.indexOf("\n", bannerEnd) + 1;
contents = contents.slice(0, insertAt) + stubDecl + "\n" + contents.slice(insertAt);
} else {
contents = `${stubDecl}\n${contents}`;
}
writeFileSync(outfile, contents);
console.log(
`🛡️ Neutralised ${matches.length} \`new Function(\` literal(s) in ${outfile}`,
);
}
// Runs the dynamic-eval neutralisation on every build and rebuild, for both dev
// and production, so the dev bundle matches what ships and the post-build step
// is exercised continuously rather than only at release time. Registered last so
// it transforms the finalised main.js.
const dynamicEvalPlugin = {
name: "neutralise-dynamic-eval",
setup(build) {
build.onEnd(() => {
neutraliseDynamicEval(build.initialOptions.outfile);
});
},
};
// Plugin to merge CSS files into styles.css and cleanup build artifacts
const cssMergerPlugin = {
name: "css-merger",
@ -106,11 +168,15 @@ const cssMergerPlugin = {
// set module.exports, so esbuild can't resolve its exports. This plugin intercepts the resolve
// and appends a CJS export line so imports work correctly.
//
// The browser bundle contains dynamic require() patterns (from its own esbuild bundling) that
// our esbuild would otherwise resolve against the `external` list, emitting require("fs") etc.
// in the final output. On Obsidian mobile there is no "fs" module, so we must neutralise those
// internal requires by replacing the dynamic-require shim with a stub that always throws, and
// stripping eval("require")("stream") calls. This keeps the bundle self-contained.
// The browser bundle contains a dynamic-require shim (from its own esbuild bundling) that our
// esbuild would otherwise resolve against the `external` list, emitting require("fs") etc. in
// the final output. On Obsidian mobile there is no "fs" module, so we replace that shim with a
// stub that always throws, keeping the bundle self-contained.
//
// The shim's variable name is minified and churns between releases (Au in v5, Vm in v6, Gs in
// v7.1.0), so the regex captures whatever identifier is used rather than hardcoding it. If the
// shim shape changes such that nothing matches, the build FAILS loudly — a silent no-op here
// previously let the stale v6 `Vm` regex match nothing on v7.
const officeParserPlugin = {
name: "officeparser-cjs-shim",
setup(build) {
@ -120,11 +186,19 @@ const officeParserPlugin = {
}));
build.onLoad({ filter: /.*/, namespace: 'officeparser-shim' }, async (args) => {
let contents = readFileSync(args.path, 'utf-8');
// Replace the dynamic require shim so esbuild doesn't see real require() calls.
// The variable name changed from Au (v5) to Vm (v6).
// Capture the (minified) shim identifier in group 1 and reuse it in the replacement.
const shimPattern =
/var ([A-Za-z_$][\w$]*)=\(r=>typeof require<"u"\?require:typeof Proxy<"u"\?new Proxy\(r,\{get:\(t,e\)=>\(typeof require<"u"\?require:t\)\[e\]\}\):r\)\(function\(r\)\{if\(typeof require<"u"\)return require\.apply\(this,arguments\);throw Error\('Dynamic require of "'\+r\+'" is not supported'\)\}\)/;
if (!shimPattern.test(contents)) {
throw new Error(
"officeparser-cjs-shim: dynamic-require shim not found in officeparser browser bundle. " +
"officeparser's bundling likely changed shape — re-audit the IIFE and update shimPattern " +
"before shipping, or require(\"fs\") may leak into the mobile bundle.",
);
}
contents = contents.replace(
/var Vm=\(r=>typeof require<"u"\?require:typeof Proxy<"u"\?new Proxy\(r,\{get:\(t,e\)=>\(typeof require<"u"\?require:t\)\[e\]\}\):r\)\(function\(r\)\{if\(typeof require<"u"\)return require\.apply\(this,arguments\);throw Error\('Dynamic require of "'\+r\+'" is not supported'\)\}\)/,
'var Vm=(function(r){throw Error(\'Dynamic require of "\'+r+\'" is not supported\')})'
shimPattern,
'var $1=(function(r){throw Error(\'Dynamic require of "\'+r+\'" is not supported\')})',
);
return {
contents: contents + '\nmodule.exports = officeParser;\n',
@ -142,6 +216,7 @@ const buildOptions = {
preprocess: sveltePreprocess(),
}),
cssMergerPlugin,
dynamicEvalPlugin,
],
banner: {
js: banner,