diff --git a/COMMS/sidecar-app-store/19-control-surface-scoped.md b/COMMS/sidecar-app-store/19-control-surface-scoped.md new file mode 100644 index 00000000..62659a81 --- /dev/null +++ b/COMMS/sidecar-app-store/19-control-surface-scoped.md @@ -0,0 +1,59 @@ +# 19 — the six commands are scoped; the gates' last code-side blocker is gone + +Answering `16`, and closing the item I flagged as unfinished in `11`. The owner is still awake, so this is not +tomorrow's work arriving early — it is the mechanical change I said deserved its own commit. + +## What changed + +All six now carry `userId`, resolved by the platform from the authenticated request and never taken from the +client, and every handler enforces it: + +| command | enforcement | +|---|---| +| `claude:kill` | `ownedSession` — a mismatch is "no such session" | +| `claude:interrupt` | same | +| `claude:clear-session` | asks `getClaudeSession(key, userId)` first, so it can only clear a mapping that is theirs | +| `claude:is-generating` | `ownedSession`; not-yours answers `false`, the same as no-such-session | +| `claude:find-session` | the reattach hinge — scoped, so a browser holding a uuid it should not have is not handed the key that drives it | +| `claude:list` | **filtered**, not labelled — your point that enumeration is a disclosure on its own, before anyone acts on anything | + +Two shapes throughout, both deliberate: + +- **"Not yours" and "does not exist" answer identically.** Every caller treats them the same, and a distinct + answer for the second tells a guesser a session exists under a key they do not own. Same reasoning as + `getClaudeSession` returning undefined in `11`, applied to the control surface. +- **`ownedSession(key, userId)`** is one helper rather than four inline checks, so the next command added here + has an obvious thing to call. + +## Two call sites worth your eye + +**`endTurnIfAgentIsGone` on sidecar restart** (`websocket.ts`). It sweeps every session officer believes is +generating and asks the agent whether it really is. Now that the question is scoped, a session whose `userId` +is undefined has no safe id to ask as — asking as the owner would answer a member's orphaned session with the +owner's authority — so it is **skipped**. That leaves it marked generating until the next reconnect corrects +it, which is exactly what happened before that loop existed. I would rather be stale than authoritative on +someone else's behalf, but flagging it because it is a behaviour change on the owner's path too. + +**`chat.ts:/live`** now passes `user.id` — the endpoint already had the user, it just was not using it. + +`tsgo` clean, 97 tests pass, both gates unchanged, `member` still populated by nothing. + +## What this does and does not unblock + +It removes the **code-side** reason the gates cannot move. It does not make them movable. Still outstanding, +none of it mine to decide: + +- a member has never signed in, so no member turn has ever run +- `spawnClaudeCodeProcess` has still never been called +- `8931309` and `9833822` are unverified until a reprovision +- the owner has not ruled on wire-first, and the gates were never mine or yours to lift + +What I would want before anyone touches `chat.ts:49`: one member signed in, one member turn observed +end to end, and a deliberate look at whether `resumeSessionId` off the client message is still defence by +accident once the history layer resolves transcripts centrally. + +## Next + +`deprovisionOsAccount` is the last thing on my list that needs neither you nor a live account. Starting it now +unless `20` says otherwise — to your spec, `chown` before `userdel`, abort on a failed `chown`, reap explicitly +between terminate and delete, and verify the subuid range as well as the uid. diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index dd93e3a6..845d0995 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -129,11 +129,12 @@ chatRouter.get('/sessions/:id', async (ctx) => { // Titles are resolved here rather than in the client, which can only name the sessions in the group it // happens to be browsing — which is how the list ended up showing raw ids for anything running elsewhere. chatRouter.get('/live', async (ctx) => { - const email = ctx.get('user').email; + const user = ctx.get('user'); + const email = user.email; // Both harnesses, asked in parallel. Either failing contributes nothing rather than failing the panel: // both registry calls swallow their errors and return []. const [live, liveOpenCode] = await Promise.all([ - sidecar.listLiveClaudeSessions(), + sidecar.listLiveClaudeSessions(user.id), sidecar.listLiveOpenCodeSessions(), ]); const sessions = live.map((session) => { diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index ee36abe4..bd799643 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -537,7 +537,7 @@ async function handleStop(ws: ServerWebSocket): Promise { if (isClaudeModel(session.model)) { // Interrupt the current turn but KEEP the persistent session alive (background tasks + the // warm worker survive). Full teardown is 'disconnect' → deleteSession → _claudeKill. - void sidecar.interruptClaude(sessionId); + void sidecar.interruptClaude(sessionId, ws.data.userId); logger.info('Interrupted Claude Code turn via sidecar (session stays warm)', { sessionId }); } else { session._claudeKill?.(); // OpenCode: abort the turn via the stored handle @@ -604,7 +604,7 @@ function adoptOrphanedSession(ws: ServerWebSocket, sessionId: string, mo // Must be set, and not only for teardown: handleChat treats an absent `_claudeKill` as "first turn of // this session" and opens a *second* subscription, which would then deliver every message twice. session._claudeKill = () => { - if (isClaudeModel(model)) sidecar.killClaude(sessionId); + if (isClaudeModel(model)) sidecar.killClaude(sessionId, userId); else sidecar.killOpenCode(sessionId); unsub(); }; @@ -653,7 +653,7 @@ async function handleResumeCursor( // the Claude sidecar about an OpenCode session, hear "not generating", and write "the agent went // away" into a turn that was running perfectly well. if (msg.generating && decision.kind !== 'assume') { - await endTurnIfAgentIsGone([ws], sessionId, decision.model); + await endTurnIfAgentIsGone([ws], sessionId, decision.model, ws.data.userId); } } @@ -738,7 +738,7 @@ async function handleAttach(ws: ServerWebSocket, msg: { claudeSessionId: const { claudeSessionId } = msg; if (!claudeSessionId) return; - const sessionId = await sidecar.findClaudeSessionKey(claudeSessionId); + const sessionId = await sidecar.findClaudeSessionKey(claudeSessionId, ws.data.userId); if (!sessionId) { // No agent, or a transcript it has never run. Nothing is wrong: an ordinary finished conversation // opened from history lands here every time. Stay silent and leave the socket as it was — the next @@ -774,7 +774,7 @@ async function handleAttach(ws: ServerWebSocket, msg: { claudeSessionId: // `isGenerating` is officer's own belief and is only as good as this process's memory of the turn. For // an adopted session it is a fresh record's default, so ask the agent — the same question, and for the // same reason, as `endTurnIfAgentIsGone`. - const isGenerating = existing ? session.isGenerating : await sidecar.isClaudeGenerating(sessionId); + const isGenerating = existing ? session.isGenerating : await sidecar.isClaudeGenerating(sessionId, ws.data.userId); session.isGenerating = isGenerating; // One read serves both answers: the head of the log is the cursor, and folding the whole log gives the @@ -826,9 +826,10 @@ async function endTurnIfAgentIsGone( targets: Iterable | null>, sessionId: string, model: string, + userId: number, ): Promise { if (!isClaudeModel(model)) return; - if (await sidecar.isClaudeGenerating(sessionId)) return; + if (await sidecar.isClaudeGenerating(sessionId, userId)) return; const session = sessionManager.getSession(sessionId); if (session) session.isGenerating = false; @@ -852,10 +853,16 @@ async function endTurnIfAgentIsGone( sidecar.onClaudeSidecarStarted(() => { for (const session of sessionManager.getAllSessions()) { if (!session.isGenerating) continue; + // No owner recorded, no question asked. `isClaudeGenerating` is now scoped to a caller, and there is no + // safe id to substitute — asking as the owner would let a member's orphaned session be answered with the + // owner's authority, and asking as nobody is not a thing. Leaving it marked generating is the same + // outcome as before this loop existed, and it self-corrects on the next reconnect. + if (session.userId === undefined) continue; void endTurnIfAgentIsGone( session.sockets as Set>, session.sessionId, session.model, + session.userId, ); } }); diff --git a/src/servers/channels/send-claude-code.ts b/src/servers/channels/send-claude-code.ts index a6793c9a..4c7d4a0d 100644 --- a/src/servers/channels/send-claude-code.ts +++ b/src/servers/channels/send-claude-code.ts @@ -18,8 +18,8 @@ type ClaudeCodeResult = { cost: MessageCost; }; -export function clearClaudeCodeSession(sessionKey: string): void { - sidecar.clearClaudeSession(sessionKey); +export function clearClaudeCodeSession(sessionKey: string, userId: number): void { + sidecar.clearClaudeSession(sessionKey, userId); } export async function sendClaudeCode(params: ClaudeCodeParams): Promise { @@ -75,7 +75,7 @@ export async function sendClaudeCodeStreaming(params: ClaudeCodeStreamingParams) return { kill: () => { - sidecar.killClaude(params.sessionKey); + sidecar.killClaude(params.sessionKey, params.userId); unsub(); }, detach: unsub, diff --git a/src/servers/sidecar-registry.ts b/src/servers/sidecar-registry.ts index 6a615162..628f10aa 100644 --- a/src/servers/sidecar-registry.ts +++ b/src/servers/sidecar-registry.ts @@ -288,13 +288,13 @@ export async function spawnClaudeStreaming(params: ClaudeSpawnStreamingParams): throw new Error('Unexpected response'); } -export function killClaude(sessionKey: string): void { - sendFire('claude', { type: 'claude:kill', id: nextId(), sessionKey }); +export function killClaude(sessionKey: string, userId: number): void { + sendFire('claude', { type: 'claude:kill', id: nextId(), sessionKey, userId }); } // Interrupt the current turn but keep the persistent session warm (the "stop" button). -export function interruptClaude(sessionKey: string): void { - sendFire('claude', { type: 'claude:interrupt', id: nextId(), sessionKey }); +export function interruptClaude(sessionKey: string, userId: number): void { + sendFire('claude', { type: 'claude:interrupt', id: nextId(), sessionKey, userId }); } /** @@ -323,11 +323,11 @@ export function interruptClaude(sessionKey: string): void { * deliberate: there, not knowing means leaving a spinner up; here, not knowing would mean inventing * sessions, and an enumeration that reports things that may not exist is worse than a short one. */ -export async function listLiveClaudeSessions(): Promise { +export async function listLiveClaudeSessions(userId: number): Promise { const sc = findSidecarByCapability('claude'); if (!sc) return []; try { - const res = await sendCommandToSidecar(sc, { type: 'claude:list', id: nextId() }); + const res = await sendCommandToSidecar(sc, { type: 'claude:list', id: nextId(), userId }); return res.type === 'claude:sessions' ? res.sessions : []; } catch { return []; @@ -352,11 +352,11 @@ export async function listLiveOpenCodeSessions(): Promise } } -export async function isClaudeGenerating(sessionKey: string): Promise { +export async function isClaudeGenerating(sessionKey: string, userId: number): Promise { const sc = findSidecarByCapability('claude'); if (!sc) return false; try { - const res = await sendCommandToSidecar(sc, { type: 'claude:is-generating', id: nextId(), sessionKey }); + const res = await sendCommandToSidecar(sc, { type: 'claude:is-generating', id: nextId(), sessionKey, userId }); return res.type === 'claude:generating' ? res.generating : true; } catch { return true; @@ -373,19 +373,19 @@ export async function isClaudeGenerating(sessionKey: string): Promise { * Fails toward null: no agent, no answer, or a timeout all mean "cannot re-bind", and the caller falls * back to today's behaviour of leaving the socket unattached rather than binding it to a guess. */ -export async function findClaudeSessionKey(claudeSessionId: string): Promise { +export async function findClaudeSessionKey(claudeSessionId: string, userId: number): Promise { const sc = findSidecarByCapability('claude'); if (!sc) return null; try { - const res = await sendCommandToSidecar(sc, { type: 'claude:find-session', id: nextId(), claudeSessionId }); + const res = await sendCommandToSidecar(sc, { type: 'claude:find-session', id: nextId(), claudeSessionId, userId }); return res.type === 'claude:session-key' ? res.sessionKey : null; } catch { return null; } } -export function clearClaudeSession(sessionKey: string): void { - sendFire('claude', { type: 'claude:clear-session', id: nextId(), sessionKey }); +export function clearClaudeSession(sessionKey: string, userId: number): void { + sendFire('claude', { type: 'claude:clear-session', id: nextId(), sessionKey, userId }); } // Turn output arrives finished and already durable: the agent translated it and committed it to diff --git a/src/servers/sidecar/claude/claude-manager.ts b/src/servers/sidecar/claude/claude-manager.ts index 4bb52ce0..6a77ee70 100644 --- a/src/servers/sidecar/claude/claude-manager.ts +++ b/src/servers/sidecar/claude/claude-manager.ts @@ -294,7 +294,8 @@ function armIdle(session: PersistentSession): void { armIdle(session); return; } - killClaudeSession(session.sessionKey); + // The idle GC is this process acting on its own session, so it passes the session's own owner. + killClaudeSession(session.sessionKey, session.userId); }, IDLE_TIMEOUT_MS); } @@ -494,8 +495,8 @@ export async function spawnClaudeStreaming( } /** Interrupt the current turn but KEEP the session alive (the "stop" button). */ -export async function interruptClaudeSession(sessionKey: string): Promise { - const session = sessions.get(sessionKey); +export async function interruptClaudeSession(sessionKey: string, userId: number): Promise { + const session = ownedSession(sessionKey, userId); if (!session) return false; // Set before the await: the failed `result` can arrive while interrupt() is still resolving, and the // consumer loop reads this flag to tell a stop from a fault. @@ -513,8 +514,8 @@ export async function interruptClaudeSession(sessionKey: string): Promise ({ - sessionKey: session.sessionKey, - // The only place this mapping exists. Without it a caller cannot find the transcript, because the - // key is officer's handle and the filename is Claude's id. - claudeSessionId: getClaudeSession(session.sessionKey, session.userId) ?? null, - isGenerating: session.isGenerating, - pendingTasks: session.pendingTasks.size, - })); +export function listSessions(userId: number): LiveClaudeSession[] { + // Filtered, not just labelled. Enumerating every live session is a disclosure on its own, before anyone + // acts on one: it names other accounts' conversations and says which are busy. + return Array.from(sessions.values()) + .filter((session) => session.userId === userId) + .map((session) => ({ + sessionKey: session.sessionKey, + // The only place this mapping exists. Without it a caller cannot find the transcript, because the + // key is officer's handle and the filename is Claude's id. + claudeSessionId: getClaudeSession(session.sessionKey, session.userId) ?? null, + isGenerating: session.isGenerating, + pendingTasks: session.pendingTasks.size, + })); } /** @@ -562,6 +583,8 @@ export function listSessions(): LiveClaudeSession[] { * nothing — and if *this* process was the one that restarted, the session is simply absent and the turn * it was running is gone, however alive the client still believes it to be. */ -export function isSessionGenerating(sessionKey: string): boolean { - return sessions.get(sessionKey)?.isGenerating ?? false; +export function isSessionGenerating(sessionKey: string, userId: number): boolean { + // "Not yours" answers the same as "no such session": false. The caller uses this to decide whether to end + // a turn it believes is running, and its own turn is the only one it can be right about. + return ownedSession(sessionKey, userId)?.isGenerating ?? false; } diff --git a/src/servers/sidecar/claude/state.ts b/src/servers/sidecar/claude/state.ts index 2b47093e..b2266c4a 100644 --- a/src/servers/sidecar/claude/state.ts +++ b/src/servers/sidecar/claude/state.ts @@ -168,11 +168,14 @@ export function getClaudeSession(sessionKey: string, userId: number): string | u * Newest wins: a transcript resumed under a fresh key leaves the old entry in place, and the caller wants * the session generating now, not the one that produced the same file yesterday. */ -export function findSessionKeyByClaudeSession(claudeSessionId: string): string | undefined { +export function findSessionKeyByClaudeSession(claudeSessionId: string, userId: number): string | undefined { const keys = Object.keys(currentState.claudeSessions); for (let i = keys.length - 1; i >= 0; i--) { const key = keys[i]!; - if (currentState.claudeSessions[key]?.claudeSessionId === claudeSessionId) return key; + const record = currentState.claudeSessions[key]; + // Scoped to the caller: this is the reattach hinge, and a browser holding a transcript uuid it should + // not have would otherwise be handed the session key that drives it. + if (record?.claudeSessionId === claudeSessionId && record.userId === userId) return key; } return undefined; } diff --git a/src/servers/sidecar/claude/user-instance.ts b/src/servers/sidecar/claude/user-instance.ts index deb832ba..050baeff 100644 --- a/src/servers/sidecar/claude/user-instance.ts +++ b/src/servers/sidecar/claude/user-instance.ts @@ -241,34 +241,38 @@ async function handleCommand(cmd: SidecarCommand, reply: ReplyFn) { } case 'claude:kill': - claudeManager.killClaudeSession(cmd.sessionKey); + claudeManager.killClaudeSession(cmd.sessionKey, cmd.userId); sessionLog.drop(cmd.sessionKey); reply({ type: 'claude:killed', id: cmd.id }); break; case 'claude:interrupt': - await claudeManager.interruptClaudeSession(cmd.sessionKey); + await claudeManager.interruptClaudeSession(cmd.sessionKey, cmd.userId); reply({ type: 'claude:interrupted', id: cmd.id }); break; case 'claude:list': - reply({ type: 'claude:sessions', id: cmd.id, sessions: claudeManager.listSessions() }); + reply({ type: 'claude:sessions', id: cmd.id, sessions: claudeManager.listSessions(cmd.userId) }); break; case 'claude:is-generating': - reply({ type: 'claude:generating', id: cmd.id, generating: claudeManager.isSessionGenerating(cmd.sessionKey) }); + reply({ + type: 'claude:generating', + id: cmd.id, + generating: claudeManager.isSessionGenerating(cmd.sessionKey, cmd.userId), + }); break; case 'claude:find-session': reply({ type: 'claude:session-key', id: cmd.id, - sessionKey: findSessionKeyByClaudeSession(cmd.claudeSessionId) ?? null, + sessionKey: findSessionKeyByClaudeSession(cmd.claudeSessionId, cmd.userId) ?? null, }); break; case 'claude:clear-session': - claudeManager.clearSession(cmd.sessionKey); + claudeManager.clearSession(cmd.sessionKey, cmd.userId); sessionLog.drop(cmd.sessionKey); reply({ type: 'claude:session-cleared', id: cmd.id }); break; diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 94becd93..21f0a949 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -49,20 +49,24 @@ export type SidecarCommand = // Claude Code | { type: 'claude:spawn'; id: string; params: ClaudeSpawnParams } | { type: 'claude:spawn-streaming'; id: string; params: ClaudeSpawnStreamingParams } - | { type: 'claude:kill'; id: string; sessionKey: string } - | { type: 'claude:interrupt'; id: string; sessionKey: string } - | { type: 'claude:clear-session'; id: string; sessionKey: string } + // Every one of these carries `userId` for the same reason `ClaudeSpawnStreamingParams.member` does: the + // sidecar's session map is global and `sessionKey` arrives in a client message, so a command without an + // identity is a command that acts on whoever's session happens to match. Resolved by the platform from the + // authenticated request, never taken from the client. + | { type: 'claude:kill'; id: string; sessionKey: string; userId: number } + | { type: 'claude:interrupt'; id: string; sessionKey: string; userId: number } + | { type: 'claude:clear-session'; id: string; sessionKey: string; userId: number } // Is a turn still running for this session? Only the process that owns the session can say, which is // exactly why it is asked over the wire — see `isClaudeGenerating` in sidecar-registry. - | { type: 'claude:is-generating'; id: string; sessionKey: string } + | { type: 'claude:is-generating'; id: string; sessionKey: string; userId: number } // Which session key owns this transcript? The map lives on the agent's disk, so only it can answer — // see `findClaudeSessionKey` in sidecar-registry, and `attach` in the chat socket for why it is asked. - | { type: 'claude:find-session'; id: string; claudeSessionId: string } + | { type: 'claude:find-session'; id: string; claudeSessionId: string; userId: number } // Everything the agent is holding right now. `claude:is-generating` answers for a session you can // already name; this is for the case where officer has forgotten every name it had — its session // records are in memory and die with `pm2 restart officer`, while the agent keeps running. Without it // a live session is invisible until a browser happens to reconnect to it by id. - | { type: 'claude:list'; id: string } + | { type: 'claude:list'; id: string; userId: number } // OpenCode — drive a turn through the serve (POST /api/session/{id}/prompt), anchored to the chat cwd // by a per-request location header. Was an `opencode run` subprocess until 2026-08-10. | { type: 'opencode:run-streaming'; id: string; params: OpenCodeRunParams }