fix: preserve shared chat agents on fork

[joeyorlando]

Summary

• preserve the shared conversation's original agent when the recipient already has access, so org-shared chats no longer force an unnecessary agent selection prompt
• keep the agent picker fallback only when the shared agent is unavailable to the recipient, with copy that explains why
• validate shared-conversation forks on the backend so recipients cannot start the fork with an agent they do not have access to

Root Cause

The shared-chat UI treated every "Start New Chat from here" action as a generic fork flow and always opened the agent selector, even when the shared conversation's own agent was already accessible to the recipient. The fork endpoint also trusted the submitted agentId without re-checking agent access.

[claude]

Claude finished @joeyorlando's task in 2m 36s —— View job

---

PR Review: fix: preserve shared chat agent on fork

• Read repository guidelines
• Review changed files
• Analyze backend fork validation changes
• Analyze frontend fork flow changes
• Review added tests
• Post review feedback

Overall assessment

Nice, targeted fix. The shared-fork UX improvement (skipping the selector when the org-shared agent is already accessible) matches user expectations, and the backend now enforces agent access on the fork endpoint, which closes a real authorization gap. The added backend + frontend tests cover the most important cases. A few nits and one potentially worth-fixing issue below.

Backend — platform/backend/src/routes/chat/routes.ts

POST /api/chat/shared/:shareId/fork (lines 1305–1378):

• Good: The new check uses the exact same hasAnyAgentTypeAdminPermission + AgentModel.findById(agentId, userId, isAgentAdmin) pattern already used by POST /api/chat/conversations (routes.ts:926–936). Consistency is a plus.
• Good: The fix correctly prevents a malicious/stale client from submitting any agentId they like and starting a conversation with an agent they don't have access to.
• Minor: the 404 "Agent not found" error is the same phrasing used elsewhere, so it's consistent. It does conflate "no such agent" with "no access", but that's intentional to avoid leaking existence, and matches the existing convention. No change needed.
• Nit: Consider caching isAgentAdmin via a helper if this pattern continues to proliferate. Not for this PR.
• Nit: agentId: agent.id (line 1350) is equivalent to agentId post-validation since findById returned that same id. Either form is fine; the rename makes the origin explicit.

Backend tests — share-routes.test.ts

• Great coverage of both the happy path (same accessible agent) and the denial path (inaccessible team-scoped agent). The response 404 "Agent not found" assertion matches the route.
• Suggestion (optional): Add a test where the recipient intentionally picks a different agent they have access to, not the shared one. That exercises the case where the frontend fallback flow fires. It's essentially the same code path as the happy path test but documents the intent more clearly.
• Suggestion (optional): Add an assertion that selectedProvider is carried over from the shared conversation, to lock the behavior on line 1352 of routes.ts.

Frontend — platform/frontend/src/app/chat/page.tsx

resolveSharedConversationForkState + useEffect (lines 544–566):

• The hardening commit (1c0d0d7) correctly gates the sync on !isForkDialogOpen so the user's in-dialog selection isn't clobbered mid-edit. Good catch.
• One subtle behavior to verify: when the dialog closes (either via cancel or successful fork), the effect re-runs and resets forkAgentId to accessibleSharedAgentId. For the cancel flow that means a user's in-progress selection is wiped, which I think is desirable (next open should restart from the shared agent). Worth confirming with product, but looks intentional.
• Nit: sharedConversationAgentId currently resolves as conversation?.agentId ?? conversation?.agent?.id ?? null. This dual fallback suggests the shape is inconsistent somewhere upstream. If you can confirm which field is canonical on the shared-conversation payload, simplifying to one source would be cleaner. Non-blocking.

Fork button / dialog (lines 1786–1800, 2035–2072):

• Button now branches on shouldPromptForForkAgentSelection and directly invokes handleForkSharedConversation() in the happy path — this is the core UX win. Good.
• handleForkSharedConversation (line 1217) returns early if !effectiveForkAgentId. In the happy path effectiveForkAgentId === accessibleSharedAgentId, so it always fires. Good.
• Nit: Consider surfacing a toast or error on fork failure. Today forkSharedConversationMutation.mutateAsync will throw and the user sees nothing visible when triggered from the direct-click path (no dialog open to show a spinner/error). If this is handled globally by the mutation's onError, disregard.
• Nit: The dialog description branches on shouldPromptForForkAgentSelection. The "original agent is not available to you" copy is good and clear.

Frontend — shared-conversation-fork.ts + tests

• Small, pure, well-tested helper. I like the split from page.tsx.
• Tests cover: shared agent accessible, shared agent inaccessible (now yielding effectiveAgentId: null after the hardening commit), and explicit user selection override. Solid.
• Gap (minor): No test for sharedConversationAgentId: null (e.g., legacy conversation with no agent). The current implementation returns accessibleSharedAgentId: null, shouldPromptForForkAgentSelection: true, effectiveAgentId: null, which is correct — a quick test would lock that behavior.
• Gap (minor): No test for selectedAgentId referencing an agent that isn't in availableAgentIds. Today the helper returns whatever selectedAgentId is, even if it's stale/inaccessible. That's probably fine because the backend re-validates, but worth noting for clarity.

Security review

• The backend authorization gap (trusting the submitted agentId in the fork endpoint) was the main concern and is now fixed correctly. [routes.ts:1337–1345]
• Behavior now matches POST /api/chat/conversations, which is a good invariant: any endpoint that creates a conversation with a given agentId must validate agent access first.
• No new injection, XSS, or data-exposure concerns introduced.

Summary

Ship-ready IMO. The main correctness and security issue (server-side agent access validation on fork) is addressed, and the UX improvement is well-scoped. Optional follow-ups:

1. Backend test asserting selectedProvider carry-over.
2. Frontend test for sharedConversationAgentId: null.
3. Confirm the fork failure path surfaces an error to the user when triggered from the direct-click flow (not via dialog).
   • Branch: codex/fix-shared-chat-agent-prompt