fix(web): hide sidebar fake sessions for Cursor resume/archive#836
Conversation
Wire deduplicateSessionsByAgentId into SessionList and resolve cursor threads via cursorSessionId so resume/archive no longer shows duplicate inactive rows for the same ACP session. Fixes tiann#833 Co-authored-by: Cursor <cursoragent@cursor.com>
SessionList only receives SessionSummary from the API; native ids like cursorSessionId are already mapped into metadata.agentSessionId by toSessionSummary. Drop resolveAgentSessionIdFromMetadata to fix typecheck. Co-authored-by: Cursor <cursoragent@cursor.com>
Scope correction + CI fixPushed a follow-up commit to fix typecheck and clarify what this PR actually fixes. What was wrong in the first commit
What this PR does now
What this PR does not fix (#833 follow-up)Live repro on test hub shows the main clutter is:
See comment on #833 for Repro A/B details. Recommend follow-up PR for hiding archived stubs / empty inactive rows. Related: #834 helps resume metadata timing; not sidebar volume. |
There was a problem hiding this comment.
Findings
- [Major] Dedupe can hide unrelated cross-flavor sessions —
SessionSummary.metadata.agentSessionIdis a flattened field, andtoSessionSummaryfills it by the first available native id (codexSessionId ?? claudeSessionId ?? ... ?? cursorSessionId) rather than the current flavor-specific id. Now thatSessionListalways renders throughdeduplicateSessionsByAgentId, a Cursor session carrying a stalecodexSessionIdcan collide with a real Codex session and disappear from the sidebar, even though hub resume/dedupe logic treats those fields as flavor-specific (resolveAgentResumeId,extractAgentSessionId). Evidence:web/src/components/SessionList.tsx:745, related contextshared/src/sessionSummary.ts:65andweb/src/lib/sessionResume.ts:13.
Suggested fix:function getSessionDedupKey(session: SessionSummary): string | null { const agentId = session.metadata?.agentSessionId if (!agentId) return null return `${session.metadata?.flavor ?? 'unknown'}:${agentId}` }
Summary
- Review mode: initial
- One Major issue found: the new rendered dedupe path should scope the flattened resume id by flavor, or the upstream summary should expose only the current flavor's native id before web-side dedupe uses it.
Testing
- Not run locally:
bunis not installed in this runner (/bin/bash: bun: command not found). Add/adjust aSessionList.test.tscase with a Codex row and a Cursor row sharing the same flattened/stale id to cover this regression.
HAPI Bot
| const allSessions = useMemo( | ||
| () => props.sessions, | ||
| [props.sessions] | ||
| () => deduplicateSessionsByAgentId(props.sessions, selectedSessionId), |
There was a problem hiding this comment.
[MAJOR] This now runs every rendered session through deduplicateSessionsByAgentId, but the helper keys only on flattened metadata.agentSessionId. toSessionSummary populates that field by first available native id (codexSessionId ?? claudeSessionId ?? ... ?? cursorSessionId), while resume logic is flavor-specific and already ignores stale cross-flavor ids. A Cursor row with a stale codexSessionId can therefore collide with and hide a real Codex row in the sidebar.
Suggested fix:
function getSessionDedupKey(session: SessionSummary): string | null {
const agentId = session.metadata?.agentSessionId
if (!agentId) return null
return `${session.metadata?.flavor ?? 'unknown'}:${agentId}`
}There was a problem hiding this comment.
Findings
- [Major] Sidebar dedupe can still hide unrelated cross-flavor sessions — the rendered list now flows through
prepareSidebarSessions, which still callsdeduplicateSessionsByAgentIdusing only the flattenedmetadata.agentSessionId.toSessionSummaryfills that value from the first native id present (codexSessionId ?? claudeSessionId ?? ... ?? cursorSessionId), but the hub/web resume paths resolve ids by current flavor, so a Cursor row that preserved a stale Codex id can collide with a real Codex row and disappear from the sidebar. Evidence:web/src/components/SessionList.tsx:773, related contextshared/src/sessionSummary.ts:66andweb/src/lib/sessionResume.ts:13.
Suggested fix:function getSessionDedupKey(session: SessionSummary): string | null { const agentId = session.metadata?.agentSessionId?.trim() if (!agentId) return null return `${session.metadata?.flavor ?? 'unknown'}:${agentId}` }
Summary
- Review mode: follow-up after new commits
- One unresolved Major issue remains from the full PR: sidebar dedupe should not use the unscoped flattened agent id as a global key.
Testing
- Not run (automation):
bunis unavailable in this runner (/bin/bash: bun: command not found). Add aSessionList.test.tscase with a Codex summary and a Cursor summary sharing the same stale flattened id to lock this down.
HAPI Bot
| const allSessions = useMemo( | ||
| () => props.sessions, | ||
| [props.sessions] | ||
| () => prepareSidebarSessions(props.sessions, selectedSessionId), |
There was a problem hiding this comment.
[MAJOR] prepareSidebarSessions still sends the rendered sidebar through deduplicateSessionsByAgentId, whose key is only metadata.agentSessionId. That field is flattened in toSessionSummary by first available native id, while resume logic is flavor-specific. If a Cursor session preserves a stale codexSessionId, it can collide with a real Codex session and be dropped from the sidebar.
Suggested fix:
function getSessionDedupKey(session: SessionSummary): string | null {
const agentId = session.metadata?.agentSessionId?.trim()
if (!agentId) return null
return `${session.metadata?.flavor ?? 'unknown'}:${agentId}`
}Prevent cross-flavor collisions when flattened agentSessionId retains a stale native id. Add regression test and relax claudeRemote CI timeout. Fixes tiann#833 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- No issues found.
Summary
- Review mode: follow-up after new commits
- Full diff reviewed. The previous Major around cross-flavor
agentSessionIddedupe is addressed by scoping the dedupe key by flavor and adding a regression case. - Residual risk: I could not independently run the test suite in this runner.
Testing
- Not run (automation):
bunis unavailable in this runner (/bin/bash: bun: command not found).
HAPI Bot
…nn#835 Revert agentCliGuard.ts + AcpStdioTransport.ts + agentCliGuard.test.ts to upstream/main. Strip the agent-acp-active lock pre-check from maybeAutoMigrateLegacyCursorSession + simplify its test suite to match. Per tiann#832 (filed by swear01), the assumption underpinning my v8 strict-mkdirSync refusal — that Cursor's `agent` binary SIGTERMs a second `agent acp` process on the same host — is incorrect. Two `agent acp` processes coexist fine. The lock's actual job is narrower: prevent `agent --list-models` from killing an active `agent acp` child. swear01's open PR tiann#835 ("fix(cursor): merge SKU catalog under ACP lock and refcount agent guard") refactors agent-acp-active from a single-PID file to a cross-process refcount with live-pid reconciliation. We expect tiann#835 to land before tiann#824 (this PR) and have added swear01's three open ACP mop-up PRs (tiann#835/tiann#836/tiann#837) to the local driver-manifest soup so the migrator is exercised against the eventual upstream shape. Net effect on tiann#824: * agent-acp-active lock concerns are someone else's problem now (the refcount lock in tiann#835 just works for our migrator flow). * Verify probe still runs in isolated HAPI_HOME (unchanged) — that isolation is independent of the host-side lock semantics and remains the correct safety boundary for the probe. * Auto-migrate trigger in resumeSession is unchanged behaviourally; only the now-obsolete lock pre-check is gone. 53/53 unit tests pass (cursorLegacyMigrator + syncEngineAutoMigrate). Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
prepareSidebarSessionsintoSessionList: dedupe bymetadata.agentSessionId, then hide inactive empty stubs (no agent id, no name/summary)lifecycleStateonSessionSummaryfor future sidebar rulesFixes #833
Test plan
SessionList.test.ts— dedup, stub filter, selected-stub visibilitysessionSummary.test.ts— lifecycleState mappingbun typecheckbun run test:web/bun run test:shared