diff --git a/README.md b/README.md index 34b1658..9571c9f 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,8 @@ EJClaw는 Discord 위에서 동작하는 Tribunal 멀티에이전트 개발 보 원본은 [qwibitai/nanoclaw](https://github.com/qwibitai/nanoclaw)에서 출발했지만, 현재는 EJClaw의 Discord/paired-runtime 구조에 맞게 독립적으로 유지되고 있습니다. +이 Gitea 저장소는 GitHub `main`을 베이스로 삼고, 운영 환경에서 검증된 로컬 변경(아래 "운영 적용 사항")을 현재 코드 구조에 맞게 재구현해 얹은 배포본입니다. + ## 개요 - 단일 `ejclaw` 서비스가 owner / reviewer / arbiter 세 역할과 세 Discord 봇을 함께 관리합니다. @@ -32,6 +34,16 @@ EJClaw는 Discord 위에서 동작하는 Tribunal 멀티에이전트 개발 보 - `assign_room` 기반 명시적 room assignment - Bun + SQLite 기반 빠른 런타임 +## 운영 적용 사항 + +GitHub `main` 베이스 위에 운영 중 검증된 변경을 현재 코드 구조에 맞춰 재구현해 반영했습니다. + +- Discord 출력 마크다운 보존: 일반 문장의 `**bold**`, `~~strike~~`, 백틱 등 emphasis 마커를 렌더링 대신 원문 그대로 표시(백슬래시 이스케이프). 코드블록과 구조화 출력(EJClaw JSON, 링크→인라인 코드)은 의도대로 보존하고, 이스케이프는 Discord 전송 직전 한 번만 적용됩니다. +- 2000자 분할 시 코드블록 보존: 코드 펜스(triple backtick)를 인식해 청크 경계에서 코드블록이 깨지지 않도록 분할(서러게이트 페어 안전 처리 포함). +- 사용량 윈도우 정렬 프라이머: Codex/Claude 초기화 시점을 맞추기 위한 정렬 신호를 조건 없이 발사. +- 리뷰어 무응답 방지: 리뷰어가 verdict 없이 연속 실패할 때 재시도 상한(기본 2회)으로 핑퐁 루프를 끊고 arbiter/사용자로 에스컬레이션. +- 대시보드: 등록된 방 별칭을 Discord 채널명과 분리해 보존. + ## Tribunal 시스템 | 역할 | 현재 기본값 | 설명 | @@ -123,7 +135,7 @@ Discord ──► SQLite (WAL) ──► GroupQueue ──┬──► Owner (ho ### 설치 ```bash -git clone https://github.com/phj1081/EJClaw.git +git clone https://git.tkrmagid.kr/claude-bot/EJClaw.git cd EJClaw bun install bun run build:all diff --git a/prompts/owner-common-paired-room.md b/prompts/owner-common-paired-room.md index f30c830..027b628 100644 --- a/prompts/owner-common-paired-room.md +++ b/prompts/owner-common-paired-room.md @@ -68,7 +68,6 @@ Every finalize message must contain, in this order: 5. **(Optional) Follow-up suggestions** — only if genuinely useful, kept short. Do **not** include: - - "리뷰어가 지적해서 정정했습니다", "PROCEED 확정", "리뷰어 피드백 반영" 같은 메타 문구. The user doesn't know there was a reviewer. - A blow-by-blow of each round. - Apologies for earlier owner mistakes that the user never saw. diff --git a/src/channels/discord-outbound.ts b/src/channels/discord-outbound.ts index cd6ae03..4752ba4 100644 --- a/src/channels/discord-outbound.ts +++ b/src/channels/discord-outbound.ts @@ -1,6 +1,7 @@ import path from 'path'; import { normalizeAgentOutput } from '../agent-protocol.js'; +import { neutralizeStrayMarkdown, sanitizeForOutbound } from '../router.js'; import type { OutboundAttachment } from '../types.js'; const LOCAL_MARKDOWN_LINK_RE = /\[[^\]\n]*\]\((\/[^)\n]+)\)/g; @@ -46,7 +47,17 @@ export function prepareDiscordOutbound( const structuredOutput = normalized.output?.visibility === 'public' ? normalized.output : null; const outboundText = structuredOutput?.text ?? normalized.result ?? text; - const cleanText = sanitizeLocalMarkdownLinks(outboundText); + // Redact/strip before escaping so secret patterns (which contain `_`) still + // match, then escape stray Discord markdown in free-form agent prose so + // emphasis markers (**bold**, ~~strike~~, …) render literally. Author- + // controlled structured envelopes are intentional, so their text is left + // untouched. Escaping runs before sanitizeLocalMarkdownLinks so the inline + // code it deliberately produces is preserved. + const safeText = sanitizeForOutbound(outboundText); + const renderedText = structuredOutput + ? safeText + : neutralizeStrayMarkdown(safeText); + const cleanText = sanitizeLocalMarkdownLinks(renderedText); const attachments = optionAttachments && optionAttachments.length > 0 ? optionAttachments diff --git a/src/channels/discord.test.ts b/src/channels/discord.test.ts index cdf4fc6..a52377d 100644 --- a/src/channels/discord.test.ts +++ b/src/channels/discord.test.ts @@ -129,7 +129,11 @@ vi.mock('discord.js', () => { }; }); -import { DiscordChannel, DiscordChannelOpts } from './discord.js'; +import { + DiscordChannel, + DiscordChannelOpts, + chunkForDiscord, +} from './discord.js'; import { logger } from '../logger.js'; // --- Test helpers --- @@ -1160,3 +1164,127 @@ describe('channel properties', () => { expect(channel.name).toBe('discord'); }); }); + +// --- chunkForDiscord --- + +describe('chunkForDiscord', () => { + it('returns the input unchanged when within the limit', () => { + const text = 'short message'; + expect(chunkForDiscord(text, 2000)).toEqual([text]); + }); + + it('returns empty array for empty input', () => { + expect(chunkForDiscord('', 2000)).toEqual([]); + }); + + it('splits on newline boundaries when possible', () => { + const text = 'aaa\nbbb\nccc\nddd'; + const chunks = chunkForDiscord(text, 8); + // Every chunk must be ≤ 8 chars and joining them must rebuild the text + for (const c of chunks) expect(c.length).toBeLessThanOrEqual(8); + expect(chunks.join('')).toBe(text); + }); + + it('reopens a fenced code block across chunks (with language)', () => { + const code = Array.from({ length: 200 }, (_, i) => `line ${i}`).join('\n'); + const text = 'intro paragraph.\n\n```ts\n' + code + '\n```\nafter'; + const chunks = chunkForDiscord(text, 500); + expect(chunks.length).toBeGreaterThan(1); + for (const c of chunks) expect(c.length).toBeLessThanOrEqual(500); + // Every chunk must have a balanced number of ``` fences (so it renders + // as valid markdown on its own). + for (const c of chunks) { + const fenceMatches = c.match(/^```/gm) ?? []; + expect(fenceMatches.length % 2).toBe(0); + } + // The middle chunks must reopen the same language. + for (let i = 1; i < chunks.length; i++) { + // It should either start with the reopened fence, or never have been + // inside the fence at this point. + if (chunks[i].startsWith('```')) { + expect(chunks[i].startsWith('```ts\n')).toBe(true); + } + } + }); + + it('reopens a language-less fenced code block', () => { + const code = Array.from({ length: 200 }, (_, i) => `payload-${i}`).join( + '\n', + ); + const text = '```\n' + code + '\n```'; + const chunks = chunkForDiscord(text, 400); + expect(chunks.length).toBeGreaterThan(1); + for (const c of chunks) { + const fenceMatches = c.match(/^```/gm) ?? []; + expect(fenceMatches.length % 2).toBe(0); + } + // Joining back should rebuild a string whose un-fenced content equals + // the original code (allowing for extra fence pairs in the middle). + const reconstructed = chunks + .join('\n') + .replace(/```\n```\n/g, '') // drop reopen-close pairs at chunk seams + .replace(/```\n```$/g, ''); + expect(reconstructed.includes('payload-0')).toBe(true); + expect(reconstructed.includes('payload-199')).toBe(true); + }); + + it('handles multiple fenced blocks in the same document', () => { + const blockA = Array.from({ length: 50 }, () => 'AAAA').join('\n'); + const blockB = Array.from({ length: 50 }, () => 'BBBB').join('\n'); + const text = + 'intro\n```js\n' + blockA + '\n```\nbetween\n```py\n' + blockB + '\n```'; + const chunks = chunkForDiscord(text, 200); + for (const c of chunks) expect(c.length).toBeLessThanOrEqual(200); + for (const c of chunks) { + const fenceMatches = c.match(/^```/gm) ?? []; + expect(fenceMatches.length % 2).toBe(0); + } + }); + + it('does not split a surrogate pair', () => { + // Emoji 🦄 is a surrogate pair (length 2 in JS). + const emoji = '🦄'; + const text = emoji.repeat(50); + const chunks = chunkForDiscord(text, 9); + for (const c of chunks) { + // No chunk should start or end mid surrogate pair. + expect(c.charCodeAt(0) >= 0xdc00 && c.charCodeAt(0) <= 0xdfff).toBe( + false, + ); + const last = c.charCodeAt(c.length - 1); + expect(last >= 0xd800 && last <= 0xdbff).toBe(false); + } + expect(chunks.join('')).toBe(text); + }); + + it('falls back gracefully when a single line exceeds maxLength', () => { + const longLine = 'x'.repeat(5000); + const chunks = chunkForDiscord(longLine, 2000); + for (const c of chunks) expect(c.length).toBeLessThanOrEqual(2000); + expect(chunks.join('')).toBe(longLine); + }); + + it('keeps a closing fence with its block when there is room', () => { + const text = '```ts\nshort code\n```'; + const chunks = chunkForDiscord(text, 2000); + expect(chunks).toEqual([text]); + }); + + it('preserves total content across chunk seams (modulo reopen overhead)', () => { + const code = Array.from( + { length: 80 }, + (_, i) => `console.log(${i});`, + ).join('\n'); + const text = '```js\n' + code + '\n```'; + const chunks = chunkForDiscord(text, 300); + // After removing the reopen-close fence pairs that the chunker inserts + // between chunks, the result should match the original text. + const stripped = chunks.join('\n').replace(/```\n```js\n/g, ''); + // The first chunk still has the original opener and the last still has + // the original closer. + expect(stripped.startsWith('```js\n')).toBe(true); + expect(stripped.endsWith('```')).toBe(true); + expect(stripped.includes('console.log(0);')).toBe(true); + expect(stripped.includes('console.log(79);')).toBe(true); + }); +}); diff --git a/src/db/bootstrap.test.ts b/src/db/bootstrap.test.ts index 6b9e7f9..37b62f7 100644 --- a/src/db/bootstrap.test.ts +++ b/src/db/bootstrap.test.ts @@ -6,8 +6,17 @@ import { afterEach, describe, expect, it } from 'vitest'; import { applyBaseSchema } from './base-schema.js'; import { initializeDatabaseSchema } from './bootstrap.js'; +import { applyVersionedSchemaMigrations } from './migrations/index.js'; import { applyLegacySchemaMigrations } from './schema.js'; +function tableColumns(database: Database, table: string): string[] { + return ( + database.prepare(`PRAGMA table_info(${table})`).all() as Array<{ + name: string; + }> + ).map((column) => column.name); +} + function getAppliedSchemaMigrations( database: Database, ): Array<{ version: number; name: string }> { @@ -45,6 +54,8 @@ function getExpectedSchemaMigrations(): Array<{ { version: 18, name: 'paired_turn_output_attachments' }, { version: 19, name: 'turn_progress_text_recovery' }, { version: 20, name: 'arbiter_intervention_count' }, + { version: 21, name: 'reviewer_failure_count' }, + { version: 22, name: 'turn_progress_text_compat' }, ]; } @@ -115,4 +126,55 @@ describe('initializeDatabaseSchema', () => { reopened.close(); } }); + + it('backfills turn_progress_text columns when version 15 was recorded under a different name', () => { + const database = new Database(':memory:'); + + try { + applyBaseSchema(database); + + // Reproduce a deployment that first shipped reviewer_failure_count as a + // local migration numbered 15, before turn_progress_text claimed that + // version upstream. The runner skips by version number, so migration 015 + // (turn_progress_text) would otherwise never run on this database. + database.exec(` + CREATE TABLE schema_migrations ( + version INTEGER PRIMARY KEY, + name TEXT NOT NULL, + applied_at TEXT NOT NULL DEFAULT (datetime('now')) + ) + `); + const collidedNames: Record = { + 15: 'reviewer_failure_count', + }; + for (let version = 1; version <= 15; version += 1) { + database + .prepare('INSERT INTO schema_migrations (version, name) VALUES (?, ?)') + .run(version, collidedNames[version] ?? `legacy_${version}`); + } + // Precondition: the collided database is missing the progress columns. + expect(tableColumns(database, 'paired_turns')).not.toContain( + 'progress_text', + ); + + applyVersionedSchemaMigrations(database, { assistantName: 'Andy' }); + + const columns = tableColumns(database, 'paired_turns'); + expect(columns).toContain('progress_text'); + expect(columns).toContain('progress_updated_at'); + + // The pre-existing version-15 row is left untouched; the compat migration + // (version 20) is what reconciles the schema. + const version15 = database + .prepare('SELECT name FROM schema_migrations WHERE version = 15') + .get() as { name: string }; + expect(version15.name).toBe('reviewer_failure_count'); + const version20 = database + .prepare('SELECT name FROM schema_migrations WHERE version = 20') + .get() as { name: string } | null; + expect(version20?.name).toBe('turn_progress_text_compat'); + } finally { + database.close(); + } + }); }); diff --git a/src/db/migrations/021_reviewer-failure-count.ts b/src/db/migrations/021_reviewer-failure-count.ts new file mode 100644 index 0000000..0ed36a6 --- /dev/null +++ b/src/db/migrations/021_reviewer-failure-count.ts @@ -0,0 +1,23 @@ +import type { Database } from 'bun:sqlite'; + +import { tableHasColumn } from './helpers.js'; +import type { SchemaMigrationDefinition } from './types.js'; + +export const REVIEWER_FAILURE_COUNT_MIGRATION: SchemaMigrationDefinition = { + version: 21, + name: 'reviewer_failure_count', + apply(database: Database) { + if (!tableHasColumn(database, 'paired_tasks', 'reviewer_failure_count')) { + database.exec(` + ALTER TABLE paired_tasks + ADD COLUMN reviewer_failure_count INTEGER NOT NULL DEFAULT 0 + `); + } + + database.exec(` + UPDATE paired_tasks + SET reviewer_failure_count = 0 + WHERE reviewer_failure_count IS NULL + `); + }, +}; diff --git a/src/db/migrations/022_turn-progress-text-compat.ts b/src/db/migrations/022_turn-progress-text-compat.ts new file mode 100644 index 0000000..616a034 --- /dev/null +++ b/src/db/migrations/022_turn-progress-text-compat.ts @@ -0,0 +1,32 @@ +import type { Database } from 'bun:sqlite'; + +import { tableHasColumn } from './helpers.js'; +import type { SchemaMigrationDefinition } from './types.js'; + +/** + * Compatibility backfill for `turn_progress_text` (migration 015). + * + * Some deployments first shipped `reviewer_failure_count` as a local migration + * numbered 15, so their `schema_migrations` table records version 15 under a + * different name than the canonical `turn_progress_text` migration. The runner + * skips migrations purely by version number, so on those databases the real + * version-15 migration never runs and `paired_turns.progress_text` / + * `progress_updated_at` are missing — yet the runtime reads them. This migration + * re-adds the columns idempotently so collided databases converge with fresh + * ones. On a fresh database migration 015 already created the columns, so the + * `tableHasColumn` guards make this a no-op. + */ +export const TURN_PROGRESS_TEXT_COMPAT_MIGRATION: SchemaMigrationDefinition = { + version: 22, + name: 'turn_progress_text_compat', + apply(database: Database) { + if (!tableHasColumn(database, 'paired_turns', 'progress_text')) { + database.exec(`ALTER TABLE paired_turns ADD COLUMN progress_text TEXT`); + } + if (!tableHasColumn(database, 'paired_turns', 'progress_updated_at')) { + database.exec( + `ALTER TABLE paired_turns ADD COLUMN progress_updated_at TEXT`, + ); + } + }, +}; diff --git a/src/db/migrations/index.ts b/src/db/migrations/index.ts index 533fcec..d3b134e 100644 --- a/src/db/migrations/index.ts +++ b/src/db/migrations/index.ts @@ -20,6 +20,8 @@ import { SCHEDULED_TASK_ROOM_ROLE_MIGRATION } from './017_scheduled-task-room-ro import { PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION } from './018_paired-turn-output-attachments.js'; import { TURN_PROGRESS_TEXT_RECOVERY_MIGRATION } from './019_turn-progress-text-recovery.js'; import { ARBITER_INTERVENTION_COUNT_MIGRATION } from './020_arbiter-intervention-count.js'; +import { REVIEWER_FAILURE_COUNT_MIGRATION } from './021_reviewer-failure-count.js'; +import { TURN_PROGRESS_TEXT_COMPAT_MIGRATION } from './022_turn-progress-text-compat.js'; import type { SchemaMigrationArgs, SchemaMigrationDefinition, @@ -48,6 +50,8 @@ const ORDERED_SCHEMA_MIGRATIONS: readonly SchemaMigrationDefinition[] = [ PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION, TURN_PROGRESS_TEXT_RECOVERY_MIGRATION, ARBITER_INTERVENTION_COUNT_MIGRATION, + REVIEWER_FAILURE_COUNT_MIGRATION, + TURN_PROGRESS_TEXT_COMPAT_MIGRATION, ]; function ensureSchemaMigrationsTable(database: Database): void { diff --git a/src/db/paired-state.ts b/src/db/paired-state.ts index d873ede..12197b1 100644 --- a/src/db/paired-state.ts +++ b/src/db/paired-state.ts @@ -50,6 +50,7 @@ export type PairedTaskUpdates = Partial< | 'review_requested_at' | 'round_trip_count' | 'owner_failure_count' + | 'reviewer_failure_count' | 'owner_step_done_streak' | 'finalize_step_done_count' | 'task_done_then_user_reopen_count' @@ -177,6 +178,7 @@ export function createPairedTaskInDatabase( review_requested_at, round_trip_count, owner_failure_count, + reviewer_failure_count, owner_step_done_streak, finalize_step_done_count, task_done_then_user_reopen_count, @@ -207,6 +209,7 @@ export function createPairedTaskInDatabase( task.review_requested_at, task.round_trip_count, task.owner_failure_count ?? 0, + task.reviewer_failure_count ?? 0, task.owner_step_done_streak ?? 0, task.finalize_step_done_count ?? 0, task.task_done_then_user_reopen_count ?? 0, @@ -342,6 +345,10 @@ export function updatePairedTaskInDatabase( fields.push('owner_failure_count = ?'); values.push(updates.owner_failure_count); } + if (updates.reviewer_failure_count !== undefined) { + fields.push('reviewer_failure_count = ?'); + values.push(updates.reviewer_failure_count); + } if (updates.owner_step_done_streak !== undefined) { fields.push('owner_step_done_streak = ?'); values.push(updates.owner_step_done_streak); @@ -425,6 +432,10 @@ export function updatePairedTaskIfUnchangedInDatabase( fields.push('owner_failure_count = ?'); values.push(updates.owner_failure_count); } + if (updates.reviewer_failure_count !== undefined) { + fields.push('reviewer_failure_count = ?'); + values.push(updates.reviewer_failure_count); + } if (updates.owner_step_done_streak !== undefined) { fields.push('owner_step_done_streak = ?'); values.push(updates.owner_step_done_streak); diff --git a/src/index.ts b/src/index.ts index 1aaabf5..7a0dd62 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,7 +34,7 @@ import { deliverCanonicalOutboundMessage, deliverIpcOutboundMessage, } from './ipc-outbound-delivery.js'; -import { findChannel, formatOutbound } from './router.js'; +import { findChannel, sanitizeForOutbound } from './router.js'; import { buildRestartAnnouncement, buildInterruptedRestartAnnouncement, @@ -55,6 +55,7 @@ import { nudgeSchedulerLoop, startSchedulerLoop } from './task-scheduler.js'; import { startUnifiedDashboard } from './unified-dashboard.js'; import { startUsagePrimer } from './usage-primer.js'; import { startWebDashboardServer } from './web-dashboard-server.js'; +import { startUsagePrimer } from './usage-primer.js'; import { Channel, NewMessage, RegisteredGroup } from './types.js'; import { logger } from './logger.js'; import { initCodexTokenRotation } from './codex-token-rotation.js'; @@ -99,7 +100,7 @@ export async function sendFormattedTrackedChannelMessage( logger.warn({ jid }, 'No channel owns JID, cannot send tracked message'); return null; } - const text = formatOutbound(rawText); + const text = sanitizeForOutbound(rawText); if (!text || !channel.sendAndTrack) return null; return channel.sendAndTrack(jid, text); } @@ -115,7 +116,7 @@ export async function editFormattedTrackedChannelMessage( logger.warn({ jid }, 'No channel owns JID, cannot edit tracked message'); return; } - const text = formatOutbound(rawText); + const text = sanitizeForOutbound(rawText); if (!text || !channel.editMessage) return; await channel.editMessage(jid, messageId, text); } @@ -146,7 +147,7 @@ async function deliverFormattedCanonicalMessage( rawText: string, deliveryRole?: 'owner' | 'reviewer' | 'arbiter', ): Promise { - const text = formatOutbound(rawText); + const text = sanitizeForOutbound(rawText); if (!text) return; await deliverCanonicalOutboundMessage( { jid, text, deliveryRole }, diff --git a/src/message-turn-controller.ts b/src/message-turn-controller.ts index 6232d59..d0a2a23 100644 --- a/src/message-turn-controller.ts +++ b/src/message-turn-controller.ts @@ -4,7 +4,7 @@ import { getAgentOutputText, } from './agent-output.js'; import { createScopedLogger, logger } from './logger.js'; -import { formatOutbound } from './router.js'; +import { sanitizeForOutbound } from './router.js'; import { shouldResetSessionOnAgentFailure } from './session-recovery.js'; import { TASK_STATUS_MESSAGE_PREFIX } from './task-watch-status.js'; import { formatElapsedKorean } from './utils.js'; @@ -169,7 +169,10 @@ export class MessageTurnController { } const raw = getAgentOutputText(result); - const text = raw ? formatOutbound(raw) : null; + // Use sanitize (no markdown escape) — the Discord channel boundary + // applies the final escape pass, and double-escaping would produce + // visible backslash garbage. + const text = raw ? sanitizeForOutbound(raw) : null; const attachments = getAgentOutputAttachments(result); if (raw) { diff --git a/src/paired-arbiter-request.ts b/src/paired-arbiter-request.ts index 83dfab0..9f573a0 100644 --- a/src/paired-arbiter-request.ts +++ b/src/paired-arbiter-request.ts @@ -25,6 +25,7 @@ export function requestArbiterOrEscalate(args: { review_requested_at?: string | null; round_trip_count?: number; owner_failure_count?: number; + reviewer_failure_count?: number; owner_step_done_streak?: number; finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index 3dcf641..441ec73 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -399,6 +399,7 @@ function maybeAutoTriggerReviewerAfterOwnerCompletion(args: { patch: { round_trip_count: task.round_trip_count + 1, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, ...args.patch, diff --git a/src/paired-execution-context-reviewer.ts b/src/paired-execution-context-reviewer.ts index aaca85c..de93e5f 100644 --- a/src/paired-execution-context-reviewer.ts +++ b/src/paired-execution-context-reviewer.ts @@ -3,7 +3,10 @@ import { getPairedWorkspace } from './db.js'; import { isTerminalCodexAccountFailure } from './agent-error-detection.js'; import { logger } from './logger.js'; import { requestArbiterOrEscalate } from './paired-arbiter-request.js'; -import { transitionPairedTaskStatus } from './paired-task-status.js'; +import { + applyPairedTaskPatch, + transitionPairedTaskStatus, +} from './paired-task-status.js'; import { resolveReviewerCompletionSignal, resolveReviewerFailureSignal, @@ -12,6 +15,17 @@ import { resolveCanonicalSourceRef } from './paired-source-ref.js'; import { parseVisibleVerdict } from './paired-verdict.js'; import type { PairedTask } from './types.js'; +/** + * Maximum consecutive reviewer executions that may produce no verdict + * before we stop the ping-pong loop. Mirrors the owner-side failure cap and + * closes the "reviewer silently fails forever" hole: codex sometimes emits a + * final result with no visible text, the runtime marks it failed, the + * next-turn scheduler re-queues another reviewer-turn, and the original + * round_trip_count never increments — so the existing round-trip / arbiter + * caps never fire. + */ +const REVIEWER_FAILURE_ESCALATION_THRESHOLD = 2; + export function handleFailedReviewerExecution(args: { task: PairedTask; taskId: string; @@ -104,6 +118,35 @@ export function handleFailedReviewerExecution(args: { } } + // Reviewer produced no usable verdict and no recognised infrastructure + // failure. Track consecutive flakes — after the threshold, stop the + // ping-pong loop instead of preserving review_ready forever. + const nextFailureCount = (task.reviewer_failure_count ?? 0) + 1; + + if (nextFailureCount >= REVIEWER_FAILURE_ESCALATION_THRESHOLD) { + requestArbiterOrEscalate({ + taskId, + currentStatus: task.status, + expectedUpdatedAt: task.updated_at, + now, + arbiterLogMessage: + 'Reviewer failed repeatedly without a visible verdict — requesting arbiter', + escalateLogMessage: + 'Reviewer failed repeatedly without a visible verdict — escalating to user', + logContext: { + taskId, + role: 'reviewer', + previousStatus: task.status, + reviewerFailureCount: nextFailureCount, + summary: summary?.slice(0, 160), + }, + patch: { + reviewer_failure_count: nextFailureCount, + }, + }); + return; + } + const fallbackStatus = task.status === 'in_review' || task.status === 'review_ready' ? 'review_ready' @@ -115,17 +158,30 @@ export function handleFailedReviewerExecution(args: { nextStatus: fallbackStatus, expectedUpdatedAt: task.updated_at, updatedAt: now, - }); - logger.warn( - { - taskId, - role: 'reviewer', - previousStatus: task.status, - nextStatus: fallbackStatus, + patch: { + reviewer_failure_count: nextFailureCount, }, - 'Preserved reviewer task in review-ready state after failed execution', - ); + }); + } else { + applyPairedTaskPatch({ + taskId, + expectedUpdatedAt: task.updated_at, + updatedAt: now, + patch: { + reviewer_failure_count: nextFailureCount, + }, + }); } + logger.warn( + { + taskId, + role: 'reviewer', + previousStatus: task.status, + reviewerFailureCount: nextFailureCount, + summary: summary?.slice(0, 160), + }, + 'Preserved reviewer task in review-ready state after failed execution', + ); } export function handleReviewerCompletion(args: { @@ -157,6 +213,7 @@ export function handleReviewerCompletion(args: { patch: { source_ref: approvedSourceRef, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, finalize_step_done_count: 0, empty_step_done_streak: 0, @@ -194,6 +251,7 @@ export function handleReviewerCompletion(args: { escalateLogMessage, logContext: { taskId, verdict, summary: summary?.slice(0, 100) }, arbiterInterventionCount: task.arbiter_intervention_count ?? 0, + patch: { reviewer_failure_count: 0 }, }); return; } @@ -205,6 +263,7 @@ export function handleReviewerCompletion(args: { nextStatus: 'active', expectedUpdatedAt: task.updated_at, updatedAt: now, + patch: { reviewer_failure_count: 0 }, }); logger.info( { taskId, verdict }, diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index 45e5843..b3c0f49 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -143,6 +143,7 @@ function buildPairedTask(overrides: Partial = {}): PairedTask { review_requested_at: null, round_trip_count: 0, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, finalize_step_done_count: 0, task_done_then_user_reopen_count: 0, @@ -1398,7 +1399,7 @@ describe('paired execution context', () => { ); }); - it('keeps reviewer tasks review_ready when reviewer execution fails without a terminal verdict', () => { + it('keeps reviewer tasks review_ready and increments reviewer_failure_count after the first silent failure', () => { vi.mocked(db.getPairedTaskById).mockReturnValue( buildPairedTask({ status: 'in_review', @@ -1416,11 +1417,38 @@ describe('paired execution context', () => { 'task-1', expect.objectContaining({ status: 'review_ready', + reviewer_failure_count: 1, updated_at: expect.any(String), }), ); }); + it('requests arbiter after repeated reviewer execution failures without a visible verdict', () => { + vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true); + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'review_ready', + reviewer_failure_count: 1, + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'reviewer', + status: 'failed', + summary: 'runtime exploded before verdict', + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + status: 'arbiter_requested', + reviewer_failure_count: 2, + arbiter_requested_at: expect.any(String), + }), + ); + }); + it('keeps arbiter tasks arbiter_requested when arbiter execution fails without a terminal verdict', () => { vi.mocked(db.getPairedTaskById).mockReturnValue( buildPairedTask({ diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index f509aa6..543e501 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -141,6 +141,7 @@ function createActiveTaskForRoom(args: { review_requested_at: null, round_trip_count: 0, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, finalize_step_done_count: 0, task_done_then_user_reopen_count: 0, @@ -493,6 +494,7 @@ export function preparePairedExecutionContext(args: { ? { round_trip_count: 0, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, arbiter_intervention_count: 0, @@ -510,6 +512,7 @@ export function preparePairedExecutionContext(args: { ? { round_trip_count: 0, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, arbiter_intervention_count: 0, diff --git a/src/paired-task-status.ts b/src/paired-task-status.ts index 848c799..c0e2ea2 100644 --- a/src/paired-task-status.ts +++ b/src/paired-task-status.ts @@ -62,6 +62,7 @@ export function transitionPairedTaskStatus(args: { review_requested_at?: string | null; round_trip_count?: number; owner_failure_count?: number; + reviewer_failure_count?: number; owner_step_done_streak?: number; finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; @@ -111,6 +112,7 @@ export function applyPairedTaskPatch(args: { review_requested_at?: string | null; round_trip_count?: number; owner_failure_count?: number; + reviewer_failure_count?: number; owner_step_done_streak?: number; finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; diff --git a/src/router.test.ts b/src/router.test.ts index f97b6ca..4127490 100644 --- a/src/router.test.ts +++ b/src/router.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from 'vitest'; import { findChannelForDeliveryRole, + neutralizeStrayBackticks, + neutralizeStrayMarkdown, resolveChannelForDeliveryRole, } from './router.js'; import { type Channel } from './types.js'; @@ -92,3 +94,134 @@ describe('findChannelForDeliveryRole', () => { ); }); }); + +describe('neutralizeStrayMarkdown', () => { + it('escapes inline backticks so they render literally', () => { + expect(neutralizeStrayMarkdown('use `foo` and `bar` here')).toBe( + 'use \\`foo\\` and \\`bar\\` here', + ); + }); + + it('preserves well-formed fenced code blocks and escapes outside prose', () => { + const input = 'before\n```ts\nconst x = `tpl`;\n```\nafter `inline`'; + expect(neutralizeStrayMarkdown(input)).toBe( + 'before\n```ts\nconst x = `tpl`;\n```\nafter \\`inline\\`', + ); + }); + + it('escapes emphasis in an unterminated fence (treated as prose)', () => { + expect(neutralizeStrayMarkdown('oops ```code without close')).toBe( + 'oops \\`\\`\\`code without close', + ); + }); + + it('passes through plain text unchanged', () => { + const s = 'plain text with no markup'; + expect(neutralizeStrayMarkdown(s)).toBe(s); + }); + + it('escapes italic asterisks', () => { + expect(neutralizeStrayMarkdown('이건 *기울임* 처리')).toBe( + '이건 \\*기울임\\* 처리', + ); + }); + + it('escapes bold asterisks', () => { + expect(neutralizeStrayMarkdown('이건 **강조** 처리')).toBe( + '이건 \\*\\*강조\\*\\* 처리', + ); + }); + + it('escapes bold-italic triple asterisks', () => { + expect(neutralizeStrayMarkdown('***hybrid***')).toBe( + '\\*\\*\\*hybrid\\*\\*\\*', + ); + }); + + it('escapes italic underscores at word boundaries', () => { + expect(neutralizeStrayMarkdown('이건 _italic_ 처리')).toBe( + '이건 \\_italic\\_ 처리', + ); + }); + + it('escapes every underscore including snake_case identifiers', () => { + expect(neutralizeStrayMarkdown('use foo_bar_baz here')).toBe( + 'use foo\\_bar\\_baz here', + ); + }); + + it('escapes underscores in absolute paths', () => { + expect( + neutralizeStrayMarkdown( + '경로: /home/claude/EJClaw/groups/mc_the_scene_plugin/src/main_file.ts', + ), + ).toBe( + '경로: /home/claude/EJClaw/groups/mc\\_the\\_scene\\_plugin/src/main\\_file.ts', + ); + }); + + it('escapes underscores and backslashes in Windows paths', () => { + expect( + neutralizeStrayMarkdown( + '경로: C:\\Custom_vscode\\minecraft_launcher\\src\\main_file.ts', + ), + ).toBe( + '경로: C:\\\\Custom\\_vscode\\\\minecraft\\_launcher\\\\src\\\\main\\_file.ts', + ); + }); + + it('escapes double-underscore underline markers', () => { + expect(neutralizeStrayMarkdown('__bold__ text')).toBe( + '\\_\\_bold\\_\\_ text', + ); + }); + + it('escapes heading hashes at line start', () => { + expect(neutralizeStrayMarkdown('## 빌드 타임 메타\n본문')).toBe( + '\\## 빌드 타임 메타\n본문', + ); + }); + + it('escapes block-quote markers at line start', () => { + expect(neutralizeStrayMarkdown('> quoted line\n> another')).toBe( + '\\> quoted line\n\\> another', + ); + }); + + it('leaves mid-line hash and gt unchanged (not heading/quote markers)', () => { + expect(neutralizeStrayMarkdown('a #tag here and a > b')).toBe( + 'a #tag here and a > b', + ); + }); + + it('escapes strikethrough tildes', () => { + expect(neutralizeStrayMarkdown('이건 ~~취소선~~ 입니다')).toBe( + '이건 \\~\\~취소선\\~\\~ 입니다', + ); + }); + + it('escapes spoiler pipes', () => { + expect(neutralizeStrayMarkdown('||비밀||')).toBe('\\|\\|비밀\\|\\|'); + }); + + it('preserves Discord mention syntax (no markdown delimiters inside)', () => { + expect(neutralizeStrayMarkdown('<@123> hi <#456>')).toBe( + '<@123> hi <#456>', + ); + }); + + it('preserves code fence content while escaping prose emphasis', () => { + const input = '**bold** outside\n```py\nx = *2\n```\nand *more*'; + expect(neutralizeStrayMarkdown(input)).toBe( + '\\*\\*bold\\*\\* outside\n```py\nx = *2\n```\nand \\*more\\*', + ); + }); + + it('escapes literal backslashes first so new escapes are not doubled', () => { + expect(neutralizeStrayMarkdown('a\\b *c*')).toBe('a\\\\b \\*c\\*'); + }); + + it('back-compat alias still exported', () => { + expect(neutralizeStrayBackticks('*x*')).toBe('\\*x\\*'); + }); +}); diff --git a/src/router.ts b/src/router.ts index 5cbc161..fcf5d65 100644 --- a/src/router.ts +++ b/src/router.ts @@ -78,7 +78,85 @@ export function stripToolCallLeaks(text: string): string { return stripped.replace(/\n{3,}/g, '\n\n').trim(); } -export function formatOutbound(rawText: string): string { +/** + * Escape Discord markdown delimiters in a prose segment so the source + * characters render literally instead of triggering formatting. Used on + * non-code segments only — callers must preserve fenced code blocks + * separately. + * + * Discord rendering reference: + * *italic*, _italic_, **bold**, ***bold-italic*** + * __underline__ ~~strike~~ ||spoiler|| + * `inline` # H1 ## H2 ### H3 (at line start) + * > quote, >>> multi-line quote (at line start) + * + * `<@id>`/`<#id>`/`<:emoji:id>` mentions and `[text](url)` links contain no + * markdown delimiters and pass through unchanged. + */ +function escapeMarkdownInProse(segment: string): string { + return ( + segment + // Escape backslashes first so we don't double-escape the backslashes + // we are about to introduce for the other markers. + .replace(/\\/g, '\\\\') + // Inline markdown delimiters — escape positionally so Discord prints + // them as literal characters. + .replace(/`/g, '\\`') + .replace(/\*/g, '\\*') + .replace(/_/g, '\\_') + .replace(/~/g, '\\~') + .replace(/\|/g, '\\|') + // Heading hashes — only meaningful at the start of a line (after + // optional indent) and followed by a space. Escape only the first + // hash; the rest are now harmless literal characters. + .replace(/^([ \t]*)(#)(?=#{0,2}[ \t])/gm, '$1\\$2') + // Block-quote markers — same line-start constraint. + .replace(/^([ \t]*)(>)(?=>{0,2}([ \t]|$))/gm, '$1\\$2') + ); +} + +/** + * Escape stray Discord markdown delimiters in prose while preserving + * well-formed triple-backtick fenced code blocks (legit code snippets). + */ +export function neutralizeStrayMarkdown(text: string): string { + if (!text) return text; + const parts: string[] = []; + let i = 0; + while (i < text.length) { + const fenceStart = text.indexOf('```', i); + if (fenceStart === -1) { + parts.push(escapeMarkdownInProse(text.slice(i))); + break; + } + parts.push(escapeMarkdownInProse(text.slice(i, fenceStart))); + const fenceEnd = text.indexOf('```', fenceStart + 3); + if (fenceEnd === -1) { + // Unterminated fence — not a real code block; treat as prose. + parts.push(escapeMarkdownInProse(text.slice(fenceStart))); + break; + } + // Preserve the entire fenced block including delimiters. + parts.push(text.slice(fenceStart, fenceEnd + 3)); + i = fenceEnd + 3; + } + return parts.join(''); +} + +/** @deprecated Kept for back-compat; use neutralizeStrayMarkdown. */ +export const neutralizeStrayBackticks = neutralizeStrayMarkdown; + +/** + * Sanitize raw agent output for internal use (storage, IPC, intermediate + * channel buffers). Strips internal tags + tool-call leaks and redacts + * secrets, but does NOT touch markdown delimiters. + * + * Use this when the text will pass through another `formatOutbound` call + * downstream (e.g., the Discord channel boundary). Applying the markdown + * escape twice would double-escape backslashes and produce visible garbage + * in Discord. + */ +export function sanitizeForOutbound(rawText: string): string { let text = stripInternalTags(rawText); if (!text) return ''; text = stripToolCallLeaks(text); @@ -86,6 +164,17 @@ export function formatOutbound(rawText: string): string { return redactSecrets(text); } +/** + * Full outbound formatting for the final Discord-send boundary: sanitize + * + escape Discord markdown delimiters. Call this exactly once per + * outbound message, at the channel boundary. + */ +export function formatOutbound(rawText: string): string { + const sanitized = sanitizeForOutbound(rawText); + if (!sanitized) return ''; + return neutralizeStrayMarkdown(sanitized); +} + export function findChannel( channels: Channel[], jid: string, diff --git a/src/session-commands.ts b/src/session-commands.ts index 1a2fd5f..3bbd0b9 100644 --- a/src/session-commands.ts +++ b/src/session-commands.ts @@ -1,7 +1,7 @@ import { getAgentOutputText } from './agent-output.js'; import type { NewMessage } from './types.js'; import { logger } from './logger.js'; -import { formatOutbound } from './router.js'; +import { sanitizeForOutbound } from './router.js'; import type { StructuredAgentOutput } from './types.js'; const SESSION_COMMAND_CONTROL_PATTERNS = [ @@ -86,7 +86,7 @@ function agentResultToText(result: AgentResult): string { result: result.result ?? null, output: result.output, }); - return raw ? formatOutbound(raw) : ''; + return raw ? sanitizeForOutbound(raw) : ''; } /** diff --git a/src/types.ts b/src/types.ts index 9ef922c..c9bc531 100644 --- a/src/types.ts +++ b/src/types.ts @@ -128,6 +128,7 @@ export interface PairedTask { review_requested_at: string | null; round_trip_count: number; owner_failure_count?: number | null; + reviewer_failure_count?: number | null; owner_step_done_streak?: number | null; finalize_step_done_count?: number | null; task_done_then_user_reopen_count?: number | null; diff --git a/src/unified-dashboard.ts b/src/unified-dashboard.ts index e9c0280..17c56b2 100644 --- a/src/unified-dashboard.ts +++ b/src/unified-dashboard.ts @@ -51,7 +51,7 @@ import { mergeClaudeDashboardAccounts, type UsageRow, } from './dashboard-usage-rows.js'; -import { getAllChats, getAllTasks, updateRegisteredGroupName } from './db.js'; +import { getAllChats, getAllTasks } from './db.js'; import type { GroupQueue } from './group-queue.js'; import { logger } from './logger.js'; import { isWatchCiTask } from './task-watch-status.js'; @@ -208,12 +208,10 @@ async function refreshChannelMeta( if (!meta.name) continue; const group = opts.roomBindings()[jid]; if (!group || group.name === meta.name) continue; - logger.info( + logger.debug( { jid, oldName: group.name, newName: meta.name }, - 'Syncing group name to Discord channel name', + 'Keeping registered group name distinct from Discord channel name', ); - updateRegisteredGroupName(jid, meta.name); - opts.onGroupNameSynced?.(jid, meta.name); } } catch (err) { logger.debug({ err }, 'Failed to refresh channel metadata');