diff --git a/COMMS/command-hq/platform.md b/COMMS/command-hq/platform.md new file mode 100644 index 00000000..4c1a6f9e --- /dev/null +++ b/COMMS/command-hq/platform.md @@ -0,0 +1,51 @@ +# Command HQ — the platform side + +Written 2026-08-18, after reading `mobile.md` and `watchers-what-broke.md`. + +## On `mobile.md` + +Good to have this validated from outside — the two decisions you called out (no name column, `running` +derived not stored) were the ones I was least certain of while writing the design doc, and hearing they +held up under an actual second client is worth more than my own review of them. + +**Agreed on the shape of `POST /command-hq/commanders/new`.** Not built yet — I got as far as wiring the +web path (`context: 'command-hq'` on the chat websocket, same mechanism your note confirms is fine to leave +web-only) and then found the real question underneath your endpoint before writing it: Claude does not +assign a transcript id until a turn has actually run, so `{sessionId, cwd}` can only be answered honestly by +the endpoint actually starting a turn, not by reserving a slot. Two things I want to settle before building +it rather than guess a second time: + +1. **`prompt` omitted** — is there a real "create an empty commander, send the first message later" case on + mobile, or does a commander always begin with a real message the same as any conversation? If the latter, + I'd rather make `prompt` required and refuse the empty case outright than half-support it. +2. **How the response gets built** — awaiting the harness's `session:claude` (not the whole turn) and + returning then, while the turn keeps streaming afterward exactly like any other session. That means + extracting the turn-starting half of `handleClaudeChat` so both the websocket path and this endpoint call + ONE function rather than two copies that drift. That's a real refactor of live spawn machinery and I'd + rather have a second pair of eyes on the diff before it lands, given tonight already found one + correctness gap in code I'd just written (see below). + +Full writeup, including what's actually built vs. still a plan: `docs/command-hq.md`, "How a commander is +created" and "Still open". + +**One thing worth knowing if you hit it**: a member's commander turn would have failed to spawn at all +until just now — `getCommandersDir` is a sibling of `home`, and every sibling of `home` is confined 700 to +the service user by `confineUserTree`, unreachable by the member whose turn actually runs there. Fixed by +chowning `commanders/` the same way `home` already is. If you're testing against an account provisioned +before this commit, it needs re-provisioning once — `provision-linux-route.ts` / `manage-users.ts` already +does this idempotently, nothing new to build. + +## On `watchers-what-broke.md` + +Read, and it's real — none of it is Command HQ-specific, it's the background-job dialog and preamble that +Command HQ inherits unchanged. Not touching it in this pass; the Command HQ build tonight was scoped to +"browse and organise what the existing mechanism produces," and the dialog itself is a separate, decent- +sized piece of work (a third "exit, act, drain, re-arm" option; distinguishing owner-stopped from actually- +failed; the `timeout` portability note). Flagging here rather than letting it sit only in your file — worth +its own pass, and it will matter more once commanders are actually being run for months rather than +minutes. + +The one item I'd weigh differently: **§5, stopped reads as failed.** For a `/chat` background task that's a +readability nit. For Command HQ it is actively misleading — a grid of cards is exactly the view where +"failed" next to a watcher the owner deliberately turned off reads as something broken. If this list gets +picked up in priority order, I'd put that one first. diff --git a/docs/command-hq.md b/docs/command-hq.md index 8d57082e..75195c42 100644 --- a/docs/command-hq.md +++ b/docs/command-hq.md @@ -3,11 +3,15 @@ Spec conversation 2026-08-18, owner + agent, back and forth. This is the write-up asked for partway through it, kept current as answers landed. -**Status:** data model and REST API built and mounted 2026-08-18 (`bunx tsgo` clean, suite at its 7 -pre-existing baseline failures — none of them this feature's). **Needs `bun db:push`** before any route -answers with real data; without it every route throws a Postgres "relation does not exist" 500. **Frontend -not built yet.** Every claim below about what exists is checked against the code, not aspirational — see -"API reference" for what a client can call today. +**Status:** data model, REST API, and the web creation path built and mounted 2026-08-18 (`bunx tsgo` clean, +suite at its 7 pre-existing baseline failures — none of them this feature's). **Needs `bun db:push`** +before any route answers with real data; without it every route throws a Postgres "relation does not +exist" 500. A member also needs `confineUserTree` re-run once if their account predates tonight — see +"A member could not reach it" under "Working directory". **Frontend not built yet on the web side.** The +mobile side is further along — `apps/commandhq` exists and typechecks against this router (see +`COMMS/command-hq/mobile.md`) — but cannot create a commander yet; that needs `POST +/command-hq/commanders/new`, not built server-side, see "Still open". Every claim in this file is checked +against the code, not aspirational — see "API reference" for what a client can call today. ## API reference — build against this @@ -46,10 +50,12 @@ poll if a client wants to know "did my watcher die" without opening it. with a special shape — it simply is not in the list. See "The /chat ↔ /command-hq relationship" below for why that is the design rather than an oversight. -**There is no "create a new commander" endpoint**, and that is deliberate, not a gap — see "How a commander -is created". A commander begins as an ordinary chat turn over the existing `/chat` websocket, in the -commanders directory, and the row is written server-side the moment a real transcript id exists. `POST -/commanders` above is the low-level primitive that write uses, not the thing a composer calls directly. +**There is no "create a new commander" endpoint yet — `POST /commanders/new` is planned, not built.** The web +composer does not need one: it sends an ordinary `/chat` websocket turn in the commanders directory, and the +row is written server-side the moment a real transcript id exists. `POST /commanders` above is the low-level +primitive that write uses internally, not something a client calls directly. A client that cannot open that +websocket with the right parameters — mobile, notably — has no way to create a commander today. See "How a +commander is created" and "Still open". ## What it is @@ -90,21 +96,37 @@ Two columns. ## How a commander is created -Resolved: there is no separate "file this into Command HQ" step. Starting a new commander **is** the -creation flow. +Resolved and **built for the web path** — there is no separate "file this into Command HQ" step. Starting a +new commander **is** the creation flow. -1. The user opens Command HQ, starts a new commander through its own composer — same send-a-prompt - mechanics as `/chat`, same background-job dialog for turning it into a watcher. -2. The moment the session has a real transcript id, a `command_hq_sessions` row is written — `user_id`, - `session_id`, and (for v1) an empty/default category. -3. **"Immediately"**, per the owner, mirrors an existing pattern rather than inventing one: `websocket.ts`'s - `createMessageHandler` already tracks a session's transcript id across a turn (seeded from - `session:claude`, confirmed on `result` — see the `transcriptId` handling used for `chat_session_models`) - because the id is not known until the harness reports it. The commander row should be written on the same - signal, gated on the request having come from Command HQ's composer (a flag on the initial chat message, - the same shape as `durable`/`member` are already threaded through `ClaudeSpawnStreamingParams`). +1. The web composer sends an ordinary `/chat` websocket turn with `context: 'command-hq'`. +2. `resolveChatCwd` (`websocket.ts`) reads that context and resolves `getCommandersDir(email)` itself — + `msg.cwd` is never consulted on this branch. The client does not choose, or even need to know, where + commanders live. +3. The moment the session has a real transcript id, `createMessageHandler` writes the `command_hq_sessions` + row — the same `session:claude`-then-`result` seed `chat_session_models`' `record()` already waits on, + with a sibling closure (`fileAsCommander`) hung off the same two triggers, gated on a `commandHq` flag + threaded in from `msg.context === 'command-hq'` at the call site. -No name is stored at creation — see "Naming", next. +**Mobile needs a different door, and it is not built yet.** `COMMS/command-hq/mobile.md` — read by the +platform side 2026-08-18, after the mobile client had already been built against the router — found the +web mechanism does not transfer: mobile sends chat turns with its own parameter set, so a context flag +would have to be carried correctly by two independent clients, and a client that gets it wrong silently +creates an ordinary `/chat` conversation in the wrong directory rather than failing loudly. Decided (with +the owner, in that conversation): a dedicated endpoint — + +``` +POST /command-hq/commanders/new + body: { categoryId?: number | null, prompt?: string } + -> { sessionId, cwd } +``` + +The server resolves the directory, runs the turn itself, and returns the real transcript id once the +harness has assigned one — the client is not responsible for constructing or guessing a path. **Not +implemented server-side yet** — see "Still open" for the design questions blocking it (mainly: what happens +when `prompt` is omitted, since Claude assigns a transcript id only once a turn has actually run). + +No name is stored at creation, on either path — see "Naming", next. ## Naming — resolved, and it changes the schema @@ -187,6 +209,30 @@ otherwise reserved for non-owner accounts and the pipeline executor. Commanders project transcripts and out of `/chat`'s history for the same underlying reason categories exist: a commander is not a project conversation, and should not be filed as if it were. +### A member could not reach it, and this is now fixed — but only going forward + +Found while wiring the creation flow, not guessed at: `getCommandersDir` is a sibling of `home`, and +`confineUserTree` (`os-user.ts`) confines **every** sibling of `home` to 700, owned by the SERVICE user, +"unreachable even by name" — correct for platform-written directories like `attachments/`, and exactly +wrong here, because a member's commander turn runs as the MEMBER's own Linux account via `setpriv`. This is +the identical shape of bug `resolveChatCwd`'s own comment already tells a story about (`general_chat_sessions`, +abandoned for the same reason) — a working directory the turn cannot `cd` into, surfacing as an agent that +fails on its own first command. + +**Fixed in `confineUserTree` itself**, not by abandoning the directory: it now chowns `commanders/` to the +member exactly as it already chowns `home`, and excludes it from the loop that confines every other +sibling. No ACL, unlike `home` — the file browser needs one because it reads a member's home from inside +the platform process as the service user; Command HQ has no equivalent, since it reads a member's commander +transcripts through `listTranscriptsAs`, the same as-the-member read `/chat` already uses for anyone's +transcripts. Plain ownership is the whole requirement, same as `~/.claude/projects` itself. + +**The gap this leaves:** `confineUserTree` runs at provisioning time. An account provisioned _before_ this +fix landed will not have `commanders/` chowned until it is provisioned again — the existing admin +reprovision action (`provision-linux-route.ts` / `manage-users.ts`) already re-runs `confineUserTree` +idempotently, so no new machinery was needed, but a member who existed before tonight needs that one +manual re-run before their first commander turn will actually spawn. Worth a release note, not a code +change. + ## The `/chat` ↔ `/command-hq` relationship **One predicate, opposite sign, both directions read from the same join.** @@ -267,4 +313,18 @@ scope for tonight."_ ## Still open -Nothing blocking as of this revision. +**`POST /command-hq/commanders/new` — the endpoint mobile needs, not yet built server-side.** The shape is +decided (see "How a commander is created"), but the one field that decides its implementation is not: + +- **When `prompt` is given**, the endpoint has to actually run a turn and wait for the harness to report a + real transcript id before it can respond — Claude does not assign one until a turn has run. That means + reusing the SAME spawn machinery `handleClaudeChat` already drives (`sendClaudeCodeStreaming`, + `sessionManager.getOrCreate`, listening for `session:claude`), awaited only up to that first event rather + than the whole turn, with the turn continuing to stream afterward exactly as any other session does. The + honest way to do this without a second, drifting copy of that logic is to extract the turn-starting half + of `handleClaudeChat` into something both call — not attempted yet, and worth review before it touches + live infrastructure a second time in one night. +- **When `prompt` is omitted**, there is nothing to run and therefore no real transcript id to return. + Whether that case is even needed for v1, or refused outright (a commander always begins with a real first + message, the same as any conversation), was not settled in `COMMS/command-hq/mobile.md` and needs a + decision before this is built rather than a guess baked into the router. diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index 17b062fe..52f1bbcb 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -15,7 +15,12 @@ import { sendClaudeCodeStreaming } from '@@/channels/send-claude-code'; import { sendOpenCodeStreaming } from '@@/channels/send-opencode'; import * as sidecar from '@@/sidecar-registry'; import { join } from 'path'; -import { getOwnerHomeDir, getEmailAccountsDir, findEmailAccountName } from '../../../servers/data-path'; +import { + getOwnerHomeDir, + getEmailAccountsDir, + findEmailAccountName, + getCommandersDir, +} from '../../../servers/data-path'; import { attachedFilePaths } from './attached-files'; import { getUserSettings, @@ -24,6 +29,7 @@ import { getUserById, setChatSessionModel, setChatSessionPlanMode, + createCommandHqSession, } from 'officerdb'; import { resolveHomeDir } from '@@/user-home'; import { claudeLoginState } from '@@/os-user-claude'; @@ -143,6 +149,19 @@ async function resolveChatCwd( userId: number, home: string, ): Promise { + // Every commander a user has shares ONE directory — never a client-chosen one, and never per-commander + // (see `getCommandersDir`). `msg.cwd` is ignored on this branch entirely, the same posture the identity + // half of a turn already takes: resolved by the platform, never trusted from the caller. + // + // `mkdirSync` — synchronous, so this returns only once the directory genuinely exists, the same pattern + // `resolveEmailCwd` below already uses. No race: unlike the rejected per-commander design, this directory + // is shared and only the very first commander ever needs the mkdir to do anything; `recursive: true` + // makes every call after that a no-op stat. + if (msg.context === 'command-hq') { + const dir = getCommandersDir(email); + mkdirSync(dir, { recursive: true }); + return dir; + } if (msg.context === 'email') return resolveEmailCwd(email, msg.contextId); if (msg.context === 'chat') { if (msg.cwd?.trim()) return resolveCwd(home, msg.cwd); @@ -319,7 +338,13 @@ function foldIntoSession(session: UserSession, msg: TurnMessage, model: string): } // Shared by both harnesses now: whichever sidecar ran the turn has already translated and committed it. -function createMessageHandler(sessionId: string, model: string, resumeSessionId?: string, planMode?: boolean) { +function createMessageHandler( + sessionId: string, + model: string, + resumeSessionId?: string, + planMode?: boolean, + commandHq?: boolean, +) { /* The model messages are ATTRIBUTED to, which is not always the model the turn asked for. @@ -403,6 +428,34 @@ function createMessageHandler(sessionId: string, model: string, resumeSessionId? }); }; + /* + File this session as a commander, once the transcript id is known — the same wait `record` above is + built around, and for the same reason: there is nothing to key the row on before the harness announces + it. Gated on `commandHq`, which the caller sets only when `msg.context === 'command-hq'`, so an + ordinary `/chat` turn never touches this table. + + Idempotent by construction (`createCommandHqSession` upserts on `(user_id, session_id)`), so calling it + again on every later turn of the same commander — which happens, because the client keeps sending + `context: 'command-hq'` for `resolveChatCwd` to keep resolving the right directory — costs one no-op + upsert rather than a duplicate row. That repetition is also a cheap self-heal: if the row were ever lost, + the next turn puts it back. + + Fire-and-forget, swallowed failure, same posture as `record`: a database hiccup here must not be a + reason a chat turn fails. + */ + const fileAsCommander = (): void => { + if (!commandHq || !transcriptId) return; + const userId = sessionManager.getSession(sessionId)?.userId; + if (!userId) return; + void createCommandHqSession(userId, transcriptId).catch((err: unknown) => { + logger.warn('Could not file this session as a commander', { + sessionId, + transcriptId, + error: err instanceof Error ? err.message : String(err), + }); + }); + }; + return (msg: TurnMessage, seq?: number): void => { const session = sessionManager.getSession(sessionId); if (!session) return; @@ -452,10 +505,12 @@ function createMessageHandler(sessionId: string, model: string, resumeSessionId? if (msg.type === 'session:claude' && msg.claudeSessionId) { transcriptId = msg.claudeSessionId; record(); + fileAsCommander(); } if (msg.type === 'result' && msg.claudeSessionId && msg.claudeSessionId !== transcriptId) { transcriptId = msg.claudeSessionId; record(); + fileAsCommander(); } /* @@ -723,8 +778,15 @@ async function handleClaudeCodeChat( session.isGenerating = true; // The resume id is passed so a tier switch on a RESUMED conversation is recorded when it happens rather - // than when the turn ends — see the seed in `createMessageHandler`. - const onMessage = createMessageHandler(sessionId, model, msg.resumeSessionId, msg.planMode); + // than when the turn ends — see the seed in `createMessageHandler`. `commandHq` is `msg.context === + // 'command-hq'`, the same signal `resolveChatCwd` above already reads to pick this session's directory. + const onMessage = createMessageHandler( + sessionId, + model, + msg.resumeSessionId, + msg.planMode, + msg.context === 'command-hq', + ); /** * One description of the turn, used by both paths below. diff --git a/src/servers/data-path.ts b/src/servers/data-path.ts index 97b11829..a504e432 100644 --- a/src/servers/data-path.ts +++ b/src/servers/data-path.ts @@ -209,6 +209,13 @@ export const getEmailAttachmentCacheDir = (ownerEmail: string) => * normal chat runs in `getOwnerHomeDir` (their real login home). Commanders use the managed directory * instead, on both sides of the account split, which is what keeps them out of the owner's own project * transcripts as surely as `command_hq_sessions` keeps them out of the `/chat` list. See `docs/command-hq.md`. + * + * **A sibling of `home`, but reachable like `home` — not confined like every other sibling.** Every other + * directory alongside a member's `home` (`attachments`, `email_accounts`, …) is 700 and owned by the + * SERVICE user, unreachable by the member "even by name" — correct for platform-written data, and exactly + * wrong for a directory a member's own `claude` must `cd` into via `setpriv`. `confineUserTree` + * (`os-user.ts`) chowns this one to the member the same way it chowns `home`. An account provisioned + * BEFORE this existed will not have that chown until it is re-provisioned once — see `docs/command-hq.md`. */ export const getCommandersDir = (email: string) => join(DATA_PATH, email, 'commanders'); diff --git a/src/servers/os-user.ts b/src/servers/os-user.ts index c4b74805..23633f39 100644 --- a/src/servers/os-user.ts +++ b/src/servers/os-user.ts @@ -2,7 +2,7 @@ import { chmod, mkdir, readdir, stat } from 'node:fs/promises'; import { existsSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { secretStorePath } from 'officerdb/secret-store'; -import { DATA_PATH, USER_DIRS, getOwnerHomeDir, toShellUsername } from './data-path'; +import { DATA_PATH, USER_DIRS, getOwnerHomeDir, getCommandersDir, toShellUsername } from './data-path'; // Real Linux accounts for members, so the surfaces that execute code can run as them. // @@ -337,10 +337,13 @@ export async function confineUserTree(params: { }; } - // Every sibling of `home` is the platform's. 700 means traversal alone does not open them. + // Every sibling of `home` is the platform's, confined 700 to the service user — EXCEPT `commanders`, + // which gets the same member ownership as `home` itself, just below. Both are excluded from this loop + // for the same reason: this pass runs unconditionally on every provision, and re-confining a directory + // a member is meant to reach would silently undo the block that grants it. const entries = await readdir(accountDir, { withFileTypes: true }); for (const entry of entries) { - if (entry.name === 'home') continue; + if (entry.name === 'home' || entry.name === 'commanders') continue; if (!entry.isDirectory()) continue; await chmod(join(accountDir, entry.name), 0o700); } @@ -404,6 +407,29 @@ export async function confineUserTree(params: { } } + // ── Commanders: a second directory the member must be able to reach directly ── + // + // Command HQ (docs/command-hq.md) gives every account ONE shared directory for the long-running jobs + // ("commanders") they start there — `getCommandersDir`, a sibling of `home` rather than something + // inside it, on purpose: keeping it out of the owner's own home is the whole reason it exists as its + // own directory rather than reusing `home`. But every OTHER sibling of `home` is confined 700 to the + // service user, unreachable "even by name" — and a member's commander turn runs as the MEMBER'S own + // Linux account via `setpriv`, so a sibling with that shape is exactly the `general_chat_sessions` + // mistake `resolveChatCwd` already tells this story about: a working directory the turn cannot even + // `cd` into, surfacing as an agent that fails on its own first command. + // + // No ACL here, unlike `home` above. `home` needs one because the file browser reads a member's home + // FROM INSIDE the platform process, as the service user. Command HQ has no equivalent: it reads a + // member's commander transcripts the same way `/chat` already reads any member's transcripts — through + // `listTranscriptsAs`, which runs the read AS the member, never as the service user directly. So plain + // ownership is the whole requirement, the same as `~/.claude/projects` itself. + const commandersDir = getCommandersDir(params.email); + if (!existsSync(commandersDir)) await mkdir(commandersDir, { recursive: true }); + const giveCommanders = await run(['sudo', '-n', 'chown', '-R', `${params.uid}:${params.gid}`, commandersDir]); + if (!giveCommanders.ok) return { ok: false, error: `chown of ${commandersDir} failed: ${giveCommanders.out}` }; + const closeCommanders = await run(['sudo', '-n', 'chmod', '700', commandersDir]); + if (!closeCommanders.ok) return { ok: false, error: `chmod of ${commandersDir} failed: ${closeCommanders.out}` }; + // ── One directory where container bind mounts can live ── // // Created for every account, not only the ones that get a daemon: it is two `install` calls, and