fix: surface remaining paired evidence loss (#203)
This commit is contained in:
@@ -5,7 +5,7 @@ export const IMAGE_TAG_RE =
|
||||
/\[Image:\s*(?:(?:[^\]\n]*?)\s*→\s*)?(\/[^\]\n]+)\]/g;
|
||||
const IMAGE_TAG_SEGMENT_RE = /\[Image:\s*(?:(.*?)\s*→\s*)?(\/[^\]\n]+)\]/g;
|
||||
const IMAGE_EXT_RE = /\.(png|jpe?g|gif|webp|bmp)$/i;
|
||||
const MARKDOWN_ABSOLUTE_LINK_RE = /!?\[[^\]\n]*\]\((\/[^)\n]+)\)/g;
|
||||
const MARKDOWN_IMAGE_ABSOLUTE_LINK_RE = /!\[[^\]\n]*\]\((\/[^)\n]+)\)/g;
|
||||
const MEDIA_TAG_RE =
|
||||
/^[ \t]*MEDIA:\s*(?:"([^"\n]+)"|'([^'\n]+)'|`([^`\n]+)`|(\/\S+))[ \t]*$/gm;
|
||||
|
||||
@@ -158,7 +158,7 @@ export function extractMarkdownImageAttachments(text: string): {
|
||||
} {
|
||||
const attachments: RunnerOutputAttachment[] = [];
|
||||
const cleanText = text.replace(
|
||||
MARKDOWN_ABSOLUTE_LINK_RE,
|
||||
MARKDOWN_IMAGE_ABSOLUTE_LINK_RE,
|
||||
(full: string, rawPath: string) => {
|
||||
const trimmed = rawPath.trim();
|
||||
if (!IMAGE_EXT_RE.test(trimmed)) return full;
|
||||
|
||||
@@ -10,10 +10,11 @@ describe('normalizeAgentOutput', () => {
|
||||
it('extracts markdown image attachments without rewriting normal links', () => {
|
||||
expect(
|
||||
extractMarkdownImageAttachments(
|
||||
'결과입니다.\n\n[code](/tmp/source.ts#L10)',
|
||||
'결과입니다.\n\n[render link](/tmp/render.png)\n[code](/tmp/source.ts#L10)',
|
||||
),
|
||||
).toEqual({
|
||||
cleanText: '결과입니다.\n\n[code](/tmp/source.ts#L10)',
|
||||
cleanText:
|
||||
'결과입니다.\n\n[render link](/tmp/render.png)\n[code](/tmp/source.ts#L10)',
|
||||
attachments: [
|
||||
{
|
||||
path: '/tmp/result.png',
|
||||
|
||||
@@ -71,6 +71,15 @@ function formatAttachmentReference(
|
||||
return `[${label}: ${att.name || 'file'} → ${filePath}]`;
|
||||
}
|
||||
|
||||
function formatUnavailableAttachmentReference(
|
||||
label: 'Audio' | 'File' | 'Image' | 'Video',
|
||||
att: Attachment,
|
||||
filePath: string,
|
||||
contentType: string,
|
||||
): string {
|
||||
return `[${label} unavailable: ${att.name || 'file'} → ${filePath} — ${contentType} is not loaded as structured model input]`;
|
||||
}
|
||||
|
||||
function formatBytes(bytes: number): string {
|
||||
return `${(bytes / (1024 * 1024)).toFixed(1)} MiB`;
|
||||
}
|
||||
@@ -112,7 +121,15 @@ export async function describeDownloadedAttachment(
|
||||
attachmentDefaultExtension(contentType),
|
||||
);
|
||||
const reference = formatAttachmentReference(label, att, filePath);
|
||||
if (!isTextAttachment(att, contentType)) return reference;
|
||||
if (!isTextAttachment(att, contentType)) {
|
||||
if (label === 'Image') return reference;
|
||||
return formatUnavailableAttachmentReference(
|
||||
label,
|
||||
att,
|
||||
filePath,
|
||||
contentType,
|
||||
);
|
||||
}
|
||||
|
||||
let text = fs.readFileSync(filePath, 'utf8');
|
||||
const MAX_TEXT_LENGTH = 32_000;
|
||||
|
||||
@@ -648,7 +648,7 @@ describe('attachments', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('stores video attachment with local path', async () => {
|
||||
it('stores video attachment with a visible unsupported marker', async () => {
|
||||
const opts = createTestOpts();
|
||||
const channel = new DiscordChannel('test-token', opts);
|
||||
await channel.connect();
|
||||
@@ -674,12 +674,14 @@ describe('attachments', () => {
|
||||
expect(opts.onMessage).toHaveBeenCalledWith(
|
||||
'dc:1234567890123456',
|
||||
expect.objectContaining({
|
||||
content: expect.stringMatching(/^\[Video: clip\.mp4 → .+\.mp4\]$/),
|
||||
content: expect.stringMatching(
|
||||
/^\[Video unavailable: clip\.mp4 → .+\.mp4 — video\/mp4 is not loaded as structured model input\]$/,
|
||||
),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('stores file attachment with local path', async () => {
|
||||
it('stores file attachment with a visible unsupported marker', async () => {
|
||||
const opts = createTestOpts();
|
||||
const channel = new DiscordChannel('test-token', opts);
|
||||
await channel.connect();
|
||||
@@ -705,12 +707,14 @@ describe('attachments', () => {
|
||||
expect(opts.onMessage).toHaveBeenCalledWith(
|
||||
'dc:1234567890123456',
|
||||
expect.objectContaining({
|
||||
content: expect.stringMatching(/^\[File: report\.pdf → .+\.pdf\]$/),
|
||||
content: expect.stringMatching(
|
||||
/^\[File unavailable: report\.pdf → .+\.pdf — application\/pdf is not loaded as structured model input\]$/,
|
||||
),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('stores zip attachment with local path', async () => {
|
||||
it('stores zip attachment with a visible unsupported marker', async () => {
|
||||
const opts = createTestOpts();
|
||||
const channel = new DiscordChannel('test-token', opts);
|
||||
await channel.connect();
|
||||
@@ -737,7 +741,7 @@ describe('attachments', () => {
|
||||
'dc:1234567890123456',
|
||||
expect.objectContaining({
|
||||
content: expect.stringMatching(
|
||||
/^\[File: display_latency_atopile_rev09\.zip → .+\.zip\]$/,
|
||||
/^\[File unavailable: display_latency_atopile_rev09\.zip → .+\.zip — application\/zip is not loaded as structured model input\]$/,
|
||||
),
|
||||
}),
|
||||
);
|
||||
|
||||
84
src/db-attachment-payload.test.ts
Normal file
84
src/db-attachment-payload.test.ts
Normal file
@@ -0,0 +1,84 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('./logger.js', () => ({
|
||||
logger: {
|
||||
warn: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
import { logger } from './logger.js';
|
||||
import { parseAttachmentPayload } from './db/work-items.js';
|
||||
|
||||
describe('attachment payload parsing', () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(logger.warn).mockReset();
|
||||
});
|
||||
|
||||
it('logs malformed JSON without throwing or breaking old rows', () => {
|
||||
const attachments = parseAttachmentPayload('{"path":', {
|
||||
table: 'work_items',
|
||||
rowId: 42,
|
||||
});
|
||||
|
||||
expect(attachments).toEqual([]);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
table: 'work_items',
|
||||
rowId: 42,
|
||||
reason: 'invalid_json',
|
||||
payloadLength: 8,
|
||||
payloadPreview: '{"path":',
|
||||
err: expect.any(SyntaxError),
|
||||
}),
|
||||
'Ignored malformed attachment payload',
|
||||
);
|
||||
});
|
||||
|
||||
it('logs non-array attachment JSON without throwing', () => {
|
||||
const payload = '{"path":"/tmp/image.png"}';
|
||||
const attachments = parseAttachmentPayload(payload, {
|
||||
table: 'paired_turn_outputs',
|
||||
rowId: 7,
|
||||
});
|
||||
|
||||
expect(attachments).toEqual([]);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
table: 'paired_turn_outputs',
|
||||
rowId: 7,
|
||||
reason: 'not_array',
|
||||
payloadLength: payload.length,
|
||||
payloadPreview: payload,
|
||||
}),
|
||||
'Ignored malformed attachment payload',
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps valid attachments while logging malformed entries', () => {
|
||||
const payload = JSON.stringify([
|
||||
{ path: '/tmp/image.png', name: 'image.png', mime: 'image/png' },
|
||||
{ name: 'missing-path.png' },
|
||||
null,
|
||||
]);
|
||||
|
||||
const attachments = parseAttachmentPayload(payload, {
|
||||
table: 'work_items',
|
||||
rowId: 43,
|
||||
});
|
||||
|
||||
expect(attachments).toEqual([
|
||||
{ path: '/tmp/image.png', name: 'image.png', mime: 'image/png' },
|
||||
]);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
table: 'work_items',
|
||||
rowId: 43,
|
||||
invalidEntryCount: 2,
|
||||
validEntryCount: 1,
|
||||
payloadLength: payload.length,
|
||||
payloadPreview: payload,
|
||||
}),
|
||||
'Ignored invalid attachment payload entries',
|
||||
);
|
||||
});
|
||||
});
|
||||
29
src/db-room-assignment.test.ts
Normal file
29
src/db-room-assignment.test.ts
Normal file
@@ -0,0 +1,29 @@
|
||||
import { beforeEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import { _initTestDatabase, assignRoom, getStoredRoomSettings } from './db.js';
|
||||
|
||||
describe('room assignment metadata', () => {
|
||||
beforeEach(() => {
|
||||
_initTestDatabase();
|
||||
});
|
||||
|
||||
it('preserves explicit trigger metadata when assigning a room', () => {
|
||||
const group = assignRoom('dc:triggered-room', {
|
||||
name: 'Triggered Room',
|
||||
roomMode: 'single',
|
||||
ownerAgentType: 'codex',
|
||||
folder: 'triggered-room',
|
||||
trigger: '@Repro',
|
||||
requiresTrigger: true,
|
||||
});
|
||||
|
||||
expect(group).toMatchObject({
|
||||
trigger: '@Repro',
|
||||
requiresTrigger: true,
|
||||
});
|
||||
expect(getStoredRoomSettings('dc:triggered-room')).toMatchObject({
|
||||
trigger: '@Repro',
|
||||
requiresTrigger: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -73,7 +73,10 @@ function hydratePairedTurnOutputRow(
|
||||
): PairedTurnOutput {
|
||||
return {
|
||||
...row,
|
||||
attachments: parseAttachmentPayload(row.attachment_payload),
|
||||
attachments: parseAttachmentPayload(row.attachment_payload, {
|
||||
table: 'paired_turn_outputs',
|
||||
rowId: row.id,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -53,6 +53,8 @@ export interface AssignRoomInput {
|
||||
reviewerAgentType?: AgentType;
|
||||
arbiterAgentType?: AgentType | null;
|
||||
folder?: string;
|
||||
trigger?: string;
|
||||
requiresTrigger?: boolean;
|
||||
isMain?: boolean;
|
||||
workDir?: string;
|
||||
addedAt?: string;
|
||||
@@ -204,8 +206,9 @@ export function assignRoomInDatabase(
|
||||
const snapshot: RoomRegistrationSnapshot = {
|
||||
name: input.name,
|
||||
folder,
|
||||
triggerPattern: existing?.trigger ?? '',
|
||||
requiresTrigger: existing?.requiresTrigger ?? false,
|
||||
triggerPattern: input.trigger ?? existing?.trigger ?? '',
|
||||
requiresTrigger:
|
||||
input.requiresTrigger ?? existing?.requiresTrigger ?? false,
|
||||
isMain: input.isMain ?? existing?.isMain ?? false,
|
||||
ownerAgentType,
|
||||
workDir: input.workDir ?? existing?.workDir ?? null,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Database } from 'bun:sqlite';
|
||||
|
||||
import { SERVICE_SESSION_SCOPE, normalizeServiceId } from '../config.js';
|
||||
import { logger } from '../logger.js';
|
||||
import {
|
||||
inferAgentTypeFromServiceShadow,
|
||||
inferRoleFromServiceShadow,
|
||||
@@ -101,31 +102,102 @@ function hydrateWorkItemRow(row: StoredWorkItemRow): WorkItem {
|
||||
...row,
|
||||
agent_type: agentType,
|
||||
service_id: readStoredWorkItemServiceId(row),
|
||||
attachments: parseAttachmentPayload(row.attachment_payload),
|
||||
attachments: parseAttachmentPayload(row.attachment_payload, {
|
||||
table: 'work_items',
|
||||
rowId: row.id,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
export interface AttachmentPayloadContext {
|
||||
table?: string;
|
||||
rowId?: string | number;
|
||||
}
|
||||
|
||||
function payloadPreview(payload: string): string {
|
||||
return payload.length > 200 ? `${payload.slice(0, 200)}...` : payload;
|
||||
}
|
||||
|
||||
function isAttachmentPayloadEntry(
|
||||
item: unknown,
|
||||
): item is { path: string; name?: unknown; mime?: unknown } {
|
||||
return (
|
||||
item !== null &&
|
||||
typeof item === 'object' &&
|
||||
!Array.isArray(item) &&
|
||||
typeof (item as { path?: unknown }).path === 'string'
|
||||
);
|
||||
}
|
||||
|
||||
function logMalformedAttachmentPayload(args: {
|
||||
payload: string;
|
||||
context?: AttachmentPayloadContext;
|
||||
reason: string;
|
||||
err?: unknown;
|
||||
}): void {
|
||||
logger.warn(
|
||||
{
|
||||
...args.context,
|
||||
reason: args.reason,
|
||||
payloadLength: args.payload.length,
|
||||
payloadPreview: payloadPreview(args.payload),
|
||||
...(args.err ? { err: args.err } : {}),
|
||||
},
|
||||
'Ignored malformed attachment payload',
|
||||
);
|
||||
}
|
||||
|
||||
export function parseAttachmentPayload(
|
||||
payload: string | null | undefined,
|
||||
context?: AttachmentPayloadContext,
|
||||
): OutboundAttachment[] {
|
||||
if (!payload) return [];
|
||||
try {
|
||||
const parsed = JSON.parse(payload) as unknown;
|
||||
if (!Array.isArray(parsed)) return [];
|
||||
return parsed
|
||||
.filter(
|
||||
(item): item is OutboundAttachment =>
|
||||
item !== null &&
|
||||
typeof item === 'object' &&
|
||||
!Array.isArray(item) &&
|
||||
typeof (item as { path?: unknown }).path === 'string',
|
||||
)
|
||||
.map((item) => ({
|
||||
path: item.path,
|
||||
...(typeof item.name === 'string' ? { name: item.name } : {}),
|
||||
...(typeof item.mime === 'string' ? { mime: item.mime } : {}),
|
||||
}));
|
||||
} catch {
|
||||
if (!Array.isArray(parsed)) {
|
||||
logMalformedAttachmentPayload({
|
||||
payload,
|
||||
context,
|
||||
reason: 'not_array',
|
||||
});
|
||||
return [];
|
||||
}
|
||||
|
||||
const attachments: OutboundAttachment[] = [];
|
||||
let invalidEntryCount = 0;
|
||||
for (const item of parsed) {
|
||||
if (isAttachmentPayloadEntry(item)) {
|
||||
attachments.push({
|
||||
path: item.path,
|
||||
...(typeof item.name === 'string' ? { name: item.name } : {}),
|
||||
...(typeof item.mime === 'string' ? { mime: item.mime } : {}),
|
||||
});
|
||||
} else {
|
||||
invalidEntryCount += 1;
|
||||
}
|
||||
}
|
||||
|
||||
if (invalidEntryCount > 0) {
|
||||
logger.warn(
|
||||
{
|
||||
...context,
|
||||
invalidEntryCount,
|
||||
validEntryCount: attachments.length,
|
||||
payloadLength: payload.length,
|
||||
payloadPreview: payloadPreview(payload),
|
||||
},
|
||||
'Ignored invalid attachment payload entries',
|
||||
);
|
||||
}
|
||||
|
||||
return attachments;
|
||||
} catch (err) {
|
||||
logMalformedAttachmentPayload({
|
||||
payload,
|
||||
context,
|
||||
reason: 'invalid_json',
|
||||
err,
|
||||
});
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1106,6 +1106,31 @@ describe('assign_room success', () => {
|
||||
expect(group!.folder).toBe('new-group');
|
||||
});
|
||||
|
||||
it('assign_room persists trigger metadata through IPC', async () => {
|
||||
await processTaskIpc(
|
||||
{
|
||||
type: 'assign_room',
|
||||
jid: 'triggered@g.us',
|
||||
name: 'Triggered Group',
|
||||
folder: 'triggered-group',
|
||||
trigger: '@Repro',
|
||||
requiresTrigger: true,
|
||||
},
|
||||
'whatsapp_main',
|
||||
true,
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(getStoredRoomSettings('triggered@g.us')).toMatchObject({
|
||||
trigger: '@Repro',
|
||||
requiresTrigger: true,
|
||||
});
|
||||
expect(getRegisteredGroup('triggered@g.us')).toMatchObject({
|
||||
trigger: '@Repro',
|
||||
requiresTrigger: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('assign_room auto-fills missing folder for single rooms without exposing trigger metadata', async () => {
|
||||
await processTaskIpc(
|
||||
{
|
||||
|
||||
@@ -35,6 +35,8 @@ export function handleAssignRoom(
|
||||
reviewerAgentType: data.reviewer_agent_type,
|
||||
arbiterAgentType: data.arbiter_agent_type,
|
||||
folder: data.folder,
|
||||
trigger: data.trigger,
|
||||
requiresTrigger: data.requiresTrigger,
|
||||
isMain: data.isMain,
|
||||
workDir: data.workDir,
|
||||
});
|
||||
@@ -66,6 +68,23 @@ function validateAssignRoomRequest(
|
||||
);
|
||||
return { ok: false };
|
||||
}
|
||||
if (data.trigger !== undefined && typeof data.trigger !== 'string') {
|
||||
logger.warn(
|
||||
{ sourceGroup, trigger: data.trigger },
|
||||
'Invalid assign_room request - trigger must be a string',
|
||||
);
|
||||
return { ok: false };
|
||||
}
|
||||
if (
|
||||
data.requiresTrigger !== undefined &&
|
||||
typeof data.requiresTrigger !== 'boolean'
|
||||
) {
|
||||
logger.warn(
|
||||
{ sourceGroup, requiresTrigger: data.requiresTrigger },
|
||||
'Invalid assign_room request - requiresTrigger must be a boolean',
|
||||
);
|
||||
return { ok: false };
|
||||
}
|
||||
if (data.room_mode !== undefined && !isRoomMode(data.room_mode)) {
|
||||
logger.warn(
|
||||
{ sourceGroup, roomMode: data.room_mode },
|
||||
|
||||
@@ -131,6 +131,8 @@ export interface TaskIpcPayload {
|
||||
jid?: string;
|
||||
name?: string;
|
||||
folder?: string;
|
||||
trigger?: string;
|
||||
requiresTrigger?: boolean;
|
||||
room_mode?: RoomMode;
|
||||
owner_agent_type?: AgentType;
|
||||
reviewer_agent_type?: AgentType;
|
||||
|
||||
@@ -170,6 +170,33 @@ describe('message-runtime-prompts output-only context', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('makes non-image turn attachments visibly unavailable instead of implying inspection', () => {
|
||||
const prompt = buildReviewerPendingPrompt({
|
||||
chatJid: 'group@test',
|
||||
timezone: 'UTC',
|
||||
turnOutputs: [
|
||||
makeTurnOutput('TASK_DONE\nPDF 증거 첨부', 'owner', {
|
||||
turn_number: 1,
|
||||
created_at: '2026-04-20T02:00:00.000Z',
|
||||
attachments: [
|
||||
{
|
||||
path: '/tmp/report.pdf',
|
||||
name: 'report.pdf',
|
||||
mime: 'application/pdf',
|
||||
},
|
||||
],
|
||||
}),
|
||||
],
|
||||
recentHumanMessages: [],
|
||||
lastHumanMessage: null,
|
||||
taskCreatedAt: '2026-04-20T01:00:00.000Z',
|
||||
});
|
||||
|
||||
expect(prompt).toContain(
|
||||
'[Attachment unavailable: report.pdf → /tmp/report.pdf — application/pdf is not loaded as structured model input]',
|
||||
);
|
||||
});
|
||||
|
||||
it('includes current task user scope in reviewer pending prompts without pulling older human messages', () => {
|
||||
const prompt = buildReviewerPendingPrompt({
|
||||
chatJid: 'group@test',
|
||||
|
||||
@@ -25,6 +25,14 @@ function attachmentLabel(attachment: OutboundAttachment): string {
|
||||
return attachment.name?.trim() || attachment.path.split('/').at(-1) || 'file';
|
||||
}
|
||||
|
||||
function nonImageAttachmentReason(attachment: OutboundAttachment): string {
|
||||
const mime = attachment.mime?.trim();
|
||||
if (mime) {
|
||||
return `${mime} is not loaded as structured model input`;
|
||||
}
|
||||
return 'non-image attachment is not loaded as structured model input';
|
||||
}
|
||||
|
||||
function formatTurnOutputAttachmentContext(
|
||||
attachments: OutboundAttachment[] | undefined,
|
||||
): string {
|
||||
@@ -33,7 +41,9 @@ function formatTurnOutputAttachmentContext(
|
||||
const label = attachmentLabel(attachment);
|
||||
return isImageAttachment(attachment)
|
||||
? `[Image: ${label} → ${attachment.path}]`
|
||||
: `[Attachment: ${label} → ${attachment.path}]`;
|
||||
: `[Attachment unavailable: ${label} → ${attachment.path} — ${nonImageAttachmentReason(
|
||||
attachment,
|
||||
)}]`;
|
||||
});
|
||||
return `\n\nAttached evidence from this turn:\n${lines.join('\n')}`;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user