Repository navigation
fix(app): refresh Home sessions across servers - #63859
limjonathan wants to merge 1 commit 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 fix works. The new regression test fails on the base (no refresh request is ever sent) and passes with this PR. App unit tests for home/sessions and tsc -b pass. I did not run the real two-server desktop setup.
Public API: unchanged.
Two concerns, both inline: the interval refreshes even when the list is fresh or the window is hidden, and the test sets up its own request mocking instead of using the shared harness.
Related: #45549
| ) | ||
| } | ||
|
|
||
| const refreshTimer = setInterval(() => { |
There was a problem hiding this comment.
Each tick fetches up to 500 root sessions (loadHomeSessionIndex), and this fires every 30s whatever state the list is in:
- It ignores
target.stale, so a tick right after a fresh load still refetches. - It keeps running while the window or tab is hidden or the desktop app is minimized, as long as Home is mounted, which means continuous background polling against local and remote servers.
Could you only dispatch when target.stale is true and document.visibilityState === "visible"? Refreshing on window focus / visibilitychange would then cover the "I come back and it's stale" case at once. shell/titlebar/recent-sessions.tsx already does this, and it would also cover what #55301 proposes.
| await expect(row(page, "Already listed")).toBeVisible() | ||
| await expect(row(page, added.title)).toHaveCount(0) | ||
|
|
||
| await page.route(/\/api\/session\?/, async (route) => { |
There was a problem hiding this comment.
packages/app/e2e/AGENTS.md asks specs not to write their own routing. mockWorkspace returns the same sessions array that the mock server's GET /api/session handler reads on every request. So this could be just workspace.sessions.push(added) before fastForward, without the custom page.route, the currentSession import or the listRequests counter. Asserting that the row appears is enough.
Closes #55098
Summary
session.created.Verification
#62298 and #60233 were reproduced against current
devand are already covered by existing fixes, so this PR does not duplicate them.Added by the opencode bot (AI-generated).
Related: #45549