fix(agents): defend AccessPicker against undefined invocation_targets (GH #4915) (#4924)

Opening the agent detail page in v0.3.37 crashed the whole route with
"Cannot read properties of undefined (reading 'some')" whenever the
cached agent record's `invocation_targets` was missing at runtime — even
though the TypeScript type declares it a required `AgentInvocationTarget[]`.

Root cause: `AccessPicker`'s `hasWorkspaceTarget` / `selectedMemberIds` /
`selectedTeamIds` helpers called `.some()` / `.filter()` directly on the
prop, and the same unguarded pattern was mirrored in `AgentMcpTab` and
the `canAssignAgentToIssue` permission gate. The field can legitimately
be undefined at runtime because:

- `packages/core/api/schemas.ts` declares `invocation_targets` as
  `.optional()`, and `MinimalAgentSchema` (used for the create-from-
  template response) also marks it optional.
- `api.listAgents` / `api.getAgent` return raw JSON without running
  through the schema, so an older self-host backend that predates
  MUL-3963 (permission_mode + invocation_targets) — the exact scenario
  in GH #4915 — yields an Agent with the field missing.

Fix (`?? []`) at every site that indexes into the list, matching the
already-established pattern in `create-agent-dialog.tsx`:

- `access-picker.tsx`: coerce inside the three helpers and widen the
  prop type to `AgentInvocationTarget[] | undefined` so the accepted
  runtime shape is documented.
- `agent-mcp-tab.tsx`: `(agent.invocation_targets ?? []).some(...)`.
- `permissions/rules.ts`: `agent.invocation_targets ?? []` before the
  workspace / member membership checks.

Adds three regression tests (AccessPicker owner + read-only paths,
AgentMcpTab shared-warning path, canAssignAgentToIssue) that pass
`undefined` for invocation_targets and assert the UI degrades to the
private / empty-allowlist state instead of throwing.

Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>

packages/core/permissions/rules.test.ts

@@ -259,6 +259,30 @@ describe("canAssignAgentToIssue", () => {
     expect(d.allowed).toBe(false);
     expect(d.reason).toBe("not_authenticated");
   });
+
+  // Regression: GH #4915. Legacy self-host backends / stale caches may
+  // return an agent without `invocation_targets` even though the modern
+  // type says required-array. The gate must degrade to "no grants" instead
+  // of throwing on `.some()` of undefined.
+  it("does not throw when invocation_targets is undefined", () => {
+    const a = makeAgent({
+      permission_mode: "public_to",
+      invocation_targets:
+        undefined as unknown as Agent["invocation_targets"],
+      owner_id: ALICE,
+    });
+    // Non-owner: no grants means denied.
+    expect(() =>
+      canAssignAgentToIssue(a, { userId: BOB, role: "member" }),
+    ).not.toThrow();
+    const d = canAssignAgentToIssue(a, { userId: BOB, role: "member" });
+    expect(d.allowed).toBe(false);
+    expect(d.reason).toBe("private_visibility");
+    // Owner path still allows.
+    expect(
+      canAssignAgentToIssue(a, { userId: ALICE, role: "member" }).allowed,
+    ).toBe(true);
+  });
 });
 
 describe("canEditSkill / canDeleteSkill", () => {

packages/core/permissions/rules.ts

@@ -75,8 +75,10 @@ export function canAssignAgentToIssue(
   }
 
   // permission_mode === "public_to": resolve the invocation grants. A
-  // workspace grant opens invocation to any workspace member.
-  const targets = agent.invocation_targets;
+  // workspace grant opens invocation to any workspace member. The `?? []`
+  // guards against legacy self-host backends / stale caches that omit the
+  // field even though the type says required-array (GH #4915).
+  const targets = agent.invocation_targets ?? [];
   if (targets.some((t) => t.target_type === "workspace")) {
     if (ctx.role === null) {
       return deny("not_member", "Join this workspace to assign agents.");

packages/views/agents/components/inspector/access-picker.test.tsx

@@ -116,4 +116,34 @@ describe("AccessPicker owner-only editing (MUL-3963)", () => {
       ],
     });
   });
+
+  // Regression: GH #4915. Older self-host backends / stale caches may return
+  // an agent without `invocation_targets` even though the modern type says
+  // required-array. The picker must degrade gracefully to the "Private" /
+  // empty-allowlist summary instead of crashing the whole agent detail
+  // route with "Cannot read properties of undefined (reading 'some')".
+  it("renders without crashing when invocationTargets is undefined", () => {
+    expect(() =>
+      renderPicker({
+        // Force the runtime shape produced by a legacy backend response.
+        invocationTargets: undefined as unknown as AgentInvocationTarget[],
+      }),
+    ).not.toThrow();
+    // Private is the fallback summary when there are no grants.
+    expect(screen.getAllByText("Only me").length).toBeGreaterThan(0);
+  });
+
+  it("read-only mode: undefined invocationTargets does not crash for non-owners", () => {
+    expect(() =>
+      renderPicker({
+        canEdit: false,
+        permissionMode: "public_to",
+        invocationTargets: undefined as unknown as AgentInvocationTarget[],
+        visibility: "workspace",
+      }),
+    ).not.toThrow();
+    // No workspace target in the (empty) list ⇒ shows the "no members" state
+    // rather than the workspace one, which is the safe degradation.
+    expect(screen.getByTestId("access-readonly")).toBeInTheDocument();
+  });
 });

packages/views/agents/components/inspector/access-picker.tsx

@@ -52,18 +52,32 @@ export type AccessChange = {
   invocation_targets: AgentInvocationTargetInput[];
 };
 
-function hasWorkspaceTarget(targets: AgentInvocationTarget[]): boolean {
-  return targets.some((t) => t.target_type === "workspace");
+// The helpers below defensively coerce a missing (`undefined` / `null`) list
+// to an empty array. `Agent.invocation_targets` is TYPED as a required array,
+// and the modern backend always serialises `[]` when there are no grants,
+// but older self-host servers / stale query caches / template create
+// responses (see `MinimalAgentSchema` in api/schemas.ts) can still surface an
+// undefined value at runtime. Without the fallback, opening an agent detail
+// page against a legacy backend crashes the whole route with
+// "Cannot read properties of undefined (reading 'some')" (GH #4915).
+function hasWorkspaceTarget(
+  targets: AgentInvocationTarget[] | undefined | null,
+): boolean {
+  return (targets ?? []).some((t) => t.target_type === "workspace");
 }
 
-function selectedMemberIds(targets: AgentInvocationTarget[]): string[] {
-  return targets
+function selectedMemberIds(
+  targets: AgentInvocationTarget[] | undefined | null,
+): string[] {
+  return (targets ?? [])
     .filter((t) => t.target_type === "member" && t.target_id !== null)
     .map((t) => t.target_id as string);
 }
 
-function selectedTeamIds(targets: AgentInvocationTarget[]): string[] {
-  return targets
+function selectedTeamIds(
+  targets: AgentInvocationTarget[] | undefined | null,
+): string[] {
+  return (targets ?? [])
     .filter((t) => t.target_type === "team" && t.target_id !== null)
     .map((t) => t.target_id as string);
 }
@@ -78,7 +92,13 @@ export function AccessPicker({
   onChange,
 }: {
   permissionMode: AgentPermissionMode;
-  invocationTargets: AgentInvocationTarget[];
+  /**
+   * The agent's invocation grants. Typed loose (may be `undefined`) because
+   * the schema is `optional()` and older self-host backends / template create
+   * responses can omit the field even though the modern shape is a
+   * required-array. The internal helpers `?? []` this before reading.
+   */
+  invocationTargets: AgentInvocationTarget[] | undefined;
   /**
    * Legacy derived visibility. No longer rendered directly (the read-only and
    * editable states both summarise permission_mode + targets), but kept in the

packages/views/agents/components/tabs/agent-mcp-tab.test.tsx

@@ -234,4 +234,25 @@ describe("AgentMcpTab", () => {
       screen.queryByText(/any workspace member may use these Composio apps/i),
     ).toBeNull();
   });
+
+  // Regression: GH #4915. Legacy self-host backends / stale caches may
+  // return an agent without `invocation_targets` even though the modern
+  // type declares a required array. The tab must degrade to the "not
+  // workspace-public" copy instead of crashing the whole detail route
+  // with "Cannot read properties of undefined (reading 'some')".
+  it("does not crash when invocation_targets is undefined", () => {
+    expect(() =>
+      renderTab({
+        permission_mode: "public_to",
+        invocation_targets:
+          undefined as unknown as Agent["invocation_targets"],
+        composio_toolkit_allowlist: ["notion"],
+      }),
+    ).not.toThrow();
+    // Falls back to the generic shared warning (no workspace target
+    // resolves to `false`), not the workspace-wide one.
+    expect(
+      screen.queryByText(/any workspace member may use these Composio apps/i),
+    ).toBeNull();
+  });
 });

packages/views/agents/components/tabs/agent-mcp-tab.tsx

@@ -90,7 +90,9 @@ export function AgentMcpTab({ agent }: { agent: Agent }) {
   const isPrivate = agent.permission_mode === "private";
   const isWorkspacePublic =
     agent.permission_mode === "public_to" &&
-    agent.invocation_targets.some((target) => target.target_type === "workspace");
+    (agent.invocation_targets ?? []).some(
+      (target) => target.target_type === "workspace",
+    );
   const showSharedWarning =
     !isPrivate && (allowlist.length > 0 || activeSlugs.length > 0);
 

server/pkg/agent/codex.go

@@ -866,13 +866,38 @@ func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOpti
 		// MUL-2339 — Trump's constraint that the three injection points
 		// must not drift independently).
 		applyCodexReasoningEffort(turnParams, opts.ThinkingLevel)
+		waitingForTurn := true
+		var timeoutDiagnostic codexTimeoutDiagnostic
+		var processExitErr error
+		finishTurn := func(aborted bool) {
+			waitingForTurn = false
+			switch {
+			case aborted:
+				finalStatus = "aborted"
+				if errMsg := c.getTurnError(); errMsg != "" {
+					finalError = errMsg
+				} else {
+					finalError = "turn was aborted"
+				}
+			default:
+				if errMsg := c.getTurnError(); errMsg != "" {
+					finalStatus = "failed"
+					finalError = errMsg
+				}
+			}
+		}
 		_, err = c.request(runCtx, "turn/start", turnParams)
 		if err != nil {
-			drainAndWait() // flush os/exec stderr goroutine before sampling Tail
-			finalStatus = "failed"
-			finalError = withAgentStderr(fmt.Sprintf("codex turn/start failed: %v", err), "codex", stderrBuf.Tail())
-			resCh <- Result{Status: finalStatus, Error: finalError, DurationMs: time.Since(startTime).Milliseconds()}
-			return
+			select {
+			case aborted := <-turnDone:
+				finishTurn(aborted)
+			default:
+				drainAndWait() // flush os/exec stderr goroutine before sampling Tail
+				finalStatus = "failed"
+				finalError = withAgentStderr(fmt.Sprintf("codex turn/start failed: %v", err), "codex", stderrBuf.Tail())
+				resCh <- Result{Status: finalStatus, Error: finalError, DurationMs: time.Since(startTime).Milliseconds()}
+				return
+			}
 		}
 
 		lastSemanticActivity := time.Now()
@@ -894,26 +919,6 @@ func (b *codexBackend) Execute(ctx context.Context, prompt string, opts ExecOpti
 		}
 		defer stopFirstTurnNoProgressTimer()
 
-		waitingForTurn := true
-		var timeoutDiagnostic codexTimeoutDiagnostic
-		var processExitErr error
-		finishTurn := func(aborted bool) {
-			waitingForTurn = false
-			switch {
-			case aborted:
-				finalStatus = "aborted"
-				if errMsg := c.getTurnError(); errMsg != "" {
-					finalError = errMsg
-				} else {
-					finalError = "turn was aborted"
-				}
-			default:
-				if errMsg := c.getTurnError(); errMsg != "" {
-					finalStatus = "failed"
-					finalError = errMsg
-				}
-			}
-		}
 		finishRunContextDone := func() {
 			waitingForTurn = false
 			if runCtx.Err() == context.DeadlineExceeded {
@@ -1434,6 +1439,11 @@ func (c *codexClient) request(ctx context.Context, method string, params any) (j
 	case res := <-pr.ch:
 		return res.result, res.err
 	case <-processDone:
+		select {
+		case res := <-pr.ch:
+			return res.result, res.err
+		default:
+		}
 		c.mu.Lock()
 		delete(c.pending, id)
 		err := c.processErr

server/pkg/agent/codex_test.go

@@ -62,6 +62,19 @@ func (f *fakeStdin) Lines() []string {
 	return lines
 }
 
+type fakeStdinWithHook struct {
+	fakeStdin
+	afterWrite func()
+}
+
+func (f *fakeStdinWithHook) Write(p []byte) (int, error) {
+	n, err := f.fakeStdin.Write(p)
+	if f.afterWrite != nil {
+		f.afterWrite()
+	}
+	return n, err
+}
+
 func splitLines(s string) []string {
 	var lines []string
 	start := 0
@@ -913,6 +926,31 @@ func TestCodexRequestPrefersContextCancellationOverProcessExit(t *testing.T) {
 	}
 }
 
+func TestCodexRequestPrefersReadyResponseOverProcessExit(t *testing.T) {
+	t.Parallel()
+
+	var c *codexClient
+	fs := &fakeStdinWithHook{}
+	fs.afterWrite = func() {
+		c.handleLine(`{"jsonrpc":"2.0","id":1,"result":{"ok":true}}`)
+		c.markProcessExited(errCodexProcessExited)
+	}
+	c = &codexClient{
+		cfg:         Config{Logger: slog.Default()},
+		stdin:       fs,
+		pending:     make(map[int]*pendingRPC),
+		processDone: make(chan struct{}),
+	}
+
+	result, err := c.request(context.Background(), "thread/start", map[string]any{})
+	if err != nil {
+		t.Fatalf("request error = %v, want ready response", err)
+	}
+	if string(result) != `{"ok":true}` {
+		t.Fatalf("response = %s, want {\"ok\":true}", result)
+	}
+}
+
 func TestCodexHandleInvalidJSON(t *testing.T) {
 	t.Parallel()
 
@@ -1880,6 +1918,35 @@ func TestCodexExecuteSemanticInactivityDoesNotAffectNormalTurnCompletion(t *test
 	}
 }
 
+func TestCodexExecuteTurnCompletionCanPrecedeTurnStartResponse(t *testing.T) {
+	t.Parallel()
+	if runtime.GOOS == "windows" {
+		t.Skip("shell-script fixture is POSIX-only")
+	}
+
+	fakePath := writeFakeCodexAppServer(t, ""+
+		`read line`+"\n"+
+		`echo '{"jsonrpc":"2.0","id":1,"result":{}}'`+"\n"+
+		`read line`+"\n"+
+		`read line`+"\n"+
+		`echo '{"jsonrpc":"2.0","id":2,"result":{"thread":{"id":"thr-reordered"}}}'`+"\n"+
+		`read line`+"\n"+
+		`echo '{"jsonrpc":"2.0","method":"turn/started","params":{"threadId":"thr-reordered","turn":{"id":"turn-reordered"}}}'`+"\n"+
+		`echo '{"jsonrpc":"2.0","method":"item/completed","params":{"threadId":"thr-reordered","item":{"type":"agentMessage","id":"msg-1","text":"Done"}}}'`+"\n"+
+		`echo '{"jsonrpc":"2.0","method":"turn/completed","params":{"threadId":"thr-reordered","turn":{"id":"turn-reordered","status":"completed"}}}'`+"\n")
+
+	result := executeFakeCodex(t, fakePath, ExecOptions{
+		Timeout:                   5 * time.Second,
+		SemanticInactivityTimeout: 100 * time.Millisecond,
+	})
+	if result.Status != "completed" {
+		t.Fatalf("expected completed, got status=%q error=%q", result.Status, result.Error)
+	}
+	if result.Output != "Done" {
+		t.Fatalf("expected output Done, got %q", result.Output)
+	}
+}
+
 func writeFakeCodexAppServer(t *testing.T, body string) string {
 	t.Helper()
 	fakePath := filepath.Join(t.TempDir(), "codex")