diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 815201c..9461d79 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -41,7 +41,10 @@ import { import { runAgentForGroup } from './message-agent-executor.js'; import { MessageTurnController } from './message-turn-controller.js'; import { createSuppressToken } from './output-suppression.js'; -import { markRoomReviewReady } from './paired-execution-context.js'; +import { + formatRoomReviewReadyMessage, + markRoomReviewReady, +} from './paired-execution-context.js'; import { buildRoomRoleContext } from './room-role-context.js'; import { extractSessionCommand, @@ -573,15 +576,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { chatJid, roomRoleContext, }); - if (!result) { - return null; - } - return [ - 'Review snapshot updated.', - `- Task: ${result.task.id}`, - `- Owner workspace: ${result.ownerWorkspace.workspace_dir}`, - `- Reviewer snapshot: ${result.reviewerWorkspace.workspace_dir}`, - ].join('\n'); + return formatRoomReviewReadyMessage(result); }, }, }); diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index 5e0c384..f9c6fa5 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -29,6 +29,7 @@ vi.mock('./logger.js', () => ({ import * as db from './db.js'; import { + formatRoomReviewReadyMessage, markRoomReviewReady, preparePairedExecutionContext, } from './paired-execution-context.js'; @@ -315,10 +316,14 @@ describe('paired execution context', () => { expect( pairedWorkspaceManager.markPairedTaskReviewReady, ).toHaveBeenCalledWith('task-1'); - expect(result?.task.status).toBe('review_ready'); + expect(result?.status).toBe('ready'); + if (result?.status !== 'ready') { + throw new Error('expected ready review result'); + } + expect(result.task.status).toBe('review_ready'); }); - it('returns null when review-ready setup cannot resolve an owner workspace', () => { + it('returns pending when review-ready setup cannot resolve an owner workspace', () => { vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ id: 'task-1', chat_jid: 'dc:test', @@ -332,6 +337,19 @@ describe('paired execution context', () => { created_at: '2026-03-28T00:00:00.000Z', updated_at: '2026-03-28T00:00:00.000Z', }); + vi.mocked(db.getPairedTaskById).mockReturnValue({ + id: 'task-1', + chat_jid: 'dc:test', + group_folder: group.folder, + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + review_requested_at: '2026-03-28T00:01:00.000Z', + status: 'review_pending', + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:01:00.000Z', + }); vi.mocked(pairedWorkspaceManager.markPairedTaskReviewReady).mockReturnValue( null, ); @@ -342,6 +360,39 @@ describe('paired execution context', () => { roomRoleContext: ownerContext, }); - expect(result).toBeNull(); + expect(result).toEqual( + expect.objectContaining({ + status: 'pending', + pendingReason: 'owner-workspace-not-ready', + task: expect.objectContaining({ + id: 'task-1', + status: 'review_pending', + }), + }), + ); + }); + + it('formats a pending review-ready result into a user-facing pending message', () => { + const message = formatRoomReviewReadyMessage({ + status: 'pending', + pendingReason: 'owner-workspace-not-ready', + task: { + id: 'task-1', + chat_jid: 'dc:test', + group_folder: group.folder, + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + review_requested_at: '2026-03-28T00:01:00.000Z', + status: 'review_pending', + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:01:00.000Z', + }, + }); + + expect(message).toBe( + 'Review request recorded, but the owner workspace is not ready yet.\n- Task: task-1\nThe task stays review_pending until the owner workspace is prepared.', + ); }); }); diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index e99ea6a..160be37 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -152,6 +152,19 @@ export interface PreparedPairedExecutionContext { blockMessage?: string; } +export type MarkRoomReviewReadyResult = + | { + status: 'ready'; + task: PairedTask; + ownerWorkspace: PairedWorkspace; + reviewerWorkspace: PairedWorkspace; + } + | { + status: 'pending'; + task: PairedTask; + pendingReason: 'owner-workspace-not-ready'; + }; + export function preparePairedExecutionContext(args: { group: RegisteredGroup; chatJid: string; @@ -231,11 +244,7 @@ export function markRoomReviewReady(args: { group: RegisteredGroup; chatJid: string; roomRoleContext?: RoomRoleContext; -}): { - task: PairedTask; - ownerWorkspace: PairedWorkspace; - reviewerWorkspace: PairedWorkspace; -} | null { +}): MarkRoomReviewReadyResult | null { const { group, chatJid, roomRoleContext } = args; if (!roomRoleContext || !group.workDir) { return null; @@ -253,14 +262,43 @@ export function markRoomReviewReady(args: { } const reviewReady = markPairedTaskReviewReady(task.id); + const latestTask = getPairedTaskById(task.id) ?? task; if (!reviewReady) { - return null; + return { + status: 'pending', + task: latestTask, + pendingReason: 'owner-workspace-not-ready', + }; } const { ownerWorkspace, reviewerWorkspace } = reviewReady; return { - task: getPairedTaskById(task.id) ?? task, + status: 'ready', + task: latestTask, ownerWorkspace, reviewerWorkspace, }; } + +export function formatRoomReviewReadyMessage( + result: MarkRoomReviewReadyResult | null, +): string | null { + if (!result) { + return null; + } + + if (result.status === 'pending') { + return [ + 'Review request recorded, but the owner workspace is not ready yet.', + `- Task: ${result.task.id}`, + 'The task stays review_pending until the owner workspace is prepared.', + ].join('\n'); + } + + return [ + 'Review snapshot updated.', + `- Task: ${result.task.id}`, + `- Owner workspace: ${result.ownerWorkspace.workspace_dir}`, + `- Reviewer snapshot: ${result.reviewerWorkspace.workspace_dir}`, + ].join('\n'); +} diff --git a/src/paired-review-command.test.ts b/src/paired-review-command.test.ts index c920bf2..80f3a1a 100644 --- a/src/paired-review-command.test.ts +++ b/src/paired-review-command.test.ts @@ -124,17 +124,74 @@ describe('paired /review command path', () => { 'owner', ); - expect(result).not.toBeNull(); - expect(result!.ownerWorkspace.workspace_dir).toBe(ownerWorkspaceDir); - expect(result!.reviewerWorkspace.snapshot_source_dir).toBe( + expect(result?.status).toBe('ready'); + if (result?.status !== 'ready') { + throw new Error('expected ready review result'); + } + expect(result.ownerWorkspace.workspace_dir).toBe(ownerWorkspaceDir); + expect(result.reviewerWorkspace.snapshot_source_dir).toBe( ownerWorkspaceDir, ); expect( fs.readFileSync( - path.join(result!.reviewerWorkspace.workspace_dir, 'README.md'), + path.join(result.reviewerWorkspace.workspace_dir, 'README.md'), 'utf-8', ), ).toBe('owner change\n'); expect(fs.existsSync(reviewerLocalOwnerDir)).toBe(false); }); + + it('returns pending and keeps review_pending when reviewer handles /review before any owner workspace exists', async () => { + const canonicalDir = path.join(tempRoot, 'canonical'); + fs.mkdirSync(canonicalDir, { recursive: true }); + runGit(['init'], canonicalDir); + runGit(['config', 'user.email', 'test@example.com'], canonicalDir); + runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir); + fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'original\n'); + runGit(['add', 'README.md'], canonicalDir); + runGit(['commit', '-m', 'initial'], canonicalDir); + + const group: RegisteredGroup = { + name: 'Paired Room', + folder: 'paired-room', + trigger: '@codex', + added_at: '2026-03-28T00:00:00.000Z', + agentType: 'codex', + workDir: canonicalDir, + }; + + process.env.EJCLAW_DATA_DIR = path.join(tempRoot, 'data-review'); + process.env.SERVICE_ID = 'codex-review'; + vi.resetModules(); + const { db, executionContext } = await loadModules(); + db.initDatabase(); + + const result = executionContext.markRoomReviewReady({ + group, + chatJid: 'dc:test', + roomRoleContext: reviewerContext, + }); + + const reviewerLocalOwnerDir = path.join( + tempRoot, + 'data-review', + 'workspaces', + 'paired-room', + 'tasks', + result?.task.id ?? 'missing-task', + 'owner', + ); + + expect(result).toEqual( + expect.objectContaining({ + status: 'pending', + pendingReason: 'owner-workspace-not-ready', + task: expect.objectContaining({ + status: 'review_pending', + }), + }), + ); + expect(result?.task.review_requested_at).toBeTruthy(); + expect(fs.existsSync(reviewerLocalOwnerDir)).toBe(false); + }); }); diff --git a/src/paired-workspace-manager.test.ts b/src/paired-workspace-manager.test.ts index e76ca81..f961932 100644 --- a/src/paired-workspace-manager.test.ts +++ b/src/paired-workspace-manager.test.ts @@ -132,6 +132,53 @@ describe('paired workspace manager', () => { ).toBeTruthy(); }); + it('keeps review_pending when /review is requested before an owner workspace exists', async () => { + const { db, manager } = await loadModules(); + db._initTestDatabase(); + + const canonicalDir = path.join(tempRoot, 'canonical'); + fs.mkdirSync(canonicalDir, { recursive: true }); + runGit(['init'], canonicalDir); + runGit(['config', 'user.email', 'test@example.com'], canonicalDir); + runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir); + fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'original\n'); + runGit(['add', 'README.md'], canonicalDir); + runGit(['commit', '-m', 'initial'], canonicalDir); + + const now = '2026-03-28T00:00:00.000Z'; + db.upsertPairedProject({ + chat_jid: 'dc:test', + group_folder: 'paired-room', + canonical_work_dir: canonicalDir, + workspace_topology: 'shadow-snapshot', + created_at: now, + updated_at: now, + }); + db.createPairedTask({ + id: 'paired-task-pending', + chat_jid: 'dc:test', + group_folder: 'paired-room', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + review_requested_at: null, + status: 'draft', + created_at: now, + updated_at: now, + }); + + const result = manager.markPairedTaskReviewReady('paired-task-pending'); + + expect(result).toBeNull(); + expect(db.getPairedTaskById('paired-task-pending')?.status).toBe( + 'review_pending', + ); + expect( + db.getPairedTaskById('paired-task-pending')?.review_requested_at, + ).toBeTruthy(); + }); + it('uses the shared DB owner workspace across service-local data dirs', async () => { const canonicalDir = path.join(tempRoot, 'canonical'); fs.mkdirSync(canonicalDir, { recursive: true }); @@ -602,6 +649,69 @@ describe('paired workspace manager', () => { ); }); + it('auto-refreshes once the owner workspace appears after review_pending was already recorded', async () => { + const { db, manager } = await loadModules(); + db._initTestDatabase(); + + const canonicalDir = path.join(tempRoot, 'canonical'); + fs.mkdirSync(canonicalDir, { recursive: true }); + runGit(['init'], canonicalDir); + runGit(['config', 'user.email', 'test@example.com'], canonicalDir); + runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir); + fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'base\n'); + runGit(['add', 'README.md'], canonicalDir); + runGit(['commit', '-m', 'initial'], canonicalDir); + + const now = '2026-03-28T00:00:00.000Z'; + db.upsertPairedProject({ + chat_jid: 'dc:test', + group_folder: 'paired-room', + canonical_work_dir: canonicalDir, + workspace_topology: 'shadow-snapshot', + created_at: now, + updated_at: now, + }); + db.createPairedTask({ + id: 'paired-task-5c', + chat_jid: 'dc:test', + group_folder: 'paired-room', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + review_requested_at: null, + status: 'draft', + created_at: now, + updated_at: now, + }); + + expect(manager.markPairedTaskReviewReady('paired-task-5c')).toBeNull(); + expect(db.getPairedTaskById('paired-task-5c')?.status).toBe( + 'review_pending', + ); + + const ownerWorkspace = + manager.provisionOwnerWorkspaceForPairedTask('paired-task-5c'); + fs.writeFileSync( + path.join(ownerWorkspace.workspace_dir, 'README.md'), + 'owner ready now\n', + ); + + const result = manager.prepareReviewerWorkspaceForExecution( + db.getPairedTaskById('paired-task-5c')!, + ); + + expect(result.autoRefreshed).toBe(true); + expect(result.blockMessage).toBeUndefined(); + expect( + fs.readFileSync( + path.join(result.workspace!.workspace_dir, 'README.md'), + 'utf-8', + ), + ).toBe('owner ready now\n'); + expect(db.getPairedTaskById('paired-task-5c')?.status).toBe('review_ready'); + }); + it('blocks once when an in-review snapshot becomes stale, then refreshes on retry', async () => { const { db, manager } = await loadModules(); db._initTestDatabase(); diff --git a/src/session-commands.test.ts b/src/session-commands.test.ts index c0c36d7..69ba849 100644 --- a/src/session-commands.test.ts +++ b/src/session-commands.test.ts @@ -91,6 +91,22 @@ describe('isSessionCommandControlMessage', () => { ).toBe(true); }); + it('matches multiline review-updated output', () => { + expect( + isSessionCommandControlMessage( + 'Review snapshot updated.\n- Task: paired-task-1', + ), + ).toBe(true); + }); + + it('matches pending review output', () => { + expect( + isSessionCommandControlMessage( + 'Review request recorded, but the owner workspace is not ready yet.\n- Task: paired-task-1\nThe task stays review_pending until the owner workspace is prepared.', + ), + ).toBe(true); + }); + it('does not match regular bot conversation', () => { expect( isSessionCommandControlMessage( @@ -237,6 +253,31 @@ describe('handleSessionCommand', () => { ); }); + it('sends a pending review message when owner workspace is not ready yet', async () => { + const deps = makeDeps({ + markReviewReady: vi + .fn() + .mockResolvedValue( + 'Review request recorded, but the owner workspace is not ready yet.\n- Task: paired-task-1\nThe task stays review_pending until the owner workspace is prepared.', + ), + }); + + const result = await handleSessionCommand({ + missedMessages: [makeMsg('/review')], + isMainGroup: true, + groupName: 'test', + triggerPattern: trigger, + timezone: 'UTC', + deps, + }); + + expect(result).toEqual({ handled: true, success: true }); + expect(deps.markReviewReady).toHaveBeenCalledTimes(1); + expect(deps.sendMessage).toHaveBeenCalledWith( + 'Review request recorded, but the owner workspace is not ready yet.\n- Task: paired-task-1\nThe task stays review_pending until the owner workspace is prepared.', + ); + }); + it('sends denial to interactable sender in non-main group', async () => { const deps = makeDeps(); const result = await handleSessionCommand({ diff --git a/src/session-commands.ts b/src/session-commands.ts index 6c57b99..6973dc6 100644 --- a/src/session-commands.ts +++ b/src/session-commands.ts @@ -10,7 +10,8 @@ const SESSION_COMMAND_CONTROL_PATTERNS = [ /^Failed to process messages before \/compact\. Try again\.$/, /^\/compact failed\. The session is unchanged\.$/, /^Conversation compacted\.$/, - /^Review snapshot updated\.$/, + /^Review snapshot updated\.(?:\n|$)/, + /^Review request recorded, but the owner workspace is not ready yet\.(?:\n|$)/, /^Review is unavailable for this room\./, ];