Skip to content

feat: improve session sharing transparency and multi-user UX#972

Merged
Gkrumbach07 merged 15 commits intomainfrom
feat/session-sharing-audit
Mar 20, 2026
Merged

feat: improve session sharing transparency and multi-user UX#972
Gkrumbach07 merged 15 commits intomainfrom
feat/session-sharing-audit

Conversation

@Gkrumbach07
Copy link
Copy Markdown
Contributor

Summary

Comprehensive session sharing improvements for multi-user workspaces:

  • Display session creator in UI (sessions table + hover card)
  • Inject sender attribution into messages for multi-user clarity
  • Document RBAC and API key behavior in audit report

Resolves: Session sharing audit findings #1 and #3

Changes

1. Session Creator Display (Finding #1)

  • Add userContext field to AgenticSessionSpec types
  • Add "Creator" column to sessions table (visible at xl+ breakpoint)
  • Show creator in session hover card with User icon
  • Fallback display: displayName || userId || "—"

Before: No way to see who created a session
After: Creator shown in table and hover card

2. Message Sender Attribution (Finding #3)

  • Backend injects senderId and senderDisplayName into user message metadata
  • Frontend extracts and displays sender above user messages
  • Format: "SenderName • timestamp" above message bubble

Before: Multi-user sessions showed confusing "ghost messages" with no sender
After: Clear attribution for who sent each message

3. Audit Report (Documentation)

  • Comprehensive 4-finding audit in SESSION_SHARING_AUDIT.md
  • Documents RBAC enforcement (workspace-scoped, not session-scoped)
  • Documents API key sharing behavior (workspace-level secrets)
  • Recommendations for future enhancements (per-session ownership, user-scoped keys)

Testing

Frontend types: TypeScript compilation successful
Backend logic: Sender metadata injected before forwarding to runner
UI rendering: Sender attribution displayed for user messages

Related Issues

See SESSION_SHARING_AUDIT.md for:

Screenshots

Session table with creator column

(Creator column visible at xl+ breakpoint, shows displayName/userId)

Message with sender attribution

(Shows "SenderName • timestamp" above user message bubble)


🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 19, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a session-sharing audit doc and UI permissions plan; backend now injects per-message sender metadata and emits an early messages snapshot SSE event; frontend types, message rendering, session UI, chat input, explorer panels, and tests were updated to surface per-message sender attribution and project-role-based UI gating; RBAC secret verbs were adjusted.

Changes

Cohort / File(s) Summary
Audit & Plans
SESSION_SHARING_AUDIT.md, UI_PERMISSIONS_AUDIT.md, .claude/CLAUDE.md
New documentation files: session sharing audit, UI permissions plan, and an orchestrator status/workflow doc.
Backend proxy & SSE
components/backend/websocket/agui_proxy.go
Extracts userID/userName from request context, injects metadata.senderId/senderDisplayName into user messages, replaces outgoing batch with modified messages, and adds emitUserMessagesSnapshot(...) to persist/publish an early messages snapshot SSE event.
Frontend types
components/frontend/src/types/agentic-session.ts, components/frontend/src/types/agui.ts, components/frontend/src/types/api/sessions.ts
Adds UserContext and MessageMetadata types; extends AgenticSessionSpec with userContext; constrains PlatformMessage, UserMessage, and AgentMessage metadata to include optional sender attribution fields.
Message UI & streaming
components/frontend/src/components/ui/message.tsx, components/frontend/src/components/ui/stream-message.tsx, components/frontend/src/components/session/MessagesTab.tsx
Message gains senderAttribution, senderId, senderDisplayName, currentUserId props and updates avatar/header logic; StreamMessage forwards attribution for non-agent messages and accepts currentUserId; MessagesTab obtains current user and passes currentUserId to stream messages.
Chat input & project access hook
components/frontend/src/components/chat/ChatInputBox.tsx, components/frontend/src/services/queries/use-project-access.ts
Adds useProjectAccess(projectName) hook and integrates it into ChatInputBox; disables interaction and hides toolbar for view role; projectName prop added.
Sessions table, header & actions
components/frontend/src/components/workspace-sections/sessions-section.tsx, components/frontend/src/app/projects/[name]/sessions/[sessionName]/session-header.tsx
Sessions table shows Creator column from session.spec.userContext; row hover shows creator; session actions and header menu items are gated by project userRole (view/edit/admin) with conditional action rendering.
Explorer & file/context tabs
components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/explorer/explorer-panel.tsx, .../files-tab.tsx, .../context-tab.tsx
Explorer props now include projectName; useProjectAccess drives canModify which is passed to FilesTab and ContextTab; mutation UI (add/upload/remove) is hidden when canModify is false.
Page wiring & props
components/frontend/src/app/projects/[name]/sessions/[sessionName]/page.tsx, components/frontend/src/components/session/MessagesTab.tsx
Passes projectName down to MessagesTab and ExplorerPanel; MessagesTab signature updated.
Tests updated / mocks added
.../session-header.test.tsx, .../explorer-panel.test.tsx, .../context-tab.test.tsx, components/frontend/src/components/chat/__tests__/ChatInputBox.test.tsx
Added/mocked useProjectAccess returns and updated test props (e.g., canModify) to reflect role-based behavior; minor test updates.
Kubernetes RBAC manifest
components/manifests/base/rbac/ambient-project-edit-clusterrole.yaml
ClusterRole ambient-project-edit secrets verbs changed from ["create"] to ["get","create"] to allow secret reads for runner token validation.
Other
many frontend TSX/typing updates (various paths)
Multiple exported types and component prop signatures were added/extended to carry user/project-context and sender attribution across the frontend.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client User
    participant Backend as Backend (agui_proxy)
    participant Runner as Runner
    participant Frontend as Frontend (UI)

    Client->>Backend: Send chat payload (input.Messages)
    Backend->>Backend: Read userID / userName from context
    Backend->>Backend: For each message: if role == user, unmarshal -> inject metadata.senderId & senderDisplayName -> remarshal
    Backend->>Backend: emitUserMessagesSnapshot(session, runID, threadID, minimalMsgs) (persist + SSE)
    Backend->>Runner: Forward enriched message batch
    Runner->>Frontend: Emit messages (may include sender metadata)
    Frontend->>Frontend: StreamMessage reads metadata.senderDisplayName / senderId
    Frontend->>Frontend: Message renders sender attribution and timestamp
    Frontend->>Client: Display message with sender/creator info
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: improve session sharing transparency and multi-user UX' directly and clearly summarizes the PR's primary objective of enhancing session visibility and user experience in multi-user scenarios.
Description check ✅ Passed The description comprehensively details the changes across session creator display, message sender attribution, and audit documentation, with clear before/after context and testing information.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/session-sharing-audit
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/frontend/src/components/ui/stream-message.tsx`:
- Around line 90-92: Combine display name and id into a single fallback
attribution string instead of leaving senderId unused: replace the separate
senderDisplayName and senderId usages with a single derived value like const
senderAttribution = m.metadata?.senderDisplayName ?? m.metadata?.senderId and
use senderAttribution wherever the message attribution is rendered (update the
occurrences around the current variables and the other places noted in the file,
e.g., the other blocks at the later occurrences that reference
senderDisplayName/senderId).

In `@components/frontend/src/components/workspace-sections/sessions-section.tsx`:
- Around line 327-332: The creator row currently is omitted when
session.spec.userContext is falsy and shows an empty value when displayName and
userId are empty; update the JSX in sessions-section.tsx so the creator
hover-row always renders (do not gate the entire div on
session.spec.userContext) and display session.spec.userContext?.displayName ||
session.spec.userContext?.userId || "—" (trim or check for empty strings if
needed) while keeping the User icon and existing styling; this ensures the row
is visible with the proper fallback even when userContext is missing or fields
are blank.

In `@components/frontend/src/types/agentic-session.ts`:
- Around line 164-178: Replace the separate inline metadata shapes in
UserMessage and AgentMessage with the shared MessageMetadata type from agui.ts
to ensure consistent typing; import MessageMetadata into
components/frontend/src/types/agentic-session.ts and change both metadata?: {
... } declarations to metadata?: MessageMetadata on the UserMessage and
AgentMessage types (preserving the optional flag), so StreamMessage can safely
read metadata?.senderDisplayName across the union.

In `@SESSION_SHARING_AUDIT.md`:
- Around line 11-18: The report still lists "Creator not visible in UI" and "No
message attribution" (and the "No per-session ownership controls" finding) as
open issues even though this PR adds creator visibility and message sender
attribution; update the document (the "Critical Findings" list, the summary
table, and all other sections that reference these findings) to mark those items
as remediated or partially remediated (or explicitly label them as pre-fix
findings shipped for historical context), and ensure the wording and status for
"Creator not visible in UI", "No message attribution", and "No per-session
ownership controls" reflect the new behavior introduced by this PR.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6c5dc091-c5f6-4242-8b23-23de3bce1b2e

📥 Commits

Reviewing files that changed from the base of the PR and between f8ac621 and ea24d3d.

📒 Files selected for processing (8)
  • SESSION_SHARING_AUDIT.md
  • components/backend/websocket/agui_proxy.go
  • components/frontend/src/components/ui/message.tsx
  • components/frontend/src/components/ui/stream-message.tsx
  • components/frontend/src/components/workspace-sections/sessions-section.tsx
  • components/frontend/src/types/agentic-session.ts
  • components/frontend/src/types/agui.ts
  • components/frontend/src/types/api/sessions.ts

Comment on lines +90 to +92
// Extract sender attribution from metadata (for multi-user sessions)
const senderDisplayName = m.metadata?.senderDisplayName;
const senderId = m.metadata?.senderId;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n components/frontend/src/components/ui/stream-message.tsx | head -150

Repository: ambient-code/platform

Length of output: 7023


🏁 Script executed:

grep -n "senderId" components/frontend/src/components/ui/stream-message.tsx

Repository: ambient-code/platform

Length of output: 112


🏁 Script executed:

cat -n components/frontend/src/components/ui/stream-message.tsx | tail -100

Repository: ambient-code/platform

Length of output: 4418


🏁 Script executed:

cat -n components/frontend/src/types/agentic-session.ts | grep -A 10 -B 10 "senderDisplayName\|senderId"

Repository: ambient-code/platform

Length of output: 701


🏁 Script executed:

cat -n components/backend/websocket/agui_proxy.go | grep -A 5 -B 5 "senderId\|senderDisplayName"

Repository: ambient-code/platform

Length of output: 1042


🏁 Script executed:

# Verify the exact changes needed - check if senderDisplayName/senderId are used anywhere else
rg "senderDisplayName|senderId" components/frontend/src/components/ui/stream-message.tsx

Repository: ambient-code/platform

Length of output: 514


Derive one sender attribution string and fall back to senderId.

The code extracts senderId from metadata but never uses it, leaving it unused and breaking lint. More importantly, users without a display name don't receive any attribution even though senderId is available from the backend. Combine both values with a fallback: m.metadata?.senderDisplayName ?? m.metadata?.senderId.

Suggested fix
-      const senderDisplayName = m.metadata?.senderDisplayName;
-      const senderId = m.metadata?.senderId;
+      const senderAttribution = m.metadata?.senderDisplayName ?? m.metadata?.senderId;-            name={isAgent ? (agentName ?? "AI Agent") : senderDisplayName}
+            name={isAgent ? (agentName ?? "AI Agent") : senderAttribution}-            senderAttribution={!isAgent && senderDisplayName ? senderDisplayName : undefined}
+            senderAttribution={!isAgent ? senderAttribution : undefined}-              name={isAgent ? (agentName ?? "AI Agent") : senderDisplayName}
+              name={isAgent ? (agentName ?? "AI Agent") : senderAttribution}-              senderAttribution={!isAgent && senderDisplayName ? senderDisplayName : undefined}
+              senderAttribution={!isAgent ? senderAttribution : undefined}

Also applies to: 116-121, 133-138

🧰 Tools
🪛 GitHub Actions: Lint

[error] 92-92: ESLint (@typescript-eslint/no-unused-vars): 'senderId' is assigned a value but never used.


[error] Command failed: "npm run lint" (exit code != 0).

🪛 GitHub Check: Frontend Lint and Type Check

[failure] 92-92:
'senderId' is assigned a value but never used

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/stream-message.tsx` around lines 90 -
92, Combine display name and id into a single fallback attribution string
instead of leaving senderId unused: replace the separate senderDisplayName and
senderId usages with a single derived value like const senderAttribution =
m.metadata?.senderDisplayName ?? m.metadata?.senderId and use senderAttribution
wherever the message attribution is rendered (update the occurrences around the
current variables and the other places noted in the file, e.g., the other blocks
at the later occurrences that reference senderDisplayName/senderId).

Comment on lines +327 to +332
{session.spec.userContext && (
<div className="flex items-center gap-1.5 text-xs text-muted-foreground">
<User className="h-3 w-3" />
<span>{session.spec.userContext.displayName || session.spec.userContext.userId}</span>
</div>
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Keep the creator row visible in the hover card.

The table cell falls back to , but this block drops the row entirely when userContext is absent and shows an empty value when both fields are blank. That makes the hover card inconsistent with the same feature and with the documented displayName || userId || "—" fallback.

Suggested fix
-                                  {session.spec.userContext && (
-                                    <div className="flex items-center gap-1.5 text-xs text-muted-foreground">
-                                      <User className="h-3 w-3" />
-                                      <span>{session.spec.userContext.displayName || session.spec.userContext.userId}</span>
-                                    </div>
-                                  )}
+                                  <div className="flex items-center gap-1.5 text-xs text-muted-foreground">
+                                    <User className="h-3 w-3" />
+                                    <span>{session.spec.userContext?.displayName || session.spec.userContext?.userId || '—'}</span>
+                                  </div>
📝 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
{session.spec.userContext && (
<div className="flex items-center gap-1.5 text-xs text-muted-foreground">
<User className="h-3 w-3" />
<span>{session.spec.userContext.displayName || session.spec.userContext.userId}</span>
</div>
)}
<div className="flex items-center gap-1.5 text-xs text-muted-foreground">
<User className="h-3 w-3" />
<span>{session.spec.userContext?.displayName || session.spec.userContext?.userId || '—'}</span>
</div>
🧰 Tools
🪛 GitHub Actions: Lint

[error] Command failed: "npm run lint" (exit code != 0).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/workspace-sections/sessions-section.tsx`
around lines 327 - 332, The creator row currently is omitted when
session.spec.userContext is falsy and shows an empty value when displayName and
userId are empty; update the JSX in sessions-section.tsx so the creator
hover-row always renders (do not gate the entire div on
session.spec.userContext) and display session.spec.userContext?.displayName ||
session.spec.userContext?.userId || "—" (trim or check for empty strings if
needed) while keeping the User icon and existing styling; this ensures the row
is visible with the proper fallback even when userContext is missing or fields
are blank.

Comment on lines +164 to +178
metadata?: {
senderId?: string;
senderDisplayName?: string;
[key: string]: unknown;
};
}
export type AgentMessage = {
type: "agent_message";
id?: string; // Message ID for feedback association
content: ContentBlock;
model: string;
timestamp: string;
metadata?: {
[key: string]: unknown;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n components/frontend/src/types/agentic-session.ts | sed -n '160,185p'

Repository: ambient-code/platform

Length of output: 813


🏁 Script executed:

cat -n components/frontend/src/types/agui.ts | grep -A 10 "MessageMetadata"

Repository: ambient-code/platform

Length of output: 875


🏁 Script executed:

cat -n components/frontend/src/components/ui/stream-message.tsx | grep -B 5 -A 5 "senderDisplayName"

Repository: ambient-code/platform

Length of output: 2083


🏁 Script executed:

fd "agentic-session.ts" components/frontend/src/types/

Repository: ambient-code/platform

Length of output: 113


Reuse one MessageMetadata type across both message variants to maintain type consistency.

StreamMessage reads m.metadata?.senderDisplayName from both UserMessage and AgentMessage. Currently, UserMessage.metadata explicitly defines senderId and senderDisplayName fields, while AgentMessage.metadata has only an index signature. In the UserMessage | AgentMessage union, this shape mismatch creates type-safety issues. A shared MessageMetadata type already exists in agui.ts with the exact fields needed by both variants.

Suggested fix
+import type { MessageMetadata } from "@/types/agui";
…
 export type UserMessage = {
 	type: "user_message";
 	id?: string;  // Message ID for feedback association
 	content: ContentBlock | string;
 	timestamp: string;
-	metadata?: {
-		senderId?: string;
-		senderDisplayName?: string;
-		[key: string]: unknown;
-	};
+	metadata?: MessageMetadata;
 }
 export type AgentMessage = {
 	type: "agent_message";
 	id?: string;  // Message ID for feedback association
 	content: ContentBlock;
 	model: string;
 	timestamp: string;
-	metadata?: {
-		[key: string]: unknown;
-	};
+	metadata?: MessageMetadata;
 }
🧰 Tools
🪛 GitHub Actions: Lint

[error] Command failed: "npm run lint" (exit code != 0).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/types/agentic-session.ts` around lines 164 - 178,
Replace the separate inline metadata shapes in UserMessage and AgentMessage with
the shared MessageMetadata type from agui.ts to ensure consistent typing; import
MessageMetadata into components/frontend/src/types/agentic-session.ts and change
both metadata?: { ... } declarations to metadata?: MessageMetadata on the
UserMessage and AgentMessage types (preserving the optional flag), so
StreamMessage can safely read metadata?.senderDisplayName across the union.

@Gkrumbach07 Gkrumbach07 force-pushed the feat/session-sharing-audit branch from ea24d3d to 3a4347c Compare March 19, 2026 16:56
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
components/frontend/src/types/api/sessions.ts (1)

6-10: 🧹 Nitpick | 🔵 Trivial

Duplicate UserContext type definition.

UserContext is defined identically here and in components/frontend/src/types/agentic-session.ts (lines 38-43). Consider consolidating into a single source and re-exporting to avoid drift.

Also applies to: 55-55

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/types/api/sessions.ts` around lines 6 - 10, Duplicate
UserContext type definitions exist (UserContext) across modules; remove the
duplicate declaration in this file and import/re-export the canonical type
instead: create or use the single authoritative export of UserContext (the one
in agentic-session.ts) and replace the local type definition with an import of
UserContext and re-export it if needed, updating any local references to use the
imported symbol; also remove the duplicate occurrence noted at the other
location (line 55) so all code consumes the single shared UserContext type.
♻️ Duplicate comments (4)
components/frontend/src/types/agentic-session.ts (1)

164-178: 🛠️ Refactor suggestion | 🟠 Major

Reuse MessageMetadata from agui.ts for type consistency.

UserMessage.metadata and AgentMessage.metadata define inline types that duplicate the MessageMetadata type in agui.ts. Since stream-message.tsx reads metadata?.senderDisplayName from both message types in a union, using the shared type ensures consistent typing.

Suggested fix
+import type { MessageMetadata } from "@/types/agui";
…
 export type UserMessage = {
 	type: "user_message";
 	id?: string;
 	content: ContentBlock | string;
 	timestamp: string;
-	metadata?: {
-		senderId?: string;
-		senderDisplayName?: string;
-		[key: string]: unknown;
-	};
+	metadata?: MessageMetadata;
 }
 export type AgentMessage = {
 	type: "agent_message";
 	id?: string;
 	content: ContentBlock;
 	model: string;
 	timestamp: string;
-	metadata?: {
-		[key: string]: unknown;
-	};
+	metadata?: MessageMetadata;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/types/agentic-session.ts` around lines 164 - 178,
UserMessage and AgentMessage define inline metadata types that duplicate the
existing MessageMetadata in agui.ts; replace those inline metadata definitions
with the shared MessageMetadata type so both UserMessage and AgentMessage use
MessageMetadata (import it from agui.ts), ensuring stream-message.tsx can safely
read metadata?.senderDisplayName from the union; update the metadata?: { ... }
declarations in the UserMessage and AgentMessage types to metadata?:
MessageMetadata and add the necessary import of MessageMetadata where the
agentic-session.ts types are declared.
components/frontend/src/components/ui/stream-message.tsx (1)

90-92: ⚠️ Potential issue | 🟠 Major

senderId is extracted but never used; attribution should fall back to it.

senderId is assigned at line 92 but never referenced, which will cause a lint error. More importantly, users without a display name won't receive any attribution even though senderId is available. Combine both with a fallback.

Suggested fix
-      const senderDisplayName = m.metadata?.senderDisplayName as string | undefined;
-      const senderId = m.metadata?.senderId as string | undefined;
+      const senderAttribution = (m.metadata?.senderDisplayName ?? m.metadata?.senderId) as string | undefined;-            name={isAgent ? (agentName ?? "AI Agent") : senderDisplayName}
+            name={isAgent ? (agentName ?? "AI Agent") : senderAttribution}-            senderAttribution={!isAgent && senderDisplayName ? senderDisplayName : undefined}
+            senderAttribution={!isAgent ? senderAttribution : undefined}

Apply similar changes to lines 133 and 138.

Also applies to: 116-122, 133-139

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/stream-message.tsx` around lines 90 -
92, The code extracts senderDisplayName and senderId but never uses senderId,
causing a lint error and missing attribution fallback; update the attribution
logic in stream-message.tsx to use senderDisplayName with a fallback to senderId
(e.g., use senderDisplayName ?? senderId or senderDisplayName || senderId)
wherever senderDisplayName is rendered or used (including the other occurrences
that mirror this pattern), or remove the unused senderId variable if you instead
inline m.metadata?.senderId; ensure the variables are typed as string |
undefined and replace instances at the other similar locations so all message
attribution falls back to senderId when display name is absent.
components/frontend/src/components/workspace-sections/sessions-section.tsx (1)

315-320: ⚠️ Potential issue | 🟡 Minor

Hover card creator row is inconsistent with table cell fallback.

The table cell at lines 357-361 correctly falls back to '—' when userContext is missing, but the hover card conditionally omits the entire creator row when userContext is absent. This creates UI inconsistency.

Suggested fix
-                                  {session.spec.userContext && (
-                                    <div className="flex items-center gap-1.5 text-xs text-muted-foreground">
-                                      <User className="h-3 w-3" />
-                                      <span>{session.spec.userContext.displayName || session.spec.userContext.userId}</span>
-                                    </div>
-                                  )}
+                                  <div className="flex items-center gap-1.5 text-xs text-muted-foreground">
+                                    <User className="h-3 w-3" />
+                                    <span>{session.spec.userContext?.displayName || session.spec.userContext?.userId || '—'}</span>
+                                  </div>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/workspace-sections/sessions-section.tsx`
around lines 315 - 320, The hover card's creator row currently hides entirely
when session.spec.userContext is falsy, causing UI inconsistency with the table
cell; change the conditional block that renders the creator row (the JSX using
session.spec.userContext, <User />, and the <span> showing displayName/userId)
to always render the row but display a fallback '—' when
session.spec.userContext is missing (i.e., use
session.spec.userContext?.displayName || session.spec.userContext?.userId ||
'—') so the hover card matches the table cell behavior.
SESSION_SHARING_AUDIT.md (1)

13-18: ⚠️ Potential issue | 🟠 Major

Update audit report to reflect fixes shipping in this PR.

Findings #1 (creator visibility) and #3 (message attribution) are described as current missing behavior, but this PR implements both features. The summary table at lines 277-282 marks them as "❌ Missing feature" — this will be incorrect once merged. Update these sections to reflect the remediated status or clearly label them as "pre-fix findings."

Suggested changes to summary table
-| 1 | Creator not shown in UI | Medium | ❌ Missing feature |
+| 1 | Creator not shown in UI | Medium | ✅ Fixed in this PR |
 | 2 | No per-session RBAC | Low-Medium | ⚠️ Design decision needed |
-| 3 | No message attribution | Medium | ❌ Confusing UX in multi-user scenarios |
+| 3 | No message attribution | Medium | ✅ Fixed in this PR |
 | 4 | Shared API keys | Low | ✅ Working as designed (document) |

Also applies to: 277-288

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@SESSION_SHARING_AUDIT.md` around lines 13 - 18, Update the audit report's
summary table and any duplicate entries to reflect that creator visibility and
message attribution are implemented in this PR: change the status for the
"Creator not visible" and "No message attribution" findings from "❌ Missing
feature" to "✅ Implemented in this PR" (or annotate them as "Remediated in this
PR / pre-fix findings" if you prefer history), and update the descriptive text
in the summary table and the detailed sections that reference those findings so
they accurately describe the new behavior; search for the "summary table" and
the findings titled "Creator not visible" and "No message attribution" to locate
the exact blocks to edit (also update the duplicate entries noted in the
report).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@SESSION_SHARING_AUDIT.md`:
- Around line 294-308: Add blank lines above and below the Markdown headings
"CRD & Backend", "Operator", and "Frontend" in SESSION_SHARING_AUDIT.md so each
heading is separated from surrounding content per Markdown conventions; update
the sections listing files under these headings (e.g.,
components/manifests/base/crds/agenticsessions-crd.yaml,
components/operator/internal/handlers/sessions.go,
components/frontend/src/types/agentic-session.ts) to be preceded and followed by
an empty line.

---

Outside diff comments:
In `@components/frontend/src/types/api/sessions.ts`:
- Around line 6-10: Duplicate UserContext type definitions exist (UserContext)
across modules; remove the duplicate declaration in this file and
import/re-export the canonical type instead: create or use the single
authoritative export of UserContext (the one in agentic-session.ts) and replace
the local type definition with an import of UserContext and re-export it if
needed, updating any local references to use the imported symbol; also remove
the duplicate occurrence noted at the other location (line 55) so all code
consumes the single shared UserContext type.

---

Duplicate comments:
In `@components/frontend/src/components/ui/stream-message.tsx`:
- Around line 90-92: The code extracts senderDisplayName and senderId but never
uses senderId, causing a lint error and missing attribution fallback; update the
attribution logic in stream-message.tsx to use senderDisplayName with a fallback
to senderId (e.g., use senderDisplayName ?? senderId or senderDisplayName ||
senderId) wherever senderDisplayName is rendered or used (including the other
occurrences that mirror this pattern), or remove the unused senderId variable if
you instead inline m.metadata?.senderId; ensure the variables are typed as
string | undefined and replace instances at the other similar locations so all
message attribution falls back to senderId when display name is absent.

In `@components/frontend/src/components/workspace-sections/sessions-section.tsx`:
- Around line 315-320: The hover card's creator row currently hides entirely
when session.spec.userContext is falsy, causing UI inconsistency with the table
cell; change the conditional block that renders the creator row (the JSX using
session.spec.userContext, <User />, and the <span> showing displayName/userId)
to always render the row but display a fallback '—' when
session.spec.userContext is missing (i.e., use
session.spec.userContext?.displayName || session.spec.userContext?.userId ||
'—') so the hover card matches the table cell behavior.

In `@components/frontend/src/types/agentic-session.ts`:
- Around line 164-178: UserMessage and AgentMessage define inline metadata types
that duplicate the existing MessageMetadata in agui.ts; replace those inline
metadata definitions with the shared MessageMetadata type so both UserMessage
and AgentMessage use MessageMetadata (import it from agui.ts), ensuring
stream-message.tsx can safely read metadata?.senderDisplayName from the union;
update the metadata?: { ... } declarations in the UserMessage and AgentMessage
types to metadata?: MessageMetadata and add the necessary import of
MessageMetadata where the agentic-session.ts types are declared.

In `@SESSION_SHARING_AUDIT.md`:
- Around line 13-18: Update the audit report's summary table and any duplicate
entries to reflect that creator visibility and message attribution are
implemented in this PR: change the status for the "Creator not visible" and "No
message attribution" findings from "❌ Missing feature" to "✅ Implemented in this
PR" (or annotate them as "Remediated in this PR / pre-fix findings" if you
prefer history), and update the descriptive text in the summary table and the
detailed sections that reference those findings so they accurately describe the
new behavior; search for the "summary table" and the findings titled "Creator
not visible" and "No message attribution" to locate the exact blocks to edit
(also update the duplicate entries noted in the report).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 13a46601-e871-40c6-91f2-f75d03f0fd5d

📥 Commits

Reviewing files that changed from the base of the PR and between ea24d3d and 3a4347c.

📒 Files selected for processing (8)
  • SESSION_SHARING_AUDIT.md
  • components/backend/websocket/agui_proxy.go
  • components/frontend/src/components/ui/message.tsx
  • components/frontend/src/components/ui/stream-message.tsx
  • components/frontend/src/components/workspace-sections/sessions-section.tsx
  • components/frontend/src/types/agentic-session.ts
  • components/frontend/src/types/agui.ts
  • components/frontend/src/types/api/sessions.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/backend/websocket/agui_proxy.go`:
- Around line 301-305: The re-marshal of the modified message uses
json.Marshal(msg) and silently ignores errors, leaving raw unchanged and
dropping sender metadata; change the block around json.Marshal(msg) so that if
err != nil you log the error with context (including msg or sender id) and
either return/propagate the error or explicitly keep the original raw while
recording the failure; specifically update the code around json.Marshal(msg)
(variables: msg, raw) to handle err != nil by calling the module's logger (e.g.,
proxyLogger or log.Printf) with the error and a descriptive message and avoid
silently swallowing the failure so multi-user attribution issues can be
debugged.

In `@components/frontend/src/components/ui/message.tsx`:
- Around line 264-269: The avatar logic in the Message component currently sets
avatarText via isBot ? "AI" : (isCurrentUser ? "U" :
getInitials(senderDisplayName)), which causes other users with undefined
senderDisplayName to render as "U" (same as current user); update the logic so
when senderId !== currentUserId and senderDisplayName is falsy you derive a
distinct fallback (e.g., initials from senderId like senderId.slice(0,2) or a
placeholder like "?") before calling getInitials, and ensure avatarText uses
isBot, isCurrentUser, then this new fallback path; reference
variables/functions: avatarText, isCurrentUser, getInitials, senderDisplayName,
senderId, currentUserId, isBot.

In `@components/frontend/src/components/ui/stream-message.tsx`:
- Around line 91-94: Replace the current logic that only uses senderDisplayName
for attribution by falling back to senderId when display name is missing: derive
senderAttribution = senderDisplayName ?? senderId (or equivalent) and pass
senderAttribution into the Message props instead of senderDisplayName; apply the
same change in the other occurrences referenced (the blocks around the sections
currently extracting senderDisplayName/senderId at the later occurrences) so
attribution shows either the display name or the senderId consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 843d3473-b73e-45b7-b3db-4c5b6c1a3169

📥 Commits

Reviewing files that changed from the base of the PR and between 3a4347c and 68ff0e7.

📒 Files selected for processing (4)
  • components/backend/websocket/agui_proxy.go
  • components/frontend/src/components/session/MessagesTab.tsx
  • components/frontend/src/components/ui/message.tsx
  • components/frontend/src/components/ui/stream-message.tsx

Comment on lines +301 to +305
// Re-serialize the modified message
if modifiedRaw, err := json.Marshal(msg); err == nil {
raw = modifiedRaw
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Silent failure on re-marshal could hide injection issues.

If json.Marshal(msg) fails (line 302), the original unmodified raw is used without logging. This silently drops sender metadata, which could make debugging multi-user attribution issues difficult.

Suggested improvement
 				// Re-serialize the modified message
 				if modifiedRaw, err := json.Marshal(msg); err == nil {
 					raw = modifiedRaw
+				} else {
+					log.Printf("AGUI Proxy: failed to re-marshal message with sender metadata: %v", err)
 				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/websocket/agui_proxy.go` around lines 301 - 305, The
re-marshal of the modified message uses json.Marshal(msg) and silently ignores
errors, leaving raw unchanged and dropping sender metadata; change the block
around json.Marshal(msg) so that if err != nil you log the error with context
(including msg or sender id) and either return/propagate the error or explicitly
keep the original raw while recording the failure; specifically update the code
around json.Marshal(msg) (variables: msg, raw) to handle err != nil by calling
the module's logger (e.g., proxyLogger or log.Printf) with the error and a
descriptive message and avoid silently swallowing the failure so multi-user
attribution issues can be debugged.

Comment on lines +264 to +269

// Avatar logic for user messages:
// - If senderId matches currentUserId (or no senderId), show "U" (current user)
// - Otherwise, show initials from senderDisplayName (other user)
const isCurrentUser = !senderId || senderId === currentUserId;
const avatarText = isBot ? "AI" : (isCurrentUser ? "U" : getInitials(senderDisplayName));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Edge case: Other users without display names show "U" avatar, same as current user.

When senderId differs from currentUserId but senderDisplayName is undefined, getInitials(undefined) returns "U". This makes another user's message visually indistinguishable from the current user's messages.

Consider falling back to initials derived from senderId (e.g., first two chars) or using a distinct placeholder like "?":

Suggested improvement
-    const avatarText = isBot ? "AI" : (isCurrentUser ? "U" : getInitials(senderDisplayName));
+    const avatarText = isBot 
+      ? "AI" 
+      : isCurrentUser 
+        ? "U" 
+        : getInitials(senderDisplayName) || (senderId ? senderId.slice(0, 2).toUpperCase() : "?");
📝 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
// Avatar logic for user messages:
// - If senderId matches currentUserId (or no senderId), show "U" (current user)
// - Otherwise, show initials from senderDisplayName (other user)
const isCurrentUser = !senderId || senderId === currentUserId;
const avatarText = isBot ? "AI" : (isCurrentUser ? "U" : getInitials(senderDisplayName));
// Avatar logic for user messages:
// - If senderId matches currentUserId (or no senderId), show "U" (current user)
// - Otherwise, show initials from senderDisplayName (other user)
const isCurrentUser = !senderId || senderId === currentUserId;
const avatarText = isBot
? "AI"
: isCurrentUser
? "U"
: getInitials(senderDisplayName) || (senderId ? senderId.slice(0, 2).toUpperCase() : "?");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/message.tsx` around lines 264 - 269,
The avatar logic in the Message component currently sets avatarText via isBot ?
"AI" : (isCurrentUser ? "U" : getInitials(senderDisplayName)), which causes
other users with undefined senderDisplayName to render as "U" (same as current
user); update the logic so when senderId !== currentUserId and senderDisplayName
is falsy you derive a distinct fallback (e.g., initials from senderId like
senderId.slice(0,2) or a placeholder like "?") before calling getInitials, and
ensure avatarText uses isBot, isCurrentUser, then this new fallback path;
reference variables/functions: avatarText, isCurrentUser, getInitials,
senderDisplayName, senderId, currentUserId, isBot.

Comment on lines +91 to +94
// Extract sender attribution from metadata (for multi-user sessions)
const senderDisplayName = m.metadata?.senderDisplayName as string | undefined;
const senderId = m.metadata?.senderId as string | undefined;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider falling back to senderId for sender attribution when display name is unavailable.

Currently, senderAttribution only uses senderDisplayName. If a user lacks a display name but has a senderId, no attribution text appears above their message (though the avatar will show initials via senderId). For consistency:

Suggested improvement
-      const senderDisplayName = m.metadata?.senderDisplayName as string | undefined;
-      const senderId = m.metadata?.senderId as string | undefined;
+      const senderDisplayName = m.metadata?.senderDisplayName as string | undefined;
+      const senderId = m.metadata?.senderId as string | undefined;
+      const senderAttribution = senderDisplayName ?? senderId;

Then use senderAttribution in the Message props:

-            senderAttribution={!isAgent && senderDisplayName ? senderDisplayName : undefined}
+            senderAttribution={!isAgent ? senderAttribution : undefined}

Also applies to: 122-125, 142-145

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/stream-message.tsx` around lines 91 -
94, Replace the current logic that only uses senderDisplayName for attribution
by falling back to senderId when display name is missing: derive
senderAttribution = senderDisplayName ?? senderId (or equivalent) and pass
senderAttribution into the Message props instead of senderDisplayName; apply the
same change in the other occurrences referenced (the blocks around the sections
currently extracting senderDisplayName/senderId at the later occurrences) so
attribution shows either the display name or the senderId consistently.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/manifests/base/rbac/ambient-project-edit-clusterrole.yaml`:
- Around line 25-28: Replace the single secrets rule that grants both "get" and
"create" with two rules: one rule that allows only "get" on secrets but
restricts it to the specific secret name "ambient-runner-secrets" via
resourceNames, and a separate rule that allows "create" on secrets without
resourceNames; update the ClusterRole's secrets rule blocks (the rule that lists
apiGroups: [""], resources: ["secrets"], verbs: ["get","create"]) to the two
described rules so "get" is scoped to resourceNames: ["ambient-runner-secrets"]
while "create" remains unrestricted for creation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5025fb33-9228-4693-95c6-cfc7f4a9729b

📥 Commits

Reviewing files that changed from the base of the PR and between 68ff0e7 and 1a74e84.

📒 Files selected for processing (1)
  • components/manifests/base/rbac/ambient-project-edit-clusterrole.yaml

Comment on lines +25 to +28
# Secrets (read ambient-runner-secrets for validation, create runner tokens)
- apiGroups: [""]
resources: ["secrets"]
verbs: ["create"]
verbs: ["get", "create"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all references to ambient-runner-secrets to confirm the exact secret name pattern
rg -n "ambient-runner-secrets" --type yaml --type go --type ts

Repository: ambient-code/platform

Length of output: 4781


🏁 Script executed:

cat -n components/manifests/base/rbac/ambient-project-edit-clusterrole.yaml | head -40

Repository: ambient-code/platform

Length of output: 1805


Security concern: get on secrets grants access to all secrets, not just ambient-runner-secrets.

The rule allows reading any secret in the namespace where this role is bound, but the codebase hardcodes only one secret name: ambient-runner-secrets. This violates the principle of least privilege and could expose other sensitive credentials.

Restrict access to the specific secret using resourceNames:

🔒 Proposed fix
 # Secrets (read ambient-runner-secrets for validation, create runner tokens)
 - apiGroups: [""]
   resources: ["secrets"]
   verbs: ["get", "create"]
+  resourceNames: ["ambient-runner-secrets"]
+# Secrets (create runner tokens)
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["create"]

Note: resourceNames cannot be applied to create verbs, so two separate rules are needed—one for get with resourceNames, one for create without it.

🧰 Tools
🪛 Trivy (0.69.3)

[error] 26-28: Manage secrets

ClusterRole 'ambient-project-edit' shouldn't have access to manage resource 'secrets'

Rule: KSV-0041

Learn more

(IaC/Kubernetes)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/manifests/base/rbac/ambient-project-edit-clusterrole.yaml` around
lines 25 - 28, Replace the single secrets rule that grants both "get" and
"create" with two rules: one rule that allows only "get" on secrets but
restricts it to the specific secret name "ambient-runner-secrets" via
resourceNames, and a separate rule that allows "create" on secrets without
resourceNames; update the ClusterRole's secrets rule blocks (the rule that lists
apiGroups: [""], resources: ["secrets"], verbs: ["get","create"]) to the two
described rules so "get" is scoped to resourceNames: ["ambient-runner-secrets"]
while "create" remains unrestricted for creation.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
components/frontend/src/components/chat/ChatInputBox.tsx (1)

486-494: ⚠️ Potential issue | 🟠 Major

Two mutating actions still bypass the view-only gate.

The new canInteract branch only wraps the toolbar. Line 489 still exposes onContinue, and Line 531 still exposes onClearQueue, so viewers can resume a session or clear queued messages even though session-header.tsx now hides those actions by role. Guard both with canInteract too.

Suggested fix
-            {onContinue && (
+            {canInteract && onContinue && (
               <button type="button" onClick={onContinue} className="text-link hover:underline font-medium">
                 Resume session
               </button>
             )}
...
-              {onClearQueue && (
+              {canInteract && onClearQueue && (
                 <button
                   type="button"
                   onClick={onClearQueue}
                   className="ml-1 flex items-center gap-0.5 hover:text-amber-800 dark:hover:text-amber-200"

Also applies to: 531-540

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/chat/ChatInputBox.tsx` around lines 486 -
494, The Resume and Clear Queue actions are rendered even for view-only
users—guard the interactive buttons with the same canInteract check used for the
toolbar: wrap the onContinue button (inside the isTerminalState block where
sessionPhase is shown) with a canInteract conditional (or add && canInteract to
the JSX conditional) so it only renders when canInteract is true, and similarly
guard the onClearQueue button (the clear-queue rendering around onClearQueue)
with canInteract; use the existing canInteract prop/variable in ChatInputBox to
prevent rendering onContinue and onClearQueue for view-only users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/CLAUDE.md:
- Line 5: The file .claude/CLAUDE.md contains a hardcoded absolute path
"/Users/gkrumbac/.../STATUS.md" which will break on other machines; replace that
absolute path with a repository-relative path (e.g., use a path starting from
the repo root like "orchestrator/mailboxes/session-sharing/STATUS.md") and
update the other occurrence on line 8 similarly, or if this file is
machine-specific add it to .gitignore instead; locate the path string in
.claude/CLAUDE.md and perform the change.
- Around line 2-35: Add missing blank lines before and after Markdown headings
to satisfy the linter: insert a blank line above and below top-level heading "#
--- WORKER PROTOCOL ---", each second-level heading like "## After each
significant milestone (task complete, PR created, blocker hit):", "##
Workflow:", and "## Rules:", and any other headings or subheadings in the file
so that there is one blank line separating headings from surrounding paragraphs
or lists; update the file ".claude/CLAUDE.md" accordingly to preserve existing
content and spacing semantics.
- Around line 1-35: The file .claude/CLAUDE.md contains a machine-specific
absolute path (/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/...) that
should not be committed; either remove or generalize those paths in
.claude/CLAUDE.md (replace with relative/example paths or placeholders) OR add
.claude/CLAUDE.md to .gitignore alongside .claude/settings.local.json so local
machine paths are excluded from version control; update the file or .gitignore
accordingly and ensure the orchestration instructions still make sense after the
change.

In
`@components/frontend/src/app/projects/`[name]/sessions/[sessionName]/components/explorer/explorer-panel.tsx:
- Around line 71-72: The current check sets canModify = access?.userRole !==
'view', which treats undefined/failed access as editable; change it to an
explicit allow-list so undefined is denied: derive canModify from
useProjectAccess() by checking access?.userRole against allowed roles (e.g.,
'admin' or 'editor') and defaulting to false when access is undefined or the
role is not in the allow-list; update references to canModify in
explorer-panel.tsx so UI controls only render when the role matches the explicit
allowed roles.

In `@components/frontend/src/components/chat/__tests__/ChatInputBox.test.tsx`:
- Around line 45-50: The test currently hardcodes useProjectAccess to return
userRole: "edit", so add a new test case in ChatInputBox.test.tsx that mocks
useProjectAccess to return userRole: "view" and asserts the ChatInputBox renders
in read-only mode: the textarea/input is disabled and write controls (e.g., send
button/toolbar components used by ChatInputBox) are in their read-only state or
not interactive; implement this by creating a separate mock for useProjectAccess
with { data: { userRole: "view" }, isLoading: false }, rendering ChatInputBox,
and using the existing queries (getByRole/getByTestId/getByLabelText) to assert
disabled textbox and disabled/hidden toolbar controls.

In `@components/frontend/src/components/chat/ChatInputBox.tsx`:
- Around line 190-192: The chat input is currently enabled by default while the
access query is unresolved because canInteract is set from access?.userRole !==
"view"; change this so the input stays disabled until the role is known by
checking the query state or guarding absence of access: in ChatInputBox use the
useProjectAccess hook's loading/error flags (or require access?.userRole to be
defined) and set canInteract to false when access is undefined or the request
errored; only set canInteract true when access.userRole exists and is not
"view".

In `@components/frontend/src/components/workspace-sections/sessions-section.tsx`:
- Around line 82-86: The permission logic currently makes canModify true when
access is unresolved because access?.userRole !== 'view' evaluates to true for
undefined; update the check so modify defaults to deny until a concrete role is
present. Replace the canModify expression (in sessions-section.tsx where
useProjectAccess, canCreate, canDelete, canModify are defined) with a
presence-aware check such as: only allow modify when access.userRole is defined
and not 'view' (e.g., access?.userRole ? access.userRole !== 'view' : false),
ensuring similar safe defaults for other permission flags if needed.

In `@UI_PERMISSIONS_AUDIT.md`:
- Around line 17-37: The table has invalid single-cell divider rows for the
section headings '**Sessions**' and '**Settings**' which break Markdown linting
(MD056); update those rows by either moving the section labels out of the table
(place a bold heading line above the table) or expand each label row to match
the four-column table (e.g., replace the single-cell row containing
'**Sessions**' and the one containing '**Settings**' with rows that have four
cells so they align with the header), ensuring the rows around the symbols
'**Sessions**' and '**Settings**' now contain four pipe-separated cells to
produce a valid 4-column table.

---

Outside diff comments:
In `@components/frontend/src/components/chat/ChatInputBox.tsx`:
- Around line 486-494: The Resume and Clear Queue actions are rendered even for
view-only users—guard the interactive buttons with the same canInteract check
used for the toolbar: wrap the onContinue button (inside the isTerminalState
block where sessionPhase is shown) with a canInteract conditional (or add &&
canInteract to the JSX conditional) so it only renders when canInteract is true,
and similarly guard the onClearQueue button (the clear-queue rendering around
onClearQueue) with canInteract; use the existing canInteract prop/variable in
ChatInputBox to prevent rendering onContinue and onClearQueue for view-only
users.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7eacd14f-c6e0-4e5b-9473-e9a7481d11f0

📥 Commits

Reviewing files that changed from the base of the PR and between 1a74e84 and b0f701e.

📒 Files selected for processing (15)
  • .claude/CLAUDE.md
  • UI_PERMISSIONS_AUDIT.md
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/__tests__/session-header.test.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/explorer/__tests__/context-tab.test.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/explorer/__tests__/explorer-panel.test.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/explorer/context-tab.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/explorer/explorer-panel.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/components/explorer/files-tab.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/page.tsx
  • components/frontend/src/app/projects/[name]/sessions/[sessionName]/session-header.tsx
  • components/frontend/src/components/chat/ChatInputBox.tsx
  • components/frontend/src/components/chat/__tests__/ChatInputBox.test.tsx
  • components/frontend/src/components/session/MessagesTab.tsx
  • components/frontend/src/components/workspace-sections/sessions-section.tsx
  • components/frontend/src/services/queries/use-project-access.ts

Comment on lines +1 to +35

# --- WORKER PROTOCOL ---
# You are a persistent worker session managed by an orchestrator.
# Your label: session-sharing
# Your status file: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md

## After each significant milestone (task complete, PR created, blocker hit):
Update `/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md` with this format:

# Status: session-sharing

Phase: <planning|implementing|testing|pr-open|blocked|done>
Updated: <ISO timestamp>
Blockers: <none | description>
PR: <none | URL>
Summary: <1-2 sentence description of current state>

## Workflow:
1. Investigate the issue thoroughly — read relevant code, understand the problem
2. Make a plan — write it out clearly
3. Ask clarifying questions if anything is ambiguous (use AskUserQuestion)
4. Implement the fix
5. Run tests: frontend (npx vitest run), backend (make test), runner (python -m pytest tests/)
6. Check e2e flows are not impacted
7. Lint: frontend (npm run build must pass with 0 errors 0 warnings), backend/operator (gofmt, go vet, golangci-lint), runner (ruff)
8. Commit with conventional commit format
9. Push to your feature branch
10. Create a PR with gh pr create
11. Open the PR in the browser with: open <PR_URL>

## Rules:
- Always update STATUS.md before stopping or when blocked
- If you hit a blocker you cannot resolve, update STATUS.md and wait for instructions
- The orchestrator may send you follow-up messages at any time — act on them
- Use Context7 MCP for any library/API documentation you need
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if .gitignore already excludes .claude directory
if [ -f .gitignore ]; then
  rg -n '\.claude' .gitignore
  exit_code=$?
  if [ $exit_code -ne 0 ]; then
    echo "No .claude exclusion found in .gitignore"
  fi
else
  echo ".gitignore does not exist"
fi

# Check for other .claude files that might indicate this is intentional
fd -t f . .claude

Repository: ambient-code/platform

Length of output: 882


🏁 Script executed:

# Get full .gitignore to understand the policy
cat -n .gitignore | head -100

Repository: ambient-code/platform

Length of output: 2017


🏁 Script executed:

# Check the actual .claude/settings.local.json entry and surrounding context
sed -n '85,95p' .gitignore

Repository: ambient-code/platform

Length of output: 174


🏁 Script executed:

# Check if other .claude files contain machine-specific paths similar to CLAUDE.md
for file in .claude/amber-config.yml .claude/commands/acp-compile.md .claude/context/backend-development.md; do
  if [ -f "$file" ]; then
    echo "=== $file ===" 
    head -20 "$file" | grep -E "(Users|Documents|home|tmp|/var)"
  fi
done

Repository: ambient-code/platform

Length of output: 184


🏁 Script executed:

# Check if there's any README or documentation explaining the .claude directory
fd -t f README .claude

Repository: ambient-code/platform

Length of output: 101


Remove machine-specific paths from this file or add it to .gitignore.

This file contains a personal developer's machine-specific path (/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/...) that should not be committed to version control. The .claude/ directory is intentionally part of the repository for team orchestration documentation, but .claude/CLAUDE.md should either have these paths generalized/removed or be added to .gitignore alongside .claude/settings.local.json.

🧰 Tools
🪛 LanguageTool

[style] ~20-~20: ‘Make a plan’ might be wordy. Consider a shorter alternative.
Context: ...elevant code, understand the problem 2. Make a plan — write it out clearly 3. Ask clarifyin...

(EN_WORDINESS_PREMIUM_MAKE_A_PLAN)

🪛 markdownlint-cli2 (0.21.0)

[warning] 2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 4-4: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


[warning] 4-4: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 5-5: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


[warning] 7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 31-31: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/CLAUDE.md around lines 1 - 35, The file .claude/CLAUDE.md contains a
machine-specific absolute path
(/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/...) that should not be
committed; either remove or generalize those paths in .claude/CLAUDE.md (replace
with relative/example paths or placeholders) OR add .claude/CLAUDE.md to
.gitignore alongside .claude/settings.local.json so local machine paths are
excluded from version control; update the file or .gitignore accordingly and
ensure the orchestration instructions still make sense after the change.

Comment on lines +2 to +35
# --- WORKER PROTOCOL ---
# You are a persistent worker session managed by an orchestrator.
# Your label: session-sharing
# Your status file: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md

## After each significant milestone (task complete, PR created, blocker hit):
Update `/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md` with this format:

# Status: session-sharing

Phase: <planning|implementing|testing|pr-open|blocked|done>
Updated: <ISO timestamp>
Blockers: <none | description>
PR: <none | URL>
Summary: <1-2 sentence description of current state>

## Workflow:
1. Investigate the issue thoroughly — read relevant code, understand the problem
2. Make a plan — write it out clearly
3. Ask clarifying questions if anything is ambiguous (use AskUserQuestion)
4. Implement the fix
5. Run tests: frontend (npx vitest run), backend (make test), runner (python -m pytest tests/)
6. Check e2e flows are not impacted
7. Lint: frontend (npm run build must pass with 0 errors 0 warnings), backend/operator (gofmt, go vet, golangci-lint), runner (ruff)
8. Commit with conventional commit format
9. Push to your feature branch
10. Create a PR with gh pr create
11. Open the PR in the browser with: open <PR_URL>

## Rules:
- Always update STATUS.md before stopping or when blocked
- If you hit a blocker you cannot resolve, update STATUS.md and wait for instructions
- The orchestrator may send you follow-up messages at any time — act on them
- Use Context7 MCP for any library/API documentation you need
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Optional: Fix markdown formatting for consistency.

The markdown linter flagged missing blank lines around headings throughout the file. While these are minor style issues, adding blank lines would improve readability and comply with standard markdown conventions.

📝 Suggested formatting improvements
 
 # --- WORKER PROTOCOL ---
+
 # You are a persistent worker session managed by an orchestrator.
+
 # Your label: session-sharing
+
 # Your status file: .claude/orchestrator/mailboxes/session-sharing/STATUS.md
 
 ## After each significant milestone (task complete, PR created, blocker hit):
+
 Update `.claude/orchestrator/mailboxes/session-sharing/STATUS.md` with this format:
 
 # Status: session-sharing
@@ -17,6 +20,7 @@
 Summary: <1-2 sentence description of current state>
 
 ## Workflow:
+
 1. Investigate the issue thoroughly — read relevant code, understand the problem
 2. Make a plan — write it out clearly
 3. Ask clarifying questions if anything is ambiguous (use AskUserQuestion)
@@ -30,6 +34,7 @@
 11. Open the PR in the browser with: open <PR_URL>
 
 ## Rules:
+
 - Always update STATUS.md before stopping or when blocked
 - If you hit a blocker you cannot resolve, update STATUS.md and wait for instructions
 - The orchestrator may send you follow-up messages at any time — act on them
📝 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
# --- WORKER PROTOCOL ---
# You are a persistent worker session managed by an orchestrator.
# Your label: session-sharing
# Your status file: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md
## After each significant milestone (task complete, PR created, blocker hit):
Update `/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md` with this format:
# Status: session-sharing
Phase: <planning|implementing|testing|pr-open|blocked|done>
Updated: <ISO timestamp>
Blockers: <none | description>
PR: <none | URL>
Summary: <1-2 sentence description of current state>
## Workflow:
1. Investigate the issue thoroughly — read relevant code, understand the problem
2. Make a plan — write it out clearly
3. Ask clarifying questions if anything is ambiguous (use AskUserQuestion)
4. Implement the fix
5. Run tests: frontend (npx vitest run), backend (make test), runner (python -m pytest tests/)
6. Check e2e flows are not impacted
7. Lint: frontend (npm run build must pass with 0 errors 0 warnings), backend/operator (gofmt, go vet, golangci-lint), runner (ruff)
8. Commit with conventional commit format
9. Push to your feature branch
10. Create a PR with gh pr create
11. Open the PR in the browser with: open <PR_URL>
## Rules:
- Always update STATUS.md before stopping or when blocked
- If you hit a blocker you cannot resolve, update STATUS.md and wait for instructions
- The orchestrator may send you follow-up messages at any time — act on them
- Use Context7 MCP for any library/API documentation you need
# --- WORKER PROTOCOL ---
# You are a persistent worker session managed by an orchestrator.
# Your label: session-sharing
# Your status file: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md
## After each significant milestone (task complete, PR created, blocker hit):
Update `/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md` with this format:
# Status: session-sharing
Phase: <planning|implementing|testing|pr-open|blocked|done>
Updated: <ISO timestamp>
Blockers: <none | description>
PR: <none | URL>
Summary: <1-2 sentence description of current state>
## Workflow:
1. Investigate the issue thoroughly — read relevant code, understand the problem
2. Make a plan — write it out clearly
3. Ask clarifying questions if anything is ambiguous (use AskUserQuestion)
4. Implement the fix
5. Run tests: frontend (npx vitest run), backend (make test), runner (python -m pytest tests/)
6. Check e2e flows are not impacted
7. Lint: frontend (npm run build must pass with 0 errors 0 warnings), backend/operator (gofmt, go vet, golangci-lint), runner (ruff)
8. Commit with conventional commit format
9. Push to your feature branch
10. Create a PR with gh pr create
11. Open the PR in the browser with: open <PR_URL>
## Rules:
- Always update STATUS.md before stopping or when blocked
- If you hit a blocker you cannot resolve, update STATUS.md and wait for instructions
- The orchestrator may send you follow-up messages at any time — act on them
- Use Context7 MCP for any library/API documentation you need
🧰 Tools
🪛 LanguageTool

[style] ~20-~20: ‘Make a plan’ might be wordy. Consider a shorter alternative.
Context: ...elevant code, understand the problem 2. Make a plan — write it out clearly 3. Ask clarifyin...

(EN_WORDINESS_PREMIUM_MAKE_A_PLAN)

🪛 markdownlint-cli2 (0.21.0)

[warning] 2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 4-4: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


[warning] 4-4: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 5-5: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


[warning] 7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 31-31: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/CLAUDE.md around lines 2 - 35, Add missing blank lines before and
after Markdown headings to satisfy the linter: insert a blank line above and
below top-level heading "# --- WORKER PROTOCOL ---", each second-level heading
like "## After each significant milestone (task complete, PR created, blocker
hit):", "## Workflow:", and "## Rules:", and any other headings or subheadings
in the file so that there is one blank line separating headings from surrounding
paragraphs or lists; update the file ".claude/CLAUDE.md" accordingly to preserve
existing content and spacing semantics.

# --- WORKER PROTOCOL ---
# You are a persistent worker session managed by an orchestrator.
# Your label: session-sharing
# Your status file: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Hardcoded absolute path will break for other developers.

Line 5 contains a hardcoded absolute path specific to the user gkrumbac's machine: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md. This path will not exist for other developers cloning the repository, making the file unusable.

🔧 Recommended fixes

Option 1 (preferred): Use a relative path from the repository root:

-# Your status file: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md
+# Your status file: .claude/orchestrator/mailboxes/session-sharing/STATUS.md

And update line 8 similarly:

-Update `/Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md` with this format:
+Update `.claude/orchestrator/mailboxes/session-sharing/STATUS.md` with this format:

Option 2: Add this file to .gitignore if it's meant to be machine-specific.

📝 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
# Your status file: /Users/gkrumbac/Documents/vTeam/.claude/orchestrator/mailboxes/session-sharing/STATUS.md
# Your status file: .claude/orchestrator/mailboxes/session-sharing/STATUS.md
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 5-5: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/CLAUDE.md at line 5, The file .claude/CLAUDE.md contains a hardcoded
absolute path "/Users/gkrumbac/.../STATUS.md" which will break on other
machines; replace that absolute path with a repository-relative path (e.g., use
a path starting from the repo root like
"orchestrator/mailboxes/session-sharing/STATUS.md") and update the other
occurrence on line 8 similarly, or if this file is machine-specific add it to
.gitignore instead; locate the path string in .claude/CLAUDE.md and perform the
change.

Comment on lines +71 to +72
const { data: access } = useProjectAccess(projectName);
const canModify = access?.userRole !== 'view';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't treat unknown access as editable.

As implemented in components/frontend/src/services/queries/use-project-access.ts, data is undefined while the role query is loading and after failed /access responses. access?.userRole !== "view" therefore resolves to true, so viewers briefly get Upload/Add/Remove controls and a failed access lookup leaves them unlocked. Use an explicit allow-list here instead of a negative check.

Suggested fix
-  const { data: access } = useProjectAccess(projectName);
-  const canModify = access?.userRole !== 'view';
+  const { data: access } = useProjectAccess(projectName);
+  const canModify =
+    access?.userRole === "edit" || access?.userRole === "admin";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@components/frontend/src/app/projects/`[name]/sessions/[sessionName]/components/explorer/explorer-panel.tsx
around lines 71 - 72, The current check sets canModify = access?.userRole !==
'view', which treats undefined/failed access as editable; change it to an
explicit allow-list so undefined is denied: derive canModify from
useProjectAccess() by checking access?.userRole against allowed roles (e.g.,
'admin' or 'editor') and defaulting to false when access is undefined or the
role is not in the allow-list; update references to canModify in
explorer-panel.tsx so UI controls only render when the role matches the explicit
allowed roles.

Comment on lines +190 to +192
// Check user access level
const { data: access } = useProjectAccess(projectName || "");
const canInteract = access?.userRole !== "view";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unknown access currently unlocks the chat box.

This has the same default-allow bug as the explorer: with a real projectName, access?.userRole !== "view" makes the input interactive until the role query resolves, and any failed /access lookup leaves it enabled. Default this path to non-interactive until the role is known.

Suggested fix
-  const { data: access } = useProjectAccess(projectName || "");
-  const canInteract = access?.userRole !== "view";
+  const { data: access } = useProjectAccess(projectName ?? "");
+  const canInteract =
+    !projectName ||
+    access?.userRole === "edit" ||
+    access?.userRole === "admin";
📝 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
// Check user access level
const { data: access } = useProjectAccess(projectName || "");
const canInteract = access?.userRole !== "view";
// Check user access level
const { data: access } = useProjectAccess(projectName ?? "");
const canInteract =
!projectName ||
access?.userRole === "edit" ||
access?.userRole === "admin";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/chat/ChatInputBox.tsx` around lines 190 -
192, The chat input is currently enabled by default while the access query is
unresolved because canInteract is set from access?.userRole !== "view"; change
this so the input stays disabled until the role is known by checking the query
state or guarding absence of access: in ChatInputBox use the useProjectAccess
hook's loading/error flags (or require access?.userRole to be defined) and set
canInteract to false when access is undefined or the request errored; only set
canInteract true when access.userRole exists and is not "view".

Comment on lines +82 to +86
// Access control
const { data: access } = useProjectAccess(projectName);
const canCreate = access?.userRole === 'edit' || access?.userRole === 'admin';
const canDelete = access?.userRole === 'admin';
const canModify = access?.userRole !== 'view';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Default permission checks to deny while access is unresolved.

canModify = access?.userRole !== 'view' becomes true while the access query is still loading and after a failed fetch, so read-only users can still see edit/stop/continue actions. For this RBAC audit work, the safe default needs to be 'view', not write access.

Suggested fix
-  const { data: access } = useProjectAccess(projectName);
-  const canCreate = access?.userRole === 'edit' || access?.userRole === 'admin';
-  const canDelete = access?.userRole === 'admin';
-  const canModify = access?.userRole !== 'view';
+  const { data: access } = useProjectAccess(projectName);
+  const userRole = access?.userRole ?? 'view';
+  const canCreate = userRole === 'edit' || userRole === 'admin';
+  const canDelete = userRole === 'admin';
+  const canModify = userRole === 'edit' || userRole === 'admin';
📝 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
// Access control
const { data: access } = useProjectAccess(projectName);
const canCreate = access?.userRole === 'edit' || access?.userRole === 'admin';
const canDelete = access?.userRole === 'admin';
const canModify = access?.userRole !== 'view';
// Access control
const { data: access } = useProjectAccess(projectName);
const userRole = access?.userRole ?? 'view';
const canCreate = userRole === 'edit' || userRole === 'admin';
const canDelete = userRole === 'admin';
const canModify = userRole === 'edit' || userRole === 'admin';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/workspace-sections/sessions-section.tsx`
around lines 82 - 86, The permission logic currently makes canModify true when
access is unresolved because access?.userRole !== 'view' evaluates to true for
undefined; update the check so modify defaults to deny until a concrete role is
present. Replace the canModify expression (in sessions-section.tsx where
useProjectAccess, canCreate, canDelete, canModify are defined) with a
presence-aware check such as: only allow modify when access.userRole is defined
and not 'view' (e.g., access?.userRole ? access.userRole !== 'view' : false),
ensuring similar safe defaults for other permission flags if needed.

Gkrumbach07 and others added 13 commits March 19, 2026 23:58
Add userContext field to AgenticSessionSpec types and display creator
information in the sessions list UI. This addresses Finding #1 from the
session sharing audit - users can now see who created each session in
shared workspaces.

Changes:
- Add UserContext type (userId, displayName, groups) to type definitions
- Add userContext field to AgenticSessionSpec in both type files
- Add "Creator" column to sessions table (visible at xl+ breakpoint)
- Add creator info to session hover card with User icon
- Move "Artifacts" column to 2xl breakpoint to accommodate new column
- Display creator displayName with fallback to userId or "—"

Resolves session sharing audit finding #1.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Inject sender identity into user messages and display it in the UI to
support multi-user collaboration. This addresses Finding #3 from the
session sharing audit - users can now see who sent each message in
shared sessions.

Backend changes:
- Extract userID and userName from authenticated context
- Inject senderId and senderDisplayName into user message metadata
- Preserve metadata through message serialization pipeline

Frontend changes:
- Add MessageMetadata type with senderId and senderDisplayName fields
- Update UserMessage and AgentMessage types to include metadata
- Extract sender info from metadata in StreamMessage component
- Display sender name above user messages with timestamp separator
- Add senderAttribution prop to Message component

Display format: "SenderName • timestamp" above user message bubble

Resolves session sharing audit finding #3.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Comprehensive audit of multi-user session access, RBAC enforcement,
message attribution, and API key security. Documents 4 key findings
with recommendations for each.

Findings:
1. Creator not shown in UI - FIXED in 98af80b
2. No per-session RBAC - Design decision needed (workspace vs session scope)
3. No message attribution - FIXED in d75fb72
4. Shared API keys - Working as designed, needs documentation

This audit was conducted to improve transparency and user experience
in shared workspace environments where multiple team members interact
with the same agentic sessions.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
TypeScript infers unknown type from index signature in metadata object.
Add explicit type assertions to fix build error after rebase.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Backend: Broadcast user messages immediately via MESSAGES_SNAPSHOT event
  before forwarding to runner, ensuring all connected clients see messages
  in real-time instead of waiting for the run to finish
- Frontend: Show sender initials in avatar for other users' messages while
  keeping "U" for current user's own messages (e.g., "JD" for "John Doe")

Fixes two shared session bugs:
1. User messages not appearing for other clients until run completes
2. All user messages showing generic "U" avatar instead of sender identity

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Edit users were unable to create sessions, getting "Failed to validate API
key configuration" error. The session creation code reads ambient-runner-secrets
to validate API keys exist before creating the session, but edit role only had
"create" permission on Secrets, not "get".

Add "get" verb to Secrets permissions for ambient-project-edit role. This
allows edit users to validate API keys during session creation while still
preventing them from modifying or deleting secrets.

Fixes session creation for edit-role users.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Hide "New Session" button for view-only users and restrict session actions based on user role. Admins can delete, edit users can create/modify/stop, view users have read-only access.

Changes:
- SessionsSection: Add useProjectAccess hook to check user role
- Gate "New Session" button with canCreate (edit/admin roles)
- Pass canDelete and canModify flags to SessionActions component
- Filter available actions per session based on user permissions
- Hide action menu entirely if no actions available for user
- Export ContextTabProps type to fix TypeScript compilation
- Add missing projectName prop to ExplorerPanel component

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updated ChatInputBox component to respect user role permissions:
- Added useProjectAccess hook to check user role
- Disabled textarea for view-only users with informative placeholder
- Conditionally rendered toolbar actions (Add, Upload, Agents, Commands, Send)
- View-only users see "View-only access - you cannot send messages" message

Updated MessagesTab to accept and pass projectName prop to ChatInputBox.
Updated page.tsx to pass projectName to MessagesTab.
Exported ExplorerPanelProps type for proper TypeScript checking.

Part of session sharing RBAC audit implementation.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add useProjectAccess hook to query user's role (view/edit/admin)
- Update session header to hide Stop/Resume/Edit/Clone for view users
- Update explorer to hide Add Repository/Upload File for view users
- Update tests to mock useProjectAccess hook
- Add comprehensive UI_PERMISSIONS_AUDIT.md documentation

Completes UI permissions audit - all actions now gated by RBAC role.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix test failures by mocking useProjectAccess hook in ChatInputBox test suite.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Move minimalMsgs append to after metadata injection so that
MESSAGES_SNAPSHOT events broadcast to other users include the
senderDisplayName and senderId fields. Previously the snapshot
contained messages without metadata, so other users couldn't see
who sent the message.

Fixes multi-user message attribution in real-time.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…to user_message

The conversion from AG-UI PlatformMessage to internal UserMessage type
was dropping the metadata field containing senderId and senderDisplayName.
This caused all user messages to show as "U" regardless of who sent them,
even though the backend was correctly injecting sender metadata.

Add `metadata: msg.metadata` to the user_message object so sender
attribution flows through to StreamMessage and renders correctly.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove .claude/CLAUDE.md (worker config), SESSION_SHARING_AUDIT.md,
and UI_PERMISSIONS_AUDIT.md (development analysis docs) from the PR.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@Gkrumbach07 Gkrumbach07 force-pushed the feat/session-sharing-audit branch from f9c6d0c to 7d5051a Compare March 20, 2026 04:59
@Gkrumbach07
Copy link
Copy Markdown
Contributor Author

Final Review - Ready for Merge ✅

All CI checks pass. All development artifacts removed from PR.

Changes Summary

20 files changed across backend, frontend, and RBAC manifests:

Multi-User Session Improvements

  • Backend: Inject sender metadata (senderId, senderDisplayName) into user messages for attribution
  • Backend: Broadcast user messages immediately via MESSAGES_SNAPSHOT events (no waiting for run to finish)
  • Frontend: Display sender name/initials in message avatars (other users show initials, current user shows "U")
  • Frontend: Pass metadata through AG-UI → internal message type conversion
  • Frontend: Display session creator in sessions table and hover card
  • Frontend types: Added UserContext, metadata fields to UserMessage, MessageMetadata type

Role-Based UI Permissions (view/edit/admin)

  • New hook: useProjectAccess() — queries /api/projects/:name/access for user's role
  • Session header: Hide Stop/Resume (view), Delete (view+edit), Edit name/Clone (view)
  • Chat input: Disabled with "view-only access" message for view users
  • Sessions list: Hide "New Session" button (view), Delete (view+edit)
  • Explorer panel: Hide Add Repository/Upload File buttons (view)

RBAC Fix

  • ambient-project-edit ClusterRole: Added get verb to Secrets (was create only, blocking session creation with "Failed to validate API key configuration")

Checks Run

  • npm run build — 0 errors, 0 warnings
  • npx vitest run — 587 passed, 12 skipped
  • gofmt -l . — clean
  • go vet ./... — clean
  • golangci-lint run — 0 issues
  • ✅ All CI checks pass

Gkrumbach07 and others added 2 commits March 20, 2026 07:51
- stream-message: fall back to senderId when senderDisplayName is absent
- agentic-session: reuse shared MessageMetadata type from agui.ts
- api/sessions: import UserContext from canonical location instead of duplicating
- sessions-section: always render creator row in hover card with "—" fallback
- permissions: default-deny until role is resolved (sessions-section, explorer, ChatInputBox)
- ChatInputBox: guard Resume/Clear Queue buttons with canInteract check
- RBAC: scope secret get to resourceNames: ["ambient-runner-secrets"]
- agui_proxy: log marshal errors instead of silently ignoring

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Gkrumbach07 Gkrumbach07 merged commit cdf2f6f into main Mar 20, 2026
37 checks passed
@Gkrumbach07 Gkrumbach07 deleted the feat/session-sharing-audit branch March 20, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant