From 9e03fc52072693f4511df148f4c5d6d6d6a27571 Mon Sep 17 00:00:00 2001 From: Nick Tabeling Date: Mon, 13 Apr 2026 09:58:57 +0200 Subject: [PATCH 1/4] =?UTF-8?q?feat(DIS-154):=20discord=20rich=20features?= =?UTF-8?q?=20=E2=80=94=20reactions,=20file=20responses,=20attachment=20in?= =?UTF-8?q?box?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Status reactions on messages: 👀 on enqueue, ✅ on success, ❌ on any error - Typing interval raised from 5s to 8s - src/discord/send-response.ts: text >4000 chars sent as response.md attachment - src/discord/attachment-inbox.ts: downloads Discord attachments into .disclaw-inbox/, cleanupInbox() removes files older than 24h - src/router.ts: input limit guard at 4000 chars (⚠️ reaction + hint message), processAttachments hints injected into prompt - Unit tests: send-response.test.ts, attachment-inbox.test.ts (107 tests, all green) Co-Authored-By: Claude Sonnet 4.6 --- src/discord/attachment-inbox.ts | 90 +++++++++++++++++ src/discord/send-response.ts | 39 ++++++++ src/router.ts | 67 +++++++++++-- tests/unit/attachment-inbox.test.ts | 144 ++++++++++++++++++++++++++++ tests/unit/send-response.test.ts | 70 ++++++++++++++ 5 files changed, 401 insertions(+), 9 deletions(-) create mode 100644 src/discord/attachment-inbox.ts create mode 100644 src/discord/send-response.ts create mode 100644 tests/unit/attachment-inbox.test.ts create mode 100644 tests/unit/send-response.test.ts diff --git a/src/discord/attachment-inbox.ts b/src/discord/attachment-inbox.ts new file mode 100644 index 0000000..4c423e3 --- /dev/null +++ b/src/discord/attachment-inbox.ts @@ -0,0 +1,90 @@ +import { Message } from "discord.js"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { childLogger } from "../runtime/logger.js"; + +const log = childLogger("attachment-inbox"); + +const INBOX_DIR = ".disclaw-inbox"; +const MAX_AGE_MS = 24 * 60 * 60 * 1000; // 24 hours + +/** + * Saves attachments from a Discord message into /.disclaw-inbox/ + * Returns a list of saved file hints to inject into the prompt context. + */ +export async function processAttachments( + message: Message, + workspacePath: string +): Promise { + if (message.attachments.size === 0) { + return []; + } + + const inboxPath = path.join(workspacePath, INBOX_DIR); + if (!fs.existsSync(inboxPath)) { + fs.mkdirSync(inboxPath, { recursive: true }); + log.debug({ inboxPath }, "Created inbox directory"); + } + + const hints: string[] = []; + const timestamp = Date.now(); + + for (const attachment of message.attachments.values()) { + const safeName = path.basename(attachment.name ?? "file"); + const fileName = `${timestamp}-${safeName}`; + const filePath = path.join(inboxPath, fileName); + + try { + const response = await fetch(attachment.url); + if (!response.ok) { + log.warn( + { url: attachment.url, status: response.status }, + "Failed to download attachment" + ); + continue; + } + const buffer = Buffer.from(await response.arrayBuffer()); + fs.writeFileSync(filePath, buffer); + log.debug({ fileName }, "Saved attachment"); + hints.push(`[Datei verfügbar: ${INBOX_DIR}/${fileName}]`); + } catch (err) { + log.warn({ err, fileName }, "Error saving attachment"); + } + } + + return hints; +} + +/** + * Deletes files in /.disclaw-inbox/ older than 24 hours. + */ +export function cleanupInbox(workspacePath: string): void { + const inboxPath = path.join(workspacePath, INBOX_DIR); + + if (!fs.existsSync(inboxPath)) { + return; + } + + const now = Date.now(); + let entries: string[]; + + try { + entries = fs.readdirSync(inboxPath); + } catch (err) { + log.warn({ err, inboxPath }, "Failed to read inbox directory for cleanup"); + return; + } + + for (const entry of entries) { + const filePath = path.join(inboxPath, entry); + try { + const stat = fs.statSync(filePath); + if (now - stat.mtimeMs > MAX_AGE_MS) { + fs.unlinkSync(filePath); + log.debug({ filePath }, "Deleted stale inbox file"); + } + } catch (err) { + log.warn({ err, filePath }, "Failed to stat/delete inbox file"); + } + } +} diff --git a/src/discord/send-response.ts b/src/discord/send-response.ts new file mode 100644 index 0000000..adf83fd --- /dev/null +++ b/src/discord/send-response.ts @@ -0,0 +1,39 @@ +import { TextChannel, Message, AttachmentBuilder } from "discord.js"; +import { splitForDiscord } from "./split.js"; +import { childLogger } from "../runtime/logger.js"; + +const log = childLogger("send-response"); + +const FILE_THRESHOLD = 4000; + +/** + * Sends an agent response to a Discord channel. + * - Text > 4000 chars is sent as a Markdown file attachment. + * - Text <= 4000 chars is split with splitForDiscord and sent as plain messages. + */ +export async function sendResponse( + channel: TextChannel, + text: string, + _triggerMessage: Message +): Promise { + if (text.length === 0) { + return; + } + + if (text.length > FILE_THRESHOLD) { + log.debug({ length: text.length }, "Response exceeds threshold — sending as file"); + await channel.send({ + files: [ + new AttachmentBuilder(Buffer.from(text, "utf-8"), { + name: "response.md", + }), + ], + }); + return; + } + + const chunks = splitForDiscord(text); + for (const chunk of chunks) { + await channel.send(chunk); + } +} diff --git a/src/router.ts b/src/router.ts index 475a7f9..062a432 100644 --- a/src/router.ts +++ b/src/router.ts @@ -3,11 +3,14 @@ import { Message, TextChannel } from "discord.js"; import { DisclawDatabase } from "./db/database"; import { DisclawConfig } from "./config/loader"; import { runAgent } from "./agent/runner"; -import { splitForDiscord } from "./discord/split"; import { sanitizeForDiscord } from "./runtime/sanitize"; import { ChannelQueue } from "./runtime/channel-queue"; +import { sendResponse } from "./discord/send-response"; +import { processAttachments } from "./discord/attachment-inbox"; import type { RunResult } from "./agent/types"; +const INPUT_LIMIT = 4000; + const channelQueue = new ChannelQueue(); /** @@ -33,6 +36,21 @@ export async function routeMessage( } const channel = message.channel as TextChannel; + + // Input-length guard — refuse oversized messages before queuing + if (message.content.length > INPUT_LIMIT) { + message.react?.("⚠️").catch(() => {}); + await channel + .send( + `Nachricht zu lang (${message.content.length} Zeichen). Maximum ist ${INPUT_LIMIT} Zeichen.` + ) + .catch(() => {}); + return true; + } + + // Acknowledge receipt immediately + message.react?.("👀").catch(() => {}); + const sanitizeOpts = { repoRoot: workspace.workspace_path, home: os.homedir(), @@ -41,16 +59,25 @@ export async function routeMessage( await channelQueue.enqueue(channelId, async () => { const typingInterval = setInterval(() => { channel.sendTyping().catch(() => {}); - }, 5000); + }, 8000); await channel.sendTyping().catch(() => {}); try { const history = db.getConversationHistory(workspace.id, 30); + // Process any file attachments and build hint lines for the prompt + const attachmentHints = message.attachments + ? await processAttachments(message, workspace.workspace_path) + : []; + let userMessage = message.content; + if (attachmentHints.length > 0) { + userMessage = userMessage + "\n\n" + attachmentHints.join("\n"); + } + const result: RunResult = await runAgent({ workspacePath: workspace.workspace_path, channelName: channel.name, - userMessage: message.content, + userMessage, conversationHistory: history, }); @@ -64,14 +91,30 @@ export async function routeMessage( ); const sanitizedResponse = sanitizeForDiscord(result.text, sanitizeOpts); - const chunks = splitForDiscord(sanitizedResponse); + let firstMsgId: string | null = null; - for (const chunk of chunks) { - const sent = await channel.send(chunk); - if (!firstMsgId) { - firstMsgId = sent.id; - } + // Use sendResponse which handles the file-vs-chunks decision + // We need to capture the first message id for DB storage. + // sendResponse sends all chunks; we wrap with a one-shot intercept. + const originalSend = channel.send.bind(channel); + const sentIds: string[] = []; + const patchedSend = async (...args: Parameters) => { + const sent = await originalSend(...(args as [Parameters[0]])); + sentIds.push(sent.id); + return sent; + }; + + // Temporarily patch channel.send for sendResponse + (channel as unknown as { send: typeof patchedSend }).send = patchedSend; + try { + await sendResponse(channel, sanitizedResponse, message); + } finally { + (channel as unknown as { send: typeof originalSend }).send = originalSend; + } + + if (sentIds.length > 0) { + firstMsgId = sentIds[0]; } if (firstMsgId) { @@ -83,20 +126,26 @@ export async function routeMessage( null ); } + + message.react?.("✅").catch(() => {}); } else if (result.status === "timeout") { + message.react?.("❌").catch(() => {}); await channel .send(`⏱️ Agent-Timeout nach ${result.timeoutMs / 1000}s`) .catch(() => {}); } else if (result.status === "cli-error") { + message.react?.("❌").catch(() => {}); await channel .send(sanitizeForDiscord(`❌ CLI-Fehler: ${result.message}`, sanitizeOpts)) .catch(() => {}); } else if (result.status === "parse-error") { + message.react?.("❌").catch(() => {}); await channel .send(`⚠️ Antwort konnte nicht gelesen werden`) .catch(() => {}); } } catch (error) { + message.react?.("❌").catch(() => {}); const msg = error instanceof Error ? error.message : "Unbekannter Fehler"; await channel .send(sanitizeForDiscord(`Fehler bei der Verarbeitung: ${msg}`, sanitizeOpts)) diff --git a/tests/unit/attachment-inbox.test.ts b/tests/unit/attachment-inbox.test.ts new file mode 100644 index 0000000..6d4bed2 --- /dev/null +++ b/tests/unit/attachment-inbox.test.ts @@ -0,0 +1,144 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import * as os from "node:os"; +import type { Message, Collection, Attachment } from "discord.js"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeMessage(attachments: Attachment[] = []): Message { + const col = new Map(attachments.map((a) => [a.id, a])); + return { + attachments: { + size: attachments.length, + values: () => col.values(), + }, + } as unknown as Message; +} + +function makeAttachment(id: string, name: string, url: string): Attachment { + return { id, name, url } as unknown as Attachment; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("processAttachments", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "disclaw-inbox-test-")); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it("returns an empty array when message has no attachments", async () => { + const { processAttachments } = await import("../../src/discord/attachment-inbox"); + const message = makeMessage([]); + const hints = await processAttachments(message, tmpDir); + expect(hints).toEqual([]); + // No inbox directory created + expect(fs.existsSync(path.join(tmpDir, ".disclaw-inbox"))).toBe(false); + }); + + it("saves an attachment to .disclaw-inbox/ and returns a hint", async () => { + // Mock global fetch + const fileContent = "print('hello')\n"; + const contentBytes = Buffer.from(fileContent, "utf-8"); + // Allocate a clean ArrayBuffer (not the Node shared pool) so Buffer.from() round-trips correctly + const cleanArrayBuffer = contentBytes.buffer.slice( + contentBytes.byteOffset, + contentBytes.byteOffset + contentBytes.byteLength + ); + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ + ok: true, + arrayBuffer: async () => cleanArrayBuffer, + }) + ); + + const { processAttachments } = await import("../../src/discord/attachment-inbox"); + const attachment = makeAttachment("att-1", "example.py", "https://cdn.discord.com/example.py"); + const message = makeMessage([attachment]); + + const hints = await processAttachments(message, tmpDir); + + expect(hints).toHaveLength(1); + expect(hints[0]).toMatch(/^\[Datei verfügbar: \.disclaw-inbox\//); + expect(hints[0]).toContain("example.py"); + + // File must exist in inbox + const inboxDir = path.join(tmpDir, ".disclaw-inbox"); + const files = fs.readdirSync(inboxDir); + expect(files).toHaveLength(1); + expect(files[0]).toMatch(/example\.py$/); + + // File content must match + const savedContent = fs.readFileSync(path.join(inboxDir, files[0]), "utf-8"); + expect(savedContent).toBe(fileContent); + }); + + it("skips an attachment when fetch fails", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ + ok: false, + status: 403, + }) + ); + + const { processAttachments } = await import("../../src/discord/attachment-inbox"); + const attachment = makeAttachment("att-1", "secret.txt", "https://cdn.discord.com/secret.txt"); + const message = makeMessage([attachment]); + + const hints = await processAttachments(message, tmpDir); + expect(hints).toEqual([]); + }); +}); + +describe("cleanupInbox", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "disclaw-cleanup-test-")); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it("does nothing when inbox directory does not exist", async () => { + const { cleanupInbox } = await import("../../src/discord/attachment-inbox"); + expect(() => cleanupInbox(tmpDir)).not.toThrow(); + }); + + it("deletes files older than 24 hours and keeps newer files", async () => { + const { cleanupInbox } = await import("../../src/discord/attachment-inbox"); + + const inboxDir = path.join(tmpDir, ".disclaw-inbox"); + fs.mkdirSync(inboxDir); + + const oldFile = path.join(inboxDir, "old-file.txt"); + const newFile = path.join(inboxDir, "new-file.txt"); + + fs.writeFileSync(oldFile, "old"); + fs.writeFileSync(newFile, "new"); + + // Backdate old file by 25 hours + const oldTime = new Date(Date.now() - 25 * 60 * 60 * 1000); + fs.utimesSync(oldFile, oldTime, oldTime); + + cleanupInbox(tmpDir); + + expect(fs.existsSync(oldFile)).toBe(false); + expect(fs.existsSync(newFile)).toBe(true); + }); +}); diff --git a/tests/unit/send-response.test.ts b/tests/unit/send-response.test.ts new file mode 100644 index 0000000..a5aa9f2 --- /dev/null +++ b/tests/unit/send-response.test.ts @@ -0,0 +1,70 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { sendResponse } from "../../src/discord/send-response"; +import type { TextChannel, Message } from "discord.js"; + +// Minimal stubs +function makeChannel() { + return { + send: vi.fn().mockResolvedValue({ id: "msg-1" }), + } as unknown as TextChannel; +} + +function makeMessage() { + return {} as Message; +} + +describe("sendResponse", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("sends chunks via channel.send() for text <= 4000 chars", async () => { + const channel = makeChannel(); + const text = "Hello, world!"; + await sendResponse(channel, text, makeMessage()); + + expect(channel.send).toHaveBeenCalledTimes(1); + expect(channel.send).toHaveBeenCalledWith(text); + // No file attachment + const call = vi.mocked(channel.send).mock.calls[0][0]; + expect(typeof call).toBe("string"); + }); + + it("sends multiple chunks for text <= 4000 chars that needs splitting", async () => { + const channel = makeChannel(); + // Build a text that exceeds the 1900 default split limit but stays <= 4000 + const line = "a".repeat(100); + const text = Array.from({ length: 30 }, () => line).join("\n"); + // 30*100 + 29 newlines = 3029 chars — within 4000 but split into chunks + expect(text.length).toBeLessThanOrEqual(4000); + + await sendResponse(channel, text, makeMessage()); + + expect(channel.send).toHaveBeenCalledTimes( + (channel.send as ReturnType).mock.calls.length + ); + // All calls must be strings, not objects with files + for (const [arg] of vi.mocked(channel.send).mock.calls) { + expect(typeof arg).toBe("string"); + } + }); + + it("sends an attachment for text > 4000 chars", async () => { + const channel = makeChannel(); + const text = "x".repeat(4001); + + await sendResponse(channel, text, makeMessage()); + + expect(channel.send).toHaveBeenCalledTimes(1); + const call = vi.mocked(channel.send).mock.calls[0][0]; + expect(typeof call).toBe("object"); + expect((call as { files: unknown[] }).files).toBeDefined(); + expect((call as { files: unknown[] }).files).toHaveLength(1); + }); + + it("does not call channel.send() for empty text", async () => { + const channel = makeChannel(); + await sendResponse(channel, "", makeMessage()); + expect(channel.send).not.toHaveBeenCalled(); + }); +}); -- 2.45.2 From 3070406d0fb4cd67df58572f778bc9222662a537 Mon Sep 17 00:00:00 2001 From: Nick Tabeling Date: Mon, 13 Apr 2026 10:04:53 +0200 Subject: [PATCH 2/4] update task --- tasks/DIS-154.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/DIS-154.md b/tasks/DIS-154.md index dcbb515..bf088de 100644 --- a/tasks/DIS-154.md +++ b/tasks/DIS-154.md @@ -1,12 +1,12 @@ --- id: DIS-154 -status: ready +status: in-progress phase: 1.5 priority: p1 labels: [phase:1.5, type:feat, priority:p1] branch: refinement/discord-rich-features -assignee: null -started: null +assignee: developer-agent +started: 2026-04-13 pr: null merged: null --- -- 2.45.2 From 00f7909e6be30d8154afa46c586056f43ddf4197 Mon Sep 17 00:00:00 2001 From: Nick Tabeling Date: Mon, 13 Apr 2026 10:16:50 +0200 Subject: [PATCH 3/4] feat(DIS-154): agent file sending via [ATTACH: path] convention - parseAttachments() extracts [ATTACH: path] markers from response text - validateAttachPath() enforces workspace boundary + readability + 8MB limit - sendResponse() sends validated files as Discord attachments before text - CLAUDE.md template teaches agents the [ATTACH: ...] syntax - Path-traversal guard: only files within workspace are allowed Co-Authored-By: Claude Sonnet 4.6 --- src/agent/identity.ts | 19 ++++ src/discord/send-response.ts | 101 ++++++++++++++++++-- src/router.ts | 2 +- tasks/DIS-154.md | 24 +++++ tests/unit/send-response.test.ts | 153 ++++++++++++++++++++++++------- 5 files changed, 260 insertions(+), 39 deletions(-) diff --git a/src/agent/identity.ts b/src/agent/identity.ts index ace8f4f..a557af0 100644 --- a/src/agent/identity.ts +++ b/src/agent/identity.ts @@ -108,6 +108,25 @@ ${role} - Be concise in your responses -- they are displayed in Discord (2000 char limit per message) - If a task is outside your area of expertise, say so clearly +## Sending Files to Discord + +To send a file from your workspace back to the user in Discord, include this marker anywhere in your response: + +\`\`\` +[ATTACH: path/to/file.jpg] +\`\`\` + +- The path can be relative to your workspace root or absolute +- Only files within your workspace directory are allowed +- Maximum file size: 8 MB +- Multiple files are supported — one marker per file +- The marker is removed from the text before display; only the file is sent + +Example: If a user asks "send me the image", respond with: +\`[ATTACH: images/result.png]\` + +Do **not** use this for files outside your workspace or for sensitive files. + ## Workspace Contents Add project-specific context below. Describe the files, conventions, diff --git a/src/discord/send-response.ts b/src/discord/send-response.ts index adf83fd..e6937ac 100644 --- a/src/discord/send-response.ts +++ b/src/discord/send-response.ts @@ -1,3 +1,5 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; import { TextChannel, Message, AttachmentBuilder } from "discord.js"; import { splitForDiscord } from "./split.js"; import { childLogger } from "../runtime/logger.js"; @@ -5,26 +7,113 @@ import { childLogger } from "../runtime/logger.js"; const log = childLogger("send-response"); const FILE_THRESHOLD = 4000; +const MAX_ATTACH_BYTES = 8 * 1024 * 1024; // 8 MB — Discord limit for regular bots + +const ATTACH_PATTERN = /\[ATTACH:\s*([^\]]+)\]/g; + +export interface ParsedResponse { + text: string; + attachPaths: string[]; +} + +/** + * Extracts [ATTACH: path] markers from agent response text. + * Returns cleaned text and a list of resolved absolute paths. + * Paths are resolved relative to workspacePath if not absolute. + */ +export function parseAttachments(raw: string, workspacePath: string): ParsedResponse { + const attachPaths: string[] = []; + const text = raw.replace(ATTACH_PATTERN, (_match, rawPath: string) => { + const trimmed = rawPath.trim(); + const resolved = path.isAbsolute(trimmed) + ? trimmed + : path.resolve(workspacePath, trimmed); + attachPaths.push(resolved); + return ""; + }).trim(); + return { text, attachPaths }; +} + +/** + * Validates an attach path: must exist, be readable, be within workspacePath, + * and be no larger than MAX_ATTACH_BYTES. + * Returns an error string if invalid, null if OK. + */ +export function validateAttachPath(filePath: string, workspacePath: string): string | null { + const workspaceRoot = path.resolve(workspacePath); + const resolved = path.resolve(filePath); + + // Path-traversal guard + if (!resolved.startsWith(workspaceRoot + path.sep) && resolved !== workspaceRoot) { + return `Datei liegt außerhalb des Workspace: \`${path.basename(resolved)}\``; + } + + // Existence + read permission + try { + fs.accessSync(resolved, fs.constants.R_OK); + } catch { + return `Datei nicht lesbar oder nicht vorhanden: \`${path.basename(resolved)}\``; + } + + // Size check + const stat = fs.statSync(resolved); + if (stat.size > MAX_ATTACH_BYTES) { + const mb = (stat.size / (1024 * 1024)).toFixed(1); + return `Datei zu groß (${mb} MB, Maximum 8 MB): \`${path.basename(resolved)}\``; + } + + return null; +} /** * Sends an agent response to a Discord channel. - * - Text > 4000 chars is sent as a Markdown file attachment. + * - Parses [ATTACH: path] markers and sends valid files as attachments. + * - Text > 4000 chars (after stripping markers) is sent as a Markdown file attachment. * - Text <= 4000 chars is split with splitForDiscord and sent as plain messages. */ export async function sendResponse( channel: TextChannel, text: string, - _triggerMessage: Message + _triggerMessage: Message, + workspacePath?: string ): Promise { if (text.length === 0) { return; } - if (text.length > FILE_THRESHOLD) { - log.debug({ length: text.length }, "Response exceeds threshold — sending as file"); + // Parse [ATTACH: ...] markers if we have a workspace context + let cleanText = text; + let attachPaths: string[] = []; + if (workspacePath) { + const parsed = parseAttachments(text, workspacePath); + cleanText = parsed.text; + attachPaths = parsed.attachPaths; + } + + // Send file attachments + for (const filePath of attachPaths) { + const err = validateAttachPath(filePath, workspacePath!); + if (err) { + log.warn({ filePath }, "Attachment rejected: " + err); + await channel.send(`⚠️ ${err}`).catch(() => {}); + continue; + } + log.debug({ filePath }, "Sending file attachment"); + await channel.send({ + files: [new AttachmentBuilder(filePath, { name: path.basename(filePath) })], + }); + } + + // Send text portion + if (cleanText.length === 0) { + return; + } + + if (cleanText.length > FILE_THRESHOLD) { + log.debug({ length: cleanText.length }, "Response exceeds threshold — sending as file"); await channel.send({ files: [ - new AttachmentBuilder(Buffer.from(text, "utf-8"), { + new AttachmentBuilder(Buffer.from(cleanText, "utf-8"), { name: "response.md", }), ], @@ -32,7 +121,7 @@ export async function sendResponse( return; } - const chunks = splitForDiscord(text); + const chunks = splitForDiscord(cleanText); for (const chunk of chunks) { await channel.send(chunk); } diff --git a/src/router.ts b/src/router.ts index 062a432..bd63d6b 100644 --- a/src/router.ts +++ b/src/router.ts @@ -108,7 +108,7 @@ export async function routeMessage( // Temporarily patch channel.send for sendResponse (channel as unknown as { send: typeof patchedSend }).send = patchedSend; try { - await sendResponse(channel, sanitizedResponse, message); + await sendResponse(channel, sanitizedResponse, message, workspace.workspace_path); } finally { (channel as unknown as { send: typeof originalSend }).send = originalSend; } diff --git a/tasks/DIS-154.md b/tasks/DIS-154.md index bf088de..71f4355 100644 --- a/tasks/DIS-154.md +++ b/tasks/DIS-154.md @@ -70,6 +70,30 @@ Aktuell senden Agents nur Plaintext. Discord bietet viel mehr: ## Abhängigkeiten Phase 1 abgeschlossen (splitForDiscord, RunResult). +### 6. Agent sendet Dateien zurück (`src/discord/send-response.ts`) +- Agent schreibt `[ATTACH: pfad/zur/datei.ext]` in seine Antwort +- Bot parst alle `[ATTACH: ...]`-Marker heraus, entfernt sie aus dem Text +- Sicherheitscheck vor dem Senden: + - Datei muss existieren und lesbar sein (`fs.accessSync(path, fs.constants.R_OK)`) + - Pfad darf nicht außerhalb des Workspace liegen (Path-Traversal-Schutz via `path.resolve`) + - Max. Dateigröße: **8 MB** (Discord-Limit für reguläre Bots) +- Erlaubte Pfade: relativ zum Workspace-Root oder absolut innerhalb des Workspace +- Dateien werden als Discord-Attachments gesendet, verbleibender Text normal +- CLAUDE.md-Template informiert den Agent über die `[ATTACH: ...]`-Konvention + +## Definition of Done +- [ ] Reactions `👀` / `✅` / `❌` werden korrekt gesetzt +- [ ] Typing-Intervall 8s, endet mit Agent-Antwort +- [ ] Antworten > 4000 Zeichen kommen als `.md`-Attachment +- [ ] Eingehende Dateien landen im `.disclaw-inbox/`-Verzeichnis +- [ ] Input > 4000 Zeichen → `⚠️` + Hinweis, kein Agent-Aufruf +- [ ] Agent kann mit `[ATTACH: pfad]` Dateien zurücksenden +- [ ] Path-Traversal-Schutz: nur Dateien im Workspace erlaubt +- [ ] Dateien > 8 MB → Fehlermeldung an Channel statt Absturz +- [ ] CLAUDE.md-Template enthält Hinweis auf `[ATTACH: ...]` +- [ ] Unit-Tests: `send-response`, `attachment-inbox` +- [ ] `npm run build && npm test` grün + ## Hinweis Reactions, Embeds, Threads und Polls (voller SF-004-Scope) können in Folge-Issues ausgebaut werden. Dieses Issue implementiert den Kern-QoL-Teil. diff --git a/tests/unit/send-response.test.ts b/tests/unit/send-response.test.ts index a5aa9f2..5508016 100644 --- a/tests/unit/send-response.test.ts +++ b/tests/unit/send-response.test.ts @@ -1,8 +1,10 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { sendResponse } from "../../src/discord/send-response"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { sendResponse, parseAttachments, validateAttachPath } from "../../src/discord/send-response"; import type { TextChannel, Message } from "discord.js"; -// Minimal stubs function makeChannel() { return { send: vi.fn().mockResolvedValue({ id: "msg-1" }), @@ -13,6 +15,69 @@ function makeMessage() { return {} as Message; } +describe("parseAttachments", () => { + it("returns empty attachPaths and original text when no markers", () => { + const result = parseAttachments("Hello world", "/workspace"); + expect(result.text).toBe("Hello world"); + expect(result.attachPaths).toHaveLength(0); + }); + + it("extracts a single relative [ATTACH: ...] marker", () => { + const result = parseAttachments("Here is the file [ATTACH: images/cat.jpg]", "/workspace"); + expect(result.text).toBe("Here is the file"); + expect(result.attachPaths).toHaveLength(1); + expect(result.attachPaths[0]).toBe(path.resolve("/workspace", "images/cat.jpg")); + }); + + it("extracts an absolute [ATTACH: ...] marker unchanged", () => { + const abs = path.resolve("/workspace/file.txt"); + const result = parseAttachments(`See [ATTACH: ${abs}]`, "/workspace"); + expect(result.attachPaths[0]).toBe(abs); + }); + + it("extracts multiple markers", () => { + const result = parseAttachments( + "Files: [ATTACH: a.txt] and [ATTACH: b.png]", + "/workspace" + ); + expect(result.attachPaths).toHaveLength(2); + expect(result.text).toBe("Files: and"); + }); +}); + +describe("validateAttachPath", () => { + let tmpDir: string; + let tmpFile: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "disclaw-test-")); + tmpFile = path.join(tmpDir, "file.txt"); + fs.writeFileSync(tmpFile, "hello"); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it("returns null for a valid readable file within workspace", () => { + expect(validateAttachPath(tmpFile, tmpDir)).toBeNull(); + }); + + it("rejects paths outside workspace (path traversal)", () => { + const outsideFile = path.join(os.tmpdir(), "outside.txt"); + fs.writeFileSync(outsideFile, "secret"); + const err = validateAttachPath(outsideFile, tmpDir); + expect(err).toMatch(/außerhalb/); + fs.unlinkSync(outsideFile); + }); + + it("rejects non-existent files", () => { + const missing = path.join(tmpDir, "ghost.txt"); + const err = validateAttachPath(missing, tmpDir); + expect(err).toMatch(/nicht lesbar/); + }); +}); + describe("sendResponse", () => { beforeEach(() => { vi.clearAllMocks(); @@ -20,45 +85,18 @@ describe("sendResponse", () => { it("sends chunks via channel.send() for text <= 4000 chars", async () => { const channel = makeChannel(); - const text = "Hello, world!"; - await sendResponse(channel, text, makeMessage()); - + await sendResponse(channel, "Hello, world!", makeMessage()); expect(channel.send).toHaveBeenCalledTimes(1); - expect(channel.send).toHaveBeenCalledWith(text); - // No file attachment const call = vi.mocked(channel.send).mock.calls[0][0]; expect(typeof call).toBe("string"); }); - it("sends multiple chunks for text <= 4000 chars that needs splitting", async () => { - const channel = makeChannel(); - // Build a text that exceeds the 1900 default split limit but stays <= 4000 - const line = "a".repeat(100); - const text = Array.from({ length: 30 }, () => line).join("\n"); - // 30*100 + 29 newlines = 3029 chars — within 4000 but split into chunks - expect(text.length).toBeLessThanOrEqual(4000); - - await sendResponse(channel, text, makeMessage()); - - expect(channel.send).toHaveBeenCalledTimes( - (channel.send as ReturnType).mock.calls.length - ); - // All calls must be strings, not objects with files - for (const [arg] of vi.mocked(channel.send).mock.calls) { - expect(typeof arg).toBe("string"); - } - }); - it("sends an attachment for text > 4000 chars", async () => { const channel = makeChannel(); - const text = "x".repeat(4001); - - await sendResponse(channel, text, makeMessage()); - + await sendResponse(channel, "x".repeat(4001), makeMessage()); expect(channel.send).toHaveBeenCalledTimes(1); const call = vi.mocked(channel.send).mock.calls[0][0]; expect(typeof call).toBe("object"); - expect((call as { files: unknown[] }).files).toBeDefined(); expect((call as { files: unknown[] }).files).toHaveLength(1); }); @@ -67,4 +105,55 @@ describe("sendResponse", () => { await sendResponse(channel, "", makeMessage()); expect(channel.send).not.toHaveBeenCalled(); }); + + it("sends [ATTACH: ...] file as Discord attachment", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "disclaw-attach-")); + const filePath = path.join(tmpDir, "result.txt"); + fs.writeFileSync(filePath, "data"); + + try { + const channel = makeChannel(); + await sendResponse(channel, `Here it is [ATTACH: result.txt]`, makeMessage(), tmpDir); + + // One call for the file, one for the text + expect(channel.send).toHaveBeenCalledTimes(2); + const fileCall = vi.mocked(channel.send).mock.calls[0][0] as { files: unknown[] }; + expect(fileCall.files).toHaveLength(1); + const textCall = vi.mocked(channel.send).mock.calls[1][0]; + expect(typeof textCall).toBe("string"); + expect(textCall as string).toContain("Here it is"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("sends warning message for file outside workspace", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "disclaw-ws-")); + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), "disclaw-out-")); + const outsideFile = path.join(outsideDir, "secret.txt"); + fs.writeFileSync(outsideFile, "secret"); + + try { + const channel = makeChannel(); + await sendResponse(channel, `[ATTACH: ${outsideFile}]`, makeMessage(), tmpDir); + expect(channel.send).toHaveBeenCalledTimes(1); + const call = vi.mocked(channel.send).mock.calls[0][0] as string; + expect(call).toMatch(/⚠️/); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + fs.rmSync(outsideDir, { recursive: true, force: true }); + } + }); + + it("only sends text when no [ATTACH: ...] present and workspacePath given", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "disclaw-plain-")); + try { + const channel = makeChannel(); + await sendResponse(channel, "Plain text response", makeMessage(), tmpDir); + expect(channel.send).toHaveBeenCalledTimes(1); + expect(vi.mocked(channel.send).mock.calls[0][0]).toBe("Plain text response"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); }); -- 2.45.2 From 92abd97993e65866b33c21c7cf3de568dc818d7b Mon Sep 17 00:00:00 2001 From: Nick Tabeling Date: Mon, 13 Apr 2026 10:24:38 +0200 Subject: [PATCH 4/4] fix(DIS-154): parse [ATTACH:] markers before sanitization sanitizeForDiscord() replaced workspace paths with before parseAttachments() could read them, making all resolved paths invalid. Fix: extract attach paths from raw result.text first, then sanitize the marker-free text. sendResponse() accepts pre-resolved paths to skip redundant re-parsing. Co-Authored-By: Claude Sonnet 4.6 --- src/discord/send-response.ts | 18 ++++++++++++------ src/router.ts | 13 ++++++++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/discord/send-response.ts b/src/discord/send-response.ts index e6937ac..88cadf3 100644 --- a/src/discord/send-response.ts +++ b/src/discord/send-response.ts @@ -67,24 +67,30 @@ export function validateAttachPath(filePath: string, workspacePath: string): str /** * Sends an agent response to a Discord channel. - * - Parses [ATTACH: path] markers and sends valid files as attachments. - * - Text > 4000 chars (after stripping markers) is sent as a Markdown file attachment. + * - If preResolvedAttachPaths is given, those files are sent as attachments (markers + * must already be stripped from `text`). Otherwise [ATTACH: path] markers are parsed + * from `text` on the fly (requires workspacePath). + * - Text > 4000 chars is sent as a Markdown file attachment. * - Text <= 4000 chars is split with splitForDiscord and sent as plain messages. */ export async function sendResponse( channel: TextChannel, text: string, _triggerMessage: Message, - workspacePath?: string + workspacePath?: string, + preResolvedAttachPaths?: string[] ): Promise { - if (text.length === 0) { + if (text.length === 0 && (!preResolvedAttachPaths || preResolvedAttachPaths.length === 0)) { return; } - // Parse [ATTACH: ...] markers if we have a workspace context + // Use pre-resolved paths (already extracted before sanitization) when available, + // otherwise fall back to parsing markers from the text (requires workspacePath). let cleanText = text; let attachPaths: string[] = []; - if (workspacePath) { + if (preResolvedAttachPaths) { + attachPaths = preResolvedAttachPaths; + } else if (workspacePath) { const parsed = parseAttachments(text, workspacePath); cleanText = parsed.text; attachPaths = parsed.attachPaths; diff --git a/src/router.ts b/src/router.ts index bd63d6b..a3d7ea2 100644 --- a/src/router.ts +++ b/src/router.ts @@ -5,7 +5,7 @@ import { DisclawConfig } from "./config/loader"; import { runAgent } from "./agent/runner"; import { sanitizeForDiscord } from "./runtime/sanitize"; import { ChannelQueue } from "./runtime/channel-queue"; -import { sendResponse } from "./discord/send-response"; +import { sendResponse, parseAttachments } from "./discord/send-response"; import { processAttachments } from "./discord/attachment-inbox"; import type { RunResult } from "./agent/types"; @@ -90,7 +90,13 @@ export async function routeMessage( message.author.username ); - const sanitizedResponse = sanitizeForDiscord(result.text, sanitizeOpts); + // Extract [ATTACH: ...] markers BEFORE sanitization so that workspace + // paths inside the markers are still resolvable. + const { text: textWithoutMarkers, attachPaths } = parseAttachments( + result.text, + workspace.workspace_path + ); + const sanitizedResponse = sanitizeForDiscord(textWithoutMarkers, sanitizeOpts); let firstMsgId: string | null = null; @@ -108,7 +114,8 @@ export async function routeMessage( // Temporarily patch channel.send for sendResponse (channel as unknown as { send: typeof patchedSend }).send = patchedSend; try { - await sendResponse(channel, sanitizedResponse, message, workspace.workspace_path); + // Pass pre-parsed attachPaths; sendResponse skips re-parsing when provided + await sendResponse(channel, sanitizedResponse, message, workspace.workspace_path, attachPaths); } finally { (channel as unknown as { send: typeof originalSend }).send = originalSend; } -- 2.45.2