feat(web): support project and global pinned sessions - #1115
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Grouped sidebar can still show unpinned sessions ahead of pinned sessions — the new sort promotes any project/machine that contains a pin, but the render still prints every session in that project before moving to the next project. With project A containing
[pinned, unpinned]and project B containing[pinned], A's unpinned row appears above B's pinned row, so the default grouped sidebar does not consistently keep pinned sessions ahead. Evidence:web/src/components/SessionList.tsx:276andweb/src/components/SessionList.tsx:1347.
Suggested fix:const pinned = Boolean(session.pinned) const key = `${machineId ?? UNKNOWN_MACHINE_ID}::${pinned ? 'pinned' : 'unpinned'}::${path}` // Each generated group now contains only pinned or only unpinned rows, so // the existing group/machine sorts can keep all pinned buckets ahead. const hasPinnedSession = group.sessions.every((session) => session.pinned)
Summary
- Review mode: initial
- Found one ordering regression in the grouped session list. Residual risk: I did not run the full test suite; focused review on the PR diff and surrounding code.
Testing
- Not run (automation); inspected diff and ran
git diff --check.
HAPI Bot
There was a problem hiding this comment.
Findings
No findings.
Summary
Review mode: follow-up after new commits
No issues found in the latest full PR diff. Residual risk: I did not execute tests; review used PR metadata, full diff, previous bot review context, compare diff, and surrounding source inspection.
Testing
Not run (automation).
HAPI Bot
03d31bd to
919f370
Compare
There was a problem hiding this comment.
Findings
No findings.
Summary
Review mode: follow-up after new commits
No issues found in the latest full PR diff. Residual risk: I did not execute tests; review used PR metadata, full diff, previous bot review context, compare diff, and surrounding hub/shared/web source inspection.
Testing
Not run (automation; PR content not executed).
HAPI Bot
There was a problem hiding this comment.
Findings
No findings.
Summary
Review mode: follow-up after new commits
No issues found in the latest full PR diff. Residual risk: I did not execute tests; review used PR metadata, previous bot review context, compare diff, root/package docs, full diff, and surrounding hub/shared/web source inspection.
Testing
Not run (automation; PR content not executed).
HAPI Bot
c824d32 to
9cee6ec
Compare
There was a problem hiding this comment.
Review mode: follow-up after new commits. One Major issue found: pinned sessions can be dropped by the existing agentSessionId dedupe path, so the new pin state may never surface in the sidebar when duplicates exist. Residual risk: tests were not run here because bun is unavailable in this environment.
Testing: Not run (bun unavailable in this environment)
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Persistent pins are lost when duplicate/resumed sessions are merged; see inline comment.
Summary
Review mode: follow-up after new commits. One Major issue found: the new persisted pinned flag is not preserved by the existing session merge path, so pin state can be lost during dedup/resume flows.
Testing
Not run (automation; PR code not executed).
HAPI Bot
There was a problem hiding this comment.
Findings
None.
Questions
None.
Summary
Review mode: follow-up after new commits. No high-confidence issues found in the current diff. Prior merge-preservation finding appears addressed by hub/src/sync/sessionCache.ts:947 and regression coverage at hub/src/sync/sessionModel.test.ts:250. Residual risk: automation was not run here; UI pin ordering is covered by unit tests but not browser/E2E.
Testing
Not run (automation; PR code not executed).
HAPI Bot
4b1326d to
2b020dc
Compare
There was a problem hiding this comment.
Findings
None.
Questions
None.
Summary
Review mode: follow-up after new commits. No high-confidence issues found in the full current diff. The rebased v15-to-v16 migration, namespace guard, persisted pin state, merge preservation, REST/SSE propagation, and sidebar ordering paths were checked statically. Residual risk: cross-client pin propagation and project/machine promotion lack browser/E2E coverage.
Testing
Not run (automation; PR code not executed).
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Schema v16 bump leaves the migration suite expecting v15 —
SCHEMA_VERSIONis now 16, but the existing migration assertions were not advanced. The current requiredtestcheck fails seven hub tests withExpected: 15 / Received: 16, includinghub/src/store/migration-v13.test.ts:13andhub/src/store/migration-v15.test.ts:20. Evidence:hub/src/store/index.ts:32.
Suggested fix:const LATEST_SCHEMA_VERSION = 16 expect(getUserVersion(store)).toBe(LATEST_SCHEMA_VERSION)
- [Minor] Pinned empty stubs disappear after becoming inactive — the new action can pin any active session, but
prepareSidebarSessionslater filters inactive metadata-only stubs becauseshouldShowSessionInSidebarexempts only selected or active rows. The pin remains persisted, yet the row disappears after navigation and cannot be unpinned from the list. Evidence:web/src/components/SessionList.tsx:957, related filter atweb/src/components/SessionList.tsx:208.
Suggested fix:export function shouldShowSessionInSidebar(session: SessionSummary, selectedSessionId?: string | null): boolean { if (session.id === selectedSessionId) return true if (session.active || session.pinned) return true return !isSidebarEmptySessionStub(session) }
Questions
None.
Summary
Review mode: follow-up after new commits. Two issues found: one CI-blocking migration-test regression and one pinned-session visibility edge case. The compare from the prior bot-reviewed commit is merge-only with no file changes; the full current diff was re-reviewed.
Testing
Not run (automation; PR code not executed). GitHub Actions run 30504742098 currently reports 7 failed hub migration tests. Add a regression case asserting an inactive pinned stub remains in prepareSidebarSessions.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Schema v16 bump still leaves migration tests expecting v15 — constructing
Storenow advancesuser_versionto 16, while existing assertions remain at 15 inhub/src/store/migration-v13.test.ts:13andhub/src/store/migration-v15.test.ts:20. This deterministically breaks the migration suite. Evidence:hub/src/store/index.ts:32.
Suggested fix:const LATEST_SCHEMA_VERSION = 16 expect(getUserVersion(store)).toBe(LATEST_SCHEMA_VERSION)
- [Minor] Pinned empty stubs still disappear after becoming inactive — pinning is available while a stub is active, but the sidebar filter ignores
pinned; after the session stops and is no longer selected, its persistent pin and unpin affordance vanish. Evidence:web/src/components/SessionList.tsx:957, related filter atweb/src/components/SessionList.tsx:208.
Suggested fix:if (session.active || session.pinned) return true
Questions
- None.
Summary
- Review mode: follow-up after new commits. The commits after the prior HAPI Bot review are tree-identical retriggers, so both prior findings remain unresolved. No additional findings met the 80% confidence threshold.
Testing
- Not run (automation; PR code was not executed). Current GitHub checks are pending; static inspection confirms the schema-version assertions still mismatch. Add coverage for an inactive pinned empty stub in
prepareSidebarSessions.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Schema v16 bump leaves migration expectations at v15 — every
Storeinitialization now advancesuser_versionto 16, while the older migration suites still assert 15. The current CI run confirms all seven affected cases fail. Evidencehub/src/store/index.ts:32, related assertions athub/src/store/migration-v13.test.ts:13andhub/src/store/migration-v15.test.ts:20.
Suggested fix:const LATEST_SCHEMA_VERSION = 16 expect(getUserVersion(store)).toBe(LATEST_SCHEMA_VERSION)
- [Minor] Pinned empty stubs disappear after becoming inactive — the new action can persist a pin for an active metadata-only stub, but the sidebar filter only preserves active or selected rows. Once inactive and unselected, the row and its unpin affordance vanish. Evidence
web/src/components/SessionList.tsx:957, related filter atweb/src/components/SessionList.tsx:208.
Suggested fix:if (session.active || session.pinned) return true
Questions
- None.
Summary
- Review mode: follow-up after new commits. The compare from the prior reviewed commit contains no file changes, so both prior findings remain unresolved. No additional correctness, security, data-loss, performance, or maintainability findings met the 80% confidence threshold.
Testing
- Not run locally (automation; PR code was not executed). GitHub
testreports 679 passed, 7 failed; all failures are the stale schema-version assertions. Add coverage for an inactive pinned empty stub inprepareSidebarSessions.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Schema v16 bump leaves migration expectations at v15 — every
Storeinitialization now advancesuser_versionto 16, but the existing migration suites still assert 15. The current GitHubtestjob fails all seven affected cases. Evidencehub/src/store/index.ts:32, related assertions athub/src/store/migration-v13.test.ts:13andhub/src/store/migration-v15.test.ts:20.
Suggested fix:expect(getUserVersion(store)).toBe(16)
- [Minor] Pinned empty stubs disappear after becoming inactive — the new action can persist a pin for an active metadata-only stub, but the sidebar filter only preserves active or selected rows. Once inactive and unselected, the durable pin and its unpin affordance disappear from the sidebar. Evidence
web/src/components/SessionList.tsx:955, related filter atweb/src/components/SessionList.tsx:208.
Suggested fix:if (session.id === selectedSessionId) return true if (session.active || session.pinned) return true return !isSidebarEmptySessionStub(session)
Questions
- None.
Summary
Review mode: follow-up after new commits. The prior and current commits have identical trees, so both prior findings remain unresolved. No additional correctness, security, data-loss, performance, or maintainability findings met the 80% confidence threshold.
Testing
- Not run (automation; PR code was not executed).
- GitHub
test: failed with 7 stale schema-version assertions. - Missing coverage: inactive pinned empty stub in
prepareSidebarSessions.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Minor] Pin/unpin is missing from the selected-session header menu - the new action renders only when
onTogglePinis supplied, but the productionSessionHeadercaller does not pass pin state or a callback. The three-dot menu used in session/detail and files views therefore omits the feature; only the sidebar row context menu exposes it. Evidenceweb/src/components/SessionActionMenu.tsx:25, related callerweb/src/components/SessionHeader.tsx:367.
Suggested fix:const { archiveSession, reopenSession, renameSession, deleteSession, setPinned, isPending } = useSessionActions(api, session.id, session.metadata?.flavor ?? null) // Add to the existing SessionActionMenu: sessionPinned={session.pinned} onTogglePin={() => void setPinned(!session.pinned)}
Questions
- None.
Summary
Review mode: follow-up after new commits. The new commits resolve the prior schema-version test failures and inactive pinned-stub visibility issue. One remaining UI integration gap affects the header action menu; no additional correctness, security, data-loss, performance, or maintainability findings met the 80% confidence threshold.
Testing
- Not run (automation; PR code was not executed).
- GitHub
testworkflow: passed onfc62184. - Missing coverage:
SessionHeadershould expose and invoke Pin/Unpin.
HAPI Bot
There was a problem hiding this comment.
Findings
No findings.
Questions
None.
Summary
Review mode: follow-up after new commits.
The follow-up wires pinning into the selected-session header and adds regression coverage, resolving the prior HAPI Bot finding. No correctness, security, regression, data-loss, performance, or maintainability issues met the 80% confidence threshold. Residual risk: cross-client live propagation and grouped project/machine promotion still lack browser/E2E coverage.
Testing
- Not run (automation; PR code was not executed).
git diff --check: passed.- GitHub
test: passed on the prior reviewed commitfc62184; no completedtestcheck is attached to the current head yet.
HAPI Bot
ca184dd to
8d34929
Compare
There was a problem hiding this comment.
Findings
No findings.
Questions
None.
Summary
Review mode: follow-up after new commits.
No correctness, security, regression, data-loss, performance, or maintainability issue met the 80% confidence threshold in the current full diff. Residual risk: cross-client live pin propagation and project/machine promotion are covered through unit-level paths but not browser/E2E coverage.
Testing
- Not run locally (automation; PR code was not executed).
- GitHub test: passed on the current head.
- git diff --check: passed for the current PR diff.
HAPI Bot
8d34929 to
3db7edb
Compare
There was a problem hiding this comment.
Findings
- [Minor] Pin/unpin request failures are silently discarded — both new menu callbacks discard the rejecting
mutateAsyncpromise, while the app QueryClient has no global mutation error handler. On a network or HTTP failure, the menu closes, the state stays unchanged, and the user receives no feedback; the same issue exists inweb/src/components/SessionHeader.tsx:416. Evidence:web/src/components/SessionList.tsx:890
Suggested fix:const handleTogglePin = async () => { try { await setPinned(!s.pinned) } catch (error) { addToast({ title: t('session.action.pinFailed'), body: error instanceof Error ? error.message : t('dialog.error.default'), sessionId: s.id, url: '' }) } } onTogglePin={() => void handleTogglePin()}
Questions
None.
Summary
Review mode: follow-up after new commits.
One minor failure-handling issue found in the current full diff. No blocker or major correctness, security, regression, data-loss, or performance issue met the 80% confidence threshold. Residual testing gap: no rejection-path coverage for the new pin actions or browser/E2E coverage for cross-client pin propagation and pinned group ordering.
Testing
- Not run (automation; PR code was not executed).
- GitHub
test: passed on the current head. git diff --check: passed for the current PR diff.- Suggested: add list/header tests where
setSessionPinnedrejects and verify visible error handling without an unhandled rejection.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Minor] Restore LF line endings in the follow-up — the latest commits convert most lines in five web files to CRLF, so
git diff --checkreports every rewritten line as trailing whitespace and the prior-to-current compare expands to 8,197 lines. This obscures the actual pin-error fix and leaves the live head failing the repository mechanical diff check. Evidence:web/src/components/SessionHeader.test.tsx:1(alsoSessionHeader.tsx,SessionList.tsx,locales/en.ts, andlocales/zh-CN.ts).
Suggested fix:sed -i 's/\r$//' \ web/src/components/SessionHeader.test.tsx \ web/src/components/SessionHeader.tsx \ web/src/components/SessionList.tsx \ web/src/lib/locales/en.ts \ web/src/lib/locales/zh-CN.ts
Questions
None.
Summary
Review mode: follow-up after new commits.
One minor mechanical/maintainability issue found in the current full diff. The prior pin-mutation rejection issue is addressed by the new toast handling. No blocker or major correctness, security, regression, data-loss, or performance issue met the 80% confidence threshold. Residual testing gaps: no browser/E2E coverage for cross-client SSE pin propagation and grouped ordering, and no rejection-path test for the sidebar action.
Testing
- Not run (automation; PR code was not executed).
git diff --check 05ba050e..f9810f0: failed because of the CRLF conversions above.- GitHub
test: in progress at review time. - Suggested: normalize the five files to LF, rerun
git diff --check, and add a sidebar rejection-path test when touching this area again.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Minor] Normalize the five mixed-line-ending web files — the current head still changes almost every pre-existing line in these files from LF to CRLF while the newly added lines remain LF. This expands a 483-line semantic change into an 8,420-line textual diff and makes
git diff --checkfail on the rewritten lines, obscuring future review and violating the repository's mechanical pre-push check. Evidence:web/src/components/SessionHeader.test.tsx:1(alsoSessionHeader.tsx,SessionList.tsx,locales/en.ts, andlocales/zh-CN.ts).
Suggested fix:sed -i 's/\r$//' \ web/src/components/SessionHeader.test.tsx \ web/src/components/SessionHeader.tsx \ web/src/components/SessionList.tsx \ web/src/lib/locales/en.ts \ web/src/lib/locales/zh-CN.ts
Questions
None.
Summary
- Review mode: follow-up after new commits.
- The previous HAPI Bot finding remains unresolved. The prior-reviewed commit and current head have identical tree IDs, so this follow-up contains no content change despite the new commit SHA. No blocker or major correctness, security, regression, data-loss, performance, or maintainability issue met the 80% confidence threshold in the full substantive diff. Residual testing gaps: no focused web test covers cross-client SSE pin propagation or machine/project promotion after a pin update.
Testing
- Not run (automation; PR code was not executed).
- Read-only
git diff --check 05ba050e...f8880eb7: failed on the mixed CRLF/LF files above. - GitHub checks at review time: only
pr-reviewwas in progress; no test check was attached to the head SHA. - Suggested: normalize line endings, rerun
git diff --check, and add an SSE cache-update/group-ordering test when touching this area again.
HAPI Bot
f8880eb to
d965ee3
Compare
There was a problem hiding this comment.
Findings
- [Major] Restore the SessionList test render contract —
SessionItemnow unconditionally callsuseToast, but existing SessionList render harnesses do not provideToastProvider. The current GitHubtestcheck fails seven web tests at this line and stops before the shared suite. Evidence:web/src/components/SessionList.tsx:788.
Suggested fix:<QueryClientProvider client={client}> <ToastProvider> <I18nProvider> <SessionList {...props} /> </I18nProvider> </ToastProvider> </QueryClientProvider>
- [Minor] Treat an already-pinned merge target as success —
newStoredis captured before the attachment-transfer awaits, whilesetSessionPinnedreturnsfalsewhen another request has already set the target totrue. A concurrent pin can therefore make this branch throw after messages or scratchlist data have already moved, leaving the merge partially applied. Evidence:hub/src/sync/sessionCache.ts:1041; related predicatehub/src/store/sessions.ts:581.
Suggested fix:if (oldStored.pinned) { const latest = this.store.sessions.getSessionByNamespace(newSessionId, namespace) if (!latest) throw new Error('Session not found for merge') if (!latest.pinned) { const updated = this.store.sessions.setSessionPinned(newSessionId, true, namespace) const nowPinned = this.store.sessions.getSessionByNamespace(newSessionId, namespace)?.pinned === true if (!updated && !nowPinned) { throw new Error('Failed to preserve session pin during merge') } } }
Questions
None.
Summary
- Review mode: follow-up after new commits.
- The previous mixed-line-ending finding is resolved;
git diff --checkpasses on the current base/head. - Two current-head issues found. Residual testing gaps: no concurrent pin-during-merge test and no focused cross-client SSE/group-promotion test.
Testing
- Not run locally (automation; PR code was not executed).
- GitHub automation:
bun typecheckpassed; terminal-wrap E2E passed;bun run testfailed with 7 SessionList web tests becauseToastProvideris missing. The shared suite was not reached. - Read-only
git diff --check f018c602...d965ee3: passed.
HAPI Bot
Support mutually exclusive project vs global pins, surface both in the session action menu, and render globally pinned sessions in a top-level sidebar group with project path labels.
Keep global pins over project pins during consolidation so a project-pinned source cannot downgrade an already or concurrently global-pinned target. Co-authored-by: Cursor <cursoragent@cursor.com>
Pinned sessions should stay easy to find after reload; do not hide them behind the default inactive-group collapse. Co-authored-by: Cursor <cursoragent@cursor.com>
fd06296 to
367a218
Compare
There was a problem hiding this comment.
Findings
- [Major] Project-pinned active sessions leave their project group when “Pin in-progress sessions” is enabled — the running bucket skips only global pins, then the group filter removes every in-progress session. A project-pinned session that is thinking or awaiting input therefore moves into the top-level In progress section instead of staying in its machine → project hierarchy. Evidence
web/src/components/SessionList.tsx:1255.
Suggested fix:for (const session of machineFilteredSessions) { if (session.globalPinned || session.pinned) continue // existing in-progress classification } // Keep project pins in their directory group. if (pinInProgressSessions && !session.pinned && isPinnedInProgressSession(session)) { return false }
Questions
- None.
Summary
- Review mode: follow-up after new commits.
- One major issue found in the full latest diff: project pin mode is overridden by the optional In progress grouping for active sessions.
Testing
- Not run (automation; PR code was not executed under the review security policy).
- GitHub
testandpr-reviewchecks were pending when reviewed. - Add a SessionList regression test with
hapi-pin-in-progress-sessions=trueand an active, thinking, project-pinned session; assert it remains only in the project group.
HAPI Bot
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits.
No issues found at the latest head. The prior project-pin/In-progress regression is no longer present in the full diff. Residual risk: pin synchronization was reviewed statically; no dedicated multi-client SSE regression test was found.
Testing
- Not run (automation; PR code was not executed under the review security policy).
- GitHub
testandpr-reviewchecks were in progress at review time.
HAPI Bot
Merged tiann#1115 (3da9f77). Remove driver/web-pinned-sessions from the soup manifest; remat will take dual-pin (v22) from upstream/main. Co-authored-by: Cursor <cursoragent@cursor.com>
…nish Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
pinned+global_pinned).Implementation
main. Keep main's usage migrations at v20/v21; add pin columns via v21→v22 and bumpSCHEMA_VERSIONto 22.PUT /api/sessions/:id/pinto accept{ mode: 'none' | 'project' | 'global' }with shared Zod validation; sync pin changes over REST/SSE.global>project>none), including concurrent target updates during merge.Testing
testandpr-reviewchecks on tip (fd06296d)bun test hub/src/sync/sessionModel.test.ts -t pin(6 passed; merge/global precedence and race coverage)bun run test src/components/SessionList.directory-action.test.tsx -t project-pinnedfromweb(inactive project-pinned group stays expanded)UI
none.Compatibility
Closes #532.
Related to the pin/favorite portion of #477.
AI assistance
Developed with assistance from Cursor. Changes were reviewed locally and validated with the checks above.