Repository navigation
feat(agent-sessions): un-conflate session ≠ conversation, restore panes/console/agent-page surfaces on session ids - #2258
Merged
Merged
Conversation
…logy Ports the columns-of-panes reducer deleted in the phase-8 teardown (623e632), whose removal the rebuild plan explicitly instructed — "Don't carry pane-surface.ts/workspace-reducer/PaneBar forward" — and which is why the agents surface ended up with a flat tab bar instead of a split grid. The transitions are unchanged: a horizontal row of columns, each an independent vertical stack; splitRight adds a column, splitDown stacks within one; deliberately not a recursive split tree. A split focuses the new pane's picker rather than leaving a blank rectangle. Every transition no-ops on an id it cannot resolve, so a stale click racing a close is never an error. What the port drops, and why: - The machine topology. MachineNodeScope/OpenTerminalScope and the projectName/branchName plumbing existed because a grid hung off a Machine at a git checkout. Git is no longer the IA. - The multi-workspace list. A Machine owned several named workspaces; the workspace unit is now the CONVERSATION, one grid keyed by its id. The sidebar's leaves are conversations, which is what its workspace leaves already were. - Server-synced layouts (useMachineWorkspaceSync, the Server*DTO types, mergeServerWorkspaces). Layout is local and persisted, as intended. A pane stores a PaneScope — kind ('chat' | 'terminal'), the id it addresses, and for a conversation which agent it belongs to. That last field is what lets one grid hold conversations with several different agents side by side, and a null one is a global-assistant conversation. Adds PANE_KINDS/paneScopeSchema to the session contract and corrects the doc block that claimed PTY-only "by construction" for the whole pane surface. That was a fact about the shells TABLE — sessions and shells are now two tables — but it was read as a fact about what a pane can show, which is what removed agent conversations from panes. 28 reducer tests + 7 contract tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
Ports PaneBar from the machine workspace grid (623e632^) unchanged apart from its one piece of git topology. One slim bar per pane: identity left, actions right, and the bar tint AS the focus state. That single bar replaced two pieces of floating chrome — the hover-revealed split/close chip, which on chat panes physically covered the chat header's own controls, and the 2px top accent line. Actions dim rather than hide (opacity, never display), so they stay clickable on every pointer type without the coarse-pointer escape hatch an opacity-0 chip needs. Pure presentational: no store, no hooks, no network. A terminal pane and a chat pane wear the same bar without either knowing about the other, which is what let the old grid host both surfaces. The topology drop: the checkout chip named a project/branch. That slot is now the agent label, so a grid holding conversations with several different agents says which is which. All 8 original tests ported and passing, including the two that pin the non-obvious rules: canSplit=false renders close only (a phone cannot hold a split grid), and control clicks stopPropagation so closing a pane never first re-selects it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
Ports pane-surface.ts from 623e632^, simplified by the model change but keeping the rule that mattered. The old version resolved a kind-less binding by looking the pane's NAME up in a session list, because layouts predating the chat pane stored no kind. Its safety rule for that lookup was that an unanswered list means 'loading', never a mounted Xterm — "opening a PTY stream registers this pane as a viewer server-side, so guessing 'pty' for what turns out to be a chat isn't a harmless flash, it's a connection." There are no kind-less bindings now: paneScopeSchema requires kind, and every binding is written by the picker that knew what it spawned. So the name lookup is gone. The rule it protected is not — a pane bound to a kind but not yet to a row renders 'loading', never a speculative terminal, and a test pins that for the terminal case specifically since that is the one where guessing costs a connection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
Ports TerminalPanes from 623e632^ as SessionPanes. Layout is two levels, never a recursive tree: a horizontal group of columns, each a vertical group of panes, using the ResizablePanelGroup primitives that survived the teardown. Surfaces are INJECTED via renderPane rather than resolved here, so the grid knows nothing about chat or terminals. That is what lets one grid serve both the console and the agent page, and lets these tests run without mounting an xterm. The two narrow-viewport rules are the reason this is a port and not a rewrite — both encode bugs that are invisible in a screenshot: 1. Inactive panes are HIDDEN, NOT UNMOUNTED. Unmounting a terminal emits a disconnect, drops this pane's viewer entry and, when it was the last, arms the idle reap. An agent finishing while its pane was off-screen would lose its final output and exit code, and returning would cold-start a fresh PTY instead of showing the completed run. 2. `invisible` (visibility:hidden), NOT `hidden` (display:none). xterm measures its character cell from the DOM at open(); in a display:none box that measurement is 0, the fit addon proposes no dimensions, and even the refit on re-show is a no-op — the pane stays blank for good. visibility:hidden also keeps offsetParent/clientWidth truthy, which is what the terminal's own visibility gate checks before it fits. Both are pinned by tests; the second is mutation-verified (swapping to `hidden` fails that test and nothing else). The pane strip is kept too: once the grid collapses to one pane it is the ONLY route back to the others. Split/close handlers are deliberately not props here — the caller already closes over them to build each pane's bar, so taking them would be a second copy of wiring this component never calls. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
… a shell The old machine grid's picker offered the two agent TYPES of one machine — pagespace (Agent) or shell. This one offers a choice that surface could not: WHICH AGENT the conversation belongs to, so a single grid can hold conversations with several different agents side by side. That is the one deliberate extension in this restoration; everything else is a port. The global assistant is a first-class choice here, reported as a null agentPageId — which is exactly what agent_sessions.agentPageId being nullable already means. It is also the groundwork for reaching the global assistant from the sidebar. Presentational, like PaneBar: it renders choices and reports them. Minting a conversation or a shell is IO and belongs to the container, which is also the only thing that knows whether a pick should reuse an existing row. Two layout decisions worth naming: - the drive's agents are listed BELOW the two fixed choices rather than merged with them. The list is unbounded, and a drive with forty agents must not push "Shell" off the top of a short pane. - loading says "Loading agents…" rather than rendering an empty list, because "not answered yet" and "this drive has no agents" are different facts and only one of them is worth acting on. autoFocus takes the first choice on mount, preserving the rule a split already encodes: the user asked for something in this pane, not for a blank rectangle with a control to go hunt for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
The IO shell for the pane grid: identity minting, persistence and subscription. Every transition delegates to the pure reducer, so the store holds no layout logic of its own — which is what keeps those rules exhaustively testable without React. Persisted, NOT synced. The old machine grid pushed layouts to the server via useMachineWorkspaceSync; that is deliberately not restored. A layout is a local view preference, and syncing it made every split a write. Keyed by conversationId, because the conversation IS the workspace unit now. Opening a conversation restores the grid you left it in, and the PTYs behind those panes are still running server-side to reattach to. Three behaviours worth naming: - ensureWorkspace is idempotent. Re-opening a conversation must never discard the layout you built in it, and "give it a grid once" is the only sane reading of a mount effect that fires on every remount. - a transition aimed at a grid that is GONE no-ops rather than throwing or fabricating one. A close can land after the conversation was deleted and its grid forgotten; there is nothing meaningful to build from a split of something absent. - id minting falls back to a counter when crypto.randomUUID is unavailable, so a non-secure context still gets distinct pane ids instead of colliding on one. 39 tests across the reducer and the store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
… off) The picker offered "Assistant" while nothing could render the pick: SessionChat resolves its display identity from an agent PAGE, and a global-assistant conversation has none. A menu item with no renderer is a dead choice — the same offered-but-unsupplied shape as the unwired measureStorage seam found earlier. The option stays built and tested; canPickAssistant (default false) turns it on when the assistant identity path lands (tracked in the epic's Phase R2). A test pins the default-off state so flipping it is a deliberate act, not a drive-by. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…s identity R0 step 1 of 3 (schema). packages/db is green; packages/lib and the apps still reference the old shape and are reworked in the next commits of this PR — this commit is not independently shippable, the PR is. A session is a drive-level workspace that owns one sandbox and hosts MANY conversations plus shells. The first cut conflated it with a conversation: conversationId was the PRIMARY KEY and the Sprite name was folded from it, forcing one environment per chat thread and making shared working contexts structurally impossible (it is why panes could not share a sandbox). That is a cardinality error, not a wiring problem. Schema: - agent_sessions.id — its own cuid PK. conversationId and agentPageId are GONE: the association runs the other way (conversations.sessionId FK), and the agent belongs to each conversation (contextId), never to the session, which can host several agents at once. - agent_sessions.driveId — nullable cascade FK; null = global-assistant session (user-scoped). Billing/tenancy resolve via the drive's owner, falling back to ownerId. - conversations.sessionId — nullable FK, ON DELETE SET NULL: ending a session releases compute, never erases history. Binding is set at creation and permanent; moving a thread is a fork, not a rebind. - agent_session_shells.sessionId re-pointed at agent_sessions.id. Migrations (unreleased feature ⇒ drop-and-recreate, no data migration): - 0235 custom: DELETE rows (not TRUNCATE — per-row AFTER DELETE triggers do not fire on TRUNCATE, and the 0233 trigger is what rescues any live Sprite pointer into the reclaim outbox on the way out). - 0236 generated: DROP both tables. Generated as a pure-drop diff because drizzle-kit's ALTER path emits broken SQL for a PK swap (ADD PRIMARY KEY while the old PK exists; FKs added before the UNIQUE they reference) and its rename prompt cannot run non-interactively. - 0237 generated: CREATE both tables in final shape + the conversations column/FK/index — correct-by-construction ordering. - 0238 custom: re-arm the reclaim trigger (DROP TABLE took 0233's with it); body unchanged, pointer columns survived the identity change. Proven on real Postgres, both acceptance paths: - fresh DB: full chain applies, end state verified column by column. - DB at current head (journal-order replay to 0234): seeded live / torn-down / never-provisioned sessions → 0235 rescued exactly the live pointer; new-shape session delete and owner-cascade delete both rescued through the re-armed trigger; conversations survived with sessionId nulled. Drift suite updated: 33 tests pin the new invariants, including the two new ones — no conversation-derived column may creep back, and threads outlive their session as history (SET NULL, never cascade). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
R0 step 2 of 3 (pure contract). Services, routes and tools are re-pointed
in the next commit of this PR.
Invariant 1 inverts. It read "sessionId ≡ conversationId — ONE id
everywhere", and FORBADE a session-binding field on the grounds that a
DTO carrying both ids would raise the "which id?" question. The question
was real; deleting one of the two ids answered it by amputation — it
made shared working contexts structurally impossible. The invariant now
states what the schema enforces: a session is a drive-level workspace
with its own id, hosting many conversations and shells, and a DTO
carries sessionId when it means the workspace and conversationId when it
means the thread — different kinds of thing, not two names for one.
Sessions are user-visible now (the sidebar lists them), so the old
vocabulary rule ("session" never in user-facing copy) is retired; the
user-facing word is simply "session".
AgentSessionDTO: sessionId is the session's own id; gains driveId
(nullable — null is a global-assistant session); loses agentPageId (a
session hosts MANY agents' conversations; the agent belongs to each
conversation). Tests pin both strips: a conversationId and an
agentPageId on the session DTO are category errors the schema removes.
Sprite-key namespace BUMPED to v2. v1 folded conversation cuids; v2
folds session cuids — the SAME keyspace, so same-namespace reuse could
let a v2 session derive the name of a v1 Sprite still awaiting reclaim.
Pinned with a known answer computed independently of the function under
test (a snapshot of its own output would pass under any namespace);
mutation-verified — reverting the namespace to v1 fails exactly that pin.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…ntity
R0 step 3a of 3 (packages/lib services). apps/web and apps/realtime are
re-pointed in the next commit of this PR; packages/lib is green
standalone (typecheck 0 errors, 8620 unit tests, 18 integration tests on
real Postgres).
Store:
- AgentSessionRecord.id replaces conversationId; driveId replaces
agentPageId (a session hosts MANY agents' conversations — the agent
belongs to each conversation, never the session).
- insertIfAbsent dies with the ensure-by-conversation semantics it
served. create() mints a session (spawning is an explicit act; two
spawns ARE two workspaces), and findByConversation() is how a chat
turn resolves its working context — reading conversations.sessionId,
answering null for an unbound thread, never a fallback. Proven on real
Postgres: two conversations bound to one session resolve the SAME row.
- list's driveId filter is a direct column read; the pages inner-join
dies.
Sessions service: ensureAgentSession → spawnAgentSession. The injected
squat-guarded ensureConversation dep dies — a session no longer FKs a
conversation, and the caller binds the first conversation through the
conversation path with sessionId set at creation.
Access: the decision is DRIVE access now. Conversation-ownership and
agent-page-permission gates dissolved with the conflation — neither one
thread's sharing state nor one page's ACL can speak for a workspace
hosting many of both. decideAgentSessionAccess takes driveMembership;
global-assistant sessions stay owner-only; unknown still denies. The END
variant keeps its no-capability rule and gains an owner short-circuit:
an owner removed from a drive loses USE of the session but keeps the
power to stop paying for it.
Sprite provisioning: authorize reads row.driveId (the resolveDriveId dep
dies — the drive is a fact of the row); the measurement seam drops
agentPageId.
Storage billing: attribution re-pointed from page to drive.
storageBillingTarget answers { driveId } | { ownerId };
lookupDriveOwnerId is a direct drives read (the page→drive join dies);
the reconcile's skip-on-failed-lookup semantics survive unchanged; the
charge carries driveId+sessionId in metadata and no pageId — a session
is not page-anchored, and pretending otherwise was a semantic lie.
Cascade tests inverted to match the schema: a page delete leaves
sessions ALIVE; a conversation delete leaves the session alive and the
thread's siblings untouched; drive and owner deletes rescue the Sprite
pointer through the re-armed trigger.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…n id R0 step 3b of 3 — the apps. The whole tree typechecks; lint clean; realtime 937/937; the touched web suites 1,650 green (the one failure is the known no-test-DB activity-tools suite). acquireSandbox (the tool layer's heart): resolves the conversation's working context through conversations.sessionId (findSessionForConversation) and provisions THAT row — whose own id folds the Sprite key. A thread with no session gets a typed no_session denial, never a lazily-minted per-conversation environment: lazy minting is exactly the conflation the session model removed, and it is what made panes unable to share a sandbox. The runtime guardrail and activity metering re-key to the SESSION id — one budget per workspace, however many threads work in it. R0's PAYOFF TEST now passes at the tool layer: two conversations bound to one session acquire the SAME sandbox, with the shared row proven to flow into both provisions and no shared id threaded anywhere. Routes: - POST /api/agent-sessions/[sessionId] no longer ensures-by-conversation; it (re-)provisions an EXISTING session. Spawn belongs to the collection route (R2's flow); a missing session is a 404, not a mint. - POST .../shells requires an existing session — a shell opens INSIDE a workspace, it never creates one, and a session is born with its first conversation, not a shell. - GET /api/agent-sessions loses ?agentId= — a session hosts many agents, so "an agent's sessions" is not a real relation; the param is ignored. Worker tools (spawn/send/read/kill_session): a worker now works in its SPAWNER's workspace — createWorkerSession resolves the caller's session and binds the worker conversation into it (same sandbox, same filesystem: the shared context is the point of spawning one), refusing with no_session rather than minting. kill_session aborts the worker's in-flight run and deliberately does NOT tear down the sandbox any more: the worker never owned one, and killing "its" sandbox would destroy the caller's own working context. spawn_shell resolves conversation→session before hanging the PTY row off the session id. Web runtime: ensureSession → spawnSession + findSessionForConversation + createConversationInSession (creates a thread already bound to its session — the binding is set once and permanent). Access deps rebuilt on resolveDriveMembership (drives.ownerId, then accepted driveMembers); tenancy reads row.driveId directly. Realtime bridge: shell access via the same shared drive-membership decision; tenant and payer resolve from the session's drive owner (falling back to the session owner); resolveDriveId dep and the page-permission/conversation-ownership gathers die. Billing attribution: PTY usage carries NO pageId any more — a session is a drive-level workspace, not page-anchored, so there is no page to group usage under. Payer stays the drive owner ?? session owner. Tests pin the absence explicitly rather than dropping the assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
Contributor
📝 WalkthroughWalkthroughThis PR rebuilds agent sessions around independent session IDs and drive-scoped authorization, adds session-bound conversations and shared sandbox tooling, introduces session APIs and pane workspaces, migrates storage billing, and adds per-agent sandbox enablement controls. ChangesSession identity and persistence
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AgentSessionsAPI
participant SessionRuntime
participant ConversationRuntime
participant SandboxRuntime
Client->>AgentSessionsAPI: spawn session request
AgentSessionsAPI->>SessionRuntime: validate access and spawnSession
SessionRuntime->>ConversationRuntime: create first session-bound conversation
AgentSessionsAPI->>SandboxRuntime: provision existing session sandbox
SandboxRuntime-->>Client: session and sandbox binding
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…he session R1: rework the pane store onto the un-conflated model (this branch now stacks on pu/session-unconflate). The workspace unit is the SESSION — the thing that owns the sandbox every pane shares by construction — so grids key on agent_sessions.id, and newWorkspace takes a sessionId with its opening pane bound to the session's FIRST conversation (a session is born with one). Closing the LAST pane ends the session. The pure reducer's closePane still no-ops on it — a WorkspaceState transition cannot delete its own container — and the STORE is the container, so the interception lives there: the grid is removed and closePane returns 'session-ended', the caller's signal to tear the sandbox down as the same act. This is the container level the old machine closePaneIn owned, dropped in the original port and restored where it belongs. 'closed' and 'noop' verdicts let the caller distinguish a layout change from a stale click. Layout transitions, PaneBar, SessionPanes, PanePicker and resolvePaneSurface survive unchanged — all sandbox-agnostic by design. 78 pane-layer tests, typecheck 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8cb425b87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+125
to
+127
| const row = await findSessionForConversation(conversationId); | ||
| if (!row) { | ||
| return { ok: false, reason: 'provision_failed', cause: 'no_session' }; |
There was a problem hiding this comment.
After resolving a conversation to row, the successful acquisition still does not return row.id. Consequently tool-runners.ts continues passing ctx.conversationId to measureStorage and notifyShellActivity (lines 520-524 and 743-752). Now that conversation and session IDs differ, measureWarmSessionStorage cannot find the session row, so post-write storage measurements silently stop and storage is underbilled; activity notifications are likewise addressed to the wrong session. Carry the resolved session ID through SandboxAcquireResult and use it for these hooks.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in c6d5164a9: SandboxAcquireResult's success arm now REQUIRES sessionId (both web acquirers return the resolved row id; the handle-bound git deps take it from the routes), openSession exposes it, and both post-run hooks — storage measurement and shell-activity notification — key on it instead of ctx.conversationId. This also un-gates the hooks from conversation presence, so sessionless-context runs measure and notify correctly. Fakes updated to a distinct workspace id; the two conversation-gated tests inverted into pins of the new keying. Leaving open for your verification.
Comment on lines
+429
to
+431
| const session = await findSessionForConversation(sessionId); | ||
| if (!session) return { ok: false, reason: 'no_session' }; | ||
| const spawned = await spawnShell({ sessionId: session.id, ownerId, name }); |
There was a problem hiding this comment.
This now stores each shell under the real workspace ID (session.id), but openOwnShell in session-tools.ts still compares shell.sessionId directly with context.conversationId. For every un-conflated session those values differ, so a shell can be spawned successfully and then every send_shell, read_shell, and kill_shell call rejects it as belonging to another session. Resolve the caller conversation through findSessionForConversation before performing that comparison.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 8e7fae707: new findOwnWorkspace(conversationId) dep resolves the caller's workspace once, and openOwnShell/kill_shell compare shell.sessionId against the workspace id — one namespace. Fixtures now use a workspace id distinct from the conversation id, with regression tests for own-shell reach, sessionless refusal, and foreign-workspace already-gone. Leaving this thread open for your verification.
Comment on lines
+380
to
+385
| await createConversationInSession({ | ||
| conversationId: sessionId, | ||
| userId: ownerId, | ||
| agentPageId, | ||
| sessionId: callerSession.id, | ||
| }); |
There was a problem hiding this comment.
When spawn_session selects an agent page from another drive, canUseAgent permits any page the caller can view and this binds that conversation to the caller's existing workspace without checking the page's drive. Active-runtime billing still resolves its payer from ctx.agentPageId in sandbox-billing.ts, so the other drive's owner is charged for compute belonging to the caller's session. Either restrict the selected agent to callerSession.driveId or propagate the session's drive/owner into the active billing seam.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in d2bcdba77: the rule is enforced at the ONE binding path — createConversationInSessionWith now gates the page arm on agent.driveId === session.driveId (fail-closed on unresolved facts), so the worker-spawn path cannot bind a foreign drive's agent into the caller's workspace; a GLOBAL session hosts only assistant threads. Active-runtime billing therefore can no longer be pointed at another drive's owner via a cross-drive worker — the agent's drive and the session's drive are equal by construction. Leaving open for your verification.
| return Promise.all( | ||
| rows.map(async (row) => ({ | ||
| sessionId: row.conversationId, | ||
| sessionId: row.id, |
There was a problem hiding this comment.
list_sessions now emits the workspace row ID, while findSession at lines 322-325 interprets every send_session, read_session, and kill_session argument as a worker conversation ID. Thus none of the IDs returned by the advertised discovery tool can be used by those tools, and worker IDs cannot be recovered after the spawn response is lost. List the worker conversations with their conversation IDs, or change the downstream tools to consistently address workspace IDs.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 8e7fae707: list_sessions now lists the caller's session's WORKERS by conversation id (the exact address send_session/read_session/kill_session take) plus shells and the shared sandbox status, with agent labels delivered and an isCaller marker — one address namespace per verb family, matching your suggested first option. Leaving open for your verification.
Comment on lines
+389
to
+390
| // `name` labels the worker CONVERSATION; the session keeps its own name. | ||
| void name; |
There was a problem hiding this comment.
The spawn_session contract promises a labeled worker, but the supplied name is explicitly discarded after creating the conversation. Since createConversationInSession also does not set the conversation title, the worker appears in conversation/sidebar views with an empty or default title even though the tool response reports the requested name. Persist name onto the newly created worker conversation.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in c64990853: the worker's name now travels through createConversationInSession into the creator's INSERT (conversations.title), so the sidebar, history and list_sessions show what the spawner named it. Labels only — nothing resolves by them. Leaving open for your verification.
R1's last structural piece. Composes the pure pane pieces into a working
grid for one session:
SessionPanes (layout only, surfaces injected)
└─ pane: PaneBar (identity + split/close) over
picker → PanePicker (choose an agent in the drive, or a shell)
chat → PaneChat (a conversation IN this session)
terminal → Shell (a PTY on this session's sandbox)
loading → spinner (bound, row not minted — never speculative)
The container owns ALL the IO a pick triggers and writes the resulting
PaneScope back through assignPane:
- Agent pick: mints a conversation id and POSTs it to the page-agents
conversations route with { sessionId } — the route (extended here)
gates the binding on checkSessionAccess and creates the thread already
BOUND to the workspace, so its tool calls resolve the shared sandbox by
construction. The pane holds `loading` (kind set, target null) during
the mint, and error paths reset it to the picker — a pane stuck on
loading forever is a dead pane.
- Shell pick: POSTs the session's shells route; the session must already
exist (a shell opens inside a workspace, never creates one).
- Close: the store's 'session-ended' verdict (last pane) triggers the
DELETE — emptying the session ends it, one act — with a toast when the
teardown IO fails, since a silent failure bills until reclaim.
PaneChat exists because SessionChat takes a resolved AgentInfo and hooks
cannot run inside a render-prop map: each pane resolves its OWN agent,
which is also what lets one grid hold conversations with different
agents side by side. A null agentPageId renders a notice (assistant
identity path is Phase R2; the picker doesn't offer it until then).
78 pane-layer tests; typecheck 0; lint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…s first conversation
R2's spawn flow, server side. One act: mint the workspace row and create
its first conversation (chosen agent) already BOUND to it — a session is
never empty, and the grid never starts on a picker.
Spawn is instant and free: nothing here provisions. The route does not
even import the provisioner, so "no sandbox until first use" is
structural, and the test says so rather than asserting a mock was quiet.
If the first conversation fails (squat guard, dead agent), the
just-minted session is ENDED before the error returns — the model says
an empty workspace cannot exist, so the failure path enforces it too.
Access runs the same shared decision every session surface uses
(drive membership + capability), applied to the row-to-be BEFORE
anything is minted. Global-assistant spawns (null drive) are refused
until the assistant identity path lands; the client picker does not
offer them.
Also extended earlier in this branch: the page-agents conversations POST
accepts { sessionId } to bind a new thread into an existing workspace,
gated on checkSessionAccess — the pane picker's "new conversation in
THIS session" path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…onversations The left sidebar's tree now matches the corrected model: a SESSION (drive-level workspace) is the second level, its CONVERSATIONS the third — panes never appear here (layout is centre-view state). Selecting a session opens its most recent conversation; selecting a conversation carries session+c+agent as one pushState transition, so nothing navigates and live shells survive every click. - lib/agents/agent-selection.ts: grammar is ?session=&c=&agent= (stable order) - stores/agents/useAgentSurfaceStore: selectSession / selectConversation, session switch clears the conversation (it belonged to the old workspace) - AgentsSurface renders AgentPanes keyed by session; degenerate deep links get prompts, never a speculative grid - AgentsSidebar rewritten: sessions via /api/agent-sessions (admin-gated null SWR key), per-session New-conversation, one-act New-session (server answers with session + first conversation — no empty-session state is ever visible) - GET /api/agent-sessions now attaches conversations per session (listSessionConversations); POST spawn already landed - tests rewritten to pin the new grammar, store API, sidebar tree and route Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
The lifecycle already ends a session when its last pane closes; this adds the sidebar-side act for a session you don't want to open first. Confirmed via AlertDialog (the sandbox dies — never one accidental hover-click). On confirm: DELETE the session, forget its local pane grid, clear the selection if it was open, refetch the list. Conversations remain as history in each agent's list (conversations.sessionId is ON DELETE SET NULL). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
… sidebar group The assistant becomes a first-class session inhabitant. The gap was display identity + creation paths; the pipeline itself (type:'global' rows, owner-only access for null-drive sessions, owner-as-tenant provisioning) already existed. - useAssistantSessionChat: useAgentSessionChat's sibling on the global chat pipeline (global channel, /api/ai/global/[id]/messages, global loaders, buildGlobalChatRequestBody; no per-hook socket — GlobalChatProvider is app-wide). Same return shape, so one view renders both. - SessionChat split into SessionChatView (presentation) + the agent wrapper; new AssistantSessionChat wrapper reads identity from the assistant settings store. PaneChat's null-agent branch now renders it instead of a notice. - PanePicker offers the Assistant in every session (canPickAssistant on); AgentPanes routes the pick through the new session-centric creator. - POST /api/agent-sessions/[sessionId]/conversations: threads born INTO a session — session access + (for agent pages) canPrincipalViewPage layered, agentPageId null = assistant thread (the only way one can join a session). - POST /api/agent-sessions accepts the both-null shape: a global-assistant session (owner-only), first conversation an assistant thread. Half-specified shapes still 400. - Sidebar: Assistant group always present in global mode (one-click spawn, no chooser), assistant new-conversation via the session route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…at tab The drive AI_CHAT page gets its good UI back, with panes as the ONE addition: - Chat | History | Settings are real tabs again: grid grid-cols-3 max-w-lg pills with icons in the p-4 border-b header block — the same Tabs defaults every other tabbed surface uses. The full-bleed rounded-none bar is gone. - History is a full-height TAB (PageAgentHistoryTab is h-full + virtualized; the 320px popover gave it no height to resolve against — deleted). - Save Settings is pinned in the header row beside the tabs; Webhooks is back to the icon-only ghost button. - The Chat tab hosts the PANE GRID for a session-bound conversation (AgentPanes, full page renderer via chatContext) — split-capable, every pane sharing the session's one sandbox by construction. Sessions are capability-shaped, so new conversations are born WITH a session for session users (spawn route; refused spawns fall back to plain) and plain for everyone else; pre-session threads render the plain chat (binding is set at creation and permanent — an old thread cannot join a workspace). - Conversations now carry sessionId through the listing (repo SQL, GET route, ConversationData) so History selection lands in the right surface, and "Open in Agents" deep-links ?session=&c=&agent= when bound. - Removed: AgentView + its tabs container, session-tabs.ts, session-status.ts (status chip copy), useAgentSession/useSessionShells, the Add-shell button and its provisioning path, the history Popover. No sandbox chrome remains — provisioning is lazy and automatic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…stored) DECIDE settled as (a): a per-agent boolean, pages.sandboxEnabled (default false — code execution is opt-in per agent, as machineAccess was). The old Machine Access card returns as a plain Sandbox card: one Switch, no machine topology (there is nothing to pick — the sandbox belongs to the conversation's SESSION and provisions automatically on first use). - schema: pages.sandboxEnabled boolean NOT NULL DEFAULT false (0239, additive) - tool-filtering: SANDBOX_TOOL_NAMES = core (bash/files) ∪ git+gh ∪ session/shell families; filterToolsForSandboxEnablement strips ALL of them (reads included — this is agent configuration, not the read-only gate). Drift-guarded against createSandboxTools' actual keys. - request-time gates: /api/ai/chat (also covers spawned workers, which dispatch through it) and both /api/v1/chat/completions branches. The allowlist cannot re-grant a stripped tool. Env kill-switch + canRunCode remain the security boundaries underneath. - agent-config GET/PATCH round-trips sandboxEnabled. - settings tab: Sandbox card + Switch; Default Tools hides the sandbox families while the switch is off (the old MACHINE_TOOL_NAMES behaviour). - also fixes a stale expectation from e8cb425 (global lifecycle never carried sessionId). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
resolveAgentPageDriveId and findSessionConversation lost their last callers when sessions stopped being addressed through conversations/agent pages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…on; baseline stack-consumed exports - resolveAgentPageDriveId / findSessionConversation lost their callers in the un-conflation itself — deleted (same cleanup already on pu/agents-restore-panes). - The global stream-lifecycle test expected a sessionId the lifecycle never carries — a stray from e8cb425's sweep. - checkAccessForSubject / spawnSession are this phase's contract surface; their callers are the very next PR in the stack (#2259, which removes these baseline entries again). Baselined rather than deleted so the phase ships its deliverable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
…gents-restore-panes # Conflicts: # apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/lib/ai/tools/session-tools.ts (1)
221-225: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale
endSessiondoc contradicts the updatedkill_sessiondescription in this same file.Line 221 still says
endSessionwill "kill its Sprite (instance-guarded)", but per this PR's behavior change, worker-session teardown deliberately never tears down the shared sandbox — thekill_sessiontool description you just updated at Line 566 says exactly that ("stopping one never tears the sandbox down"). The interface doc for the dependency it calls wasn't updated to match.📝 Proposed doc fix
- /** End the session: abort its runs, kill its Sprite (instance-guarded), keep the row. */ + /** End the session: abort its in-flight runs. Never tears down the shared sandbox — a worker runs in its spawner's session, so its sandbox survives. Keeps the row. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/tools/session-tools.ts` around lines 221 - 225, Update the JSDoc for endSession to remove the claim that it kills or tears down the Sprite, and describe that it aborts the session’s runs while preserving the shared sandbox. Keep the return type unchanged.apps/web/src/app/api/agent-sessions/[sessionId]/route.ts (1)
11-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale docstrings still describe the removed "ensure/lazily mint a session" behavior.
Both routes were rewritten so a session must already exist — POST now 404s with "Session not found" rather than creating/anchoring one — but their module docstrings weren't updated to match, and still reference the removed
sessionAnchorForConversationanchoring and lazy-ensure semantics.
apps/web/src/app/api/agent-sessions/[sessionId]/route.ts#L11-L17: update the POST doc to state the session must already exist (spawning lives on the collection route) and drop thesessionAnchorForConversation/conversation-identity language.apps/web/src/app/api/agent-sessions/[sessionId]/shells/route.ts#L7-L12: update the POST doc to state a cold/nonexistent session 404s rather than being lazily ensured; only the sandbox is provisioned here, on an already-existing session.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/agent-sessions/`[sessionId]/route.ts around lines 11 - 17, Update the module docstrings to match the existing route behavior: in apps/web/src/app/api/agent-sessions/[sessionId]/route.ts lines 11-17, document that POST requires an existing session, returns 404 when absent, and that spawning occurs on the collection route; remove the sessionAnchorForConversation and lazy-ensure/conversation-identity claims. In apps/web/src/app/api/agent-sessions/[sessionId]/shells/route.ts lines 7-12, document that POST returns 404 for a nonexistent session and only provisions the sandbox for an existing session.apps/realtime/src/terminal/shell-access.ts (1)
61-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale "agent page" wording left in the
resolvePayerdoc comment.The comment still says "Payer = the agent page's drive owner, falling back to the session's own owner when there is no page... or the page's owner cannot be resolved", mixing old page-based language with the new drive-scoped rule tacked on at the end ("the drive-owner ?? session-owner attribution rule"). The session model no longer has an agent-page concept for this attribution (per
agent-session-access.ts'ssession.driveId-based logic and the parallel, already-cleaned-up comment inshell-handler.tsLine 105-106: "Billing attribution scope: the session's drive, or null (owner-attributed) for a global-assistant session."). This is likely to confuse future readers about what actually drives payer resolution.📝 Suggested comment fix
/** - * Who pays for this session's runtime, and which drive an audit row lands - * under. Payer = the agent page's drive owner, falling back to the session's - * own owner when there is no page (a global-assistant session) or the page's - * owner cannot be resolved — the drive-owner ?? session-owner attribution rule. + * Who pays for this session's runtime, and which drive an audit row lands + * under. Payer = the session's drive owner, falling back to the session's + * own owner when there is no drive (a global-assistant session) or the + * drive's owner cannot be resolved. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/terminal/shell-access.ts` around lines 61 - 67, Update the JSDoc for resolvePayer to remove all agent-page references and accurately describe the drive-scoped attribution rule: use the session’s drive when present, otherwise use null for a global-assistant session, with payer ownership falling back to the session owner when needed.apps/web/src/app/api/agent-sessions/route.ts (1)
1-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale JSDoc still advertises the removed
?agentId=<id>filter.The header comment says
GET ?driveId=<id> | ?agentId=<id> | (none = mine), but the handler below no longer parses or filters onagentIdat all (onlydriveIdis read from search params, and the filter construction at Lines 47-50 has noagentIdbranch). This will mislead anyone reading the route's contract.📝 Suggested fix
- * GET ?driveId=<id> | ?agentId=<id> | (none = mine) + * GET ?driveId=<id> | (none = mine)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/agent-sessions/route.ts` around lines 1 - 15, Update the route header JSDoc to remove the obsolete ?agentId filter and document only the supported driveId query parameter and default behavior, keeping the remaining session-listing contract description unchanged.
🧹 Nitpick comments (5)
packages/lib/src/services/agent-sessions/__tests__/agent-session-access.test.ts (1)
32-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting
canRunCode's inputs too.The membership wiring is captured (Line 44), but nothing pins
canRunCodereceiving the requester id and the row'sdriveId— including thenull-drive case, where it must still be consulted withdriveId: nullper the deps contract inpackages/lib/src/services/agent-sessions/agent-session-access.ts(lines 26-39). That's wrapper-only wiring with no coverage indecide-session-access.test.ts.♻️ Optional: capture the capability input in the global-assistant test
it('given a global-assistant session, should NOT fetch a membership at all', async () => { + const capabilityInputs: Array<{ userId: string; driveId: string | null }> = []; const result = await checkAgentSessionAccess({ requesterId: OWNER_ID, sessionId: SESSION_ID, deps: makeDeps({ findSession: async () => ({ ...subject, driveId: null }), resolveDriveMembership: async () => { throw new Error('must not resolve a membership for a session with no drive'); }, + canRunCode: async (input) => { + capabilityInputs.push(input); + return true; + }, }), }); expect(result).toEqual({ allowed: true }); + expect(capabilityInputs).toEqual([{ userId: OWNER_ID, driveId: null }]); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/src/services/agent-sessions/__tests__/agent-session-access.test.ts` around lines 32 - 59, Extend the tests around checkAgentSessionAccess to capture and assert canRunCode inputs, verifying it receives the requesterId and the session row’s driveId for both drive and global-assistant sessions. In the null-drive case, ensure canRunCode is still called with driveId: null while preserving the existing membership non-call assertion.packages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.ts (1)
450-457: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
reclaimedOutsideduplicatesreclaimedverbatim.Only the closure scoping differs. Hoist the single helper to module scope and drop the inner copy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.ts` around lines 450 - 457, Hoist the existing reclaimed helper to module scope so it can be used both inside and outside the cascade describe closure, then remove the duplicate reclaimedOutside function. Preserve the current database query and boolean result behavior.packages/lib/src/services/sandbox/__tests__/sandbox-storage-reconcile.test.ts (1)
174-179: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLeftover page/conversation-scoped naming in the re-scoped tests. The attribution model moved to drive/session scoping, but several identifiers, sentinels and assertion messages still describe pages and conversation ids — correct values, misleading labels, and in the mock-sentinel case capable of hiding a wrong-column read.
packages/lib/src/services/sandbox/__tests__/sandbox-storage-reconcile.test.ts#L174-L179: reword thegiven/shouldstrings (and the titles/fixtures on Lines 198 and 233) from "agent page" to drive.packages/lib/src/services/sandbox/__tests__/sandbox-storage-billing.test.ts#L21-L22: change the mockeddriveIdsentinel to'agent_sessions.driveId'and retire the'agent-page-1'fixture value on Line 58.packages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.ts#L190-L190: rename theconversationIdvariable used as the session id tosessionId(also Lines 213, 232, 250, 265, 301, 315).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/src/services/sandbox/__tests__/sandbox-storage-reconcile.test.ts` around lines 174 - 179, Re-scope the test naming to drive/session terminology: in packages/lib/src/services/sandbox/__tests__/sandbox-storage-reconcile.test.ts lines 174-179, 198, and 233, update titles, fixtures, and given/should messages from agent page to drive; in packages/lib/src/services/sandbox/__tests__/sandbox-storage-billing.test.ts lines 21-22 and 58, use the driveId sentinel 'agent_sessions.driveId' and remove the 'agent-page-1' fixture; in packages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.ts lines 190, 213, 232, 250, 265, 301, and 315, rename the session-id variable from conversationId to sessionId.apps/realtime/src/terminal/__tests__/shell-access.test.ts (1)
166-204: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider a test case for the "drive vanished" fallback branch.
Coverage here spans a real drive and an explicit
driveId: nullsession, but not the case wheresession.driveIdis non-null yet the drive lookup misses (theresolvePayerfallback attributing to the owner). Adding that case would pin down whether the returned top-leveldriveIdand the audit-loggeddriveIdstay consistent in that branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/realtime/src/terminal/__tests__/shell-access.test.ts` around lines 166 - 204, Add a test alongside the existing happy-path and null-drive cases for a session with a non-null driveId whose resolvePayer lookup returns the owner fallback with driveId null. Assert authorization succeeds, the returned top-level driveId is null, and the audit-log record also uses null for driveId, using the existing buildDeps and audit-log assertions.knip-baseline.json (1)
5-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve these unused-export suppressions instead of growing the baseline.
checkAccessForSubjectandspawnSessionare only referenced insideapps/web/src/lib/agent-sessions/agent-sessions-runtime.ts, so these exports should be converted to internal helpers/removed rather than permanently baselined as unused exports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@knip-baseline.json` around lines 5 - 7, Remove the unused-export suppressions for checkAccessForSubject and spawnSession from knip-baseline.json, then update their declarations in agent-sessions-runtime.ts to be internal helpers rather than exported symbols. Preserve all existing internal call sites and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts`:
- Around line 273-278: Update the conversation binding logic in
resolveOrCreateConversation to only update conversations whose sessionId is null
by adding isNull to the database-operators import and including that guard
alongside conversations.id. Ensure the caller detects a zero-row update as a
binding conflict rather than treating an already-bound conversation as
successfully rebound; preserve the permanent session binding invariant.
---
Outside diff comments:
In `@apps/realtime/src/terminal/shell-access.ts`:
- Around line 61-67: Update the JSDoc for resolvePayer to remove all agent-page
references and accurately describe the drive-scoped attribution rule: use the
session’s drive when present, otherwise use null for a global-assistant session,
with payer ownership falling back to the session owner when needed.
In `@apps/web/src/app/api/agent-sessions/`[sessionId]/route.ts:
- Around line 11-17: Update the module docstrings to match the existing route
behavior: in apps/web/src/app/api/agent-sessions/[sessionId]/route.ts lines
11-17, document that POST requires an existing session, returns 404 when absent,
and that spawning occurs on the collection route; remove the
sessionAnchorForConversation and lazy-ensure/conversation-identity claims. In
apps/web/src/app/api/agent-sessions/[sessionId]/shells/route.ts lines 7-12,
document that POST returns 404 for a nonexistent session and only provisions the
sandbox for an existing session.
In `@apps/web/src/app/api/agent-sessions/route.ts`:
- Around line 1-15: Update the route header JSDoc to remove the obsolete
?agentId filter and document only the supported driveId query parameter and
default behavior, keeping the remaining session-listing contract description
unchanged.
In `@apps/web/src/lib/ai/tools/session-tools.ts`:
- Around line 221-225: Update the JSDoc for endSession to remove the claim that
it kills or tears down the Sprite, and describe that it aborts the session’s
runs while preserving the shared sandbox. Keep the return type unchanged.
---
Nitpick comments:
In `@apps/realtime/src/terminal/__tests__/shell-access.test.ts`:
- Around line 166-204: Add a test alongside the existing happy-path and
null-drive cases for a session with a non-null driveId whose resolvePayer lookup
returns the owner fallback with driveId null. Assert authorization succeeds, the
returned top-level driveId is null, and the audit-log record also uses null for
driveId, using the existing buildDeps and audit-log assertions.
In `@knip-baseline.json`:
- Around line 5-7: Remove the unused-export suppressions for
checkAccessForSubject and spawnSession from knip-baseline.json, then update
their declarations in agent-sessions-runtime.ts to be internal helpers rather
than exported symbols. Preserve all existing internal call sites and behavior.
In
`@packages/lib/src/services/agent-sessions/__tests__/agent-session-access.test.ts`:
- Around line 32-59: Extend the tests around checkAgentSessionAccess to capture
and assert canRunCode inputs, verifying it receives the requesterId and the
session row’s driveId for both drive and global-assistant sessions. In the
null-drive case, ensure canRunCode is still called with driveId: null while
preserving the existing membership non-call assertion.
In
`@packages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.ts`:
- Around line 450-457: Hoist the existing reclaimed helper to module scope so it
can be used both inside and outside the cascade describe closure, then remove
the duplicate reclaimedOutside function. Preserve the current database query and
boolean result behavior.
In
`@packages/lib/src/services/sandbox/__tests__/sandbox-storage-reconcile.test.ts`:
- Around line 174-179: Re-scope the test naming to drive/session terminology: in
packages/lib/src/services/sandbox/__tests__/sandbox-storage-reconcile.test.ts
lines 174-179, 198, and 233, update titles, fixtures, and given/should messages
from agent page to drive; in
packages/lib/src/services/sandbox/__tests__/sandbox-storage-billing.test.ts
lines 21-22 and 58, use the driveId sentinel 'agent_sessions.driveId' and remove
the 'agent-page-1' fixture; in
packages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.ts
lines 190, 213, 232, 250, 265, 301, and 315, rename the session-id variable from
conversationId to sessionId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cbab643-c6f1-4035-ae00-dc3d545f5561
📒 Files selected for processing (61)
apps/realtime/src/index.tsapps/realtime/src/terminal/__tests__/shell-access.test.tsapps/realtime/src/terminal/__tests__/shell-handler.test.tsapps/realtime/src/terminal/shell-access.tsapps/realtime/src/terminal/shell-handler.tsapps/web/src/app/api/agent-sessions/[sessionId]/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/[sessionId]/route.tsapps/web/src/app/api/agent-sessions/[sessionId]/shells/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/[sessionId]/shells/route.tsapps/web/src/app/api/agent-sessions/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/route.tsapps/web/src/app/api/ai/global/[id]/messages/__tests__/stream-socket-events.test.tsapps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/__tests__/route.test.tsapps/web/src/app/api/ai/page-agents/[agentId]/conversations/[conversationId]/messages/__tests__/route.test.tsapps/web/src/app/api/v1/chat/completions/__tests__/route-backfill.test.tsapps/web/src/app/api/v1/chat/completions/__tests__/route.test.tsapps/web/src/app/api/v1/conversations/__tests__/route.test.tsapps/web/src/components/agents/__tests__/useAgentSession.test.tsxapps/web/src/lib/agent-sessions/agent-session-orphan-reconcile-runtime.tsapps/web/src/lib/agent-sessions/agent-sessions-runtime.tsapps/web/src/lib/agents/__tests__/session-status.test.tsapps/web/src/lib/ai/tools/__tests__/sandbox-tools-runtime.test.tsapps/web/src/lib/ai/tools/__tests__/session-tools.test.tsapps/web/src/lib/ai/tools/sandbox-tools-runtime.tsapps/web/src/lib/ai/tools/session-tools-runtime.tsapps/web/src/lib/ai/tools/session-tools.tsknip-baseline.jsonpackages/db/drizzle/0235_session_unconflate_clear_rows.sqlpackages/db/drizzle/0236_marvelous_tyger_tiger.sqlpackages/db/drizzle/0237_overrated_susan_delgado.sqlpackages/db/drizzle/0238_session_unconflate_recreate_reclaim_trigger.sqlpackages/db/drizzle/meta/0235_snapshot.jsonpackages/db/drizzle/meta/0236_snapshot.jsonpackages/db/drizzle/meta/0237_snapshot.jsonpackages/db/drizzle/meta/0238_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/__tests__/agent-sessions.test.tspackages/db/src/schema/agent-sessions.tspackages/db/src/schema/conversations.tspackages/lib/src/agent-sessions/__tests__/contract.test.tspackages/lib/src/agent-sessions/__tests__/decide-session-access.test.tspackages/lib/src/agent-sessions/__tests__/session-sprite-key.test.tspackages/lib/src/agent-sessions/contract.tspackages/lib/src/agent-sessions/decide-session-access.tspackages/lib/src/agent-sessions/session-sprite-key.tspackages/lib/src/billing/sandbox-payer.tspackages/lib/src/services/agent-sessions/__tests__/agent-session-access.test.tspackages/lib/src/services/agent-sessions/__tests__/agent-session-sprite.test.tspackages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.tspackages/lib/src/services/agent-sessions/__tests__/agent-sessions.test.tspackages/lib/src/services/agent-sessions/__tests__/fakes.tspackages/lib/src/services/agent-sessions/agent-session-access.tspackages/lib/src/services/agent-sessions/agent-session-sprite.tspackages/lib/src/services/agent-sessions/agent-sessions-store.tspackages/lib/src/services/agent-sessions/agent-sessions.tspackages/lib/src/services/sandbox/__tests__/sandbox-storage-attribution.test.tspackages/lib/src/services/sandbox/__tests__/sandbox-storage-billing.test.tspackages/lib/src/services/sandbox/__tests__/sandbox-storage-reconcile.test.tspackages/lib/src/services/sandbox/sandbox-storage-attribution.tspackages/lib/src/services/sandbox/sandbox-storage-billing.tspackages/lib/src/services/sandbox/sandbox-storage-reconcile.ts
2witstudios
commented
Jul 30, 2026
2witstudios
commented
Jul 30, 2026
2witstudios
commented
Jul 30, 2026
2witstudios
changed the title
Owner
Author
CodeQL alert #273 (
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/src/lib/agent-sessions/create-conversation-in-session.ts (1)
103-122: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNormalize conversation-creation errors consistently.
Infrastructure failures are currently disguised as 409 conflicts, while the new cross-drive domain error falls through as 502.
apps/web/src/lib/agent-sessions/create-conversation-in-session.ts#L103-L122: translate only known ownership/binding conflicts, explicitly preserve/mapAgentNotInSessionDriveError, and rethrow infrastructure failures.apps/web/src/app/api/agent-sessions/[sessionId]/conversations/__tests__/route.test.ts#L142-L157: add a regression test for the intended cross-drive HTTP response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/agent-sessions/create-conversation-in-session.ts` around lines 103 - 122, The createConversationInSession error handling must distinguish domain conflicts from infrastructure failures: in createGlobalConversation handling, translate only known ownership or binding conflicts, preserve/map AgentNotInSessionDriveError explicitly, and rethrow unexpected infrastructure errors. Update the regression coverage in apps/web/src/app/api/agent-sessions/[sessionId]/conversations/__tests__/route.test.ts:142-157 to assert the intended HTTP response for a cross-drive AgentNotInSessionDriveError; the anchor implementation change is in apps/web/src/lib/agent-sessions/create-conversation-in-session.ts:103-122.apps/web/src/app/api/ai/global/[id]/messages/resolve-or-create-conversation.ts (1)
103-117: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the racing winner with the same global-conversation invariants.
Unlike Lines 78–83, this path accepts an inactive or non-global row when its owner and session match. A concurrent page insert can therefore be returned as a global conversation. Reject winners where
winner.type !== 'global'or!winner.isActive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/ai/global/`[id]/messages/resolve-or-create-conversation.ts around lines 103 - 117, Validate the concurrently selected winner in the resolve-or-create flow using the same global-conversation invariants as the initial lookup. In the winner checks after selecting by conversationId, reject and throw the existing appropriate error when winner.type is not global or winner.isActive is false, before returning the conversation; preserve the existing ownership and session-binding validations.apps/web/src/lib/ai/tools/session-tools-runtime.ts (1)
357-370: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject unbound target conversations.
findSessionaccepts any existing conversation as a worker address. CheckfindSessionForConversation(sessionId)before returning the descriptor; otherwisesend_session/read_session/kill_sessioncan address a thread with no workspace session instead of returningno_session.Suggested fix
const conversation = await conversationRepository.getConversation(sessionId); if (!conversation) return null; +const workspaceSession = await findSessionForConversation(sessionId); +if (!workspaceSession) return null; return {As per PR objectives, threads without sessions must receive a typed
no_sessiondenial. [pr_objectives]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/ai/tools/session-tools-runtime.ts` around lines 357 - 370, Update findSession to call findSessionForConversation(sessionId) before resolving and returning the conversation descriptor. If no bound workspace session exists, return null so send_session, read_session, and kill_session produce the typed no_session denial; preserve the existing conversation-derived descriptor for bound sessions.apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx (1)
67-92: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReplace the app-admin-only gate with drive/session authorization.
The sidebar currently blocks every non-app-admin before the server can apply drive membership and code-execution permissions.
apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx#L67-L92: derive visibility from the applicable session capability, or allow the authenticated request to reach server authorization instead of usinguser.role.apps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx#L161-L172: replace the blanket non-admin refusal assertion with authorized-member and unauthorized-user cases.Based on the PR objectives, “Session access and billing are based on drive ownership or membership.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx` around lines 67 - 92, Replace the app-admin-only gating in AgentsSidebar’s sessionsKey and usePageAgents calls with drive/session authorization, allowing authenticated requests to reach server-side drive membership and code-execution checks instead of relying on user.role. Update apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx lines 67-92 accordingly, and revise apps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx lines 161-172 to cover both authorized drive members and unauthorized users rather than asserting blanket non-admin refusal.apps/web/src/components/agents/AgentPageView.tsx (1)
87-112: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse one SWR-backed source for client-side server state.
The manual effects create stale, independent caches; the agent-config request also duplicates the same SWR request already made by
useResolvedAgent.
apps/web/src/components/agents/AgentPageView.tsx#L87-L112: consume the existing agent-config SWR state or share its key/fetcher instead of maintaining a second fetch and cache.apps/web/src/components/agents/usePermissionsCheck.ts#L11-L30: replace the effect with a keyed SWR hook and validate/type the JSON payload before readingcanEdit.As per coding guidelines, “Use SWR for server state and caching in client components” and “Never use
anytypes.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/agents/AgentPageView.tsx` around lines 87 - 112, The manual agent-config fetch in AgentPageView’s effect creates a duplicate cache; consume the existing agent-config SWR state or reuse its key and fetcher instead, while preserving agentConfig loading behavior. In apps/web/src/components/agents/AgentPageView.tsx lines 87-112, remove the independent fetch effect and use the shared SWR source. In apps/web/src/components/agents/usePermissionsCheck.ts lines 11-30, replace the effect with keyed SWR state and validate/type the response before reading canEdit; do not use any.Source: Coding guidelines
🧹 Nitpick comments (2)
apps/web/src/app/api/v1/chat/completions/__tests__/route.test.ts (1)
120-120: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd route-level coverage for the sandbox gate.
This identity mock keeps existing tests passing without proving that
sandboxEnabled: falseremoves sandbox tools in server-only and mixed modes. Add focused assertions for both branches, while confirming client-only tools remain available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/v1/chat/completions/__tests__/route.test.ts` at line 120, Add route-level tests in the completions route test suite that exercise the sandbox gate with sandboxEnabled: false for both server-only and mixed tool sets. Assert sandbox tools are removed while client-only tools remain available, and update the filterToolsForSandboxEnablement mock only as needed to support these focused branch assertions.apps/web/src/components/agents/panes/AgentPanes.tsx (1)
128-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated pane-reset-on-error logic.
handlePickAgent's andhandlePickShell's catch blocks both reset the pane to the same sentinel shape ({ kind: 'chat', name: '', targetId: null, agentPageId: null }) thatshowPicker(Line 198) specifically pattern-matches on. Extracting a shared helper avoids the two call sites drifting out of sync with that sentinel contract.♻️ Proposed refactor
+ const resetPaneToPicker = useCallback( + (paneId: string, description: string, error: unknown) => { + toast.error(description, { + description: error instanceof Error ? error.message : 'Please try again.', + }); + assignPane(sessionId, paneId, { kind: 'chat', name: '', targetId: null, agentPageId: null }); + dismissPicker(sessionId, paneId); + }, + [assignPane, dismissPicker, sessionId], + ); + const handlePickAgent = useCallback( async (paneId: string, agentPageId: string | null) => { ... } catch (error) { console.error('Failed to start a conversation in this pane:', error); - toast.error('Could not start a conversation', { - description: error instanceof Error ? error.message : 'Please try again.', - }); - assignPane(sessionId, paneId, { kind: 'chat', name: '', targetId: null, agentPageId: null }); - dismissPicker(sessionId, paneId); + resetPaneToPicker(paneId, 'Could not start a conversation', error); } }, - [assignPane, dismissPicker, sessionId], + [assignPane, resetPaneToPicker, sessionId], );(similarly for
handlePickShell)Also applies to: 161-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/agents/panes/AgentPanes.tsx` around lines 128 - 159, Extract the shared pane-reset sentinel into a helper near showPicker, then reuse it in the catch blocks of handlePickAgent and handlePickShell before dismissPicker. Ensure both paths continue resetting the pane to the exact shape recognized by showPicker, avoiding duplicated object literals that could drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/api/agent-sessions/`[sessionId]/conversations/route.ts:
- Around line 107-136: Update the catch around createConversationInSession to
handle AgentNotInSessionDriveError explicitly before the generic error path.
Return the same 400 cross-drive response used by session spawning, while
preserving the existing ConversationUnavailableError handling and 502 response
for unexpected failures.
In `@apps/web/src/app/api/agent-sessions/route.ts`:
- Around line 239-248: Update the catch path around endSession in the
agent-session spawn flow so cleanup failures are not discarded: inspect the
returned result and handle rejected errors, then durably record or otherwise
propagate an unsuccessful rollback while preserving the existing error response
and logging context. Ensure a failed initial conversation cannot leave the newly
spawned session active without a durable cleanup outcome.
- Around line 135-147: Update the request-body validation in
apps/web/src/app/api/agent-sessions/route.ts lines 135-147 to return 400 for
JSON parse failures, non-object bodies, and any present non-string or empty
driveId, agentPageId, or name fields; only omitted fields should become null.
Apply the same validation in
apps/web/src/app/api/agent-sessions/[sessionId]/conversations/route.ts lines
57-64, returning 400 for parse failures, non-object bodies, or invalid present
agentPageId while preserving omission or explicit null for assistant creation.
- Around line 192-223: Make the active-session limit atomic by moving
enforcement from the route’s preflight check into the owner-scoped creation path
used by spawnSession/spawnAgentSession. Within the same transaction, CAS, or row
lock as deps.store.create, recheck the owner’s active-session count and reject
creation when it reaches MAX_ACTIVE_SESSIONS_PER_OWNER; preserve the existing
quota-exceeded response behavior.
In `@apps/web/src/app/api/pages/`[pageId]/agent-config/route.ts:
- Around line 233-235: Update the sandboxEnabled handling in the agent-config
route to accept only actual boolean values, rejecting or otherwise handling
strings, objects, and other types without enabling sandbox tools. When valid,
assign sandboxEnabled directly to updateData.sandboxEnabled instead of coercing
it with Boolean().
In `@apps/web/src/components/agents/AgentPageView.tsx`:
- Around line 76-83: Scope the conversation override used by AgentPageView to
the current page.id so it cannot persist across agent page changes. Update both
setOverride call sites to store the associated pageId, and make the current
conversation use the override only when its pageId matches page.id; otherwise
fall back to initialResolved. Add a rerender regression test covering a page-id
change.
In `@apps/web/src/components/agents/chat/SessionChat.tsx`:
- Around line 154-159: Update the SessionChat component’s handleUndoFromHere
prop to be undefined when isReadOnly is true, matching the existing conditional
behavior for handleEdit, handleDelete, and handleRetry. Preserve the current
setUndoMessageId callback for editable views.
In `@apps/web/src/components/agents/panes/PaneChat.tsx`:
- Line 35: Update PaneChat’s useResolvedAgent integration to destructure and
handle error and retry alongside agent and isLoading. When resolution fails,
render an error state with a retry affordance instead of allowing the existing
loading condition to show a permanent spinner; preserve the loading and
successfully resolved agent rendering paths.
In `@apps/web/src/components/agents/useResolvedConversation.ts`:
- Around line 60-78: Model conversation-creation outcomes explicitly: in
apps/web/src/components/agents/useResolvedConversation.ts lines 60-78, only fall
back to plain chat for a confirmed access/capability refusal, while propagating
ambiguous or network/5xx session failures; in lines 112-128, do not resolve a
client-only ID when createAgentConversation fails, but return a typed error or
retry outcome; in apps/web/src/components/agents/AgentPageView.tsx lines
122-131, consume that outcome or catch and report failures so void
newConversation() callers do not produce unhandled rejections.
In `@apps/web/src/components/ai/page-agents/PageAgentSettingsTab.tsx`:
- Around line 362-373: Update the selected-tools count to use only the
intersection of enabledTools and visibleTools, so hidden sandbox tools are
excluded when sandboxEnabled is false. Locate the footer/count logic associated
with visibleTools and preserve the existing total visible-tool count while
preventing values such as “Selected 5 of 3.”
In `@apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx`:
- Around line 291-346: Add a dedicated pending state for new-conversation
creation near the existing expanded, confirmingEnd, and ending state in the
session row, guard newConversation against re-entry, and set/reset the state
around both post request branches using finally. Disable the new-conversation
button while that state is active, following the existing session spawning and
ending patterns so repeated clicks cannot create duplicates.
In `@apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts`:
- Around line 329-359: Update listSessionConversationsBulk so the database query
limits results to the newest 100 active conversations per session using a
partitioned row_number() or lateral-query strategy. Apply deterministic
newest-first ordering, including a stable tie-breaker, in SQL; remove the
JavaScript bucket-length truncation while preserving the returned grouping and
entry mapping.
In `@packages/lib/src/services/agent-sessions/__tests__/fakes.ts`:
- Around line 104-121: Update the fake store’s `list` method to import and apply
`SESSION_LIST_LIMIT` from the store module after filtering and sorting,
returning no more than the production limit of matching rows while preserving
the existing ordering.
---
Outside diff comments:
In
`@apps/web/src/app/api/ai/global/`[id]/messages/resolve-or-create-conversation.ts:
- Around line 103-117: Validate the concurrently selected winner in the
resolve-or-create flow using the same global-conversation invariants as the
initial lookup. In the winner checks after selecting by conversationId, reject
and throw the existing appropriate error when winner.type is not global or
winner.isActive is false, before returning the conversation; preserve the
existing ownership and session-binding validations.
In `@apps/web/src/components/agents/AgentPageView.tsx`:
- Around line 87-112: The manual agent-config fetch in AgentPageView’s effect
creates a duplicate cache; consume the existing agent-config SWR state or reuse
its key and fetcher instead, while preserving agentConfig loading behavior. In
apps/web/src/components/agents/AgentPageView.tsx lines 87-112, remove the
independent fetch effect and use the shared SWR source. In
apps/web/src/components/agents/usePermissionsCheck.ts lines 11-30, replace the
effect with keyed SWR state and validate/type the response before reading
canEdit; do not use any.
In `@apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx`:
- Around line 67-92: Replace the app-admin-only gating in AgentsSidebar’s
sessionsKey and usePageAgents calls with drive/session authorization, allowing
authenticated requests to reach server-side drive membership and code-execution
checks instead of relying on user.role. Update
apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx lines 67-92
accordingly, and revise
apps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsx
lines 161-172 to cover both authorized drive members and unauthorized users
rather than asserting blanket non-admin refusal.
In `@apps/web/src/lib/agent-sessions/create-conversation-in-session.ts`:
- Around line 103-122: The createConversationInSession error handling must
distinguish domain conflicts from infrastructure failures: in
createGlobalConversation handling, translate only known ownership or binding
conflicts, preserve/map AgentNotInSessionDriveError explicitly, and rethrow
unexpected infrastructure errors. Update the regression coverage in
apps/web/src/app/api/agent-sessions/[sessionId]/conversations/__tests__/route.test.ts:142-157
to assert the intended HTTP response for a cross-drive
AgentNotInSessionDriveError; the anchor implementation change is in
apps/web/src/lib/agent-sessions/create-conversation-in-session.ts:103-122.
In `@apps/web/src/lib/ai/tools/session-tools-runtime.ts`:
- Around line 357-370: Update findSession to call
findSessionForConversation(sessionId) before resolving and returning the
conversation descriptor. If no bound workspace session exists, return null so
send_session, read_session, and kill_session produce the typed no_session
denial; preserve the existing conversation-derived descriptor for bound
sessions.
---
Nitpick comments:
In `@apps/web/src/app/api/v1/chat/completions/__tests__/route.test.ts`:
- Line 120: Add route-level tests in the completions route test suite that
exercise the sandbox gate with sandboxEnabled: false for both server-only and
mixed tool sets. Assert sandbox tools are removed while client-only tools remain
available, and update the filterToolsForSandboxEnablement mock only as needed to
support these focused branch assertions.
In `@apps/web/src/components/agents/panes/AgentPanes.tsx`:
- Around line 128-159: Extract the shared pane-reset sentinel into a helper near
showPicker, then reuse it in the catch blocks of handlePickAgent and
handlePickShell before dismissPicker. Ensure both paths continue resetting the
pane to the exact shape recognized by showPicker, avoiding duplicated object
literals that could drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d073d5c-318e-4623-8aef-8abf82791506
📒 Files selected for processing (113)
apps/web/src/app/api/agent-sessions/[sessionId]/conversations/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/[sessionId]/conversations/route.tsapps/web/src/app/api/agent-sessions/[sessionId]/diff/route.tsapps/web/src/app/api/agent-sessions/[sessionId]/git-blob/route.tsapps/web/src/app/api/agent-sessions/[sessionId]/route.tsapps/web/src/app/api/agent-sessions/__tests__/route.test.tsapps/web/src/app/api/agent-sessions/route.tsapps/web/src/app/api/ai/chat/__tests__/credit-gate.test.tsapps/web/src/app/api/ai/chat/__tests__/mcp-scope.test.tsapps/web/src/app/api/ai/chat/__tests__/sandbox-github-suppression.test.tsapps/web/src/app/api/ai/chat/__tests__/stream-socket-events.test.tsapps/web/src/app/api/ai/chat/messages/[messageId]/__tests__/route.test.tsapps/web/src/app/api/ai/chat/route.tsapps/web/src/app/api/ai/global/[id]/messages/__tests__/conversation-id-resolution.test.tsapps/web/src/app/api/ai/global/[id]/messages/__tests__/credit-gate.test.tsapps/web/src/app/api/ai/global/[id]/messages/__tests__/stream-socket-events.test.tsapps/web/src/app/api/ai/global/[id]/messages/resolve-or-create-conversation.tsapps/web/src/app/api/ai/page-agents/[agentId]/conversations/__tests__/route.test.tsapps/web/src/app/api/ai/page-agents/[agentId]/conversations/route.tsapps/web/src/app/api/pages/[pageId]/agent-config/route.tsapps/web/src/app/api/tasks/__tests__/route.test.tsapps/web/src/app/api/v1/chat/completions/__tests__/route-backfill.test.tsapps/web/src/app/api/v1/chat/completions/__tests__/route.test.tsapps/web/src/app/api/v1/chat/completions/route.tsapps/web/src/components/agents/AgentPageView.tsxapps/web/src/components/agents/AgentView.tsxapps/web/src/components/agents/AgentsSurface.tsxapps/web/src/components/agents/__tests__/AgentPageView.test.tsxapps/web/src/components/agents/__tests__/AgentView.test.tsxapps/web/src/components/agents/__tests__/AgentsSurface.test.tsxapps/web/src/components/agents/__tests__/useAgentSession.test.tsxapps/web/src/components/agents/__tests__/useResolvedConversation.test.tsapps/web/src/components/agents/__tests__/useSessionShells.test.tsxapps/web/src/components/agents/chat/AssistantSessionChat.tsxapps/web/src/components/agents/chat/SessionChat.tsxapps/web/src/components/agents/chat/useAgentSessionChat.tsapps/web/src/components/agents/chat/useAssistantSessionChat.tsapps/web/src/components/agents/panes/AgentPanes.tsxapps/web/src/components/agents/panes/PaneBar.tsxapps/web/src/components/agents/panes/PaneChat.tsxapps/web/src/components/agents/panes/PanePicker.tsxapps/web/src/components/agents/panes/SessionPanes.tsxapps/web/src/components/agents/panes/__tests__/PaneBar.test.tsxapps/web/src/components/agents/panes/__tests__/PanePicker.test.tsxapps/web/src/components/agents/panes/__tests__/SessionPanes.test.tsxapps/web/src/components/agents/panes/__tests__/pane-surface.test.tsapps/web/src/components/agents/panes/pane-surface.tsapps/web/src/components/agents/shell/Shell.tsxapps/web/src/components/agents/useAgentSession.tsapps/web/src/components/agents/usePermissionsCheck.tsapps/web/src/components/agents/useResolvedAgent.tsapps/web/src/components/agents/useResolvedConversation.tsapps/web/src/components/agents/useSessionShells.tsapps/web/src/components/ai/page-agents/PageAgentSettingsTab.tsxapps/web/src/components/layout/left-sidebar/AgentsSidebar.tsxapps/web/src/components/layout/left-sidebar/__tests__/AgentsSidebar.test.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsxapps/web/src/hooks/__tests__/useUserActiveStreams.test.tsxapps/web/src/hooks/useUserActiveStreams.tsapps/web/src/lib/agent-sessions/__tests__/create-conversation-in-session.test.tsapps/web/src/lib/agent-sessions/__tests__/session-anchor.test.tsapps/web/src/lib/agent-sessions/agent-sessions-runtime.tsapps/web/src/lib/agent-sessions/create-conversation-in-session.tsapps/web/src/lib/agent-sessions/session-anchor.tsapps/web/src/lib/agent-sessions/session-sandbox-runtime.tsapps/web/src/lib/agent-sessions/session-shells-runtime.tsapps/web/src/lib/agents/__tests__/agent-selection.test.tsapps/web/src/lib/agents/__tests__/running-badges.test.tsapps/web/src/lib/agents/__tests__/session-status.test.tsapps/web/src/lib/agents/__tests__/session-tabs.test.tsapps/web/src/lib/agents/agent-selection.tsapps/web/src/lib/agents/build-session-chat-request.tsapps/web/src/lib/agents/running-badges.tsapps/web/src/lib/agents/session-status.tsapps/web/src/lib/agents/session-tabs.tsapps/web/src/lib/ai/core/__tests__/tool-filtering.test.tsapps/web/src/lib/ai/core/system-prompt.tsapps/web/src/lib/ai/core/tool-filtering.tsapps/web/src/lib/ai/shared/agent-conversations.tsapps/web/src/lib/ai/shared/chat-types.tsapps/web/src/lib/ai/shared/hooks/useConversations.tsapps/web/src/lib/ai/tools/__tests__/sandbox-git-tools.test.tsapps/web/src/lib/ai/tools/__tests__/sandbox-tools-runtime.test.tsapps/web/src/lib/ai/tools/__tests__/sandbox-tools.test.tsapps/web/src/lib/ai/tools/sandbox-tools-runtime.tsapps/web/src/lib/ai/tools/sandbox-tools.tsapps/web/src/lib/ai/tools/session-tools-runtime.tsapps/web/src/lib/repositories/__tests__/conversation-repository.test.tsapps/web/src/lib/repositories/conversation-repository.tsapps/web/src/stores/agent-workspace/__tests__/pane-reducer.test.tsapps/web/src/stores/agent-workspace/__tests__/useAgentWorkspaceStore.test.tsapps/web/src/stores/agent-workspace/pane-reducer.tsapps/web/src/stores/agent-workspace/useAgentWorkspaceStore.tsapps/web/src/stores/agents/__tests__/useAgentSurfaceStore.test.tsapps/web/src/stores/agents/useAgentSurfaceStore.tsapps/web/src/stores/useOptimisticConversationsStore.tspackages/db/drizzle/0239_faithful_klaw.sqlpackages/db/drizzle/meta/0239_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/core.tspackages/lib/src/agent-sessions/__tests__/contract.test.tspackages/lib/src/agent-sessions/contract.tspackages/lib/src/services/agent-sessions/__tests__/agent-sessions-store.integration.test.tspackages/lib/src/services/agent-sessions/__tests__/fakes.tspackages/lib/src/services/agent-sessions/agent-sessions-store.tspackages/lib/src/services/agent-sessions/agent-sessions.tspackages/lib/src/services/sandbox/__tests__/git-tool-runners.test.tspackages/lib/src/services/sandbox/__tests__/machine-diff.test.tspackages/lib/src/services/sandbox/__tests__/machine-git-blob.test.tspackages/lib/src/services/sandbox/__tests__/tool-runners.test.tspackages/lib/src/services/sandbox/git-tool-runners.tspackages/lib/src/services/sandbox/sandbox-storage-measure.tspackages/lib/src/services/sandbox/tool-runners.ts
💤 Files with no reviewable changes (16)
- apps/web/src/hooks/tests/useUserActiveStreams.test.tsx
- apps/web/src/lib/agent-sessions/tests/session-anchor.test.ts
- apps/web/src/components/agents/tests/AgentView.test.tsx
- apps/web/src/lib/agents/running-badges.ts
- apps/web/src/lib/agent-sessions/session-anchor.ts
- apps/web/src/lib/agents/tests/session-tabs.test.ts
- apps/web/src/hooks/useUserActiveStreams.ts
- apps/web/src/components/agents/tests/useSessionShells.test.tsx
- apps/web/src/components/agents/useSessionShells.ts
- apps/web/src/lib/agents/session-status.ts
- apps/web/src/lib/agents/tests/running-badges.test.ts
- apps/web/src/lib/agents/tests/session-status.test.ts
- apps/web/src/lib/agents/session-tabs.ts
- apps/web/src/components/agents/AgentView.tsx
- apps/web/src/components/agents/useAgentSession.ts
- apps/web/src/components/agents/tests/useAgentSession.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/lib/src/services/agent-sessions/agent-sessions.ts
- apps/web/src/app/api/v1/chat/completions/tests/route-backfill.test.ts
👮 Files not reviewed due to content moderation or server errors (12)
- apps/web/src/lib/ai/shared/hooks/useConversations.ts
- apps/web/src/lib/repositories/conversation-repository.ts
- apps/web/src/stores/agent-workspace/tests/useAgentWorkspaceStore.test.ts
- apps/web/src/stores/agent-workspace/useAgentWorkspaceStore.ts
- apps/web/src/stores/agents/useAgentSurfaceStore.ts
- apps/web/src/stores/useOptimisticConversationsStore.ts
- packages/db/drizzle/0239_faithful_klaw.sql
- packages/db/drizzle/meta/0239_snapshot.json
- packages/db/drizzle/meta/_journal.json
- packages/lib/src/agent-sessions/tests/contract.test.ts
- packages/lib/src/agent-sessions/contract.ts
- packages/lib/src/services/sandbox/tests/git-tool-runners.test.ts
Comment on lines
+107
to
+136
| try { | ||
| await createConversationInSession({ | ||
| conversationId, | ||
| userId: auth.userId, | ||
| agentPageId, | ||
| sessionId, | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof ConversationUnavailableError) { | ||
| // The id names a conversation that cannot be claimed WITH this binding | ||
| // (someone else's, a legacy conflict, or a different session's) — a | ||
| // conflict with existing state, not a service failure. One message for | ||
| // every cause: distinguishing them would tell an id-guessing caller | ||
| // which one it hit. | ||
| auditRequest(request, { | ||
| eventType: 'authz.access.denied', | ||
| userId: auth.userId, | ||
| resourceType: 'agent_session', | ||
| resourceId: sessionId, | ||
| details: { reason: 'conversation_unavailable', conversationId, route: 'agent-sessions/[sessionId]/conversations' }, | ||
| riskScore: 0.5, | ||
| }); | ||
| return NextResponse.json({ error: 'That conversation id is not available' }, { status: 409 }); | ||
| } | ||
| loggers.api.error( | ||
| 'Session conversation create failed', | ||
| error instanceof Error ? error : undefined, | ||
| { sessionId, agentPageId }, | ||
| ); | ||
| return NextResponse.json({ error: 'Could not start a conversation' }, { status: 502 }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Map cross-drive agent mismatches as client errors.
createConversationInSession throws AgentNotInSessionDriveError when the selected agent belongs to another drive, but this catch treats it as a 502 service failure. Catch it explicitly and return the same 400 cross-drive response used by session spawning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/agent-sessions/`[sessionId]/conversations/route.ts
around lines 107 - 136, Update the catch around createConversationInSession to
handle AgentNotInSessionDriveError explicitly before the generic error path.
Return the same 400 cross-drive response used by session spawning, while
preserving the existing ConversationUnavailableError handling and 502 response
for unexpected failures.
Comment on lines
+135
to
+147
| let body: { driveId?: unknown; agentPageId?: unknown; name?: unknown } = {}; | ||
| try { | ||
| body = await request.json(); | ||
| } catch { | ||
| // Body is required — a spawn names its drive and agent. | ||
| } | ||
| const driveId = typeof body.driveId === 'string' && body.driveId.length > 0 ? body.driveId : null; | ||
| const agentPageId = | ||
| typeof body.agentPageId === 'string' && body.agentPageId.length > 0 ? body.agentPageId : null; | ||
| const rawName = typeof body.name === 'string' ? body.name.trim() : ''; | ||
| // A label, never an address — but still bounded: it is stored, listed and | ||
| // rendered everywhere the session appears. | ||
| const name = rawName.length > 0 ? rawName.slice(0, MAX_SESSION_NAME_LENGTH) : null; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not normalize invalid request bodies into the global-assistant shape.
Both handlers interpret malformed or incorrectly typed input as omitted fields, causing unintended persistent writes.
apps/web/src/app/api/agent-sessions/route.ts#L135-L147: return 400 for parse failures, non-object bodies, and invalid presentdriveId,agentPageId, ornamefields.apps/web/src/app/api/agent-sessions/[sessionId]/conversations/route.ts#L57-L64: return 400 for parse failures, non-object bodies, and invalid presentagentPageIdfields; reserve omission or explicit null for assistant creation.
📍 Affects 2 files
apps/web/src/app/api/agent-sessions/route.ts#L135-L147(this comment)apps/web/src/app/api/agent-sessions/[sessionId]/conversations/route.ts#L57-L64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/agent-sessions/route.ts` around lines 135 - 147, Update
the request-body validation in apps/web/src/app/api/agent-sessions/route.ts
lines 135-147 to return 400 for JSON parse failures, non-object bodies, and any
present non-string or empty driveId, agentPageId, or name fields; only omitted
fields should become null. Apply the same validation in
apps/web/src/app/api/agent-sessions/[sessionId]/conversations/route.ts lines
57-64, returning 400 for parse failures, non-object bodies, or invalid present
agentPageId while preserving omission or explicit null for assistant creation.
Comment on lines
+192
to
+223
| const activeCount = await countActiveSessionsForOwner(auth.userId); | ||
| if (activeCount >= MAX_ACTIVE_SESSIONS_PER_OWNER) { | ||
| return sessionQuotaExceeded( | ||
| request, | ||
| auth.userId, | ||
| 'about-to-be-minted', | ||
| 'agent-sessions', | ||
| `You have ${activeCount} active sessions — end some before starting more.`, | ||
| ); | ||
| } | ||
|
|
||
| const access = await checkAccessForSubject(auth.userId, { | ||
| sessionId: 'about-to-be-minted', | ||
| ownerId: auth.userId, | ||
| driveId, | ||
| }); | ||
| if (!access.allowed) { | ||
| auditRequest(request, { | ||
| eventType: 'authz.access.denied', | ||
| userId: auth.userId, | ||
| resourceType: 'agent_session', | ||
| resourceId: driveId ?? 'global', | ||
| details: { reason: access.reason, method: 'POST', route: 'agent-sessions' }, | ||
| riskScore: 0.5, | ||
| }); | ||
| return NextResponse.json( | ||
| { error: driveId ? 'You cannot start a session in this drive' : 'You cannot start an assistant session' }, | ||
| { status: 403 }, | ||
| ); | ||
| } | ||
|
|
||
| const spawned = await spawnSession({ userId: auth.userId, driveId, name }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts \
--match 'spawnSession|countActiveSessionsForOwner' --view expanded
rg -n -C5 '\b(spawnSession|countActiveSessionsForOwner)\b' \
apps/web/src/lib/agent-sessions packages/lib/src/services/agent-sessionsRepository: 2witstudios/PageSpace
Length of output: 2401
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- route snippet ---\n'
sed -n '170,240p' apps/web/src/app/api/agent-sessions/route.ts
printf '\n--- agent-sessions-runtime relevant snippets ---\n'
sed -n '1,180p' apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts
sed -n '260,380p' apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts
printf '\n--- store definitions/usages ---\n'
rg -n -C4 'class .*Store|countActive|createSession|agentSession|AgentSessionStore' apps/web/src/lib/agent-sessions packages/lib/src/services/agent-sessionsRepository: 2witstudios/PageSpace
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- route snippet ---'
sed -n '170,240p' apps/web/src/app/api/agent-sessions/route.ts
echo
echo '--- agent-sessions-runtime relevant snippets ---'
sed -n '1,180p' apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts
sed -n '260,380p' apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts
echo
echo '--- store definitions/usages ---'
rg -n -C4 'class .*Store|countActive|createSession|agentSession|AgentSessionStore' apps/web/src/lib/agent-sessions packages/lib/src/services/agent-sessionsRepository: 2witstudios/PageSpace
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- package lib agent-sessions-store interface and implementation ---'
sed -n '1,220p' packages/lib/src/services/agent-sessions/agent-sessions-store.ts
sed -n '220,420p' packages/lib/src/services/agent-sessions/agent-sessions-store.ts
echo
echo '--- package lib spawnAgentSession implementation ---'
sed -n '1,170p' packages/lib/src/services/agent-sessions/agent-sessions.ts
echo
echo '--- DB transaction / countActive lock references ---'
rg -n -C3 '\b(countActive|MAX_ACTIVE_SESSIONS_PER_OWNER|MAX_.*SESSION|transaction\(|forShare|FOR SHARE|selectForUpdate|FOR UPDATE|Advisory|PG_LOCK|lock\(|locks|lock)' \
apps/web/src/lib/agent-sessions packages/lib/src/agent-sessions packages/lib/src/services/agent-sessions packages/lib/src/applications packages/lib/src/services \
--glob '*.ts'Repository: 2witstudios/PageSpace
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- focused file sizes ---'
wc -l packages/lib/src/services/agent-sessions/agent-sessions-store.ts packages/lib/src/services/agent-sessions/agent-sessions.ts
echo
echo '--- agent-sessions-store interface and create/countActive live snippets ---'
sed -n '80,125p' packages/lib/src/services/agent-sessions/agent-sessions-store.ts
sed -n '330,355p' packages/lib/src/services/agent-sessions/agent-sessions-store.ts
echo
echo '--- spawnAgentSession exact implementation ---'
sed -n '28,76p' packages/lib/src/services/agent-sessions/agent-sessions.ts
sed -n '130,186p' packages/lib/src/services/agent-sessions/agent-sessions.ts
echo
echo '--- focused concurrency/lock search within agent-sessions service area ---'
rg -n -C3 'countActive|MAX_ACTIVE|transaction\(|select\(|forShare|FOR SHARE|FOR UPDATE|advisory|withAdvisoryLock|lock\(' \
packages/lib/src/agent-sessions packages/lib/src/services/agent-sessions \
--glob '*.ts'Repository: 2witstudios/PageSpace
Length of output: 31449
Make the active-session ceiling atomic.
spawnAgentSession only delegates to deps.store.create(...) and does not recheck the owner’s active-session count, so the countActive(ownerId) >= MAX_ACTIVE_SESSIONS_PER_OWNER check in the route can race. Move the creation through the same owner-scoped transaction/CAS or row lock that enforces the limit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/agent-sessions/route.ts` around lines 192 - 223, Make
the active-session limit atomic by moving enforcement from the route’s preflight
check into the owner-scoped creation path used by
spawnSession/spawnAgentSession. Within the same transaction, CAS, or row lock as
deps.store.create, recheck the owner’s active-session count and reject creation
when it reaches MAX_ACTIVE_SESSIONS_PER_OWNER; preserve the existing
quota-exceeded response behavior.
Comment on lines
+239
to
+248
| } catch (error) { | ||
| // The session row exists but its first conversation failed: end it rather | ||
| // than leave an empty workspace the model says cannot exist. | ||
| await endSession(spawned.session.id).catch(() => {}); | ||
| loggers.api.error( | ||
| 'Agent session spawn: first conversation failed', | ||
| error instanceof Error ? error : undefined, | ||
| { sessionId: spawned.session.id }, | ||
| ); | ||
| return NextResponse.json({ error: 'Could not start a session' }, { status: 502 }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not ignore failed rollback of the newly spawned session.
endSession returns a failure result without necessarily rejecting, and both that result and thrown errors are discarded. A failed first-conversation insert can therefore leave an active empty session. Make spawn plus initial conversation atomic, or durably handle an unsuccessful cleanup.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 242-246: Avoid logging sensitive data
Context: loggers.api.error(
'Agent session spawn: first conversation failed',
error instanceof Error ? error : undefined,
{ sessionId: spawned.session.id },
)
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/agent-sessions/route.ts` around lines 239 - 248, Update
the catch path around endSession in the agent-session spawn flow so cleanup
failures are not discarded: inspect the returned result and handle rejected
errors, then durably record or otherwise propagate an unsuccessful rollback
while preserving the existing error response and logging context. Ensure a
failed initial conversation cannot leave the newly spawned session active
without a durable cleanup outcome.
Comment on lines
+233
to
+235
| if (sandboxEnabled !== undefined) { | ||
| updateData.sandboxEnabled = Boolean(sandboxEnabled); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require a real boolean for the sandbox switch.
Boolean('false') and Boolean({}) both evaluate to true, so malformed JSON can enable sandbox tools. Reject non-boolean values and assign the boolean directly.
Suggested fix
if (sandboxEnabled !== undefined) {
- updateData.sandboxEnabled = Boolean(sandboxEnabled);
+ if (typeof sandboxEnabled !== 'boolean') {
+ return NextResponse.json({ error: 'sandboxEnabled must be a boolean' }, { status: 400 });
+ }
+ updateData.sandboxEnabled = sandboxEnabled;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (sandboxEnabled !== undefined) { | |
| updateData.sandboxEnabled = Boolean(sandboxEnabled); | |
| } | |
| if (sandboxEnabled !== undefined) { | |
| if (typeof sandboxEnabled !== 'boolean') { | |
| return NextResponse.json({ error: 'sandboxEnabled must be a boolean' }, { status: 400 }); | |
| } | |
| updateData.sandboxEnabled = sandboxEnabled; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/pages/`[pageId]/agent-config/route.ts around lines 233 -
235, Update the sandboxEnabled handling in the agent-config route to accept only
actual boolean values, rejecting or otherwise handling strings, objects, and
other types without enabling sandbox tools. When valid, assign sandboxEnabled
directly to updateData.sandboxEnabled instead of coercing it with Boolean().
Comment on lines
+60
to
+78
| try { | ||
| const created = await post<{ session: { sessionId: string }; conversationId: string }>( | ||
| '/api/agent-sessions', | ||
| { driveId, agentPageId: agentId }, | ||
| ); | ||
| conversationMessagesActions.seedConversation(created.conversationId); | ||
| return { conversationId: created.conversationId, sessionId: created.session.sessionId }; | ||
| } catch (error) { | ||
| // The role gate is a cheap client-side approximation; the server's | ||
| // access decision (drive membership + code-execution) is the truth. | ||
| // A refusal means "no workspace for you", not "no conversation". | ||
| console.warn('Session spawn refused; falling back to a plain conversation:', error); | ||
| } | ||
| } | ||
|
|
||
| const conversationId = createId(); | ||
| conversationMessagesActions.seedConversation(conversationId); | ||
| await createAgentConversation(agentId, conversationId); | ||
| return { conversationId, sessionId: null }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Model conversation-creation failures instead of treating them as success.
The flow does not distinguish a confirmed session denial from an ambiguous or hard failure, creating duplicate, phantom, or unhandled conversation states.
apps/web/src/components/agents/useResolvedConversation.ts#L60-L78: only downgrade to plain chat for a confirmed capability/access refusal. A network or 5xx failure may occur after the session POST committed; falling back then creates a second conversation.apps/web/src/components/agents/useResolvedConversation.ts#L112-L128: do not mark a client-only ID as resolved after plain creation fails. Return an explicit error/retry state instead.apps/web/src/components/agents/AgentPageView.tsx#L122-L131: consume that typed outcome or catch and report failure so thevoid newConversation()callers cannot produce unhandled rejections.
📍 Affects 2 files
apps/web/src/components/agents/useResolvedConversation.ts#L60-L78(this comment)apps/web/src/components/agents/useResolvedConversation.ts#L112-L128apps/web/src/components/agents/AgentPageView.tsx#L122-L131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/agents/useResolvedConversation.ts` around lines 60 -
78, Model conversation-creation outcomes explicitly: in
apps/web/src/components/agents/useResolvedConversation.ts lines 60-78, only fall
back to plain chat for a confirmed access/capability refusal, while propagating
ambiguous or network/5xx session failures; in lines 112-128, do not resolve a
client-only ID when createAgentConversation fails, but return a typed error or
retry outcome; in apps/web/src/components/agents/AgentPageView.tsx lines
122-131, consume that outcome or catch and report failures so void
newConversation() callers do not produce unhandled rejections.
Comment on lines
+362
to
+373
| // The sandbox switch gates the sandbox tool families out of the Default | ||
| // Tools list (the old machineAccess/MACHINE_TOOL_NAMES behaviour). The | ||
| // request-time filter in tool-filtering.ts is the real gate; this keeps the | ||
| // picker from offering tools the agent will never receive. | ||
| const sandboxEnabled = watch('sandboxEnabled', false); | ||
|
|
||
| const visibleTools = useMemo( | ||
| () => config?.availableTools || [], | ||
| [config] | ||
| () => | ||
| (config?.availableTools || []).filter( | ||
| (tool) => sandboxEnabled || !SANDBOX_TOOL_NAMES.has(tool.name) | ||
| ), | ||
| [config, sandboxEnabled] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Count only visible selected tools.
Disabling sandbox hides its tools but leaves them in enabledTools, so the footer can display values such as “Selected 5 of 3.” Count the intersection with visibleTools, or clear sandbox selections when disabling the toggle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/ai/page-agents/PageAgentSettingsTab.tsx` around lines
362 - 373, Update the selected-tools count to use only the intersection of
enabledTools and visibleTools, so hidden sandbox tools are excluded when
sandboxEnabled is false. Locate the footer/count logic associated with
visibleTools and preserve the existing total visible-tool count while preventing
values such as “Selected 5 of 3.”
Comment on lines
+291
to
+346
| const [expanded, setExpanded] = useState(false); | ||
| const [confirmingEnd, setConfirmingEnd] = useState(false); | ||
| const [ending, setEnding] = useState(false); | ||
| const isSelected = selectedSessionId === session.sessionId; | ||
| const isRunning = session.sandboxStatus === 'running' || session.sandboxStatus === 'starting'; | ||
|
|
||
| const onConversationCreate = useCallback( | ||
| (conversationId: string) => { | ||
| // A just-minted id has no server rows — mark it loaded-empty so nothing | ||
| // fetches for it and no loading state shows. (Same move as | ||
| // `usePageAgentDashboardStore.createNewConversation`, which this mirrors.) | ||
| conversationMessagesActions.seedConversation(conversationId); | ||
| selectConversation(conversationId, agent.id); | ||
| const openConversation = useCallback( | ||
| (conversation: SessionConversationEntry) => { | ||
| selectConversation({ | ||
| sessionId: session.sessionId, | ||
| conversationId: conversation.conversationId, | ||
| agentId: conversation.agentPageId, | ||
| }); | ||
| }, | ||
| [agent.id, selectConversation], | ||
| [selectConversation, session.sessionId], | ||
| ); | ||
|
|
||
| const { conversations, isLoading, createConversation } = useConversations({ | ||
| agentId: agent.id, | ||
| currentConversationId: selectedConversationId, | ||
| enabled: expanded, | ||
| onConversationCreate, | ||
| }); | ||
| const openSession = useCallback(() => { | ||
| // Selecting a SESSION opens its most recent conversation — the row is a | ||
| // workspace, and a workspace opens on its work, not on a placeholder. | ||
| setExpanded(true); | ||
| const first = session.conversations[0]; | ||
| if (first) openConversation(first); | ||
| }, [openConversation, session.conversations]); | ||
|
|
||
| const conversationIds = useMemo(() => conversations.map((c) => c.id), [conversations]); | ||
| const conversationBadges = useMemo( | ||
| () => deriveRunningBadges(streams, conversationIds).byConversation, | ||
| [streams, conversationIds], | ||
| ); | ||
| const newConversation = useCallback(async () => { | ||
| // A new thread defaults to the session's most recent conversation's | ||
| // counterpart — the full drive picker lives in the pane grid. A null | ||
| // agent (a global session, or a drive session whose latest thread is an | ||
| // assistant thread) means the ASSISTANT, created through the | ||
| // session-centric route since it has no agent page. | ||
| const agentPageId = session.conversations[0]?.agentPageId ?? null; | ||
| try { | ||
| const created = | ||
| agentPageId === null | ||
| ? await post<{ conversationId: string }>( | ||
| `/api/agent-sessions/${encodeURIComponent(session.sessionId)}/conversations`, | ||
| {}, | ||
| ) | ||
| : await post<{ conversationId: string }>( | ||
| `/api/ai/page-agents/${encodeURIComponent(agentPageId)}/conversations`, | ||
| { sessionId: session.sessionId }, | ||
| ); | ||
| onChanged(); | ||
| selectConversation({ | ||
| sessionId: session.sessionId, | ||
| conversationId: created.conversationId, | ||
| agentId: agentPageId, | ||
| }); | ||
| } catch (error) { | ||
| console.error('Failed to start a conversation:', error); | ||
| toast.error('Could not start a conversation', { | ||
| description: error instanceof Error ? error.message : 'Please try again.', | ||
| }); | ||
| } | ||
| }, [onChanged, selectConversation, session.conversations, session.sessionId]); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Guard the non-idempotent conversation creation request.
The button remains enabled while post() is pending, so repeated clicks can create duplicate conversations and race which one becomes selected. Add a pending guard and disable the button, as done for session spawning and ending.
Proposed fix
const [expanded, setExpanded] = useState(false);
const [confirmingEnd, setConfirmingEnd] = useState(false);
const [ending, setEnding] = useState(false);
+ const [creatingConversation, setCreatingConversation] = useState(false);
const newConversation = useCallback(async () => {
+ if (creatingConversation) return;
+ setCreatingConversation(true);
const agentPageId = session.conversations[0]?.agentPageId ?? null;
try {
// ...
} catch (error) {
// ...
+ } finally {
+ setCreatingConversation(false);
}
- }, [onChanged, selectConversation, session.conversations, session.sessionId]);
+ }, [creatingConversation, onChanged, selectConversation, session.conversations, session.sessionId]);
<button
type="button"
+ disabled={creatingConversation}
aria-label="New conversation in this session"Also applies to: 389-396
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/layout/left-sidebar/AgentsSidebar.tsx` around lines
291 - 346, Add a dedicated pending state for new-conversation creation near the
existing expanded, confirmingEnd, and ending state in the session row, guard
newConversation against re-entry, and set/reset the state around both post
request branches using finally. Disable the new-conversation button while that
state is active, following the existing session spawning and ending patterns so
repeated clicks cannot create duplicates.
Comment on lines
+329
to
+359
| export async function listSessionConversationsBulk( | ||
| sessionIds: string[], | ||
| ): Promise<Map<string, SessionConversationEntry[]>> { | ||
| const grouped = new Map<string, SessionConversationEntry[]>(); | ||
| if (sessionIds.length === 0) return grouped; | ||
| const rows = await db | ||
| .select({ | ||
| sessionId: conversations.sessionId, | ||
| conversationId: conversations.id, | ||
| title: conversations.title, | ||
| type: conversations.type, | ||
| contextId: conversations.contextId, | ||
| lastMessageAt: conversations.lastMessageAt, | ||
| }) | ||
| .from(conversations) | ||
| .where(and(inArray(conversations.sessionId, sessionIds), eq(conversations.isActive, true))) | ||
| .orderBy(desc(conversations.lastMessageAt)); | ||
| for (const row of rows) { | ||
| if (row.sessionId === null) continue; | ||
| const bucket = grouped.get(row.sessionId) ?? []; | ||
| if (bucket.length >= 100) continue; | ||
| bucket.push({ | ||
| conversationId: row.conversationId, | ||
| title: row.title, | ||
| agentPageId: row.type === 'page' ? row.contextId : null, | ||
| lastMessageAt: row.lastMessageAt, | ||
| }); | ||
| grouped.set(row.sessionId, bucket); | ||
| } | ||
| return grouped; | ||
| } |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Enforce the per-session cap in SQL, not after fetching.
This query loads every active conversation for every selected session and discards rows only in JavaScript. A single large session can still make each sidebar poll unbounded. Use a partitioned row_number()/lateral query capped at 100 per session, with deterministic newest-first ordering.
This is the same bounded-listing risk previously raised under M3/M4; batching removed the N+1 pattern but did not bound the child query.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/agent-sessions/agent-sessions-runtime.ts` around lines 329 -
359, Update listSessionConversationsBulk so the database query limits results to
the newest 100 active conversations per session using a partitioned row_number()
or lateral-query strategy. Apply deterministic newest-first ordering, including
a stable tie-breaker, in SQL; remove the JavaScript bucket-length truncation
while preserving the returned grouping and entry mapping.
Comment on lines
+104
to
+121
| async list(filter) { | ||
| // Mirrors the real store: active rows only, newest activity first. | ||
| return [...rows.values()] | ||
| .filter((row) => { | ||
| if ('driveId' in filter && row.driveId !== filter.driveId) return false; | ||
| if (filter.ownerId !== undefined && row.ownerId !== filter.ownerId) return false; | ||
| return row.endedAt === null; | ||
| }) | ||
| .sort((a, b) => { | ||
| const aAt = a.lastActiveAt?.getTime() ?? -1; | ||
| const bAt = b.lastActiveAt?.getTime() ?? -1; | ||
| if (aAt !== bAt) return bAt - aAt; | ||
| return b.createdAt.getTime() - a.createdAt.getTime(); | ||
| }); | ||
| }, | ||
|
|
||
| async countActive(ownerId) { | ||
| return [...rows.values()].filter((row) => row.ownerId === ownerId && row.endedAt === null).length; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply SESSION_LIST_LIMIT in the fake store.
The production contract caps list() at 100, but this fake returns every matching row. Tests using it can miss pagination and spawn-limit regressions.
Proposed fix
- });
+ })
+ .slice(0, SESSION_LIST_LIMIT);Import SESSION_LIST_LIMIT from the store module.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async list(filter) { | |
| // Mirrors the real store: active rows only, newest activity first. | |
| return [...rows.values()] | |
| .filter((row) => { | |
| if ('driveId' in filter && row.driveId !== filter.driveId) return false; | |
| if (filter.ownerId !== undefined && row.ownerId !== filter.ownerId) return false; | |
| return row.endedAt === null; | |
| }) | |
| .sort((a, b) => { | |
| const aAt = a.lastActiveAt?.getTime() ?? -1; | |
| const bAt = b.lastActiveAt?.getTime() ?? -1; | |
| if (aAt !== bAt) return bAt - aAt; | |
| return b.createdAt.getTime() - a.createdAt.getTime(); | |
| }); | |
| }, | |
| async countActive(ownerId) { | |
| return [...rows.values()].filter((row) => row.ownerId === ownerId && row.endedAt === null).length; | |
| async list(filter) { | |
| // Mirrors the real store: active rows only, newest activity first. | |
| return [...rows.values()] | |
| .filter((row) => { | |
| if ('driveId' in filter && row.driveId !== filter.driveId) return false; | |
| if (filter.ownerId !== undefined && row.ownerId !== filter.ownerId) return false; | |
| return row.endedAt === null; | |
| }) | |
| .sort((a, b) => { | |
| const aAt = a.lastActiveAt?.getTime() ?? -1; | |
| const bAt = b.lastActiveAt?.getTime() ?? -1; | |
| if (aAt !== bAt) return bAt - aAt; | |
| return b.createdAt.getTime() - a.createdAt.getTime(); | |
| }) | |
| .slice(0, SESSION_LIST_LIMIT); | |
| }, | |
| async countActive(ownerId) { | |
| return [...rows.values()].filter((row) => row.ownerId === ownerId && row.endedAt === null).length; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/services/agent-sessions/__tests__/fakes.ts` around lines 104
- 121, Update the fake store’s `list` method to import and apply
`SESSION_LIST_LIMIT` from the store module after filtering and sorting,
returning no more than the production limit of matching rows while preserving
the existing ordering.
This was referenced Jul 30, 2026
Merged
2witstudios
added a commit
that referenced
this pull request
Jul 30, 2026
…ing, sidebar polish Fixes the follow-up audit findings from PR #2258 (issue #2264), plus a live P1 the point guard surfaced from stale PR #2253. - useResolvedConversation: gate first resolution on authLoading === false. On a hard refresh, useAuth starts out loading, so canUseSessions reads false before the role is known — resolving anyway would mint a fresh agent's first conversation as permanently session-less (thread→session binding is congenital). A startedForAgentId ref ensures this only waits out the FIRST load, so a later auth-loading blip (token refresh) never restarts resolution and clobbers a conversation the user is in. - Split spawn refusals into quota (429, worth interrupting the user for — they have the capability and simply ran out of allowance) vs capability (everything else — the existing silent degrade to a plain conversation). New spawn-refusal.ts pure module + ApiRequestError (auth-fetch.ts) to carry the status code through the throw. - quota-response.ts P1: `{ error: detail ?? SESSION_QUOTA_MESSAGE }` used `detail` for both a diagnostic enum (quota.reason, e.g. 'concurrency_limit', ALWAYS set on denial) and an occasional human override — so the enum always won and users saw 'concurrency_limit' rendered as their error message, with the human sentence unreachable. Split into {reasonCode, message}: reasonCode goes only into the audit row, the response body is always human-worded. Addresses the quota-delivery half of PR #2253. Verified the new UI's 429 consumers (AgentPanes.tsx handlePickShell, AgentsSidebar.tsx spawn/newConversation/ endSession) already catch and toast the rejection — no repeat of the deleted AgentView.tsx handleAddShell's silent-spinner bug. - usePermissionsCheck: reset isReadOnly at effect start so a page change doesn't inherit the previous page's read-only verdict when its own check errors. - AgentsSidebar: extracted session-groups.ts (groupSessionsByDrive) — the Assistant group now sorts first deterministically instead of depending on Map insertion order during the sessions fetch. Status dot gets role="img" so it's announced by screen readers (a bare aria-label span is not). Coordination note: useResolvedConversation now takes an authLoading param; AgentPageView.tsx (owned by the pane-system agent, issue #2263) got the smallest possible touch — destructuring isLoading from useAuth and passing it through — per the task's stated seam. Closes #2264 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WZ55BHXrze46tyZDA9eNhU
2witstudios
added a commit
that referenced
this pull request
Jul 30, 2026
…de sweep; retire machine vocabulary Follow-up from the post-merge audit of PR #2258 (realtime + cross-tier consistency slice). - Hoist resolveSessionTenantId (fail-closed on vanished drive) and resolveDriveMembership/canRunCode reduction into ONE new lib module (packages/lib/src/services/agent-sessions/agent-session-tenant.ts), imported by both apps/realtime and apps/web. Kills the web tier's fail-open `drive?.ownerId ?? session.ownerId` fallback. - Delete dead imports (checkMachineRuntimeGuardrail, recordMachineActivity, conversations schema) and the unused resolveDriveOwnerContext helper in apps/realtime/src/index.ts. - Fix five stale doc comments left by prior re-keying (index.ts tenantId docblock, shell-access.ts payer prose, XtermTerminal.tsx, and the useAgentSessionChat test header). - Delete the permanently-undefined TerminalSession.pageId field; attribute driveId on shell trackUsage calls instead (SandboxBillingDeps.trackUsage gains an optional driveId alongside the AI-tool-runner path's pageId). - Export composeSocketKey from shell-handler instead of re-spelling the socket-key format inline in index.ts. - Rename SandboxSurface's 'machine' value and quota.ts's guardrail identifiers to session vocabulary (checkSessionRuntimeGuardrail, recordSessionActivity, SESSION_ACTIVITY_GRACE_MS, etc.), touching both tiers' call sites and tests. Keeps the machine_sprite_reclaims table name. Closes #2265 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018DrdwiwAmX57oXSpiBcmAF
2witstudios
added a commit
that referenced
this pull request
Jul 30, 2026
…e error-mapping Post-merge audit follow-up from PR #2258 (issue #2261). 1. CAS-guard lifecycle stamps that could race a concurrent identity write: - endAgentSession's never-provisioned noop now CASes its `endedAt` stamp on `sandboxId` still being null, with a bounded retry that re-plans against the fresh row if a concurrent `ensure` won the race. - The resume/attach arm's activity-refresh stamp now CASes on `endedAt` still being null, so it can no longer erase a concurrent `end`'s teardown intent (the mirror-image race). Together these close the acceptance case: concurrent end+ensure can no longer land a row with endedAt set, a live sandboxId, and no teardownRequestedAt. 2. Make the spawn ceiling atomic and structural: a new `AgentSessionStore.createIfUnderLimit` serializes count-then-insert under a per-owner Postgres advisory lock (same primitive credit-gate.ts uses), closing the TOCTOU window a count-then-insert pre-check left open. `spawnAgentSession` now takes `maxActiveSessions` as a required dep, mirroring `checkConcurrency` in agent-session-sprite.ts, so a future caller cannot spawn without the ceiling applying. 3. `endAgentSession` now reports `spriteTornDown` honestly: when a concurrent ensure revived the session and the teardown CAS refused, the caller is told the sprite was not torn down rather than that the session ended. 4. Fixed the `end` planner: a NORMALLY-ended session (provisioned, then confirmed-killed — the common shape) now checked on `spriteTornDownAt` before `sandboxId`, since teardown never clears the pointer. Previously only the never-provisioned shape was idempotent; the common shape re-requested teardown and re-killed an already-dead Sprite on every re-end. Added the missing test for the common shape at both the planner and service level. 5. One consistent not-found/denied policy across the whole `/api/agent-sessions/[sessionId]/**` route family (new session-unavailable-response.ts): an unknown session id and one this requester may not touch now answer IDENTICALLY everywhere (the null-session 200 on GET, empty-list 200 on shells GET, a uniform 404 elsewhere) instead of the previous 404-vs-403 split that let a 403 confirm a session id was real. Denials are still audited internally; only the wire response stopped distinguishing. 6. Map AgentNotInSessionDriveError to 400 on both conversation-binding routes (agent-sessions/[sessionId]/conversations and ai/page-agents/[agentId]/conversations) — was falling through to a generic 502/500 as if it were a service failure. Extended packages/lib/src/services/agent-sessions/__tests__/fakes.ts to implement the same CAS semantics (applyStamps cas guards, createIfUnderLimit) the real store now has, and added a real-Postgres concurrency test proving the spawn ceiling holds under N concurrent callers. Closes #2261 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QUXe3ULG4vU3Etz7QumCBS
2witstudios
added a commit
that referenced
this pull request
Jul 30, 2026
…e error-mapping Post-merge audit follow-up from PR #2258 (issue #2261). 1. CAS-guard lifecycle stamps that could race a concurrent identity write: - endAgentSession's never-provisioned noop now CASes its `endedAt` stamp on `sandboxId` still being null, with a bounded retry that re-plans against the fresh row if a concurrent `ensure` won the race. - The resume/attach arm's activity-refresh stamp now CASes on `endedAt` still being null, so it can no longer erase a concurrent `end`'s teardown intent (the mirror-image race). Together these close the acceptance case: concurrent end+ensure can no longer land a row with endedAt set, a live sandboxId, and no teardownRequestedAt. 2. Make the spawn ceiling atomic and structural: a new `AgentSessionStore.createIfUnderLimit` serializes count-then-insert under a per-owner Postgres advisory lock (same primitive credit-gate.ts uses), closing the TOCTOU window a count-then-insert pre-check left open. `spawnAgentSession` now takes `maxActiveSessions` as a required dep, mirroring `checkConcurrency` in agent-session-sprite.ts, so a future caller cannot spawn without the ceiling applying. 3. `endAgentSession` now reports `spriteTornDown` honestly: when a concurrent ensure revived the session and the teardown CAS refused, the caller is told the sprite was not torn down rather than that the session ended. 4. Fixed the `end` planner: a NORMALLY-ended session (provisioned, then confirmed-killed — the common shape) now checked on `spriteTornDownAt` before `sandboxId`, since teardown never clears the pointer. Previously only the never-provisioned shape was idempotent; the common shape re-requested teardown and re-killed an already-dead Sprite on every re-end. Added the missing test for the common shape at both the planner and service level. 5. One consistent not-found/denied policy across the whole `/api/agent-sessions/[sessionId]/**` route family (new session-unavailable-response.ts): an unknown session id and one this requester may not touch now answer IDENTICALLY everywhere (the null-session 200 on GET, empty-list 200 on shells GET, a uniform 404 elsewhere) instead of the previous 404-vs-403 split that let a 403 confirm a session id was real. Denials are still audited internally; only the wire response stopped distinguishing. 6. Map AgentNotInSessionDriveError to 400 on both conversation-binding routes (agent-sessions/[sessionId]/conversations and ai/page-agents/[agentId]/conversations) — was falling through to a generic 502/500 as if it were a service failure. Extended packages/lib/src/services/agent-sessions/__tests__/fakes.ts to implement the same CAS semantics (applyStamps cas guards, createIfUnderLimit) the real store now has, and added a real-Postgres concurrency test proving the spawn ceiling holds under N concurrent callers. Closes #2261 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QUXe3ULG4vU3Etz7QumCBS
This was referenced Jul 30, 2026
Merged
5 of 7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase R0 of the corrective epic (
ta6prjapfdcrfjyg3wi7gsv8). A session is NOT a conversation.The first cut made them one object:
agent_sessions.conversationIdwas the PRIMARY KEY and the Sprite name was folded from it. That is a cardinality error — a session and a conversation have different lifecycles and a genuine one-to-many relationship — and it forced one environment per chat thread, making shared working contexts structurally impossible (it is why panes could not share a sandbox).The model
agent_sessions.id— its own cuid PK.conversationIdandagentPageIdare GONE: the association runs the other way (conversations.sessionIdFK, set at creation and permanent — moving a thread is a fork, never a rebind), and the agent belongs to each conversation, never to the session.sessionKey = HMAC(tenant, sessionId)folds the SESSION id, namespace bumped to v2 (v1 folded conversation cuids in the same keyspace — reuse could let a v2 session derive a v1 Sprite's name). Sharing is by construction: every conversation and shell in a session resolves the same row, so their tool calls land in one sandbox with no shared id threaded anywhere.conversations.sessionIdis ON DELETE SET NULL: ending a session releases compute, never erases history.no_sessiondenial from the tool layer — never a lazily-minted per-conversation environment.spawn_session) now work in their spawner's workspace: same session, same sandbox, same filesystem.kill_sessionaborts the run and no longer tears down a sandbox the worker never owned.pageId— a session is not page-anchored, and pretending otherwise was a semantic lie.The payoff test
Two conversations bound to one session resolve ONE sandboxId — proven at the store contract, against real Postgres (18 integration tests), and at the tool layer (
acquireSandbox).Migrations (unreleased ⇒ drop-and-recreate, no data migration)
0235custom clear (DELETE, not TRUNCATE — the AFTER-DELETE trigger rescues live Sprite pointers into the reclaim outbox) →0236generated DROP →0237generated CREATE (drizzle-kit's ALTER path emits broken SQL for PK swaps: ADD PRIMARY KEY beside the old PK, FKs before their UNIQUE) →0238custom trigger re-arm.Proven on real Postgres both ways: fresh DB end-to-end, and a head-state DB where the clear rescued exactly the live pointer (torn-down and never-provisioned correctly skipped) and the re-armed trigger rescued through both direct delete and owner cascade. Cascade tests inverted and pinned: a page delete leaves sessions alive; a conversation delete leaves the session alive.
The restored surfaces (R1–R3 — #2259, merged into this branch)
#2259 stacked on this PR and has been merged in, so this branch now carries the full corrective arc:
stores/agent-workspace/,components/agents/panes/): columns-of-panes reducer, split-right/split-down/close, pane picker offering an agent conversation in this session or a shell. Closing the last pane ends the session (store-levelclosePaneInsemantics — the reducer no-ops, the store ends the workspace).AgentsSurface+AgentPanes): selection lives in the query string (?session=&c=&agent=), the centre is the session's pane grid keyed by session id, and the grid receives the session's drive (fetched per session), not the surface's.canUseSessions; read-only viewers get history without send/edit/delete/retry affordances.sandboxEnabledon pages, migration0239): a Sandbox switch on agent settings; provisioning stays lazy on first tool use; sandbox/git/session tools hidden from Default Tools while off (canRunCoderemains the non-UI security boundary).Review-hardening pass (from the PR review cycle)
conversations.sessionId+titleride inside the creators' INSERTs; no UPDATE path exists, so rebinding is unrepresentable.createConversationreturns'created' | 'exists' | 'message_owner_conflict'with RETURNING-based race detection.createConversationInSessionWithenforcesagent.driveId === session.driveIdcentrally (fail-closed); global sessions host only assistant threads.findOwnWorkspace(conversationId)resolves once.SandboxAcquireResult.sessionIdis required on success — storage measurement and PTY activity are keyed by the acquired session, never the conversation.canRunCode.Validation
R4 (E2E coverage) remains tracked on the epic (
ta6prjapfdcrfjyg3wi7gsv8).🤖 Generated with Claude Code
https://claude.ai/code/session_017V3eqwRX5cFy3Tdu2Syoro
Summary by CodeRabbit