a member's turn ran as the owner after any idle gap

This is the cause. Everything before it was symptom.

`handleChat` had two spawn call sites. The first, for a new session, passed `member`.
The second — "session already live" — was a separate object literal that did not.

That branch is taken whenever OFFICER still holds the session, and officer holds it
long after the SIDECAR has let go: the sidecar reaps a session after 30 minutes idle,
`_claudeKill` stays set here regardless. So a gap sent a turn with no identity to a
sidecar with nothing to push it onto, and it created a fresh session from those
params. No member means the owner.

    [claude:spawn:…] resume=49527b33… cwd=…/data/jg@pertento.ai/home as=owner

green is a Developer with their own Linux account, their own `claude` and their own
login. Every turn after a break ran as the OWNER: owner's HOME, owner's Anthropic
credential, owner's MCP config — which carries a JWT that signs as the owner. The
entire per-member boundary, undone by an omitted line in the branch nobody reads.

It surfaced as breakage rather than as silent escalation only by luck. The owner's
`claude` was handed the member's cwd — a 700 home it cannot enter — and the member's
transcript id, so it exited 1 instead of quietly working. Reported as "he went for a
walk and it stopped working", which is the idle timeout to the minute.

── The fix, and what it then found ──

Both paths now build from one `turnParams`, so they cannot drift again.

Then `member` was made REQUIRED-though-possibly-undefined in the protocol, and the
compiler immediately found three more sites that had left it out:

  - `agent-handoff/deliver.ts` — the same bug, worse. No member AND a cwd from
    `getOwnerHomeDir`, which ignores the email it is given. A member's agent panel ran
    as the owner in the owner's home. Now resolves identity like chat does.
  - `agents/agent-runner.ts`, `tasks/pipeline-executor.ts` — correct as they were:
    both are `kind: 'execution'`, owner-only and never grantable. They now say
    `member: undefined` explicitly, because a field you must fill in is a decision and
    a field you may omit is an accident.

`resolveTurnIdentity` is exported so "whose turn is this" has one implementation
rather than three.

── What I got wrong on the way ──

Three wrong answers first — the credential, the working directory, then a recovery
whose condition could never fire — each from reading code and reasoning instead of
making the system say what it was doing. The log line that found this took five
minutes to write and should have been the first thing I did, not the fourth.

Also: `--resume` by hand appeared to work twice, which is what sent me down the wrong
paths. It proved nothing. The id I tested was not the id the platform passed.

tsgo clean, 950 pass / 7 fail unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-16 20:59:08 +00:00
co-authored by Claude Opus 5
parent 9203472be8
commit 690a726f55
6 changed files with 79 additions and 31 deletions
+17 -2
View File
@@ -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,
+5
View File
@@ -195,6 +195,11 @@ export async function startAgentRun(params: StartAgentRunParams): Promise<StartA
// replay rather than the output. That matters more here than in a pipeline step, because an agent
// run is long and, before long, unattended.
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: params.user.id,
email: params.user.email,
username: params.user.username,
+39 -26
View File
@@ -52,7 +52,7 @@ type TurnIdentity =
| { kind: 'member'; run: { osUser: string; home: string } }
| { kind: 'refuse'; reason: string };
async function resolveTurnIdentity(userId: number): Promise<TurnIdentity> {
export async function resolveTurnIdentity(userId: number): Promise<TurnIdentity> {
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) });
@@ -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,
+7 -2
View File
@@ -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;
};
+6 -1
View File
@@ -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 = {