diff --git a/AIPrompts/ExecutionPrompt.ts b/AIPrompts/ExecutionPrompt.ts index 668d438..b79a546 100644 --- a/AIPrompts/ExecutionPrompt.ts +++ b/AIPrompts/ExecutionPrompt.ts @@ -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: diff --git a/AIPrompts/OrchestrationPrompt.ts b/AIPrompts/OrchestrationPrompt.ts index bf2328f..301ebc9 100644 --- a/AIPrompts/OrchestrationPrompt.ts +++ b/AIPrompts/OrchestrationPrompt.ts @@ -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 - 1–3 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]]\`. diff --git a/AIPrompts/PlanningPrompt.ts b/AIPrompts/PlanningPrompt.ts index ec3054f..515d5c3 100644 --- a/AIPrompts/PlanningPrompt.ts +++ b/AIPrompts/PlanningPrompt.ts @@ -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) diff --git a/AIPrompts/SystemPrompt.ts b/AIPrompts/SystemPrompt.ts index 6cd7f7d..012072d 100644 --- a/AIPrompts/SystemPrompt.ts +++ b/AIPrompts/SystemPrompt.ts @@ -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. --- diff --git a/Enums/Copy.ts b/Enums/Copy.ts index f169617..79acb97 100644 --- a/Enums/Copy.ts +++ b/Enums/Copy.ts @@ -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. diff --git a/Helpers/DocumentHelper.ts b/Helpers/DocumentHelper.ts index eb86a38..4a01b13 100644 --- a/Helpers/DocumentHelper.ts +++ b/Helpers/DocumentHelper.ts @@ -6,7 +6,7 @@ import { parseOffice } from 'officeparser'; // Handles PDF format export async function readPDF(arrayBuffer: ArrayBuffer): Promise { 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) => ({ diff --git a/Services/AIServices/AIToolService.ts b/Services/AIServices/AIToolService.ts index 12d9793..b5ae492 100644 --- a/Services/AIServices/AIToolService.ts +++ b/Services/AIServices/AIToolService.ts @@ -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 }; diff --git a/__tests__/Services/AIToolService.test.ts b/__tests__/Services/AIToolService.test.ts index 9537210..0da4ca5 100644 --- a/__tests__/Services/AIToolService.test.ts +++ b/__tests__/Services/AIToolService.test.ts @@ -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 () => { diff --git a/esbuild.config.mjs b/esbuild.config.mjs index afdd44d..d84dbd9 100644 --- a/esbuild.config.mjs +++ b/esbuild.config.mjs @@ -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,