a commander is identified by its row, not by where it lives
Groundwork for choosing a commander's working directory. Identity was the CWD — every commander shared one directory, so the directory was the answer and it needed no query. The moment a commander can choose its own, that test is wrong in BOTH directions: a commander in the platform checkout reads as an ordinary chat, and an ordinary chat in the old shared directory reads as a commander. So identity is now the `command_hq_sessions` row, which is the only thing that survives the cwd being free. The isolation the shared directory provided is no longer carrying anything — the surfaces that needed it now filter explicitly, which is what made this possible. Four surfaces moved, and the shape of the change is the same in each: the transcript scanner stops inferring it and the ROUTE decorates from the rows. - `listClaudeSessions` says nothing about commanders any more. It is synchronous over a directory scan and identity now needs a database read, so `withCommanderFlag` applies it at the caller. - the sessions list, `/chat/live` and `GET /chat/sessions/:id` each read `listCommandHqSessionIds` once per request. A set membership test per row rather than a lookup per row: commanders are few, and the alternative turns one list into fifty queries. - `commanderBlock` resolves the row instead of parsing the account out of the path. It needs the email for the record paths, so that is a second read — once per session spawn, beside the affordances and the skills index, which are read there for the same reason. `commanderBlock` had to be hoisted out of `createSession`, which is synchronous, into `spawnClaudeStreaming`, which is async and already holds the account. Passing it in as a parameter rather than making the whole session constructor async. `isCommandersDir` survives but is documented as no longer being the identity test — it remains only for the places that legitimately care about that specific path: creating it for a new OS account, and as the historical home of commanders created before this. A commander mid-first-turn has no row and so reports false for one turn. That is the same turn it must not arm anything on, and it corrects itself the moment it is filed. Verified against real data: a filed commander renders the block, an unknown id and a null id both return null, and the live gate still answers 401. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -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<T extends { id: string }>(rows: T[], commanderIds: ReadonlySet<string>): 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
|
||||
|
||||
@@ -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/<email>/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<string | null> {
|
||||
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',
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user