fix(site/src/pages/WorkspacesPage): show only parent agent apps in workspaces table#26568
Conversation
…rkspaces table The workspaces table shortcuts row selected resources[0].agents[0], so a sub-agent that ended up first (for example the Claude/Task sub-agent created on a workspace) could replace the parent agent's launcher icons, and the choice depended on agent ordering. Select the parent agent (parent_id === null) of the first non-hidden resource instead, matching the convention already used on the workspace detail page. This keeps the shortcuts row deterministic and excludes sub-agent apps.
|
/coder-agents-review |
|
Chat: Review posted | View chat Review history
deep-review v0.9.0 | Round 1 | Last posted: Round 1, 3 findings (2 Nit, 1 Note), COMMENT. Review Finding inventoryFinding inventoryFindings
Round logRound 1Panel (12 reviewers: Bisky, Hisoka, Mafu-san, Mafuuu, Pariston, Ging-ts, Ging-react, Gon, Leorio, Nami, Komugi, Meruem). Netero: no findings. 2 Nit, 1 Note posted. 2 dropped. Reviewed against 335d6bd..a455723. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
There was a problem hiding this comment.
Clean, well-scoped fix. The one-line production change (.at(0) to .find(a => a.parent_id === null)) replaces positional agent access with semantic selection, matching the existing convention in Workspace.tsx:218. The Storybook story is genuine: sub-agent listed first, parent second, play function asserts the parent's app renders and the sub-agent's does not. 87% test density on a 9-line production change.
Pariston tried to construct a scenario where this fix silently regresses (all-sub-agent resource, empty agents), and confirmed the null guard on line 672 handles every case. Meruem independently verified that sub-agents always share a resource with their parent (agentapi/subagent.go:156), so a resource with agents always has at least one parent. Hisoka confirmed parent_id is string | null (never undefined) in the generated types, so strict === null is safe.
Severity summary: 2 Nit, 1 Note. No correctness, security, or performance concerns.
Mafu-san flagged for visibility that sibling code paths use positional agent indexing (getMatchingAgentOrFirst in workspace.tsx:263, chatHelpers.ts:122). Those serve different use cases (terminal connection, chat agent selection) and are out of scope per DEVEX-459. Noting in case they need the same treatment later.
"The old comment was speculative, 'Lets test this getting the apps...and see what customers...think about that,' like a doctor scribbling 'tried something, let's see if the patient lives.'" (Leorio)
site/src/pages/WorkspacesPage/WorkspacesTable.tsx:655-667
Note [CRF-3] The first paragraph ("Coder is pretty flexible...") is background context that could be distilled. The essential information, first visible resource plus parent agent selection, starts at line 662. Gon proposed:
/**
* Most workspaces have one compute resource with one agent. We take the
* first visible resource (sorted compute-first) and its parent agent
* (`parent_id === null`) so sub-agent apps don't replace the parent's
* depending on ordering.
*/Not wrong as-is; noting for author judgment.
(Gon)
🤖
🤖 This review was automatically generated with Coder Agents.
| * | ||
| * For multi-agent workspaces we show the apps from the parent agent, the one | ||
| * without a `parent_id`, instead of whichever agent happens to be discovered | ||
| * first. Sub-agents, such as those created by devcontainers, are skipped so |
There was a problem hiding this comment.
Nit [CRF-1] "their apps do not replace the parent agent's apps depending on ordering" parses two ways: (1) the ordering-dependent replacement no longer happens, or (2) whether replacement happens depends on ordering. The follow-up sentence resolves it, but a reader shouldn't have to read ahead to parse one sentence.
Suggestion:
* Sub-agents, such as those created by devcontainers, are skipped so
* agent ordering does not determine which apps appear.
(Leorio)
🤖
| * their apps do not replace the parent agent's apps depending on ordering. | ||
| * This keeps the shortcuts row deterministic. | ||
| */ | ||
| const agent = workspace.latest_build.resources |
There was a problem hiding this comment.
Nit [CRF-2] agent is now selected by parent_id === null, not by position. Renaming to parentAgent would carry the selection semantics to downstream uses (parentAgent.display_apps, parentAgent.apps) where the .find predicate is no longer visible.
(Gon)
🤖
| * For multi-agent workspaces we show the apps from the parent agent, the one | ||
| * without a `parent_id`, instead of whichever agent happens to be discovered | ||
| * first. Sub-agents, such as those created by devcontainers, are skipped so |
There was a problem hiding this comment.
This is only valid for multi-agent workspaces with child agents. There are cases when there could be multiple parent-level agents.
The workspaces table shortcuts row selected
resources[0].agents[0], so a sub-agent that ended up first (for example the Claude/Task sub-agent created on a workspace) could replace the parent agent's launcher icons, and which apps showed depended on agent ordering.Select the parent agent (
parent_id === null) of the first non-hidden resource instead, matching the convention already used on the workspace detail page (Workspace.tsx). This keeps the shortcuts row deterministic and excludes sub-agent apps.Refs DEVEX-459
Decision context and scope
Per the discussion on DEVEX-459, this is the agreed short-term fix:
Out of scope (tracked as a longer-term backlog item on DEVEX-459):
WORKSPACE_APPS_SLOTS).For workspaces with multiple top-level agents, the first parent agent's apps are shown. This is deterministic but not aggregated.
A
ParentAgentAppsStorybook story was added (sub-agent listed first) with aplayfunction asserting the parent agent's app renders and the sub-agent's app does not.This PR was created by Coder Agents on behalf of @uzair-coder07.