From 890991394dbad9ae8eb7c6d8153ae0e62e6dae80 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 27 May 2026 12:43:15 +0900 Subject: [PATCH] remove one-shot reviewer mode --- src/message-runtime.ts | 15 +-- src/one-shot-review.test.ts | 138 -------------------------- src/one-shot-review.ts | 127 ------------------------ src/workspace-package-manager.test.ts | 18 ---- src/workspace-package-manager.ts | 34 ------- 5 files changed, 1 insertion(+), 331 deletions(-) delete mode 100644 src/one-shot-review.test.ts delete mode 100644 src/one-shot-review.ts diff --git a/src/message-runtime.ts b/src/message-runtime.ts index b7c1cd8..9f241ab 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -45,10 +45,6 @@ import { enqueuePairedFollowUpAfterEvent, schedulePairedFollowUpWithMessageCheck, } from './message-runtime-follow-up.js'; -import { - clearOneShotReviewLeaseIfIdle, - maybeActivateOneShotReviewLease, -} from './one-shot-review.js'; import { type ScheduledPairedFollowUpIntentKind } from './paired-follow-up-scheduler.js'; import { transitionPairedTaskStatus } from './paired-execution-context-shared.js'; import { @@ -342,7 +338,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { : 'Skipped duplicate paired follow-up after successful reviewer/arbiter delivery while task state was unchanged', ); } - clearOneShotReviewLeaseIfIdle(chatJid); }, }); @@ -421,7 +416,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { 'Paired room reviewer/arbiter channel resolution', ); } - clearOneShotReviewLeaseIfIdle(chatJid); // Delivery retries can come from forced fallback runs whose agent_type // differs from the room owner's registered agent type. @@ -655,16 +649,9 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { return gateResult.success; } - const oneShotReviewGroup = - maybeActivateOneShotReviewLease({ - chatJid, - group, - messages: missedMessages, - }) ?? group; - return runQueuedGroupTurn({ chatJid, - group: oneShotReviewGroup, + group, runId, log, timezone: deps.timezone, diff --git a/src/one-shot-review.test.ts b/src/one-shot-review.test.ts deleted file mode 100644 index a4f025d..0000000 --- a/src/one-shot-review.test.ts +++ /dev/null @@ -1,138 +0,0 @@ -import { beforeEach, describe, expect, it } from 'vitest'; - -import { - _initTestDatabase, - assignRoom, - getChannelOwnerLease, - getEffectiveRuntimeRoomMode, -} from './db.js'; -import { - clearOneShotReviewLeaseIfIdle, - isOneShotReviewRequest, - maybeActivateOneShotReviewLease, - ONE_SHOT_REVIEW_LEASE_REASON, -} from './one-shot-review.js'; -import { - hasReviewerLease, - refreshChannelOwnerCache, -} from './service-routing.js'; -import type { NewMessage } from './types.js'; - -function humanMessage(content: string): NewMessage { - return { - id: 'msg-1', - chat_jid: 'dc:single-room', - sender: 'user-1', - sender_name: 'user', - content, - timestamp: '2026-05-27T01:50:00.000Z', - seq: 1, - is_bot_message: false, - }; -} - -describe('one-shot review', () => { - beforeEach(() => { - _initTestDatabase(); - refreshChannelOwnerCache(true); - }); - - it('detects explicit reviewer requests in user text', () => { - expect(isOneShotReviewRequest('이번 요청만 리뷰어 사용해서 검증해줘')).toBe( - true, - ); - expect(isOneShotReviewRequest('리뷰어로 확인해줘')).toBe(true); - expect(isOneShotReviewRequest('그냥 테스트만 돌려줘')).toBe(false); - }); - - it('temporarily enables a reviewer lease without changing single room mode', () => { - const group = assignRoom('dc:single-room', { - name: 'single room', - folder: 'single_room', - roomMode: 'single', - ownerAgentType: 'claude-code', - workDir: '/tmp/ejclaw-single-room', - }); - - expect(group).toBeDefined(); - expect(getEffectiveRuntimeRoomMode('dc:single-room')).toBe('single'); - expect(hasReviewerLease('dc:single-room')).toBe(false); - - const reviewGroup = maybeActivateOneShotReviewLease({ - chatJid: 'dc:single-room', - group: group!, - messages: [humanMessage('이번 요청만 리뷰어 사용해서 검증해줘')], - }); - - expect(reviewGroup).toMatchObject({ - folder: 'single_room', - workDir: '/tmp/ejclaw-single-room', - }); - expect(getEffectiveRuntimeRoomMode('dc:single-room')).toBe('single'); - expect(getChannelOwnerLease('dc:single-room')).toMatchObject({ - reason: ONE_SHOT_REVIEW_LEASE_REASON, - owner_agent_type: 'claude-code', - }); - expect(hasReviewerLease('dc:single-room')).toBe(true); - }); - - it('does not enable review for ordinary single-mode messages', () => { - const group = assignRoom('dc:single-room', { - name: 'single room', - folder: 'single_room', - roomMode: 'single', - ownerAgentType: 'claude-code', - workDir: '/tmp/ejclaw-single-room', - }); - - expect( - maybeActivateOneShotReviewLease({ - chatJid: 'dc:single-room', - group: group!, - messages: [humanMessage('그냥 구현해줘')], - }), - ).toBeNull(); - expect(getChannelOwnerLease('dc:single-room')).toBeUndefined(); - }); - - it('uses the group folder as the workDir for one-shot review in rooms without workDir', () => { - const group = assignRoom('dc:single-room', { - name: 'single room', - folder: 'single_room', - roomMode: 'single', - ownerAgentType: 'claude-code', - }); - - const reviewGroup = maybeActivateOneShotReviewLease({ - chatJid: 'dc:single-room', - group: group!, - messages: [humanMessage('리뷰어로 검증해줘')], - }); - - expect(reviewGroup?.folder).toBe('single_room'); - expect(reviewGroup?.workDir).toMatch(/\/groups\/single_room$/); - expect(getChannelOwnerLease('dc:single-room')).toMatchObject({ - reason: ONE_SHOT_REVIEW_LEASE_REASON, - }); - expect(hasReviewerLease('dc:single-room')).toBe(true); - }); - - it('clears an idle one-shot reviewer lease', () => { - const group = assignRoom('dc:single-room', { - name: 'single room', - folder: 'single_room', - roomMode: 'single', - ownerAgentType: 'claude-code', - workDir: '/tmp/ejclaw-single-room', - }); - maybeActivateOneShotReviewLease({ - chatJid: 'dc:single-room', - group: group!, - messages: [humanMessage('리뷰어로 확인해줘')], - }); - - expect(clearOneShotReviewLeaseIfIdle('dc:single-room')).toBe(true); - expect(getChannelOwnerLease('dc:single-room')).toBeUndefined(); - expect(hasReviewerLease('dc:single-room')).toBe(false); - }); -}); diff --git a/src/one-shot-review.ts b/src/one-shot-review.ts deleted file mode 100644 index 4112140..0000000 --- a/src/one-shot-review.ts +++ /dev/null @@ -1,127 +0,0 @@ -import { - ARBITER_AGENT_TYPE, - REVIEWER_AGENT_TYPE, - SERVICE_ID, -} from './config.js'; -import { - clearChannelOwnerLease, - getChannelOwnerLease, - getEffectiveRuntimeRoomMode, - getLatestOpenPairedTaskForChat, - getStoredRoomRoleAgentPlan, - setChannelOwnerLease, -} from './db.js'; -import { logger } from './logger.js'; -import { resolveGroupFolderPath } from './group-folder.js'; -import { resolveRoleAgentPlan } from './role-agent-plan.js'; -import { resolveRoleServiceShadow } from './role-service-shadow.js'; -import { - getEffectiveChannelLease, - hasReviewerLease, - refreshChannelOwnerCache, - resolveLeaseServiceId, -} from './service-routing.js'; -import type { NewMessage, RegisteredGroup } from './types.js'; - -export const ONE_SHOT_REVIEW_LEASE_REASON = 'one-shot-review'; - -const REVIEW_ONCE_PATTERNS = [ - /(이번\s*요청만|이번만|한\s*번만|임시로|필요할\s*때만).{0,30}(리뷰어|reviewer).{0,30}(사용|켜|돌려|붙여|검증|확인|리뷰)/i, - /(리뷰어|reviewer).{0,20}(사용해서|사용해|켜서|돌려서|붙여서).{0,40}(검증|확인|리뷰|체크|봐줘|봐)/i, - /(리뷰어|reviewer).{0,30}(검증해줘|확인해줘|리뷰해줘|체크해줘|봐줘|돌려줘)/i, - /(검증|확인|리뷰|체크).{0,30}(리뷰어|reviewer).{0,30}(사용|켜|돌려|붙여)/i, -]; - -export function isOneShotReviewRequest(content: string): boolean { - const normalized = content.replace(/\s+/g, ' ').trim(); - if (!normalized) return false; - return REVIEW_ONCE_PATTERNS.some((pattern) => pattern.test(normalized)); -} - -function hasHumanOneShotReviewRequest(messages: NewMessage[]): boolean { - return messages.some( - (message) => - message.is_from_me !== true && - !message.is_bot_message && - isOneShotReviewRequest(message.content), - ); -} - -export function maybeActivateOneShotReviewLease(args: { - chatJid: string; - group: RegisteredGroup; - messages: NewMessage[]; -}): RegisteredGroup | null { - if (getEffectiveRuntimeRoomMode(args.chatJid) !== 'single') return null; - if (hasReviewerLease(args.chatJid)) return null; - if (!hasHumanOneShotReviewRequest(args.messages)) return null; - - let effectiveGroup = args.group; - if (!effectiveGroup.workDir) { - effectiveGroup = { - ...effectiveGroup, - workDir: resolveGroupFolderPath(effectiveGroup.folder), - }; - } - - const lease = getEffectiveChannelLease(args.chatJid); - const storedPlan = getStoredRoomRoleAgentPlan(args.chatJid); - const rolePlan = resolveRoleAgentPlan({ - paired: true, - groupAgentType: - lease.owner_agent_type ?? - storedPlan?.ownerAgentType ?? - args.group.agentType, - configuredReviewer: storedPlan?.reviewerAgentType ?? REVIEWER_AGENT_TYPE, - configuredArbiter: storedPlan?.arbiterAgentType ?? ARBITER_AGENT_TYPE, - }); - const ownerServiceId = - resolveLeaseServiceId(lease, 'owner') ?? - resolveRoleServiceShadow('owner', rolePlan.ownerAgentType) ?? - SERVICE_ID; - const reviewerServiceId = resolveRoleServiceShadow( - 'reviewer', - rolePlan.reviewerAgentType, - ); - if (!reviewerServiceId || !rolePlan.reviewerAgentType) { - return null; - } - - setChannelOwnerLease({ - chat_jid: args.chatJid, - owner_service_id: ownerServiceId, - reviewer_service_id: reviewerServiceId, - arbiter_service_id: resolveRoleServiceShadow( - 'arbiter', - rolePlan.arbiterAgentType, - ), - owner_agent_type: rolePlan.ownerAgentType, - reviewer_agent_type: rolePlan.reviewerAgentType, - arbiter_agent_type: rolePlan.arbiterAgentType, - reason: ONE_SHOT_REVIEW_LEASE_REASON, - }); - refreshChannelOwnerCache(true); - logger.info( - { - chatJid: args.chatJid, - groupFolder: effectiveGroup.folder, - workDir: effectiveGroup.workDir, - usedDefaultWorkDir: !args.group.workDir, - ownerAgentType: rolePlan.ownerAgentType, - reviewerAgentType: rolePlan.reviewerAgentType, - }, - 'Activated one-shot reviewer lease for a single-mode room', - ); - return effectiveGroup; -} - -export function clearOneShotReviewLeaseIfIdle(chatJid: string): boolean { - const lease = getChannelOwnerLease(chatJid); - if (lease?.reason !== ONE_SHOT_REVIEW_LEASE_REASON) return false; - if (getLatestOpenPairedTaskForChat(chatJid)) return false; - - clearChannelOwnerLease(chatJid); - refreshChannelOwnerCache(true); - logger.info({ chatJid }, 'Cleared idle one-shot reviewer lease'); - return true; -} diff --git a/src/workspace-package-manager.test.ts b/src/workspace-package-manager.test.ts index 14dd666..0989542 100644 --- a/src/workspace-package-manager.test.ts +++ b/src/workspace-package-manager.test.ts @@ -218,24 +218,6 @@ describe('workspace package manager helpers', () => { expect(execFileSyncMock).not.toHaveBeenCalled(); }); - it('prepends the current Bun binary directory for bun workspace commands', () => { - const repoDir = path.join(tempRoot, 'bun-workspace'); - fs.mkdirSync(repoDir, { recursive: true }); - fs.writeFileSync( - path.join(repoDir, 'package.json'), - JSON.stringify({ packageManager: 'bun@1.3.13' }), - ); - - const env = buildWorkspaceCommandEnvironment(repoDir, 'bun', { - PATH: '/tmp/bin', - }); - - const expectedBunBinDir = path.basename(process.execPath).startsWith('bun') - ? path.dirname(process.execPath) - : path.join(os.homedir(), '.bun', 'bin'); - expect(env.PATH?.split(path.delimiter)[0]).toBe(expectedBunBinDir); - }); - it('disables corepack project specs only for lockfile-selected pnpm workspaces under a conflicting ancestor', () => { const parentDir = path.join(tempRoot, 'parent'); fs.mkdirSync(parentDir, { recursive: true }); diff --git a/src/workspace-package-manager.ts b/src/workspace-package-manager.ts index 379d444..d8022af 100644 --- a/src/workspace-package-manager.ts +++ b/src/workspace-package-manager.ts @@ -118,36 +118,6 @@ function buildCorepackCommand( }; } -function prependPathEntry( - baseEnv: NodeJS.ProcessEnv, - entry: string | null, -): NodeJS.ProcessEnv { - if (!entry) { - return baseEnv; - } - const currentPath = baseEnv.PATH ?? ''; - const entries = currentPath.split(path.delimiter).filter(Boolean); - if (entries.includes(entry)) { - return baseEnv; - } - return { - ...baseEnv, - PATH: [entry, ...entries].join(path.delimiter), - }; -} - -function resolveCurrentBunBinDir(): string | null { - if (path.basename(process.execPath).startsWith('bun')) { - return path.dirname(process.execPath); - } - const home = process.env.HOME; - if (!home) { - return null; - } - const candidate = path.join(home, '.bun', 'bin'); - return fs.existsSync(path.join(candidate, 'bun')) ? candidate : null; -} - function findNearestAncestorPackageManager( repoDir: string, ): WorkspacePackageManager | null { @@ -174,10 +144,6 @@ export function buildWorkspaceCommandEnvironment( packageManager: WorkspacePackageManager, baseEnv: NodeJS.ProcessEnv = process.env, ): NodeJS.ProcessEnv { - if (packageManager === 'bun') { - return prependPathEntry(baseEnv, resolveCurrentBunBinDir()); - } - if (packageManager !== 'pnpm') { return baseEnv; }