commanders: a commander is a directory, not a session
Implements phase 1 of docs/commander-directory.md. The row owns its own identity
and its own directory; session_id becomes a nullable pointer to whichever
conversation is current.
The ordering was the bug. POST /command-hq/commanders took a sessionId the caller
already held, so a chat existed first and was filed as a commander second — and
the working directory is fixed when that session spawns, before any id exists. It
now takes {name, referencePath}, creates the row, its directory and its CLAUDE.md,
and returns the cwd to start the chat in. Nothing waits on the CLI.
commander-block.ts is deleted. Claude Code reads a CLAUDE.md from the working
directory, so what it injected per turn is written once at creation and the
commander extends it as the job is agreed. resolveChatCwd no longer trusts
msg.cwd at all: it looks the row up server-side. The containment reasoning that
made a client-chosen path safe still holds — it just no longer applies, because
the client has nothing left to choose.
Four things found in review, none of which the build had.
Ownership. confineUserTree chowns commanders/ once, at provisioning, so anything
created afterwards belongs to the service user — a MEMBER's commander directory
would be one their own claude cannot write into, failing as an agent that dies on
its first command in a directory it can see. Invisible because it works perfectly
for the owner, who has no osUser. Handed over on creation, best effort, and the
error is returned as well as logged.
getCommanderDir is guarded. Two callers rm -rf what it returns, and createCommander
inserts dir_name as '' before fixing it up from the serial id — an empty name
resolves to the commanders ROOT and would take every commander the account has.
The transaction closes that window; the guard means it does not have to.
The fallback in resolveChatCwd was silent. A commander whose row is not found runs
in the account's home with no CLAUDE.md, which reads as an agent that forgot its
job rather than a lookup that missed. It says so now.
And sv-SE is renamed alongside the other five locales. It did not exist when this
work was written.
Verified: SQL applied against a scratch database first — idempotent, several
commanders can hold a null pointer at once, one-per-transcript still enforced.
Then here: db:push emits no DDL and does not prompt, officer restarts clean, and
POST /commanders returns a real directory holding state/ and a CLAUDE.md.
1428 pass, tsgo clean.
The single existing commander was deleted rather than migrated, per the design.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -3,7 +3,9 @@ export {
|
||||
createCommandHqCategory,
|
||||
renameCommandHqCategory,
|
||||
deleteCommandHqCategory,
|
||||
createCommandHqSession,
|
||||
createCommander,
|
||||
setCommanderSessionId,
|
||||
getCommandHqSessionById,
|
||||
listCommandHqSessions,
|
||||
listCommandHqSessionIds,
|
||||
getCommandHqSession,
|
||||
@@ -31,6 +33,7 @@ export {
|
||||
export type {
|
||||
CommandHqCategory,
|
||||
CommandHqSessionRow,
|
||||
NewCommander,
|
||||
CommandHqTag,
|
||||
CommandHqWatcherFire,
|
||||
CommandHqTurnCost,
|
||||
|
||||
@@ -54,7 +54,12 @@ export async function deleteCommandHqCategory(userId: number, id: number): Promi
|
||||
|
||||
export type CommandHqSessionRow = {
|
||||
id: number;
|
||||
sessionId: string;
|
||||
/** Claude's transcript uuid — null until the first turn reports one. See the schema file's docstring. */
|
||||
sessionId: string | null;
|
||||
/** `command_hq_sessions.dir_name` — the directory under `getCommandersDir(email)` that IS this commander. */
|
||||
dirName: string;
|
||||
/** What this commander operates on. Seeded with one path at creation; grown in conversation. */
|
||||
referencePaths: string[];
|
||||
/** What the owner called it. Null for rows filed before the column existed — fall back to the transcript. */
|
||||
name: string | null;
|
||||
categoryId: number | null;
|
||||
@@ -69,6 +74,8 @@ export type CommandHqSessionRow = {
|
||||
const SESSION_ROW_COLUMNS = {
|
||||
id: commandHqSessions.id,
|
||||
sessionId: commandHqSessions.sessionId,
|
||||
dirName: commandHqSessions.dirName,
|
||||
referencePaths: commandHqSessions.referencePaths,
|
||||
name: commandHqSessions.name,
|
||||
categoryId: commandHqSessions.categoryId,
|
||||
watcherArmed: commandHqSessions.watcherArmed,
|
||||
@@ -78,30 +85,67 @@ const SESSION_ROW_COLUMNS = {
|
||||
createdAt: commandHqSessions.createdAt,
|
||||
};
|
||||
|
||||
export type NewCommander = { userId: number; name: string; referencePath: string; categoryId?: number | null };
|
||||
|
||||
/**
|
||||
* File a session as a commander. Upsert on `(user_id, session_id)` — a second call (a retried request, a
|
||||
* client that raced itself) is a no-op rather than a duplicate-key error, and does NOT move an
|
||||
* already-categorised commander back to uncategorised.
|
||||
* Create a commander row — no transcript, no directory. The directory is the CALLER's job
|
||||
* (`api/command-hq/directory.ts`): this file is Postgres only, and `dirName` has to come back from HERE
|
||||
* before anything can be created on disk, since it is derived from the row's own id.
|
||||
*
|
||||
* Two statements in one transaction rather than a single `INSERT ... RETURNING`: `dirName` is derived FROM
|
||||
* the id Postgres assigns on insert, so the row has to exist — with some not-null placeholder — before its
|
||||
* own directory name is knowable. Nothing else here is a candidate: a name can collide (two commanders
|
||||
* called "Backups"), a random string is one more thing to keep unique, and the id already IS unique and
|
||||
* stable the moment it exists.
|
||||
*/
|
||||
export async function createCommandHqSession(
|
||||
userId: number,
|
||||
sessionId: string,
|
||||
categoryId: number | null = null,
|
||||
name: string | null = null,
|
||||
): Promise<CommandHqSessionRow> {
|
||||
export async function createCommander(input: NewCommander): Promise<CommandHqSessionRow> {
|
||||
return db.transaction(async (tx) => {
|
||||
const [inserted] = await tx
|
||||
.insert(commandHqSessions)
|
||||
.values({
|
||||
userId: input.userId,
|
||||
sessionId: null,
|
||||
dirName: '', // fixed up below, once the id it is derived from exists — never visible outside this transaction
|
||||
referencePaths: [input.referencePath],
|
||||
name: input.name,
|
||||
categoryId: input.categoryId ?? null,
|
||||
})
|
||||
.returning({ id: commandHqSessions.id });
|
||||
const [row] = await tx
|
||||
.update(commandHqSessions)
|
||||
.set({ dirName: String(inserted!.id) })
|
||||
.where(eq(commandHqSessions.id, inserted!.id))
|
||||
.returning(SESSION_ROW_COLUMNS);
|
||||
return row!;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Point a commander at its current conversation — the transcript id, once the harness reports one. Called
|
||||
* from `fileAsCommander` (`api/chat/websocket.ts`) the moment `session:claude` arrives, correlated by the
|
||||
* commander's own row id (`msg.commanderId`), not by upserting on the transcript id the way this table used
|
||||
* to be filed: the row already exists by the time any turn spawns, per `docs/commander-directory.md`, so
|
||||
* there is nothing left to upsert.
|
||||
*
|
||||
* A plain UPDATE, not gated on the column currently being null: a dead session replaced by a fresh one in
|
||||
* the same directory repoints the same row, which is the whole reason this is a pointer and not an identity.
|
||||
*/
|
||||
export async function setCommanderSessionId(userId: number, id: number, sessionId: string): Promise<boolean> {
|
||||
const rows = await db
|
||||
.update(commandHqSessions)
|
||||
.set({ sessionId })
|
||||
.where(and(eq(commandHqSessions.id, id), eq(commandHqSessions.userId, userId)))
|
||||
.returning({ id: commandHqSessions.id });
|
||||
return rows.length > 0;
|
||||
}
|
||||
|
||||
/** One commander's row, by its OWN id — for a caller (the composer, mid-creation) that has no transcript yet. */
|
||||
export async function getCommandHqSessionById(userId: number, id: number): Promise<CommandHqSessionRow | null> {
|
||||
const [row] = await db
|
||||
.insert(commandHqSessions)
|
||||
.values({ userId, sessionId, categoryId, name })
|
||||
.onConflictDoNothing({ target: [commandHqSessions.userId, commandHqSessions.sessionId] })
|
||||
.returning(SESSION_ROW_COLUMNS);
|
||||
// `onConflictDoNothing` returns nothing on the conflict path, so the existing row is fetched explicitly —
|
||||
// the caller still needs an id back (to attach tags, say) even when this call raced an earlier one.
|
||||
if (row) return row;
|
||||
const [existing] = await db
|
||||
.select(SESSION_ROW_COLUMNS)
|
||||
.from(commandHqSessions)
|
||||
.where(and(eq(commandHqSessions.userId, userId), eq(commandHqSessions.sessionId, sessionId)));
|
||||
return existing!;
|
||||
.where(and(eq(commandHqSessions.userId, userId), eq(commandHqSessions.id, id)));
|
||||
return row ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -129,15 +173,22 @@ const isCategoryNull = () => isNull(commandHqSessions.categoryId);
|
||||
* Just the session ids this user has filed — the whole shape `/chat`'s list needs to exclude them, and
|
||||
* `/command-hq`'s grid needs to include only them. One query, no join against transcripts (they are not in
|
||||
* Postgres).
|
||||
*
|
||||
* A commander between `POST /commanders` and its first turn has no transcript yet — `sessionId` is null —
|
||||
* and is filtered out here rather than returned as a literal `null` in the set: there is no transcript id
|
||||
* to exclude from `/chat`'s list, and a null in a `Set<string>` would only ever fail to match anything.
|
||||
*/
|
||||
export async function listCommandHqSessionIds(userId: number): Promise<Set<string>> {
|
||||
const rows = await db
|
||||
.select({ sessionId: commandHqSessions.sessionId })
|
||||
.from(commandHqSessions)
|
||||
.where(eq(commandHqSessions.userId, userId));
|
||||
return new Set(rows.map((r) => r.sessionId));
|
||||
.where(and(eq(commandHqSessions.userId, userId), isNotNullSessionId()));
|
||||
return new Set(rows.map((r) => r.sessionId!));
|
||||
}
|
||||
|
||||
// drizzle's `eq` cannot express `IS NOT NULL` either.
|
||||
const isNotNullSessionId = () => sql`${commandHqSessions.sessionId} is not null`;
|
||||
|
||||
export async function getCommandHqSession(userId: number, sessionId: string): Promise<CommandHqSessionRow | null> {
|
||||
const [row] = await db
|
||||
.select(SESSION_ROW_COLUMNS)
|
||||
@@ -352,7 +403,13 @@ export async function deleteCommandHqSession(userId: number, sessionId: string):
|
||||
*/
|
||||
export async function purgeCommandHqSessions(userId: number, liveSessionIds: Set<string>): Promise<number> {
|
||||
const all = await listCommandHqSessions(userId);
|
||||
const dead = all.filter((row) => !liveSessionIds.has(row.sessionId)).map((row) => row.sessionId);
|
||||
// A row with `sessionId: null` has no transcript YET — it is mid-creation, not dead — so it is never a
|
||||
// purge candidate regardless of what `liveSessionIds` holds. Only a row that HAD a transcript and lost it
|
||||
// is "dead" in the sense this sweep exists to clean up.
|
||||
const dead = all
|
||||
.filter((row): row is CommandHqSessionRow & { sessionId: string } => row.sessionId !== null)
|
||||
.filter((row) => !liveSessionIds.has(row.sessionId))
|
||||
.map((row) => row.sessionId);
|
||||
if (!dead.length) return 0;
|
||||
const rows = await db
|
||||
.delete(commandHqSessions)
|
||||
@@ -508,7 +565,11 @@ export async function parentCommanderOf(spawnedSessionId: string): Promise<Spawn
|
||||
.innerJoin(commandHqSessions, eq(commandHqSessions.id, commandHqSpawned.sessionRowId))
|
||||
.where(eq(commandHqSpawned.spawnedSessionId, spawnedSessionId))
|
||||
.limit(1);
|
||||
if (!row) return null;
|
||||
// `row.sessionId` is null for a commander mid-creation — no live loop exists to have spawned anything yet
|
||||
// at that point, so a row in this state here would mean the query above found something it should not
|
||||
// have been able to. Treated as "no parent" rather than asserted away, for the same reason every other
|
||||
// null here is: an unexpected fact is not this function's to paper over.
|
||||
if (!row || row.sessionId === null) return null;
|
||||
return {
|
||||
sessionId: row.sessionId,
|
||||
name: row.name,
|
||||
|
||||
@@ -65,13 +65,17 @@ export const commandHqCategories = pgTable(
|
||||
);
|
||||
|
||||
/**
|
||||
* One row per commander — the mapping from a Claude transcript to Command HQ's own metadata about it.
|
||||
* One row per commander. The ROW is the commander now — `docs/commander-directory.md` — with its own
|
||||
* directory on disk (`dirName`) and its own id, not a mapping keyed on a Claude transcript. `sessionId` is
|
||||
* a nullable POINTER to whichever conversation is current, set once the harness reports one and free to be
|
||||
* repointed if that conversation dies and a fresh one replaces it in the same directory.
|
||||
*
|
||||
* `/chat` and `/command-hq` read the SAME underlying session list (transcripts on disk) and this table is
|
||||
* the filter between them: `/chat` excludes any session with a row here, `/command-hq` includes only
|
||||
* sessions that have one. The table is never itself the list of what exists — a row whose transcript is
|
||||
* gone is simply absent from both lists, on purpose. See `docs/command-hq.md` → "The /chat ↔ /command-hq
|
||||
* relationship".
|
||||
* `/chat` and `/command-hq` still read the SAME underlying session list (transcripts on disk) once a row
|
||||
* has a `sessionId`, and this table is still the filter between them: `/chat` excludes any session with a
|
||||
* row here, `/command-hq` includes rows that have a live transcript. A row whose transcript is gone is
|
||||
* simply absent from both lists, on purpose. See `docs/command-hq.md` → "The /chat ↔ /command-hq
|
||||
* relationship" — written before the identity change, still correct for a row that already has a
|
||||
* transcript.
|
||||
*/
|
||||
export const commandHqSessions = pgTable(
|
||||
'command_hq_sessions',
|
||||
@@ -85,8 +89,38 @@ export const commandHqSessions = pgTable(
|
||||
* for the same reason: everything that joins back to a session on disk (models, events, this table)
|
||||
* uses the id the transcript itself carries, because that is the one Officer's `sessionId` and Claude's
|
||||
* id spaces both agree on only sometimes.
|
||||
*
|
||||
* **Nullable, since 2026-08-24 (`docs/commander-directory.md`).** This used to be the commander's
|
||||
* IDENTITY — the row was created only once a transcript existed, which is what forced a commander to be
|
||||
* created after its own working directory rather than before. The row's own `id`/`dirName` is the
|
||||
* identity now; this is a POINTER to whichever conversation is current, null from the moment the row is
|
||||
* created until the first turn reports a real transcript id, and free to be repointed later if a dead
|
||||
* session is replaced by a fresh one in the same directory.
|
||||
*/
|
||||
sessionId: text('session_id').notNull(),
|
||||
sessionId: text('session_id'),
|
||||
/**
|
||||
* The directory under `getCommandersDir(email)` that IS this commander —
|
||||
* `$DATA_PATH/<email>/commanders/<dirName>/`, holding `CLAUDE.md` and `state/`. See
|
||||
* `docs/commander-directory.md`.
|
||||
*
|
||||
* Derived from this row's own serial `id` (`String(id)`) the moment the row is inserted — nothing else
|
||||
* is guaranteed unique and stable before a name is even chosen. Not the transcript id: that does not
|
||||
* exist yet at creation, which is the entire point of this table gaining its own identity.
|
||||
*/
|
||||
dirName: text('dir_name').notNull(),
|
||||
/**
|
||||
* What the commander operates ON — not where it lives. Seeded with one path at creation (the start
|
||||
* dialog collects exactly one; asking for a list up front gets a worse answer than asking later) and
|
||||
* grown as the commander and the owner agree on more in conversation.
|
||||
*
|
||||
* A Postgres `text[]` rather than `jsonb`: every element is a plain path string with no structure of
|
||||
* its own, and an array supports appending in place (`array_append`) without reading the whole column
|
||||
* back to modify it — `jsonb` would buy nothing here but a heavier read-modify-write for the same list.
|
||||
*/
|
||||
referencePaths: text('reference_paths')
|
||||
.array()
|
||||
.notNull()
|
||||
.default([]),
|
||||
/**
|
||||
* What the owner called it, held here rather than only in the transcript's summary record.
|
||||
*
|
||||
@@ -130,6 +164,14 @@ export const commandHqSessions = pgTable(
|
||||
watcherStoppedAt: timestamp('watcher_stopped_at', { withTimezone: true }),
|
||||
createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
},
|
||||
// `sessionId` is nullable now, and this is left as a PLAIN unique index rather than a partial one
|
||||
// (`.where(isNotNull(...))`) deliberately, not by default: Postgres already treats every NULL as
|
||||
// distinct from every other NULL in a unique index, so any number of a user's commanders can sit with
|
||||
// `session_id: null` at once — which is exactly the window between `POST /commanders` creating the row
|
||||
// and the first turn reporting a real transcript id, for however many commanders are mid-creation at
|
||||
// the same time. The moment `session_id` is set, uniqueness still applies: one commander per transcript,
|
||||
// per user, same guarantee this index always gave. A partial index would enforce the identical rule with
|
||||
// an extra clause to keep in sync — nothing here needs it.
|
||||
(table) => [uniqueIndex('uq_command_hq_sessions_user_session').on(table.userId, table.sessionId)],
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user