diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index 0333ebe7..b06aa246 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -19,6 +19,7 @@ import { listClaudePwds, listClaudeSessions, listAllClaudeSessions, + withCommanderFlag, loadClaudeSession, loadClaudeSessionById, deleteClaudeSession, @@ -27,7 +28,6 @@ import { stopBackgroundTask, claudeSessionContext, liveSessionTitle, - isCommandersDir, } from './claude-sessions'; import { listOpenCodeSessions, @@ -129,10 +129,15 @@ chatRouter.get('/sessions', async (ctx) => { // the row renderer) is indifferent to which directories the rows came from. const all = ctx.req.query('all') === '1'; - const claude = (all ? listAllClaudeSessions(who) : listClaudeSessions(who, cwd)).map((s) => ({ - ...s, - harness: 'claude' as const, - })); + // Which of these are commanders comes from the ROWS, not from where the transcript lives — a commander + // chooses its own working directory now, so the directory answers nothing. One query for the whole set. + const commanderIds = await listCommandHqSessionIds(user.id); + const claude = withCommanderFlag(all ? listAllClaudeSessions(who) : listClaudeSessions(who, cwd), commanderIds).map( + (s) => ({ + ...s, + harness: 'claude' as const, + }), + ); // OpenCode's store is shared and unscoped, so for anyone but the owner this list is other people's // conversations. Empty rather than filtered: there is no per-user field to filter ON. // @@ -266,7 +271,7 @@ chatRouter.get('/sessions/:id', async (ctx) => { commander wherever it is rendered, and `detail.cwd` is the transcript's own directory, so this answer travels with the conversation. */ - isCommander: isCommandersDir(who.email, detail.cwd), + isCommander: (await listCommandHqSessionIds(user.id)).has(id), }); }); @@ -292,6 +297,7 @@ chatRouter.get('/live', async (ctx) => { sidecar.listLiveClaudeSessions(user.id), who.isOwner ? sidecar.listLiveOpenCodeSessions() : Promise.resolve([]), ]); + const commanderIds = await listCommandHqSessionIds(user.id); const sessions = live.map((session) => { // Resolve by Claude's id, never by the session key — the key is officer's handle and the transcript // is named after Claude's. Null until the first turn reports one, which is a conversation that has @@ -304,14 +310,16 @@ chatRouter.get('/live', async (ctx) => { harness: 'claude' as const, title: resolved?.title ?? null, cwd, - // Derived from the cwd rather than looked up in `command_hq_sessions`, for the same reason the - // history listing does it: every commander shares one directory, so the directory IS the answer and - // it needs no query. It also stays right for a commander mid-first-turn, before any row exists. + // From the `command_hq_sessions` rows, because a commander chooses its own working directory and the + // directory therefore answers nothing. One query for the whole set, before the map below. + // + // A commander mid-first-turn has no row yet and so reports false here for one turn. That is the same + // turn on which it must not arm anything, and it corrects itself the moment it is filed. // // This field is why the route changed at all. The iPhone app swipes between LIVE chats and has no // room for tabs, so it can only exclude commanders if the payload says which ones they are — and a // deploy commander turning up in that swipe order is exactly what it must not do. - isCommander: cwd ? isCommandersDir(who.email, cwd) : false, + isCommander: transcriptId ? commanderIds.has(transcriptId) : false, }; }); diff --git a/src/servers/api/chat/claude-sessions.ts b/src/servers/api/chat/claude-sessions.ts index 4a19d92a..dd8218f4 100644 --- a/src/servers/api/chat/claude-sessions.ts +++ b/src/servers/api/chat/claude-sessions.ts @@ -984,20 +984,42 @@ function scanGroup(who: ChatIdentity, cwd: string): TranscriptSummary[] { return applyLineage(sessions); } -/** Conversations Claude has stored for a working directory, newest first, one row per `/clear` chain. */ +/** + * Conversations Claude has stored for a working directory, newest first, one row per `/clear` chain. + * + * **Says nothing about commanders**, deliberately. It used to: every commander shared one directory, so the + * directory was the answer and this computed it once per group. That stopped being true the moment a + * commander could choose its own working directory — a commander in the platform checkout would have been + * reported as an ordinary chat, and an ordinary chat in the old commanders directory as a commander. + * + * Identity is now the `command_hq_sessions` row, which is the only thing that survives the cwd being free. + * It is applied by the CALLER, because that requires a database read and this function is synchronous over a + * directory scan — see `withCommanderFlag`. + */ export function listClaudeSessions(who: ChatIdentity, cwd: string): ClaudeSessionSummary[] { - // Every commander a user has lives in ONE directory, so this is a property of the group rather than of - // each transcript — computed once here instead of per row. See `isCommander` on the summary type for - // why the flag has to reach the payload at all. - const commander = isCommandersDir(who.email, cwd); return mergeChains(scanGroup(who, cwd)) .map(publish) - .map((entry) => (commander ? { ...entry, isCommander: true } : entry)) .sort((a, b) => b.updatedAt.localeCompare(a.updatedAt)); } /** - * Is this directory the one every commander shares? + * Mark which of these are commanders, from the rows rather than from where they live. + * + * One query per request rather than one per session: `listCommandHqSessionIds` returns the whole set for an + * account, and a set membership test is what the per-row decoration needs. Commanders are few, so the set is + * small; the alternative — a lookup per transcript — would turn one list into fifty queries. + */ +export function withCommanderFlag(rows: T[], commanderIds: ReadonlySet): T[] { + return rows.map((row) => (commanderIds.has(row.id) ? { ...row, isCommander: true } : row)); +} + +/** + * Is this the default commanders directory? + * + * **No longer the test for whether something is a commander** — that is the `command_hq_sessions` row, since + * a commander now chooses its own working directory. This remains only for the places that legitimately care + * about that specific path: creating it for a new OS account, and as the historical location of commanders + * created before the cwd was choosable. * * Compared after `resolve`, so a trailing slash or a `..` cannot make the same directory look like a * different one — the cwd here comes from a transcript rather than from code, and transcripts record diff --git a/src/servers/api/command-hq/commander-block.ts b/src/servers/api/command-hq/commander-block.ts index 44d5eafb..fc4eb04a 100644 --- a/src/servers/api/command-hq/commander-block.ts +++ b/src/servers/api/command-hq/commander-block.ts @@ -1,5 +1,4 @@ -import { basename, dirname } from 'node:path'; -import { isCommandersDir } from '../chat/claude-sessions'; +import { getCommandHqSession, getUserById } from 'officerdb'; import { loopPidPath, spawnedRecordPath } from './spawned'; /* @@ -27,46 +26,26 @@ import { loopPidPath, spawnedRecordPath } from './spawned'; */ type CommanderBlockParams = { + /** The Officer account, for the row lookup and for the paths. */ + userId: number; /** Claude's transcript id — what a commander is keyed on everywhere. */ sessionId: string | null; - /** The session's working directory, which is what makes it a commander at all — and names the account. */ - cwd: string | null; }; -/** - * The account this cwd belongs to, or null if the cwd is not a commanders directory. - * - * Derived from the path rather than taken as an argument, because the caller does not have it: the sidecar - * knows a member's osUser and home, and the owner's turn carries no email at all. The cwd is - * `DATA_PATH//commanders`, so the email is right there — and it is verified by reconstructing the - * directory from it, so a coincidentally-shaped path cannot pass. - */ -function accountFromCommandersCwd(cwd: string): string | null { - const email = basename(dirname(cwd)); - if (!email) return null; - return isCommandersDir(email, cwd) ? email : null; -} +export async function commanderBlock(params: CommanderBlockParams): Promise { + if (!params.sessionId) return null; -/** - * The block, or null when this session is not a commander. - * - * **A brand-new commander's FIRST turn gets no block**, because there is no transcript id yet — the harness - * mints it as that turn begins, and nothing can be told a path keyed on an id that does not exist. - * - * That is not a gap, because arming on the first turn is not permitted: the first turn is for agreeing what - * to watch, and a watcher armed before that agreement is a guess. So the two facts interlock rather than - * conflict — an agent with no paths is by definition on its first turn, which is the turn it must not arm on - * anyway. The block says so explicitly, so the absence reads as a signal rather than as something missing. - * - * Keyed on the cwd rather than a database lookup, and deliberately: `isCommandersDir` is the same test the - * sessions list, `/chat/live` and the detail route all use, so a session cannot be a commander in one place - * and not in another. It also works before the row exists — a brand-new commander's first turn is the one - * that starts its loop, and a lookup would come back empty exactly then. - */ -export function commanderBlock(params: CommanderBlockParams): string | null { - if (!params.cwd || !params.sessionId) return null; - const email = accountFromCommandersCwd(params.cwd); - if (!email) return null; + // The ROW decides, not the working directory. A commander chooses its own cwd, so the directory answers + // nothing — and a session in the old shared directory is not a commander unless it was filed as one. + const row = await getCommandHqSession(params.userId, params.sessionId).catch(() => null); + if (!row) return null; + + // The paths need the account's email and the row carries a user id, so this is a second read. Once per + // session spawn, alongside the affordances and the skills index, which are also read here rather than + // cached — the same trade for the same reason. + const user = await getUserById(params.userId).catch(() => null); + if (!user?.email) return null; + const email = user.email; return [ '# You are a commander', diff --git a/src/servers/sidecar/claude/claude-manager.ts b/src/servers/sidecar/claude/claude-manager.ts index 37c97ece..53d105be 100644 --- a/src/servers/sidecar/claude/claude-manager.ts +++ b/src/servers/sidecar/claude/claude-manager.ts @@ -541,6 +541,11 @@ function createSession( params: ClaudeSpawnStreamingParams, onEvent: (event: ChatEvent) => void, onNotify: (kind: 'turn-ended' | 'input-needed') => void, + /** + * The commander block, or null. Passed IN because building it needs two database reads and this function + * is synchronous — hoisted to `spawnClaudeStreaming`, which is async and already has the account. + */ + commander: string | null, ): PersistentSession { const { sessionKey } = params; const input = makeInputQueue(); @@ -616,10 +621,7 @@ function createSession( preamble, which is where this used to live and which an agent arming itself never sees. See `api/command-hq/commander-block.ts`. */ - const commander = commanderBlock({ - sessionId: resumeId ?? null, - cwd: params.cwd ?? resumeCwd ?? null, - }); + // `commander` is a parameter, not computed here — see the note on it. const preamble = [affordances, skills, commander].filter(Boolean).join('\n\n') || null; const q = query({ @@ -892,7 +894,16 @@ export async function spawnClaudeStreaming( await applyModelChange(session, params.model); await applyPlanModeChange(session, params.planMode); } else { - session = createSession(params, onEvent, onNotify); + /* + The commander block is built here, before the session exists, because it needs the database and + `createSession` is synchronous. It resolves the row for this transcript id and returns null for + anything that is not a commander — which is every ordinary chat, at the cost of one indexed lookup. + */ + const commander = await commanderBlock({ + userId: params.userId, + sessionId: getClaudeSession(params.sessionKey, params.userId) ?? params.resumeSessionId ?? null, + }); + session = createSession(params, onEvent, onNotify, commander); } session.pushTurn(params.prompt, params.images); }