From cca1f5ce0a65793e28f7ab2a818decea6f530b27 Mon Sep 17 00:00:00 2001 From: Sam Xu Date: Fri, 26 Jun 2026 18:00:34 -0700 Subject: [PATCH] fix(agents): stop false-attach warning on legit post-attach narration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The false-attach guard appended a '⚠️ no [[upload:]] directive' system note to any agent message saying 'attached the deck' without a directive. Now that commonly_attach_file works, agents post the clean directive message AND a natural 'Done — attached X' follow-up — and the guard false-positived on that follow-up, surfacing a scary warning on every successful attach (smoke 2026-06-26). Suppress the warning when THIS agent uploaded a File to this pod within the last 5 minutes (the tool's upload lands the File row moments before the narration). Lookup failure falls back to the original behaviour; informational only, never blocks the message. Adds regression tests for suppress / warn / lookup-throws. Co-Authored-By: Claude Opus 4.8 --- ...ntMessageService.phantom-directive.test.js | 63 +++++++++++++++++++ backend/services/agentMessageService.ts | 42 ++++++++++++- 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/unit/services/agentMessageService.phantom-directive.test.js b/backend/__tests__/unit/services/agentMessageService.phantom-directive.test.js index 95884dc3..101f03e2 100644 --- a/backend/__tests__/unit/services/agentMessageService.phantom-directive.test.js +++ b/backend/__tests__/unit/services/agentMessageService.phantom-directive.test.js @@ -36,6 +36,7 @@ jest.mock('../../../models/Summary', () => ({ jest.mock('../../../services/agentIdentityService', () => ({ getOrCreateAgentUser: jest.fn(), ensureAgentInPod: jest.fn(), + buildAgentUsername: jest.fn((agentName, instanceId) => `${agentName}-${instanceId}`), })); jest.mock('../../../services/podAssetService', () => ({ createChatSummaryAsset: jest.fn(), @@ -60,6 +61,10 @@ jest.mock('../../../models/User', () => ({ select: jest.fn().mockReturnThis(), lean: jest.fn().mockResolvedValue([]), })), + findOne: jest.fn(() => ({ + select: jest.fn().mockReturnThis(), + lean: jest.fn().mockResolvedValue(null), + })), })); jest.mock('../../../models/Pod', () => ({ findById: jest.fn(() => ({ @@ -69,6 +74,10 @@ jest.mock('../../../models/Pod', () => ({ })); jest.mock('../../../models/File', () => ({ find: jest.fn(), + findOne: jest.fn(() => ({ + select: jest.fn().mockReturnThis(), + lean: jest.fn().mockResolvedValue(null), + })), })); // Configure File.find to return the rows whose fileName/originalName are @@ -242,3 +251,57 @@ describe('AgentMessageService phantom-upload-directive footer', () => { expect(result).toBeTruthy(); }); }); + +describe('AgentMessageService false-attach footer (recent-attach suppression)', () => { + const FALSE_ATTACH_NOTE = 'this message claims an attachment but no'; + + it('appends the false-attach footer when the agent did NOT recently attach a file', async () => { + // Default mocks: User.findOne → null, File.findOne → null → no recent attach. + await AgentMessageService.postMessage({ + agentName: 'openclaw', + instanceId: 'pixel', + podId: '6a0da39bae757028b39f87a6', + content: 'Done — attached the launch deck here.', + }); + + expect(persistedDoc.content).toContain(FALSE_ATTACH_NOTE); + }); + + it('suppresses the footer when THIS agent uploaded a file to the pod moments ago', async () => { + // The agent's `commonly_attach_file` call already landed a File row; the + // follow-up narration ("Done — attached…") legitimately has no directive. + User.findOne.mockReturnValueOnce({ + select: jest.fn().mockReturnThis(), + lean: jest.fn().mockResolvedValue({ _id: 'bot-user-1' }), + }); + File.findOne.mockReturnValueOnce({ + select: jest.fn().mockReturnThis(), + lean: jest.fn().mockResolvedValue({ _id: 'recent-file-1' }), + }); + + await AgentMessageService.postMessage({ + agentName: 'openclaw', + instanceId: 'pixel', + podId: '6a0da39bae757028b39f87a6', + content: 'Done — attached the launch deck here.', + }); + + expect(persistedDoc.content).not.toContain(FALSE_ATTACH_NOTE); + expect(persistedDoc.content).toContain('Done — attached the launch deck here.'); + }); + + it('still appends the footer when the recent-attach lookup throws', async () => { + User.findOne.mockImplementationOnce(() => { + throw new Error('mongo connection lost'); + }); + + await AgentMessageService.postMessage({ + agentName: 'openclaw', + instanceId: 'pixel', + podId: '6a0da39bae757028b39f87a6', + content: 'Done — attached the report file here.', + }); + + expect(persistedDoc.content).toContain(FALSE_ATTACH_NOTE); + }); +}); diff --git a/backend/services/agentMessageService.ts b/backend/services/agentMessageService.ts index 3d3740b0..9e65ae97 100644 --- a/backend/services/agentMessageService.ts +++ b/backend/services/agentMessageService.ts @@ -40,6 +40,13 @@ const FILE_DIRECTIVE_SCAN_LIMIT = (() => { return Number.isFinite(parsed) && parsed > 0 ? Math.floor(parsed) : 500; })(); +// How recently THIS agent must have uploaded a file to this pod for a +// "Done — attached the deck" narration (which carries no `[[upload:]]` +// directive of its own) to be treated as a legitimate post-attach follow-up +// rather than a false-attach claim. The tool posts the directive message and +// the narration within the same turn, so a few minutes is ample. +const RECENT_ATTACH_WINDOW_MS = 5 * 60 * 1000; + let PGMessage: unknown = null; try { // eslint-disable-next-line global-require @@ -814,8 +821,39 @@ class AgentMessageService { const hasUploadDirective = /\[\[upload:/i.test(sanitizedContent); const claimsAttachment = /(?:\b(?:i(?:'ve| have)?|done\s*[—-]?\s*i|i\s+just|i\s+already)\s+|(?:^|[.!?]\s+|[\r\n]+)(?:done\s*[—-]?\s*)?(?:just\s+)?)(?:attached|uploaded|posted)\b[^.\n]{0,80}\b(?:file|deck|attachment|runbook|pptx|docx|xlsx|pdf|csv|image|artifact)\b/i.test(sanitizedContent); if (claimsAttachment && !hasUploadDirective) { - sanitizedContent += '\n\n⚠️ _(system note: this message claims an attachment but no `[[upload:...]]` directive is in the body. The agent may not have actually called `commonly_attach_file`. Check the agent\'s workspace for the file and re-attach if needed.)_'; - console.warn(`[agent-msg] false-attach-claim from agent=${agentName} instance=${instanceId} pod=${podId} — appended system note`); + // Suppress the warning when THIS agent genuinely attached a file to + // this pod moments ago. `commonly_attach_file` posts the clean + // directive message AND the agent then posts a natural-language + // "Done — attached the deck" follow-up; that follow-up legitimately + // has no `[[upload:]]` directive of its own, but the attachment + // really happened. Without this check the guard false-positives on + // every successful tool-driven attach (smoke 2026-06-26). We look + // for a File row this agent uploaded to this pod inside a short + // window — informational-only, so a miss just restores the old + // (over-eager) behaviour rather than blocking the message. + let recentlyAttached = false; + try { + if (podId && mongoose.Types.ObjectId.isValid(podId)) { + const botUsername = AgentIdentityService.buildAgentUsername(agentName, instanceId); + const botUser = await User.findOne({ username: botUsername }).select('_id').lean(); + if (botUser && botUser._id) { + const since = new Date(Date.now() - RECENT_ATTACH_WINDOW_MS); + const recentFile = await File.findOne({ + podId: new mongoose.Types.ObjectId(String(podId)), + uploadedBy: botUser._id, + createdAt: { $gte: since }, + }).select('_id').lean(); + recentlyAttached = !!recentFile; + } + } + } catch (recentAttachErr) { + // Lookup failure → fall back to the original warning behaviour. + console.warn(`[agent-msg] recent-attach lookup failed: ${(recentAttachErr as Error).message}`); + } + if (!recentlyAttached) { + sanitizedContent += '\n\n⚠️ _(system note: this message claims an attachment but no `[[upload:...]]` directive is in the body. The agent may not have actually called `commonly_attach_file`. Check the agent\'s workspace for the file and re-attach if needed.)_'; + console.warn(`[agent-msg] false-attach-claim from agent=${agentName} instance=${instanceId} pod=${podId} — appended system note`); + } } // Round 2 of the false-attach defence (smoke 2026-05-20): the previous