Repository navigation
feat(app): pin Home sessions - #61647
s63388951-design wants to merge 3 commits into
Conversation
github-actions
Bot
added
the
needs:compliance
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach looks sound, but there's one pin-state problem worth fixing before merge. I checked it by reading the code and by running the src/home/sessions unit tests (26 pass) and tsc -b for packages/app (clean). I couldn't run the UI or the new Playwright test here (no browser), so a short before/after screenshot or video of Home with a pinned session would help.
Public API: unchanged. Pins are client-only state in a new persisted key, home.sessions.pinned.
The concern: the pin store is created once in the Home controller and again in the recently-closed-tabs menu. Their copies don't sync with each other (see the inline comment).
| return { | ||
| ...homeSessionsQuery(conn, ctx), | ||
| enabled: state.open && recent().length < 10 && ctx?.sdk.connection.status() === "connected", | ||
| ...homeSessionsQuery(conn, ctx, () => new Set(pins.ids(ServerConnection.key(conn)))), |
There was a problem hiding this comment.
This creates a second persisted(Persist.global("home.sessions.pinned")) copy, separate from the one in the Home controller. Persist.global doesn't set sync, so the two copies never share updates. This one lives in the titlebar for the whole session and keeps the pins it read when it started.
When the menu's fetch runs, it writes the same ["home-sessions", conn] cache entry with that stale keep set. A session pinned since then, and later pushed past the 64-row window, gets dropped from Home until the controller fetches again. Separate desktop windows have the same problem: each window writes its own full pinned record, so one window's pin can wipe out another's.
Possible fixes: share a single pin store (a context or provider, or sync: true on the target), or have the menu not depend on pins at all.
There was a problem hiding this comment.
The new commits add tab-strip pinning and then revert it, so the code is the same as at my last review. The point about the recently-closed-tabs menu keeping its own copy of the pin store, which doesn't sync with Home's, is still open. Public API: unchanged.
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 72-hour window. Feel free to open a new pull request that follows our guidelines. |
github-actions
Bot
removed
the
needs:compliance
Summary
Adds a per-session Pin to the desktop/web Home session list. Pinned sessions appear in a dedicated
Pinnedgroup at the top, with a pin icon and a quiet accent bar. Pinning is client UI state, kept per server; it does not mutate session data or server-side ordering.Changes
home/sessions/pins.tspersisted store:Persist.global("home.sessions.pinned"), keyed byServerConnection.key.drafts.sqlitestatetable.localStorage(opencode.global.dat:home.sessions.pinned).Pinnedgroup;isPinned/togglePin;keepset threaded throughhomeSessionsQuery,loadHomeSessionIndex, andretainHomeSessions, so a pinned session survives the per-directory 64-row window.Pin/Unpincontext-menu item;data-pinnedattribute and left accent bar;Pinnedgroup header with pin icon.home.sessions.group.pinned,common.pin,common.unpin.pinicon to the UI artwork catalog.Verification
bun typecheck: no type errors in 1,896 files.packages/apphome session unit tests: 26 pass.packages/app/e2e/regression/home.spec.ts: 15 passed.data-pinned; reload → still pinned; unpin → group removed.home-session-index-benchmarkrun before/after. The no-pin path keeps the originalslice/sortedfast paths; no structural regression observed. Benchmark numbers vary on this machine.Notes
Added by the opencode bot (AI-generated).
Fixes #55764
Fixes #59906
Related: #59220