[fix] Session lists only show rows they can open - #5993
Conversation
dosubot
Bot
added
the
size:XL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR documents workflow-reference propagation and session persistence. It adds keyed session-reference types, centralizes agent resolution, filters unopenable sessions, updates list hooks, and adds unit tests. ChangesSession attribution and openability
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Railway Preview Environment
Updated at 2026-08-12T20:10:37.430Z |
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Inline notes to explain the intent behind each part of this change. They are explanations for the reviewer, not change requests.
| if (workflow?.id) return workflow.id | ||
| if (usable.some((ref) => Boolean(ref.key))) return null | ||
| return usable[0]?.id ?? null | ||
| } |
There was a problem hiding this comment.
This decides where a row opens, and it is the only place that decides it now.
The references arrive as a flat list, one element per member of the workflow family, each naming its member in key. Only the workflow element holds an app id. A variant or revision id sends the playground to a route that does not exist. So the code prefers the workflow element.
| const usable = (row.references ?? []).filter((ref) => ref.id && isValidUUID(ref.id)) | ||
| const workflow = usable.find((ref) => ref.key === "workflow") | ||
| if (workflow?.id) return workflow.id | ||
| if (usable.some((ref) => Boolean(ref.key))) return null |
There was a problem hiding this comment.
This line is why a keyed row without a workflow element returns null instead of an id.
If any element carries a key, the writer labelled the family. A missing workflow element then means the row genuinely has no app id, and returning the first uuid would send the user to a dead route. Rendering the row inert is the correct outcome.
Note that ANY non-empty key counts as keyed, not only the three we act on. A key this code does not recognise still proves the writer labelled the family. Treating it as unkeyed would drop through to the old guess.
| const workflow = usable.find((ref) => ref.key === "workflow") | ||
| if (workflow?.id) return workflow.id | ||
| if (usable.some((ref) => Boolean(ref.key))) return null | ||
| return usable[0]?.id ?? null |
There was a problem hiding this comment.
The old heuristic survives, but only for rows that carry no key at all. Those rows were written before the runner stamped the family, and the first uuid is the only thing on offer for them. Removing this line would hide history that opens correctly today.
| // Which family member this id is. Absent on rows written before the runner stamped it; open | ||
| // string, see `SessionReferenceKey`. `.catch(undefined)` keeps a non-string from failing the | ||
| // whole page's parse. | ||
| key: z.string().nullish().catch(undefined), |
There was a problem hiding this comment.
key is parsed as an open string, not as an enum of the three known values.
The backend stores reference keys permissively, so a future family name can reach the frontend. If this schema narrowed an unrecognised key to undefined, the element would read as unkeyed. The row would then fall back to the first-uuid guess and its dead route, which is what this stack is removing.
.catch(undefined) keeps a non-string value from failing the whole page's parse.
| export const sessionGroupRows = <T extends SessionStream>( | ||
| group: "main" | "pinned" | "waiting", | ||
| rows: readonly T[], | ||
| ): T[] => (group === "main" ? openableSessions(startedSessions(rows)) : [...rows]) |
There was a problem hiding this comment.
One function now decides what each group shows.
main is browsing, so it hides chats nobody started and rows with nowhere to open. pinned and waiting apply neither rule. Both hold rows someone asked for by name, where a missing row reads as a fault rather than as tidiness.
Concretely: a pin that silently disappears reads as data loss, and hiding an approval-gated row would ask its owner to approve a tool call they cannot see. An inert row is the lesser harm in both cases.
|
|
||
| /** The automation carve-out, on the same two signals `isStartedSession` accepts. */ | ||
| const hasAutomationIdentity = (row: SessionStream): boolean => | ||
| isAutomationSession(row) || Boolean(row.trigger) |
There was a problem hiding this comment.
The automation carve-out, on the same two signals isStartedSession already accepts.
A trigger row IS its schedule. It has a name, an identity and its own menu before its first turn ever lands. Filtering it out on openability would blank the automations list.
| */ | ||
| export function sessionOpenTarget(row: SessionStream): PendingSessionOpen | null { | ||
| const appId = row.references?.find((ref) => ref.id && isValidUUID(ref.id))?.id | ||
| const appId = sessionAgentId(row) |
There was a problem hiding this comment.
This used to run its own "first valid uuid" search. It now calls sessionAgentId, so the list row and the open target cannot disagree about which reference is the agent.
|
@coderabbitai review |
✅ Action performedReview finished.
|
4075881 to
3439b9e
Compare
182bc36 to
ef48d73
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 27515e13-0aef-4319-b4c2-c141ad98264c
📒 Files selected for processing (20)
docs/design/agent-workflows/interfaces/cross-service/service-to-agent-runner.mddocs/design/agent-workflows/projects/build-kit-tools-cleanup/research.mddocs/design/agent-workflows/projects/sessions-takeover/architecture.mddocs/design/agenta-sessions-ux/plan.mddocs/design/untitled-sessions-investigation/HANDOFF.mddocs/design/untitled-sessions-investigation/findings.mddocs/design/untitled-sessions-investigation/plan.mdweb/oss/src/components/AgentChatSlice/assets/sessionOpenTarget.test.tsweb/oss/src/components/AgentChatSlice/assets/sessionOpenTarget.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-sessions/src/row/index.tsweb/packages/agenta-sessions/src/row/sessionAgent.tsweb/packages/agenta-sessions/src/row/viewModel.tsweb/packages/agenta-sessions/src/state/index.tsweb/packages/agenta-sessions/src/state/sessionListPolicy.tsweb/packages/agenta-sessions/src/state/useSessionCardList.tsweb/packages/agenta-sessions/src/state/useSessionsList.tsweb/packages/agenta-sessions/tests/unit/sessionAgent.test.tsweb/packages/agenta-sessions/tests/unit/sessionListPolicy.test.ts
A session row was shown if it had references OR a name OR a last message, but was openable only when an agent id could be derived from its references, so headless sessions rendered and swallowed the click (or navigated to /apps/<variant-id>/playground, a dead route, when the first UUID was a variant). sessionAgentId now prefers the reference with key=workflow, treats any keyed family without one as unopenable instead of guessing, and keeps the first-UUID fallback only for legacy untagged rows. The main lists on the sessions page and home cards drop unopenable rows like the sidebar always did; pinned and waiting groups stay visible (a vanishing pin reads as data loss, and a hidden waiting row would ask users to approve a tool call they cannot see); automations keep their carve-out. Includes the docs sync for the new wire fields and the untitled-sessions investigation workspace.
3439b9e to
06477eb
Compare
ef48d73 to
e83347e
Compare
1 participant
Context
This is the top PR of the untitled-sessions stack. It sits on #5991 and on the runner PR #5992.
What the user sees. The session lists show rows that they cannot open. Two cases:
/apps/<variant-id>/playground, which is not a real route.Why it happens. The rule for showing a row and the rule for opening it disagreed. A row was shown if it had references, or a name, or a last message, or a trigger. But opening it needed an agent id taken from the references. The sidebar already dropped such rows. The sessions page and the home cards did not.
Changes
The open target follows the key
sessionAgentIdnow prefers the reference element whosekeyis"workflow". A keyed family with no workflow element resolves to null, so the code no longer guesses. The old "first UUID wins" fallback stays, but only for old rows that carry no key at all.The wire
keyis parsed as an open string, not as a fixed set of values. If the backend adds a new family name later, such a row reads as "cannot open". That is safer than falling back to the guess and its dead route.One rule for what each group shows
sessionGroupRows(group, rows)insessionListPolicy.tsnow decides this in one place.Pinned rows stay, because a pin that silently disappears reads as data loss. Waiting rows stay, because hiding one would ask the user to approve a tool call they cannot see.
Automations keep their existing exemption. A scheduled run has a row before its first turn lands, and that row must still show.
No copy changed anywhere. An unopenable pinned row renders inert, exactly as it does today.
Known divergence, and it predates this PR. The sidebar drops unopenable rows in every group, pinned included. This PR leaves the sidebar alone.
This PR also carries the documentation for the stack's wire changes. That is the sessions-takeover architecture page, the runner interface page, and two superseded design notes. It also carries the investigation workspace
docs/design/untitled-sessions-investigation/, which holds the findings, the plan and the handoff.Tests
@agenta/sessionstests, 974@agenta/entitiestests and 396 app-level tests pass.tsc --noEmitis clean. Scoped lint changed nothing.What to QA