From 712664ca003e95efdc20e8639654f99aa3468497 Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 25 Aug 2026 18:40:14 +0900 Subject: [PATCH] Fix owner context loss after finalize; green the test suite Owner continuity: - Seed a freshly created owner task with the previous task's latest owner final on a cold start after the previous task already closed, so a user reply to a finalized TASK_DONE no longer produces a "no context" answer. Activated for this deployment via PAIRED_CARRY_FORWARD_LATEST_OWNER_FINAL (.env); carried text is injected as clearly-marked background only. - Skip intermediate STEP_DONE outputs when picking the carry-forward anchor. Single-mode routing: - enforceRoomModeOnLease strips a stale reviewer/arbiter lease from a room switched back to single, preventing single-mode messages from stalling in the paired path on a stuck execution lease. Session auth / credentials: - Pre-sync Claude credentials into each session dir before the agent spawns. - Honor CLAUDE_CREDENTIALS_PATH in setup/login.ts (per-service isolation). - Add a relogin-required gate so a permanently logged-out claude-code room asks the user to re-login instead of spawning a doomed agent. Other: - Arbiter verdicts written in the user's language (verdict keyword stays EN). - status-dashboard chatName field; runtime-inventory credential path resolver. Tests (make suite fully green: 1595 pass / 3 skip): - service-routing: default owner is now the claude service and reviewer is codex-review; update the 7 failover/default expectations accordingly. - migrate-room-registrations: owner inferred as claude-code (configured OWNER_AGENT_TYPE) for a dual legacy room; reviewer becomes codex. - register: mock paired-workspace provisioning + reload signal (registration now provisions a workspace and hot-reloads); assert RELOADED status. - paired-execution-context: force a claude-code reviewer to exercise the Claude read-only branch regardless of the deployment default. Co-Authored-By: Claude Opus 4.7 --- prompts/arbiter-paired-room.md | 5 + setup/login.ts | 41 ++++++--- setup/migrate-room-registrations.test.ts | 6 +- setup/register.test.ts | 14 +++ src/agent-runner-environment.ts | 23 ++++- src/claude-usage-backoff.test.ts | 26 ++++++ src/claude-usage.ts | 92 +++++++++++-------- src/dashboard.test.ts | 30 ++++++ src/message-runtime-group-processing.ts | 55 +++++++++++ src/message-runtime-rules.test.ts | 7 +- ...ed-execution-context-carry-forward.test.ts | 85 +++++++++++++++++ src/paired-execution-context.test.ts | 4 +- src/paired-execution-context.ts | 56 +++++++---- src/runtime-inventory.ts | 4 +- src/service-routing.test.ts | 68 ++++++++++++-- src/service-routing.ts | 31 ++++++- src/status-dashboard.ts | 1 + 17 files changed, 457 insertions(+), 91 deletions(-) diff --git a/prompts/arbiter-paired-room.md b/prompts/arbiter-paired-room.md index 43a2f71..777ad22 100644 --- a/prompts/arbiter-paired-room.md +++ b/prompts/arbiter-paired-room.md @@ -43,3 +43,8 @@ You may receive reference opinions from external models appended to your prompt. - If both sides are saying the same thing but not acting on it, call it out and direct the owner to act - If the conversation shows the owner asking the user a question (not the reviewer), always ESCALATE — the arbiter cannot answer on behalf of the user - If you see a prior arbiter verdict of PROCEED in the history but the same issue persists, do NOT repeat PROCEED — use ESCALATE instead + +## Language + +- Write your verdict in the user's language (Korean for this deployment) unless the user wrote in another language. The user must be able to read your verdict. +- Keep ONLY the leading verdict keyword in English — `PROCEED` / `REVISE` / `RESET` / `ESCALATE` — exactly as specified above, because the system parses that first token. Write everything after it (reasoning, evidence, the required action for the owner) in Korean. diff --git a/setup/login.ts b/setup/login.ts index 96d7a4f..6bca947 100644 --- a/setup/login.ts +++ b/setup/login.ts @@ -5,13 +5,13 @@ * builds (captured from the official CLI). This is the manual / paste-code * variant: the user visits the authorize URL, completes login in their * browser, and pastes the code back. We exchange it for tokens and write - * `~/.claude/.credentials.json`. + * `CLAUDE_CREDENTIALS_PATH` or `~/.claude/.credentials.json`. * * Usage: * bun setup/index.ts --step login # phase 1: print authorize URL * bun setup/index.ts --step login --code # phase 2: exchange the code * - * Phase 1 stashes the PKCE verifier + state in /tmp/ejclaw-claude-login.json + * Phase 1 stashes the PKCE verifier + state in a /tmp login state file * (mode 0600). Phase 2 reads it back, exchanges the code, writes credentials. * * This step is run by hand for re-auth. Once `.credentials.json` exists, the @@ -39,8 +39,21 @@ const SCOPES = [ 'user:file_upload', ]; -const STATE_FILE = path.join(os.tmpdir(), 'ejclaw-claude-login.json'); -const CREDS_PATH = path.join(os.homedir(), '.claude', '.credentials.json'); +function credentialsPath(): string { + const configured = process.env.CLAUDE_CREDENTIALS_PATH?.trim(); + return configured + ? path.resolve(configured) + : path.join(os.homedir(), '.claude', '.credentials.json'); +} + +function stateFilePath(): string { + const hash = crypto + .createHash('sha256') + .update(credentialsPath()) + .digest('hex') + .slice(0, 12); + return path.join(os.tmpdir(), `ejclaw-claude-login-${hash}.json`); +} interface PendingState { verifier: string; @@ -90,13 +103,14 @@ function buildAuthorizeUrl(state: string, challenge: string): string { } function writePendingState(p: PendingState): void { - fs.writeFileSync(STATE_FILE, JSON.stringify(p), { mode: 0o600 }); + fs.writeFileSync(stateFilePath(), JSON.stringify(p), { mode: 0o600 }); } function readPendingState(): PendingState | null { - if (!fs.existsSync(STATE_FILE)) return null; + const stateFile = stateFilePath(); + if (!fs.existsSync(stateFile)) return null; try { - return JSON.parse(fs.readFileSync(STATE_FILE, 'utf-8')) as PendingState; + return JSON.parse(fs.readFileSync(stateFile, 'utf-8')) as PendingState; } catch { return null; } @@ -136,6 +150,7 @@ async function exchangeCode( } function writeCredentials(resp: ExchangeResponse): void { + const credsPath = credentialsPath(); const expiresAt = Date.now() + resp.expires_in * 1000; const creds = { claudeAiOauth: { @@ -150,11 +165,11 @@ function writeCredentials(resp: ExchangeResponse): void { : '', }, }; - const dir = path.dirname(CREDS_PATH); + const dir = path.dirname(credsPath); fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); - const tmp = `${CREDS_PATH}.tmp`; + const tmp = `${credsPath}.tmp`; fs.writeFileSync(tmp, JSON.stringify(creds, null, 2), { mode: 0o600 }); - fs.renameSync(tmp, CREDS_PATH); + fs.renameSync(tmp, credsPath); } interface Args { @@ -208,15 +223,15 @@ export async function run(args: string[]): Promise { try { const resp = await exchangeCode(code, pending.verifier, pending.state); writeCredentials(resp); - fs.unlinkSync(STATE_FILE); + fs.unlinkSync(stateFilePath()); const newScopes = (resp.scope || SCOPES.join(' ')).split(' ').sort(); logger.info( { scopes: newScopes, expiresInMin: Math.round(resp.expires_in / 60) }, - 'Wrote ~/.claude/.credentials.json', + 'Wrote Claude credentials', ); emitStatus('LOGIN', { STATUS: 'success', - CREDENTIALS_PATH: CREDS_PATH, + CREDENTIALS_PATH: credentialsPath(), SCOPES: newScopes.join(','), }); } catch (err) { diff --git a/setup/migrate-room-registrations.test.ts b/setup/migrate-room-registrations.test.ts index 4eeae23..45c1303 100644 --- a/setup/migrate-room-registrations.test.ts +++ b/setup/migrate-room-registrations.test.ts @@ -126,7 +126,7 @@ describe('migrate room registrations step', () => { chat_jid: 'dc:legacy-room', room_mode: 'tribunal', mode_source: 'inferred', - owner_agent_type: 'codex', + owner_agent_type: 'claude-code', }, ]); expect( @@ -141,12 +141,12 @@ describe('migrate room registrations step', () => { { chat_jid: 'dc:legacy-room', role: 'owner', - agent_type: 'codex', + agent_type: 'claude-code', }, { chat_jid: 'dc:legacy-room', role: 'reviewer', - agent_type: 'claude-code', + agent_type: 'codex', }, ]); expect( diff --git a/setup/register.test.ts b/setup/register.test.ts index df83157..4d5a7bb 100644 --- a/setup/register.test.ts +++ b/setup/register.test.ts @@ -28,6 +28,19 @@ vi.mock('../src/group-folder.js', () => ({ isValidGroupFolder: isValidGroupFolderMock, })); +// Registration now provisions the paired workspace (which shells out to git) +// and hot-reloads the running service. Stub both so the unit test stays hermetic +// and only verifies the registration delegation, not workspace/git side effects. +vi.mock('../src/paired-workspace-manager.js', () => ({ + ensurePairedWorkspaceProvisioned: vi.fn( + () => '/tmp/ejclaw-groups/test-room/owner', + ), +})); + +vi.mock('../src/runtime-reload-signal.js', () => ({ + signalEjclawReload: vi.fn(() => false), +})); + vi.mock('../src/logger.js', () => ({ logger: { info: loggerInfoMock, @@ -84,6 +97,7 @@ describe('register step', () => { FOLDER: 'test-room', CHANNEL: 'discord', STATUS: 'success', + RELOADED: 'no', LOG: 'logs/setup.log', }); }); diff --git a/src/agent-runner-environment.ts b/src/agent-runner-environment.ts index 32e2683..44e72e5 100644 --- a/src/agent-runner-environment.ts +++ b/src/agent-runner-environment.ts @@ -21,6 +21,7 @@ import { } from './codex-token-rotation.js'; import { readCodexFeatureFromFile } from './codex-config-features.js'; import { ensureClaudeSessionSettings } from './claude-session-settings.js'; +import { getClaudeCredentialsPath } from './claude-credentials-path.js'; import { getConfiguredClaudeTokens, getCurrentToken, @@ -53,16 +54,35 @@ function syncDirectoryEntries(sources: string[], destination: string): void { for (const entry of fs.readdirSync(source)) { const srcPath = path.join(source, entry); const dstPath = path.join(destination, entry); + fs.mkdirSync(destination, { recursive: true }); if (fs.statSync(srcPath).isDirectory()) { fs.cpSync(srcPath, dstPath, { recursive: true }); } else { - fs.mkdirSync(destination, { recursive: true }); fs.copyFileSync(srcPath, dstPath); } } } } +function syncClaudeCredentialsToSessionDir(sessionClaudeDir: string): void { + // The agent spawns with CLAUDE_CONFIG_DIR=. Without this + // pre-sync, a freshly created session dir has no credentials.json and the + // child Claude process fails to authenticate on its first run. token- + // refresh.ts's syncToSessionDirs only kicks in on the next refresh cycle. + const srcPath = getClaudeCredentialsPath(0, { allowHomeFallback: true }); + if (!srcPath || !fs.existsSync(srcPath)) return; + const dest = path.join(sessionClaudeDir, '.credentials.json'); + try { + fs.copyFileSync(srcPath, dest); + fs.chmodSync(dest, 0o600); + } catch (err) { + logger.warn( + { err, srcPath, dest }, + 'Failed to pre-sync Claude credentials to session dir', + ); + } +} + type SkillSyncScope = 'codex-user' | 'claude-user' | 'runner' | 'workdir'; interface SkillSyncSource { @@ -595,6 +615,7 @@ export function prepareGroupEnvironment( const groupSessionsDir = path.join(sessionRootDir, '.claude'); fs.mkdirSync(groupSessionsDir, { recursive: true }); ensureClaudeSessionSettings(groupSessionsDir); + syncClaudeCredentialsToSessionDir(groupSessionsDir); const workDirClaude = group.workDir ? path.join(group.workDir, '.claude') diff --git a/src/claude-usage-backoff.test.ts b/src/claude-usage-backoff.test.ts index bc5f260..e9e118a 100644 --- a/src/claude-usage-backoff.test.ts +++ b/src/claude-usage-backoff.test.ts @@ -102,6 +102,32 @@ describe('Claude usage 429 Retry-After backoff', () => { expect(second[0].usageRateLimited).toBe(true); }); + it('falls back to the default cooldown when Retry-After is 0 (does not disable backoff)', async () => { + // Production regression: the usage endpoint was returning 429 with + // `retry-after: 0`, which made `retryAfterMs ?? DEFAULT` evaluate to 0 — a + // ~5s cooldown. The poller then re-hit the endpoint every 60s and re-tripped + // the 429 indefinitely. A non-positive Retry-After must use the default. + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-06-20T00:00:00Z')); + + const fetchMock = vi.fn(async () => + makeResponse(429, { 'retry-after': '0' }), + ); + vi.stubGlobal('fetch', fetchMock); + + const { fetchAllClaudeUsage } = await import('./claude-usage.js'); + + await fetchAllClaudeUsage(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + // +70s: past the 60s throttle. With the bug (0 cooldown) this would refetch; + // with the default 5-min cooldown it must still be held. + vi.setSystemTime(Date.now() + 70_000); + const held = await fetchAllClaudeUsage(); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(held[0].usageRateLimited).toBe(true); + }); + it('keeps backing off after the 60s throttle elapses but before the cooldown closes', async () => { // This is the core regression: a 93s Retry-After must outlast the 60s // MIN_FETCH_INTERVAL throttle. Without the cooldown the poller would refetch diff --git a/src/claude-usage.ts b/src/claude-usage.ts index f20daed..6245abf 100644 --- a/src/claude-usage.ts +++ b/src/claude-usage.ts @@ -6,9 +6,12 @@ */ import fs from 'fs'; -import os from 'os'; import path from 'path'; +import { + getClaudeCredentialsPath, + hasExplicitClaudeCredentialsPath, +} from './claude-credentials-path.js'; import { DATA_DIR } from './config.js'; import { logger } from './logger.js'; import { @@ -32,6 +35,14 @@ export interface ClaudeUsageData { const USAGE_ENDPOINT = 'https://api.anthropic.com/api/oauth/usage'; const FETCH_TIMEOUT_MS = 10_000; +// Master gate for credentials-backed usage queries. When false, the module +// uses only env-supplied tokens and skips all credentials-file reads, per- +// account cache keys, and on-401 refresh attempts. Enabled when either an +// explicit CLAUDE_CREDENTIALS_PATH is set or the home fallback is allowed. +const USE_CLAUDE_CREDENTIALS_FOR_USAGE = + hasExplicitClaudeCredentialsPath() || + process.env.CLAUDE_USAGE_USE_HOME_CREDENTIALS !== 'false'; + interface UsageApiResponse { five_hour?: { utilization: number; resets_at?: string }; seven_day?: { utilization: number; resets_at?: string }; @@ -112,7 +123,7 @@ export function getUsageCacheWriteKey( token: string, accountIndex?: number, ): string { - return accountIndex != null + return USE_CLAUDE_CREDENTIALS_FOR_USAGE && accountIndex != null ? accountCacheKey(accountIndex) : legacyTokenCacheKey(token); } @@ -123,9 +134,11 @@ export function getUsageCacheReadKeys( credentialsAccessToken?: string | null, ): string[] { const keys: string[] = []; - if (accountIndex != null) keys.push(accountCacheKey(accountIndex)); + if (USE_CLAUDE_CREDENTIALS_FOR_USAGE && accountIndex != null) { + keys.push(accountCacheKey(accountIndex)); + } - if (credentialsAccessToken) { + if (USE_CLAUDE_CREDENTIALS_FOR_USAGE && credentialsAccessToken) { const credsKey = legacyTokenCacheKey(credentialsAccessToken); if (!keys.includes(credsKey)) keys.push(credsKey); } @@ -265,7 +278,11 @@ async function fetchUsageForToken( // 401 = token expired; 403 = token lacks `user:profile` scope. // Both are recoverable by exchanging refresh_token for a fresh access // token (mint includes current default scope set). Retry once. - if (accountIndex != null && !refreshAttempted) { + if ( + USE_CLAUDE_CREDENTIALS_FOR_USAGE && + accountIndex != null && + !refreshAttempted + ) { try { const { forceRefreshToken } = await import('./token-refresh.js'); const newToken = await forceRefreshToken(accountIndex); @@ -303,7 +320,14 @@ async function fetchUsageForToken( if (res.status === 429) { const staleMs = cached ? Date.now() - cached.fetchedAt : 0; const retryAfterMs = parseRetryAfterMs(res.headers.get('retry-after')); - const cooldownMs = retryAfterMs ?? DEFAULT_RATE_LIMIT_COOLDOWN_MS; + // A `Retry-After` of 0 (or a past HTTP-date that parses to 0) must NOT + // disable the backoff — honoring it literally makes the poller retry on + // the next 60s tick and re-trip the 429 indefinitely. Treat any + // non-positive value as "no usable hint" and fall back to the default. + const cooldownMs = + retryAfterMs != null && retryAfterMs > 0 + ? retryAfterMs + : DEFAULT_RATE_LIMIT_COOLDOWN_MS; const cooldownUntil = Date.now() + cooldownMs + RATE_LIMIT_MARGIN_MS; logger.warn( { @@ -403,12 +427,11 @@ async function fetchUsageForToken( * Uses the current active token from rotation. */ export async function fetchClaudeUsage(): Promise { - // Prefer the access token in ~/.claude/.credentials.json when present. - // The static .env token (CLAUDE_CODE_OAUTH_TOKEN) was issued before the - // `user:profile` scope was required for /api/oauth/usage and so always 403s. - // The credentials file is the canonical source written by `claude auth - // login` and kept fresh by token-refresh.ts — its accessToken carries the - // full scope set including user:profile. + // Prefer the access token in credentials.json when present. The static .env + // token (CLAUDE_CODE_OAUTH_TOKEN) was issued before the `user:profile` scope + // was required for /api/oauth/usage and so always 403s. The credentials file + // is the canonical source written by `claude auth login` and kept fresh by + // token-refresh.ts — its accessToken carries the full scope set. const credsToken = readCredentialsAccessToken(0); const token = credsToken || getCurrentToken() || getConfiguredClaudeTokens()[0]; @@ -416,7 +439,8 @@ export async function fetchClaudeUsage(): Promise { logger.debug('No Claude OAuth token available for usage check'); return null; } - return (await fetchUsageForToken(token, undefined)).usage; + // Pass accountIndex=0 so 401/403 → forceRefreshToken retry path engages. + return (await fetchUsageForToken(token, 0)).usage; } export interface ClaudeAccountProfile { @@ -428,21 +452,18 @@ const profileCache = new Map(); /** * Read planType from credentials file as fallback when profile API fails. - * Account 0: ~/.claude/.credentials.json - * Account 1+: ~/.claude-accounts/{index}/.credentials.json + * Path is resolved by getClaudeCredentialsPath (honours CLAUDE_CREDENTIALS_PATH + * / CLAUDE_ACCOUNTS_DIR / home fallback). Returns null when the credentials + * gate is off or no file is found. */ function readCredentialsPlanType(accountIndex: number): string | null { + if (!USE_CLAUDE_CREDENTIALS_FOR_USAGE) return null; try { - const credsPath = - accountIndex === 0 - ? path.join(os.homedir(), '.claude', '.credentials.json') - : path.join( - os.homedir(), - '.claude-accounts', - String(accountIndex), - '.credentials.json', - ); - if (!fs.existsSync(credsPath)) return null; + const credsPath = getClaudeCredentialsPath(accountIndex, { + allowHomeFallback: + process.env.CLAUDE_USAGE_USE_HOME_CREDENTIALS !== 'false', + }); + if (!credsPath || !fs.existsSync(credsPath)) return null; const data = readJsonFile<{ claudeAiOauth?: { subscriptionType?: string }; }>(credsPath); @@ -453,17 +474,13 @@ function readCredentialsPlanType(accountIndex: number): string | null { } function readCredentialsAccessToken(accountIndex: number): string | null { + if (!USE_CLAUDE_CREDENTIALS_FOR_USAGE) return null; try { - const credsPath = - accountIndex === 0 - ? path.join(os.homedir(), '.claude', '.credentials.json') - : path.join( - os.homedir(), - '.claude-accounts', - String(accountIndex), - '.credentials.json', - ); - if (!fs.existsSync(credsPath)) return null; + const credsPath = getClaudeCredentialsPath(accountIndex, { + allowHomeFallback: + process.env.CLAUDE_USAGE_USE_HOME_CREDENTIALS !== 'false', + }); + if (!credsPath || !fs.existsSync(credsPath)) return null; const data = readJsonFile<{ claudeAiOauth?: { accessToken?: string }; }>(credsPath); @@ -519,7 +536,10 @@ async function fetchProfileForToken( export async function fetchAllClaudeProfiles(): Promise { const allTokens = getAllTokens(); for (const t of allTokens) { - let profile = await fetchProfileForToken(t.token); + // Prefer the per-account credentials access token; the static env token + // may lack the `user:profile` scope. + const credsToken = readCredentialsAccessToken(t.index); + let profile = await fetchProfileForToken(credsToken || t.token); // Fallback: if profile API failed or returned unknown plan, use credentials file if (!profile || profile.planType === '?') { diff --git a/src/dashboard.test.ts b/src/dashboard.test.ts index 32074cf..3ba34d8 100644 --- a/src/dashboard.test.ts +++ b/src/dashboard.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import type { DashboardOptions } from './dashboard-status-content.js'; +import { formatRoomName } from './unified-dashboard.js'; function makeOptions(sessionId?: string): DashboardOptions { const sessions: Record = sessionId @@ -85,3 +86,32 @@ describe('buildStatusContent', () => { expect(buildStatusContent(makeOptions())).not.toContain('**clone-test**'); }); }); + +describe('formatRoomName', () => { + it('uses the stored chat name without adding an extra Discord hash', () => { + expect( + formatRoomName( + 'dc:123', + undefined, + 'registered-room-name', + 'My Server #bot-chat', + ), + ).toBe('My Server #bot-chat'); + }); + + it('adds a Discord hash for raw channel metadata names', () => { + expect( + formatRoomName( + 'dc:123', + { + name: 'bot-chat', + position: 1, + category: 'Bots', + categoryPosition: 1, + }, + 'registered-room-name', + 'My Server #bot-chat', + ), + ).toBe('#bot-chat'); + }); +}); diff --git a/src/message-runtime-group-processing.ts b/src/message-runtime-group-processing.ts index c8a8294..2e80c79 100644 --- a/src/message-runtime-group-processing.ts +++ b/src/message-runtime-group-processing.ts @@ -36,6 +36,10 @@ import { hasHumanMessageAfterWorkItem, } from './message-runtime-preflight-messages.js'; import { deliverCanonicalOutboundMessage } from './ipc-outbound-delivery.js'; +import { + isReloginRequired, + RELOGIN_REQUIRED_MESSAGE, +} from './token-refresh.js'; import { findChannel, formatMessages } from './router.js'; import { createScopedLogger, logger } from './logger.js'; import type { AgentOutput } from './agent-runner.js'; @@ -308,6 +312,15 @@ async function processMissedMessages( return gateOutcome; } + const reloginOutcome = await runReloginRequiredGate( + args, + runtime, + missedMessages, + ); + if (reloginOutcome !== null) { + return reloginOutcome; + } + return runQueuedGroupTurn({ chatJid: runtime.chatJid, group: runtime.group, @@ -415,6 +428,48 @@ function advancePastBotOnlyCollaboration( ); } +/** + * When Claude OAuth auto-refresh has permanently given up (login lost and the + * refresh token is dead — see token-refresh.ts), don't spawn a doomed agent. + * Instead reply once, in this chat, asking the user to re-login, then advance + * the cursor so a still-logged-out bot doesn't re-notify on every poll. The + * next fresh incoming request re-triggers the notice. Recovers automatically + * once a manual re-login writes a valid token (isReloginRequired() clears). + * + * Returns true when the notice was sent (turn handled), null to fall through. + */ +async function runReloginRequiredGate( + args: ProcessGroupMessagesDeps, + runtime: RuntimeContext, + missedMessages: NewMessage[], +): Promise { + // Only claude-code turns depend on the Claude OAuth token. Codex-backed + // groups authenticate separately and must not be blocked here. + const agentType = runtime.group.agentType ?? 'claude-code'; + if (agentType !== 'claude-code') return null; + if (!isReloginRequired()) return null; + + await deliverSessionCommandMessage( + args, + runtime.chatJid, + RELOGIN_REQUIRED_MESSAGE, + ); + + const lastMessage = missedMessages[missedMessages.length - 1]; + if (lastMessage?.seq != null) { + advanceLastAgentCursor( + args.getLastAgentTimestamps(), + args.saveState, + runtime.chatJid, + lastMessage.seq, + ); + } + runtime.log.warn( + 'Claude OAuth login lost (gave up refreshing) — asked user to re-login instead of running the agent', + ); + return true; +} + async function runQueuedRunGates( args: ProcessGroupMessagesDeps, runtime: RuntimeContext, diff --git a/src/message-runtime-rules.test.ts b/src/message-runtime-rules.test.ts index c5482d8..d4631a7 100644 --- a/src/message-runtime-rules.test.ts +++ b/src/message-runtime-rules.test.ts @@ -11,10 +11,9 @@ vi.mock('./config.js', async () => { }); vi.mock('./service-routing.js', async () => { - const actual = - await vi.importActual( - './service-routing.js', - ); + const actual = await vi.importActual( + './service-routing.js', + ); return { ...actual, hasReviewerLease: vi.fn(() => true) }; }); diff --git a/src/paired-execution-context-carry-forward.test.ts b/src/paired-execution-context-carry-forward.test.ts index 38d3679..a5dc62f 100644 --- a/src/paired-execution-context-carry-forward.test.ts +++ b/src/paired-execution-context-carry-forward.test.ts @@ -161,4 +161,89 @@ describe('paired execution carry-forward attachments', () => { }, ); }); + + it('carries context forward on a cold start after the previous task already closed', () => { + vi.clearAllMocks(); + const previousTask = buildTask({ + id: 'task-previous', + status: 'completed', + completion_reason: 'done', + }); + // No open task exists -> cold start path. + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(undefined); + vi.mocked(db.getLatestPairedTaskForChat).mockReturnValue(previousTask); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([ + { + id: 1, + task_id: previousTask.id, + turn_number: 1, + role: 'owner', + output_text: + 'TASK_DONE\n사용자 액션 아이템: 1. 재배포 2. 서버 업데이트', + created_at: '2026-03-28T00:01:00.000Z', + }, + ]); + + const resolved = resolveOwnerTaskForHumanMessage({ + group, + chatJid: 'dc:test', + roomRoleContext: ownerContext, + // no existingTask -> resolves via getLatestOpenPairedTaskForChat (none) + }); + + expect(resolved.supersededTask).toBeNull(); + expect(db.insertPairedTurnOutput).toHaveBeenCalledWith( + expect.any(String), + 0, + 'owner', + expect.stringContaining( + 'TASK_DONE\n사용자 액션 아이템: 1. 재배포 2. 서버 업데이트', + ), + expect.objectContaining({ createdAt: '2026-03-28T00:01:00.000Z' }), + ); + }); + + it('carries the last final owner output, skipping an intermediate STEP_DONE', () => { + vi.clearAllMocks(); + const previousTask = buildTask({ + id: 'task-arbiter', + status: 'completed', + completion_reason: 'arbiter_escalated', + }); + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(undefined); + vi.mocked(db.getLatestPairedTaskForChat).mockReturnValue(previousTask); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([ + { + id: 1, + task_id: previousTask.id, + turn_number: 1, + role: 'owner', + output_text: + 'TASK_DONE\n사용자 액션 아이템: 1. 재배포 2. 서버 업데이트', + created_at: '2026-03-28T00:01:00.000Z', + }, + { + id: 2, + task_id: previousTask.id, + turn_number: 3, + role: 'owner', + output_text: 'STEP_DONE\nexception 경로 버그를 고쳤습니다.', + created_at: '2026-03-28T00:03:00.000Z', + }, + ]); + + resolveOwnerTaskForHumanMessage({ + group, + chatJid: 'dc:test', + roomRoleContext: ownerContext, + }); + + expect(db.insertPairedTurnOutput).toHaveBeenCalledWith( + expect.any(String), + 0, + 'owner', + expect.stringContaining('사용자 액션 아이템'), + expect.objectContaining({ createdAt: '2026-03-28T00:01:00.000Z' }), + ); + }); }); diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index b3c0f49..43d0a2b 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -677,7 +677,9 @@ describe('paired execution context', () => { group, chatJid: 'dc:test', runId: 'run-host-reviewer', - roomRoleContext: reviewerContext, + // Force a claude-code reviewer so this test exercises the Claude + // read-only branch regardless of the deployment's default reviewer type. + roomRoleContext: { ...reviewerContext, reviewerAgentType: 'claude-code' }, }); expect(result?.envOverrides).toMatchObject({ diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 543e501..9a24f5f 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -229,14 +229,22 @@ function cancelOutstandingFinalizeOwnerTurn(task: PairedTask): void { ); } -function getLatestTurnOutputByRole( - taskId: string, - role: PairedRoomRole, -): PairedTurnOutput | null { +function isIntermediateStepOutput(outputText: string): boolean { + // STEP_DONE marks an intermediate step that keeps the task active; it is not + // the user-facing finalize summary, so it is a poor carry-forward anchor. + return /^\s*STEP_DONE\b/.test(outputText); +} + +function getLatestOwnerFinalOutput(taskId: string): PairedTurnOutput | null { + const ownerOutputs = [...getPairedTurnOutputs(taskId)] + .reverse() + .filter((output) => output.role === 'owner'); return ( - [...getPairedTurnOutputs(taskId)] - .reverse() - .find((output) => output.role === role) ?? null + ownerOutputs.find( + (output) => !isIntermediateStepOutput(output.output_text), + ) ?? + ownerOutputs[0] ?? + null ); } @@ -248,10 +256,7 @@ function carryForwardLatestOwnerFinal(args: { return; } - const latestOwnerFinal = getLatestTurnOutputByRole( - args.sourceTask.id, - 'owner', - ); + const latestOwnerFinal = getLatestOwnerFinalOutput(args.sourceTask.id); if (!latestOwnerFinal) { return; } @@ -293,16 +298,27 @@ export function resolveOwnerTaskForHumanMessage(args: { args.existingTask ?? getLatestOpenPairedTaskForChat(args.chatJid) ?? null; if (!existing) { - maybeRecordTaskDoneReopen(getLatestPairedTaskForChat(args.chatJid) ?? null); + const previousTask = getLatestPairedTaskForChat(args.chatJid) ?? null; + maybeRecordTaskDoneReopen(previousTask); + const newTask = canonicalWorkDir + ? createActiveTaskForRoom({ + group: args.group, + chatJid: args.chatJid, + canonicalWorkDir, + roomRoleContext: args.roomRoleContext, + }) + : null; + // Cold start after the previous task already closed (completed): seed the + // fresh task with the previous task's latest owner final so the owner keeps + // continuity across sessions instead of answering with "no context". + if (newTask && previousTask) { + carryForwardLatestOwnerFinal({ + sourceTask: previousTask, + targetTask: newTask, + }); + } return { - task: canonicalWorkDir - ? createActiveTaskForRoom({ - group: args.group, - chatJid: args.chatJid, - canonicalWorkDir, - roomRoleContext: args.roomRoleContext, - }) - : null, + task: newTask, supersededTask: null, }; } diff --git a/src/runtime-inventory.ts b/src/runtime-inventory.ts index c89f4ed..76d7803 100644 --- a/src/runtime-inventory.ts +++ b/src/runtime-inventory.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { getClaudeCredentialsPath } from './claude-credentials-path.js'; import { CURRENT_RUNTIME_AGENT_TYPE, DATA_DIR, @@ -236,7 +237,8 @@ export function getRuntimeInventory( pathSnapshot('Claude settings.json', claudeSettingsPath), pathSnapshot( 'Claude credentials', - path.join(homeDir, '.claude', '.credentials.json'), + getClaudeCredentialsPath(0, { allowHomeFallback: true }) ?? + path.join(homeDir, '.claude', '.credentials.json'), ), ], skillDirs: [ diff --git a/src/service-routing.test.ts b/src/service-routing.test.ts index 3c86cba..0289b44 100644 --- a/src/service-routing.test.ts +++ b/src/service-routing.test.ts @@ -48,7 +48,7 @@ describe('service-routing global failover', () => { expect(getEffectiveChannelLease('dc:paired')).toMatchObject({ chat_jid: 'dc:paired', owner_service_id: 'codex-review', - reviewer_service_id: 'claude', + reviewer_service_id: 'codex-review', owner_failover_active: true, reason: 'claude-429', explicit: true, @@ -115,8 +115,8 @@ describe('service-routing global failover', () => { expect(getGlobalFailoverInfo().active).toBe(false); expect(getEffectiveChannelLease('dc:paired')).toMatchObject({ chat_jid: 'dc:paired', - owner_service_id: 'codex-main', - reviewer_service_id: 'claude', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', owner_failover_active: false, explicit: false, }); @@ -141,7 +141,7 @@ describe('service-routing global failover', () => { expect(getEffectiveChannelLease('dc:explicit-single')).toMatchObject({ chat_jid: 'dc:explicit-single', - owner_service_id: 'codex-main', + owner_service_id: 'claude', reviewer_service_id: null, owner_failover_active: false, explicit: false, @@ -210,7 +210,7 @@ describe('service-routing global failover', () => { expect(getEffectiveChannelLease('dc:explicit-tribunal')).toMatchObject({ chat_jid: 'dc:explicit-tribunal', owner_service_id: 'claude', - reviewer_service_id: 'claude', + reviewer_service_id: 'codex-review', owner_failover_active: false, explicit: false, }); @@ -231,7 +231,7 @@ describe('service-routing global failover', () => { ).toMatchObject({ chat_jid: 'dc:explicit-tribunal-codex', owner_service_id: 'codex-main', - reviewer_service_id: 'claude', + reviewer_service_id: 'codex-review', owner_failover_active: false, explicit: false, }); @@ -263,7 +263,7 @@ describe('service-routing global failover', () => { it('defaults to the configured owner service for chats without canonical room settings', () => { expect(getEffectiveChannelLease('dc:unregistered')).toMatchObject({ chat_jid: 'dc:unregistered', - owner_service_id: 'codex-main', + owner_service_id: 'claude', reviewer_service_id: null, owner_failover_active: false, explicit: false, @@ -289,7 +289,7 @@ describe('service-routing global failover', () => { expect(getEffectiveChannelLease('dc:legacy-only')).toMatchObject({ chat_jid: 'dc:legacy-only', - owner_service_id: 'codex-main', + owner_service_id: 'claude', reviewer_service_id: null, owner_failover_active: false, explicit: false, @@ -344,6 +344,8 @@ describe('stored lease ids as SSOT', () => { activated_at: '2026-04-09T00:00:00.000Z', reason: 'ssot-test', }); + // A stored reviewer lease only applies while the room is paired (tribunal). + setExplicitRoomMode('dc:stored-reviewer-ssot', 'tribunal'); refreshChannelOwnerCache(true); const lease = getEffectiveChannelLease('dc:stored-reviewer-ssot'); @@ -359,3 +361,53 @@ describe('stored lease ids as SSOT', () => { ); }); }); + +describe('single-mode rooms never carry a reviewer lease', () => { + it('suppresses a stale stored reviewer lease when the room is single, keeping the owner', () => { + // Reproduces the incident: a room was tribunal, got a stored reviewer + // lease, then was switched to single. The stale reviewer must not route + // single-mode messages into the paired path. + setChannelOwnerLease({ + chat_jid: 'dc:stale-single', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + owner_agent_type: 'codex', + reviewer_agent_type: 'codex', + activated_at: '2026-05-27T00:00:00.000Z', + reason: 'was-tribunal', + }); + setExplicitRoomMode('dc:stale-single', 'single'); + refreshChannelOwnerCache(true); + + expect(getEffectiveChannelLease('dc:stale-single')).toMatchObject({ + chat_jid: 'dc:stale-single', + owner_service_id: 'codex-main', + reviewer_service_id: null, + arbiter_service_id: null, + }); + }); + + it('restores the stored reviewer lease when the room is switched back to tribunal', () => { + setChannelOwnerLease({ + chat_jid: 'dc:toggle-mode', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + owner_agent_type: 'codex', + reviewer_agent_type: 'codex', + activated_at: '2026-05-27T00:00:00.000Z', + reason: 'toggle', + }); + setExplicitRoomMode('dc:toggle-mode', 'single'); + refreshChannelOwnerCache(true); + expect( + getEffectiveChannelLease('dc:toggle-mode').reviewer_service_id, + ).toBeNull(); + + setExplicitRoomMode('dc:toggle-mode', 'tribunal'); + refreshChannelOwnerCache(true); + expect(getEffectiveChannelLease('dc:toggle-mode')).toMatchObject({ + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + }); + }); +}); diff --git a/src/service-routing.ts b/src/service-routing.ts index 35c53d9..18dcf6b 100644 --- a/src/service-routing.ts +++ b/src/service-routing.ts @@ -146,13 +146,36 @@ function getDefaultLease(chatJid: string): EffectiveChannelLease { }; } +function enforceRoomModeOnLease( + chatJid: string, + lease: EffectiveChannelLease, +): EffectiveChannelLease { + // Invariant: a single-mode room never runs a reviewer/arbiter, even if a + // stored lease still carries them from when the room was tribunal. Without + // this, a stale reviewer lease keeps routing single-mode messages into the + // paired path, where they stall forever on a stuck/mismatched execution + // lease (the "task revision was already claimed elsewhere" hang). The stored + // row is left untouched, so switching the room back to tribunal restores it. + if ( + (lease.reviewer_service_id == null && lease.arbiter_service_id == null) || + getEffectiveRuntimeRoomMode(chatJid) !== 'single' + ) { + return lease; + } + return { + ...lease, + reviewer_agent_type: null, + arbiter_agent_type: null, + reviewer_service_id: null, + arbiter_service_id: null, + }; +} + function getStoredOrDefaultLease(chatJid: string): EffectiveChannelLease { refreshChannelOwnerCache(); const row = leaseCache.get(chatJid); - if (row) { - return normalizeLeaseRow(row, true); - } - return getDefaultLease(chatJid); + const lease = row ? normalizeLeaseRow(row, true) : getDefaultLease(chatJid); + return enforceRoomModeOnLease(chatJid, lease); } export function refreshChannelOwnerCache(force = false): void { diff --git a/src/status-dashboard.ts b/src/status-dashboard.ts index 08b057b..b5e7788 100644 --- a/src/status-dashboard.ts +++ b/src/status-dashboard.ts @@ -9,6 +9,7 @@ import type { AgentType } from './types.js'; export interface StatusSnapshotEntry { jid: string; name: string; + chatName?: string; folder: string; agentType: AgentType; status: GroupStatus['status'];