Fix structured attachment rendering (#69)
* review: harden readonly git checks and lint verification * test: satisfy readonly reviewer sandbox typing * test: fix readonly reviewer sandbox assertions * test: remove impossible readonly sandbox guards * test: relax readonly sandbox assertions * test: cast readonly sandbox expectation shape * test: cast readonly sandbox expectation through unknown * Phase 0 — STEP_DONE/TASK_DONE split + owner-follow-up integration smoke * Add STEP_DONE guards, verdict storage, and stale delivery suppression * style: format paired stepdone telemetry files * Route STEP_DONE through reviewer * Add structured Discord attachments * style: format structured attachment test * Fix room thread bot output parity * Remove duplicate work item attachment migration from owner branch * Remove legacy dashboard rooms renderer * Extract dashboard parsed body renderer * Extract dashboard redaction helpers * Extract dashboard RoomCardV2 component * Include RoomCardV2 test in vitest suite * Extract dashboard RoomBoardV2 component * Extract dashboard EmptyState component * Extract dashboard InboxPanel component * Split dashboard InboxPanel card renderer * Extract dashboard TaskPanel component * Extract dashboard UsagePanel component * Fix dashboard room thread chunk rendering * Extract dashboard ServicePanel component * Render dashboard live progress markdown * Extract dashboard SettingsPanel component * Fix structured attachment rendering
This commit is contained in:
101
src/channels/discord-outbound.ts
Normal file
101
src/channels/discord-outbound.ts
Normal file
@@ -0,0 +1,101 @@
|
||||
import path from 'path';
|
||||
|
||||
import {
|
||||
extractImageTagPaths,
|
||||
normalizeEjclawStructuredOutput,
|
||||
} from '../agent-protocol.js';
|
||||
import type { OutboundAttachment } from '../types.js';
|
||||
|
||||
const IMAGE_EXTS = /\.(png|jpe?g|gif|webp|bmp)$/i;
|
||||
const MD_LINK_RE = /\[[^\]]*\]\((\/[^)]+)\)/g;
|
||||
|
||||
export interface PreparedDiscordOutbound {
|
||||
text: string;
|
||||
cleanText: string;
|
||||
attachments: OutboundAttachment[];
|
||||
attachmentSource: 'structured' | 'md-link' | 'image-tag' | 'none';
|
||||
silent: boolean;
|
||||
}
|
||||
|
||||
function extractMarkdownImageAttachments(text: string): {
|
||||
cleanText: string;
|
||||
attachments: OutboundAttachment[];
|
||||
} {
|
||||
const attachments: OutboundAttachment[] = [];
|
||||
const seen = new Set<string>();
|
||||
|
||||
const cleanText = text.replace(MD_LINK_RE, (_full, rawPath: string) => {
|
||||
const trimmed = rawPath.trim();
|
||||
if (IMAGE_EXTS.test(trimmed)) {
|
||||
if (!seen.has(trimmed)) {
|
||||
attachments.push({
|
||||
path: trimmed,
|
||||
name: path.basename(trimmed),
|
||||
});
|
||||
seen.add(trimmed);
|
||||
}
|
||||
return '';
|
||||
}
|
||||
|
||||
const basename = path.basename(trimmed.replace(/#.*$/, ''));
|
||||
const lineMatch = trimmed.match(/#L(\d+)/);
|
||||
return lineMatch ? `\`${basename}:${lineMatch[1]}\`` : `\`${basename}\``;
|
||||
});
|
||||
|
||||
return { cleanText, attachments };
|
||||
}
|
||||
|
||||
function imageTagPathsToAttachments(paths: string[]): OutboundAttachment[] {
|
||||
return paths
|
||||
.filter((filePath) => IMAGE_EXTS.test(filePath))
|
||||
.map((filePath) => ({
|
||||
path: filePath,
|
||||
name: path.basename(filePath),
|
||||
}));
|
||||
}
|
||||
|
||||
export function prepareDiscordOutbound(
|
||||
text: string,
|
||||
optionAttachments: OutboundAttachment[] | undefined,
|
||||
): PreparedDiscordOutbound {
|
||||
const normalized = normalizeEjclawStructuredOutput(text);
|
||||
if (normalized.output?.visibility === 'silent') {
|
||||
return {
|
||||
text: '',
|
||||
cleanText: '',
|
||||
attachments: [],
|
||||
attachmentSource: 'none',
|
||||
silent: true,
|
||||
};
|
||||
}
|
||||
|
||||
const structuredOutput =
|
||||
normalized.output?.visibility === 'public' ? normalized.output : null;
|
||||
const outboundText = structuredOutput?.text ?? normalized.result ?? text;
|
||||
const structuredAttachments =
|
||||
optionAttachments && optionAttachments.length > 0
|
||||
? optionAttachments
|
||||
: (structuredOutput?.attachments ?? []);
|
||||
const hasStructuredAttachments = structuredAttachments.length > 0;
|
||||
const markdownExtracted = extractMarkdownImageAttachments(outboundText);
|
||||
const imageTagExtracted = extractImageTagPaths(markdownExtracted.cleanText);
|
||||
const legacyImageTagAttachments = imageTagPathsToAttachments(
|
||||
imageTagExtracted.imagePaths,
|
||||
);
|
||||
|
||||
return {
|
||||
text: outboundText,
|
||||
cleanText: imageTagExtracted.cleanText,
|
||||
attachments: hasStructuredAttachments
|
||||
? structuredAttachments
|
||||
: [...markdownExtracted.attachments, ...legacyImageTagAttachments],
|
||||
attachmentSource: hasStructuredAttachments
|
||||
? 'structured'
|
||||
: markdownExtracted.attachments.length > 0
|
||||
? 'md-link'
|
||||
: legacyImageTagAttachments.length > 0
|
||||
? 'image-tag'
|
||||
: 'none',
|
||||
silent: false,
|
||||
};
|
||||
}
|
||||
171
src/channels/discord-structured-output.test.ts
Normal file
171
src/channels/discord-structured-output.test.ts
Normal file
@@ -0,0 +1,171 @@
|
||||
import fs from 'fs';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('./registry.js', () => ({
|
||||
registerChannel: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../env.js', () => ({
|
||||
readEnvFile: vi.fn(() => ({})),
|
||||
getEnv: vi.fn(() => undefined),
|
||||
}));
|
||||
|
||||
vi.mock('../config.js', () => ({
|
||||
ASSISTANT_NAME: 'Andy',
|
||||
TRIGGER_PATTERN: /^@Andy\b/i,
|
||||
DATA_DIR: '/tmp/ejclaw-test-data',
|
||||
CACHE_DIR: '/tmp/ejclaw-test-cache',
|
||||
}));
|
||||
|
||||
vi.mock('../logger.js', () => ({
|
||||
logger: {
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../service-routing.js', () => ({
|
||||
hasReviewerLease: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
type Handler = (...args: any[]) => any;
|
||||
|
||||
const clientRef = vi.hoisted(() => ({ current: null as any }));
|
||||
|
||||
vi.mock('discord.js', () => {
|
||||
class MockClient {
|
||||
eventHandlers = new Map<string, Handler[]>();
|
||||
user: any = { id: '999888777', tag: 'Andy#1234' };
|
||||
private _ready = false;
|
||||
|
||||
constructor(_opts: any) {
|
||||
clientRef.current = this;
|
||||
}
|
||||
|
||||
on(event: string, handler: Handler) {
|
||||
this.eventHandlers.set(event, [
|
||||
...(this.eventHandlers.get(event) ?? []),
|
||||
handler,
|
||||
]);
|
||||
return this;
|
||||
}
|
||||
|
||||
once(event: string, handler: Handler) {
|
||||
return this.on(event, handler);
|
||||
}
|
||||
|
||||
async login(_token: string) {
|
||||
this._ready = true;
|
||||
for (const handler of this.eventHandlers.get('ready') ?? []) {
|
||||
handler({ user: this.user });
|
||||
}
|
||||
}
|
||||
|
||||
isReady() {
|
||||
return this._ready;
|
||||
}
|
||||
|
||||
channels = {
|
||||
fetch: vi.fn().mockResolvedValue({
|
||||
send: vi.fn().mockResolvedValue(undefined),
|
||||
sendTyping: vi.fn().mockResolvedValue(undefined),
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
Client: MockClient,
|
||||
Events: {
|
||||
MessageCreate: 'messageCreate',
|
||||
ClientReady: 'ready',
|
||||
Error: 'error',
|
||||
},
|
||||
GatewayIntentBits: {
|
||||
Guilds: 1,
|
||||
GuildMessages: 2,
|
||||
MessageContent: 4,
|
||||
DirectMessages: 8,
|
||||
},
|
||||
MessageFlags: { SuppressEmbeds: 1 << 2, IsVoiceMessage: 1 << 13 },
|
||||
TextChannel: class TextChannel {},
|
||||
};
|
||||
});
|
||||
|
||||
import { DiscordChannel, type DiscordChannelOpts } from './discord.js';
|
||||
|
||||
const ONE_PIXEL_PNG = Buffer.from(
|
||||
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII=',
|
||||
'base64',
|
||||
);
|
||||
|
||||
const tempFiles: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
for (const file of tempFiles.splice(0)) {
|
||||
fs.rmSync(file, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
function createTestOpts(): DiscordChannelOpts {
|
||||
return {
|
||||
onMessage: vi.fn(),
|
||||
onChatMetadata: vi.fn(),
|
||||
roomBindings: vi.fn(() => ({})),
|
||||
};
|
||||
}
|
||||
|
||||
describe('DiscordChannel structured output', () => {
|
||||
it('normalizes raw EJClaw JSON and sends direct temp images as files', async () => {
|
||||
const channel = new DiscordChannel('test-token', createTestOpts());
|
||||
await channel.connect();
|
||||
const filePath = path.join(
|
||||
os.tmpdir(),
|
||||
`bar-chart-label-fit-playwright-${Date.now()}.png`,
|
||||
);
|
||||
fs.writeFileSync(filePath, ONE_PIXEL_PNG);
|
||||
tempFiles.push(filePath);
|
||||
const mockChannel = {
|
||||
send: vi.fn().mockResolvedValue({ id: 'discord-message-1' }),
|
||||
sendTyping: vi.fn(),
|
||||
};
|
||||
clientRef.current.channels.fetch.mockResolvedValue(mockChannel);
|
||||
|
||||
await channel.sendMessage(
|
||||
'dc:1234567890123456',
|
||||
JSON.stringify({
|
||||
ejclaw: {
|
||||
visibility: 'public',
|
||||
text: '라벨 좌측 클리핑 회귀 수정했습니다.',
|
||||
verdict: 'done',
|
||||
attachments: [
|
||||
{
|
||||
path: filePath,
|
||||
name: 'bar-chart-label-fit-playwright.png',
|
||||
mime: 'image/png',
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(mockChannel.send).toHaveBeenCalledWith({
|
||||
content: '라벨 좌측 클리핑 회귀 수정했습니다.',
|
||||
files: [
|
||||
{
|
||||
attachment: fs.realpathSync(filePath),
|
||||
name: 'bar-chart-label-fit-playwright.png',
|
||||
},
|
||||
],
|
||||
flags: 1 << 2,
|
||||
});
|
||||
expect(JSON.stringify(mockChannel.send.mock.calls)).not.toContain(
|
||||
'"ejclaw"',
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -19,11 +19,11 @@ import {
|
||||
} from '../config.js';
|
||||
import { getEnv } from '../env.js';
|
||||
import { logger } from '../logger.js';
|
||||
import { extractImageTagPaths } from '../agent-protocol.js';
|
||||
import { validateOutboundAttachments } from '../outbound-attachments.js';
|
||||
import { formatOutbound } from '../router.js';
|
||||
import { hasReviewerLease } from '../service-routing.js';
|
||||
import type { OutboundAttachment, SendMessageOptions } from '../types.js';
|
||||
import type { SendMessageOptions } from '../types.js';
|
||||
import { prepareDiscordOutbound } from './discord-outbound.js';
|
||||
|
||||
const ATTACHMENTS_DIR = path.join(DATA_DIR, 'attachments');
|
||||
const TRANSCRIPTION_CACHE_DIR = path.join(CACHE_DIR, 'transcriptions');
|
||||
@@ -33,45 +33,6 @@ const DISCORD_ARBITER_CHANNEL = 'discord-arbiter';
|
||||
const DISCORD_OWNER_TOKEN_KEY = 'DISCORD_OWNER_BOT_TOKEN';
|
||||
const DISCORD_REVIEWER_TOKEN_KEY = 'DISCORD_REVIEWER_BOT_TOKEN';
|
||||
const DISCORD_ARBITER_TOKEN_KEY = 'DISCORD_ARBITER_BOT_TOKEN';
|
||||
const IMAGE_EXTS = /\.(png|jpe?g|gif|webp|bmp)$/i;
|
||||
const MD_LINK_RE = /\[[^\]]*\]\((\/[^)]+)\)/g;
|
||||
|
||||
function extractMarkdownImageAttachments(text: string): {
|
||||
cleanText: string;
|
||||
attachments: OutboundAttachment[];
|
||||
} {
|
||||
const attachments: OutboundAttachment[] = [];
|
||||
const seen = new Set<string>();
|
||||
|
||||
const cleanText = text.replace(MD_LINK_RE, (_full, rawPath: string) => {
|
||||
const trimmed = rawPath.trim();
|
||||
if (IMAGE_EXTS.test(trimmed)) {
|
||||
if (!seen.has(trimmed)) {
|
||||
attachments.push({
|
||||
path: trimmed,
|
||||
name: path.basename(trimmed),
|
||||
});
|
||||
seen.add(trimmed);
|
||||
}
|
||||
return '';
|
||||
}
|
||||
|
||||
const basename = path.basename(trimmed.replace(/#.*$/, ''));
|
||||
const lineMatch = trimmed.match(/#L(\d+)/);
|
||||
return lineMatch ? `\`${basename}:${lineMatch[1]}\`` : `\`${basename}\``;
|
||||
});
|
||||
|
||||
return { cleanText, attachments };
|
||||
}
|
||||
|
||||
function imageTagPathsToAttachments(paths: string[]): OutboundAttachment[] {
|
||||
return paths
|
||||
.filter((filePath) => IMAGE_EXTS.test(filePath))
|
||||
.map((filePath) => ({
|
||||
path: filePath,
|
||||
name: path.basename(filePath),
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
* Download a Discord attachment to local disk.
|
||||
@@ -466,26 +427,15 @@ export class DiscordChannel implements Channel {
|
||||
|
||||
const textChannel = channel as TextChannel;
|
||||
|
||||
const structuredAttachments = options.attachments ?? [];
|
||||
const hasStructuredAttachments = structuredAttachments.length > 0;
|
||||
const markdownExtracted = extractMarkdownImageAttachments(text);
|
||||
const imageTagExtracted = extractImageTagPaths(
|
||||
markdownExtracted.cleanText,
|
||||
);
|
||||
const legacyImageTagAttachments = imageTagPathsToAttachments(
|
||||
imageTagExtracted.imagePaths,
|
||||
);
|
||||
const outboundAttachments = hasStructuredAttachments
|
||||
? structuredAttachments
|
||||
: [...markdownExtracted.attachments, ...legacyImageTagAttachments];
|
||||
const attachmentSource = hasStructuredAttachments
|
||||
? 'structured'
|
||||
: markdownExtracted.attachments.length > 0
|
||||
? 'md-link'
|
||||
: legacyImageTagAttachments.length > 0
|
||||
? 'image-tag'
|
||||
: 'none';
|
||||
const validation = validateOutboundAttachments(outboundAttachments, {
|
||||
const outbound = prepareDiscordOutbound(text, options.attachments);
|
||||
if (outbound.silent) {
|
||||
logger.debug(
|
||||
{ jid, channelName: this.name },
|
||||
'Skipping silent structured Discord outbound message',
|
||||
);
|
||||
return;
|
||||
}
|
||||
const validation = validateOutboundAttachments(outbound.attachments, {
|
||||
baseDirs: options.attachmentBaseDirs,
|
||||
});
|
||||
const files = validation.files;
|
||||
@@ -495,14 +445,14 @@ export class DiscordChannel implements Channel {
|
||||
{
|
||||
jid,
|
||||
channelName: this.name,
|
||||
attachmentSource,
|
||||
attachmentSource: outbound.attachmentSource,
|
||||
rejected: validation.rejected,
|
||||
},
|
||||
'Rejected outbound Discord attachments',
|
||||
);
|
||||
}
|
||||
|
||||
let cleaned = imageTagExtracted.cleanText
|
||||
let cleaned = outbound.cleanText
|
||||
.replace(/^[ \t]*[•\-\*][ \t]*$/gm, '') // remove empty bullet lines
|
||||
.replace(/\n{3,}/g, '\n\n') // collapse excessive blank lines
|
||||
.trim();
|
||||
@@ -588,11 +538,11 @@ export class DiscordChannel implements Channel {
|
||||
{
|
||||
jid,
|
||||
channelName: this.name,
|
||||
length: text.length,
|
||||
length: outbound.text.length,
|
||||
deliveryMode: 'send',
|
||||
chunkCount,
|
||||
attachmentCount: files.length,
|
||||
attachmentSource,
|
||||
attachmentSource: outbound.attachmentSource,
|
||||
messageId: sentMessageIds[0] ?? null,
|
||||
messageIds: sentMessageIds,
|
||||
botUserId: this.client.user?.id ?? null,
|
||||
|
||||
Reference in New Issue
Block a user