From e68afdd2ac5df7319f0efacc165d086914371c06 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 4 Nov 2026 17:29:08 +0100 Subject: [PATCH] fix: bound the Discord table conversion input and keep received files off the watcher (release review, #741) A table costs its widest cell times its rows, so one wide cell in an agent's last message built a 202 MB string on the event loop although only 8 posts can ever go out. Only what can be posted is converted now. A file a person sent from Discord also fired attachment:detected as if the agent had produced it. Co-Authored-By: Claude Opus 5.5 --- src/discord-bridge.ts | 12 +++++++++--- src/image-watcher.ts | 8 ++++---- src/web/paste-image-gc.ts | 6 ++++++ test/discord-bridge.test.ts | 12 ++++++++++++ test/image-watcher.test.ts | 1 + 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/discord-bridge.ts b/src/discord-bridge.ts index 56fd34c0..a8703371 100644 --- a/src/discord-bridge.ts +++ b/src/discord-bridge.ts @@ -25,6 +25,7 @@ import fs from 'node:fs/promises'; import { randomUUID } from 'node:crypto'; import { join } from 'node:path'; import { probePathKind } from './utils/bounded-path-probe.js'; +import { DISCORD_RECEIVED_DIR } from './web/paste-image-gc.js'; import WebSocket from 'ws'; import { WEBHOOK_SCOPES, type WebhookUrgency } from './types/push.js'; import { @@ -62,6 +63,12 @@ const DISCORD_EPOCH_MS = 1420070400000; /** A Markdown table wider or longer than this is posted as it was written, not padded into a code block. */ const TABLE_MAX_COLS = 24; const TABLE_MAX_ROWS = 200; +/** + * The most of a last message worth converting: what 8 posts can carry, twice over, since a table can + * shrink when formatted. The rest could never be posted, and a padded table costs its widest cell + * times its rows, so an agent-steerable message must not reach the conversion whole. + */ +const MAX_CONVERTED_TEXT = MAX_CONTENT * 8 * 2; /** Reading a CLI's transcript must not hold an alert back for long. */ const FULL_TEXT_TIMEOUT_MS = 4000; @@ -165,7 +172,7 @@ export const MAX_FILE_BYTES = 8 * 1024 * 1024; const MAX_FILES_PER_REPLY = 5; const DOWNLOAD_TIMEOUT_MS = 30_000; /** Where a session's received files go, relative to its working folder. */ -export const RECEIVED_DIR = '.codeman-discord'; +export const RECEIVED_DIR = DISCORD_RECEIVED_DIR; /** * What a session is told about the bridge. Claude gets it as `--append-system-prompt`; no other CLI @@ -827,7 +834,6 @@ export class DiscordBridge { private lastReply = new Map(); /** The last name seen per session, so only a real rename touches the thread. */ private names = new Map(); - /** Brand-new sessions that have not yet been sent the startup hint (non-Claude CLIs). */ private pending = new Map>(); /** Tabs closed while their thread was still being created: the creation applies `onSessionDelete` to it. */ private closedWhilePending = new Set(); @@ -1102,7 +1108,7 @@ export class DiscordBridge { const key = `${got?.version ?? ''}\u0000${text}`; msg = { ...msg, tail: undefined }; if (this.lastFull.get(sid) !== key) { - chunks = splitForDiscord(tablesToCodeBlocks(text)); + chunks = splitForDiscord(tablesToCodeBlocks(text.slice(0, MAX_CONVERTED_TEXT))); posted = key; } // else: said already, so only the header goes } diff --git a/src/image-watcher.ts b/src/image-watcher.ts index 8f8dc560..072a0ee3 100644 --- a/src/image-watcher.ts +++ b/src/image-watcher.ts @@ -14,7 +14,7 @@ import { basename, extname, relative } from 'node:path'; import { statSync } from 'node:fs'; import type { AttachmentDetectedEvent, AttachmentDetectedType, ImageDetectedEvent } from './types.js'; import { KeyedDebouncer } from './utils/index.js'; -import { UPLOAD_DIR_NAMES } from './web/paste-image-gc.js'; +import { DISCORD_RECEIVED_DIR, UPLOAD_DIR_NAMES } from './web/paste-image-gc.js'; // ========== Types ========== @@ -159,14 +159,14 @@ export class ImageWatcher extends EventEmitter { // Ignore common heavy directories for performance ignored: (path: string) => { // Skip node_modules, .git, and other heavy directories, and Codeman's - // own upload folders: a pdf the user handed to the agent is not a file - // the agent produced. + // own upload folders: a pdf the user handed to the agent, by upload or + // from Discord, is not a file the agent produced. if ( path.includes('/node_modules/') || path.includes('/.git/') || path.includes('/dist/') || path.includes('/.next/') || - UPLOAD_DIR_NAMES.some((name) => path.includes(`/${name}/`)) + [...UPLOAD_DIR_NAMES, DISCORD_RECEIVED_DIR].some((name) => path.includes(`/${name}/`)) ) { return true; } diff --git a/src/web/paste-image-gc.ts b/src/web/paste-image-gc.ts index 462fce35..8fef5806 100644 --- a/src/web/paste-image-gc.ts +++ b/src/web/paste-image-gc.ts @@ -38,6 +38,12 @@ export const UPLOADS_DIR = '.codeman-uploads'; export const LEGACY_UPLOADS_DIR = '.claude-images'; /** Every upload dir name, current first. Retiring the legacy one here retires it for every reader. */ export const UPLOAD_DIR_NAMES = [UPLOADS_DIR, LEGACY_UPLOADS_DIR]; +/** + * Where files a person sent from Discord land (discord-bridge.ts). Named here with the others so the + * image watcher can skip it without importing the bridge. Not an upload dir: neither the sweep nor a + * session delete touches it. + */ +export const DISCORD_RECEIVED_DIR = '.codeman-discord'; /** * Every directory a session's uploads sit in, current first. Both consumers diff --git a/test/discord-bridge.test.ts b/test/discord-bridge.test.ts index d42ce749..32db32d7 100644 --- a/test/discord-bridge.test.ts +++ b/test/discord-bridge.test.ts @@ -638,6 +638,18 @@ describe('DiscordBridge', () => { b.stop(); }); + it('converts only what can be posted, so one very wide table cell cannot stall the server', async () => { + const b = bridge(); + const wide = ['| a |', '|---|', `| ${'x'.repeat(1024 * 1024)} |`, ...Array(190).fill('| y |')].join('\n'); + const t0 = Date.now(); + await b.notify({ ...MSG, event: 'hook:idle_prompt', fullText: async () => ({ text: wide, version: 'turn-1' }) }); + expect(Date.now() - t0).toBeLessThan(500); + const said = posts.filter((p) => p.path.endsWith('/messages')).map((p) => String(p.body?.content)); + expect(said.length).toBeLessThanOrEqual(9); // the title post plus at most 8 parts + expect(said.at(-1)).toContain('continues in the tab'); + b.stop(); + }); + it('does not post the same message twice when two alerts for one tab overlap', async () => { let release!: () => void; const gate = new Promise((r) => (release = r)); diff --git a/test/image-watcher.test.ts b/test/image-watcher.test.ts index 267157c6..22a14305 100644 --- a/test/image-watcher.test.ts +++ b/test/image-watcher.test.ts @@ -97,6 +97,7 @@ describe('ImageWatcher', () => { watcher.watchSession('session-1', '/home/user/project'); const [, opts] = vi.mocked(watch).mock.calls.at(-1) as unknown as [string, { ignored: (p: string) => boolean }]; expect(opts.ignored('/home/user/project/.codeman-uploads/paste-1-ab.pdf')).toBe(true); + expect(opts.ignored('/home/user/project/.codeman-discord/1234-report.pdf')).toBe(true); expect(opts.ignored('/home/user/project/.claude-images/paste-1-ab.png')).toBe(true); expect(opts.ignored('/home/user/project/docs/report.pdf')).toBe(false); });