diff --git a/src/servers/api/agent-handoff/deliver.ts b/src/servers/api/agent-handoff/deliver.ts index a53b2ed3..12bae31f 100644 --- a/src/servers/api/agent-handoff/deliver.ts +++ b/src/servers/api/agent-handoff/deliver.ts @@ -1,6 +1,6 @@ import { getUserById, markAgentPanelIntroduced, type AgentPanel } from 'officerdb'; import * as sidecar from '@@/sidecar-registry'; -import { resolveBaseCwd } from '../chat/websocket'; +import { resolveBaseCwd, resolveTurnIdentity } from '../chat/websocket'; import { logger } from '../chat/logger'; import { getOwnerHomeDir } from '@@/data-path'; @@ -28,13 +28,28 @@ export async function deliverToAgentPanel(target: AgentPanel, prompt: string): P const user = await getUserById(target.userId); if (!user) throw new Error(`agent panel ${target.id} belongs to a user that no longer exists`); + /** + * Whose turn this is — the same question chat asks, which this path was not asking at all. + * + * It passed no `member`, so a panel belonging to a member ran as the owner; and it derived the cwd from + * `getOwnerHomeDir`, which ignores the email it is handed and returns the OWNER's home regardless. Both + * halves pointed the same way, so a member's handoff ran as the owner, in the owner's home, with the + * owner's credential. Found by the compiler when `member` stopped being optional, not by reading. + */ + const identity = await resolveTurnIdentity(user.id); + if (identity.kind === 'refuse') { + throw new Error(`cannot deliver to ${target.name}: ${identity.reason}`); + } + const home = identity.kind === 'member' ? identity.run.home : getOwnerHomeDir(user.email); + await sidecar.spawnClaudeStreaming({ userId: user.id, email: user.email, username: user.name ?? user.email, prompt, sessionKey: target.sessionKey, - cwd: resolveBaseCwd(getOwnerHomeDir(user.email), target.cwd ?? undefined), + cwd: resolveBaseCwd(home, target.cwd ?? undefined), + member: identity.kind === 'member' ? identity.run : undefined, // Deliberately no `model`: a live session ignores it anyway, and a resumed one keeps whatever the // panel started with. Passing one here would only look like it worked. durable: true, diff --git a/src/servers/api/agents/agent-runner.ts b/src/servers/api/agents/agent-runner.ts index 6d372d06..7b9d8889 100644 --- a/src/servers/api/agents/agent-runner.ts +++ b/src/servers/api/agents/agent-runner.ts @@ -195,6 +195,11 @@ export async function startAgentRun(params: StartAgentRunParams): Promise { +export async function resolveTurnIdentity(userId: number): Promise { const resolved = await resolveHomeDir(userId); if (!resolved.ok) return { kind: 'refuse', reason: resolved.reason }; if (resolved.isOwner) return { kind: 'owner' }; @@ -534,41 +534,54 @@ async function handleClaudeCodeChat( const onMessage = createMessageHandler(sessionId, model); + /** + * One description of the turn, used by both paths below. + * + * They were two object literals, and they had drifted: the first carried `member`, the second did not. + * The second is taken whenever officer already holds a session — which it does long after the SIDECAR has + * let go, because the sidecar tears a session down after 30 minutes idle while `_claudeKill` stays set + * here. So an idle gap sent a turn with no identity to a sidecar that no longer had a session to push it + * onto, and it created a new one. With no member, that means the OWNER. + * + * What that cost, on a real account: a `Developer` with their own Linux user, their own `claude` and their + * own login had every turn after a break run as the owner — in the owner's HOME, with the owner's Anthropic + * credential and the owner's MCP config, which carries a JWT that signs as the owner. The whole point of + * per-member accounts, undone by an omitted line in the branch nobody looks at. + * + * It surfaced as breakage rather than as a silent escalation only by luck: the owner's `claude` was handed + * the member's cwd — a 700 home it cannot enter — and their transcript id, so it exited 1 instead of + * quietly working. Reported as "the chat stopped after he went for a walk", which is the idle timeout. + * + * Built once now, so the two paths cannot disagree again. + */ + const turnParams = { + userId, + email, + username, + prompt: effectivePrompt, + images: msg.images, + sessionKey: sessionId, + cwd, + model, + // Never from the client: whose account a turn runs as is resolved here and nowhere else. + member: identity.kind === 'member' ? identity.run : undefined, + resumeSessionId: msg.resumeSessionId, + }; + try { if (!session._claudeKill) { // First turn of this session: open the persistent session + a SESSION-scoped event subscription // (survives turn-end so background task:notifications keep flowing). `kill` tears both down for an // explicit disconnect; `detach` drops only the listener, which is what the idle GC uses so an // absent browser stops taking a live agent with it. - const handle = await sendClaudeCodeStreaming({ - userId, - email, - username, - prompt: effectivePrompt, - images: msg.images, - sessionKey: sessionId, - cwd, - model, - member: identity.kind === 'member' ? identity.run : undefined, - resumeSessionId: msg.resumeSessionId, - onMessage, - }); + const handle = await sendClaudeCodeStreaming({ ...turnParams, onMessage }); session.piProcess = sessionId as any; session._claudeKill = handle.kill; session._sidecarUnsub = handle.detach; } else { - // Session already live: push this turn onto the existing persistent session (no new subscription). - await sidecar.spawnClaudeStreaming({ - userId, - email, - username, - prompt: effectivePrompt, - images: msg.images, - sessionKey: sessionId, - cwd, - model, - resumeSessionId: msg.resumeSessionId, - }); + // Session already live HERE — which does not mean it is live in the sidecar. If it is not, the sidecar + // creates one from these params, so they must be complete. + await sidecar.spawnClaudeStreaming(turnParams); } } catch (err) { logger.error('Failed to start Claude Code streaming', { sessionId, error: String(err) }); diff --git a/src/servers/api/tasks/pipeline-executor.ts b/src/servers/api/tasks/pipeline-executor.ts index f8e220a6..70f8ec73 100644 --- a/src/servers/api/tasks/pipeline-executor.ts +++ b/src/servers/api/tasks/pipeline-executor.ts @@ -236,6 +236,11 @@ async function runAgenticStep({ try { const handle = await sendClaudeCodeStreaming({ + // No member, and that is correct rather than an omission: this surface is `kind: 'execution'` in the + // permission registry, which is owner-only and never grantable at any level. Stated explicitly + // because the field is required — the branch that could leave it out is how a member's chat turn + // came to run as the owner. + member: undefined, userId, email, username, diff --git a/src/servers/channels/send-claude-code.ts b/src/servers/channels/send-claude-code.ts index de0636b7..c0e8c59a 100644 --- a/src/servers/channels/send-claude-code.ts +++ b/src/servers/channels/send-claude-code.ts @@ -40,8 +40,13 @@ type ClaudeCodeStreamingParams = { model?: string; resumeSessionId?: string; durable?: boolean; - /** Whose Linux account the turn runs as. Undefined for the owner; resolved by the caller, never a client. */ - member?: { osUser: string; home: string }; + /** + * Whose Linux account the turn runs as. Undefined for the owner; resolved by the caller, never a client. + * + * Required, though it may be undefined — see the note in `protocol.ts`. Optional here meant a caller + * could leave it out, and leaving it out means the owner. + */ + member: { osUser: string; home: string } | undefined; // Finished turn messages, already committed by the sidecar; `seq` is the cursor id to deliver them under. onMessage: (msg: TurnMessage, seq?: number) => void; }; diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 14b9f666..6a932122 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -200,7 +200,12 @@ export type ClaudeSpawnStreamingParams = { // Resolved by the platform from `resolveHomeDir`, NEVER taken from a client. Same rule as the pty sidecar, // where `server.tsx` deletes any client-supplied `osUser`/`home` from the query string before setting its // own — a member's identity is not a parameter they get to choose. - member?: { osUser: string; home: string }; + // + // REQUIRED, though it may be undefined. Optional (`member?:`) let a call site simply omit it, and one + // did: the "session already live" branch in `handleChat` built its own literal without it, so every turn + // after the sidecar's 30-minute idle timeout re-created the session as the OWNER. Spelling `undefined` + // is a decision; leaving the key out was an accident the compiler had no opinion about. + member: { osUser: string; home: string } | undefined; }; export type ClaudeCodeResult = {