10 (amended again): chat sessions have no identity, and it should block the gates
Appending what is still missing to close per-user Claude, at the owner's request, into the
same unread file rather than opening 12.
The one worth reordering around: chat sessions never got the identity fix that 4d4a253f gave
the pty sidecar, and the reasoning in that commit applies word for word. claudeSessions in
state.ts:8 is a flat global map with no user dimension; the in-memory sessions Map is keyed by
sessionKey alone; and both sessionKey and resumeSessionId arrive straight off the client
message at websocket.ts:362, :379 and :469, feeding claude-manager.ts:319. So once member is
populated and the gates come off, a member can hand over another account's session id and
resume their transcript, or reach a live session and push turns into it. Invisible today only
because the gates refuse everyone. It belongs before the history layer, and no gate should
move until it is done — a member reading the owner's transcripts is worse than a member having
no chat.
Also named: no server-side precondition on loggedIn, so a turn spawned without credentials
fails as "the agent is broken", which is what /agent-status exists to prevent; members get no
MCP at all, which is a product decision sitting in an undefined branch; no per-member cap on
concurrent turns; and the interactive OAuth login is untested inside the pty sidecar, which is
the first thing every member will do and the place the empty state sends them.
And the shape risk: spawnClaudeCodeProcess has still never been called, verified from type
declarations only. With provisionClaudeCli also never executed, the two riskiest assumptions
in the feature both get their first test from one account creation — which is the argument for
doing that before building further on top of them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -149,6 +149,70 @@ inheriting another member's uid. Yours to implement, mine to try to break.
|
||||
|
||||
---
|
||||
|
||||
## ALSO APPENDED — what is still missing to close per-user Claude
|
||||
|
||||
The owner asked what remains. Your `03` list is right and I think it is incomplete. Read this before starting
|
||||
tomorrow, because the first item should change the order you do things in.
|
||||
|
||||
### The one I would treat as a gate-blocker: chat sessions have no identity
|
||||
|
||||
`4d4a253f` fixed exactly this for the pty sidecar, and the reasoning in that commit applies here word for
|
||||
word — *"re-attaching to a session belonging to another account is refused, otherwise a member resumes someone
|
||||
else's shell by guessing an id that travels in a query string."* **Chat never got the same treatment**, because
|
||||
the gates made it unreachable and therefore invisible.
|
||||
|
||||
What I found reading it:
|
||||
|
||||
- `state.ts:8` — `claudeSessions: Record<string, string>`, a **flat global map** of `sessionKey → claude
|
||||
session id`. No user dimension, one file for the whole sidecar.
|
||||
- `claude-manager.ts:475`, `:486`, `:505` — the in-memory `sessions` Map is keyed by `sessionKey` alone.
|
||||
- `claude-manager.ts:319` — `const resumeId = getClaudeSession(sessionKey) ?? params.resumeSessionId`
|
||||
- `websocket.ts:362`, `:379`, `:469` — `resumeSessionId: msg.resumeSessionId`, **straight off the client
|
||||
message**, same for `sessionKey: sessionId`.
|
||||
|
||||
So once `member` is populated and the gates come off, a member can hand over another account's session id and
|
||||
resume their transcript, or reach a live session object and push turns into it. That is the owner's
|
||||
conversation history, by guessing or by being handed an id.
|
||||
|
||||
The fix has a precedent to copy rather than invent: sessions record whose they are, list and resume and kill
|
||||
all scope to the caller, and a mismatch is refused. `registry.test.ts` should pin it the way it pins the two
|
||||
gates.
|
||||
|
||||
**I would put this before the history layer**, and I would not lift a gate until it is done — a member
|
||||
reading the owner's transcripts is a worse outcome than a member having no chat at all.
|
||||
|
||||
### Also missing, in rough order of how much they matter
|
||||
|
||||
**No server-side precondition on `loggedIn`.** The empty state is UI. If a turn is spawned while
|
||||
`.credentials.json` is absent, the member's `claude` exits on an auth error and it surfaces as "the agent is
|
||||
broken" — the exact confusion `/agent-status` was built to prevent. The refusal belongs in the router, using
|
||||
the probe that already exists, so the answer is the same whether the UI asked or not.
|
||||
|
||||
**Members get no MCP at all.** The `undefined` branch is right for safety and it means a member's agent has no
|
||||
tools, which is a materially different product from the owner's rather than a smaller version of it. Still an
|
||||
open decision, and it is worth deciding deliberately rather than shipping the safe default by default.
|
||||
|
||||
**No per-member resource bound.** Every turn spawns a `claude` as that member. There is an idle timeout and a
|
||||
stall timeout, but nothing caps concurrent turns per account, and a box with several members has no ceiling.
|
||||
|
||||
**The login flow is untested in the web terminal.** `claude` authenticates through an interactive OAuth flow.
|
||||
Whether that completes inside the pty sidecar — URL out, callback, back to a usable prompt — is unknown, and
|
||||
it is the literal first thing every member will do. If it does not work there, the empty state has nowhere to
|
||||
send them.
|
||||
|
||||
### The unknown that could still change the shape
|
||||
|
||||
`spawnClaudeCodeProcess` **has never been called.** It is verified from the type declarations and nothing
|
||||
else. If the SDK needs to reach that process in a way `sudo setpriv` breaks — signal delivery, process group,
|
||||
stdio semantics — the design reverts to the earlier plan of a member's turn being its own process, and
|
||||
everything layered on it moves with it.
|
||||
|
||||
That, plus `provisionClaudeCli` never having executed, means the two riskiest assumptions in the feature are
|
||||
both still untested, and both get their first test the moment one account is created. Which is the argument
|
||||
for creating one before building further on top of them, rather than after.
|
||||
|
||||
---
|
||||
|
||||
## Unchanged
|
||||
|
||||
Both gates up, `member` populated by nothing, 84 tests. The two docker handbacks stay mine and stay parked.
|
||||
|
||||
Reference in New Issue
Block a user