[PR] fix(desktop): keep Workbar visibility until the first session list lands [maka]

["chihumyum (via GitHub)" <gi...@apache.org>]

chihumyum opened a new pull request, #5951:
URL: https://github.com/apache/maka/pull/5951

   ## Summary
   
   `e2e/session-workbar.spec.ts:179` ("Terminal survives navigation and reload…") is flaky on main. It always fails at line 224: after `page.reload()`, the owner Session's right Workbar comes back collapsed, so the terminal region never appears. This is a product bug, not a harness problem. A reload can drop every other Session's per-Session Workbar visibility.
   
   The cause is a race at startup. After a reload, a `sessions:changed` event from a Session whose turn is still finishing (the test does not wait for the replacement Session's turn to end) is read by the patch drain. `catalog.commitPatch` commits it before `bootstrapSessions()`'s `sessions.list()` does. `commitPatch` moves the catalog `revision` off 0, and `selectAuthoritativeSessionIds` read any `revision > 0` as a membership observation. It therefore published a one-row set. `useWorkbarLayoutState` dispatched `retain-sessions` with it. No Session is active yet at that point, so every other Session's `collapsedBySession` entry was dropped, and the right-visibility effect persisted the loss to `maka-session-workbar-collapsed-v2`. The rate went from 9/40 at 8ad836ce1 to 19/40 at 255ae23ae (Fisher p = 0.034) but the root cause is older. Row patches have been able to land before the list since #5532. The range 8ad836ce1..255ae23ae changes nothing on the catalog, Workbar layout, main, p
 reload or runtime path, so #5934/#5935 only moved the timing.
   
   The fix: the catalog now records `listed`, meaning a list read has committed. `selectAuthoritativeSessionIds` keys on it instead of `revision`. So does the `commitSessions` no-op check. Without that second change, a first list that only repeats rows patches already admitted would return early and never mark the catalog as listed. The E2E test is unchanged.
   
   Refs #5936 (where the flake was raised)
   
   ## Root cause evidence
   
   A diagnostic copy of the test, not committed, hooked `localStorage.setItem` and `sessions.subscribeChanges` through `context.addInitScript`. It ran 12 times on 8ad836ce1 and 12 times on 255ae23ae:
   
   | | first sidebar render after reload | `collapsed-v2` rewritten to the replacement Session only |
   |---|---|---|
   | 7 failing runs | 1 row (the patched replacement), full list 50–70 ms later | 7/7 |
   | 17 passing runs | 2 rows (the list) | 0/17 |
   
   ## Verification
   
   All Electron counts below were run on one machine, alternating builds in ABBA order, 8 rounds × `--repeat-each=5` per build. The command was `npx playwright test --config e2e/playwright.config.ts e2e/session-workbar.spec.ts:179`. Every failure was at line 224.
   
   | build | failed |
   |---|---|
   | 8ad836ce1 (before #5934/#5935) | 9/40 |
   | 255ae23ae (main after #5935) | 19/40 (p = 0.034 vs 8ad836ce1) |
   | 255ae23ae | 20/40 |
   | 255ae23ae + this fix | **0/40** (p = 7.8e-8) |
   | 229e1b466 (main after #5937) | 13/40 |
   | 229e1b466 + this fix | **0/40** (p = 7.6e-5) |
   
   - Two node-tier tests in `workbar-model.test.ts`. Each was mutation-checked: restoring `revision > 0` in the selector fails the first test, and restoring it in the `commitSessions` no-op check fails the second. Each mutation fails only its own test.
   - `npm --workspace @maka/desktop run test:dist`: 3208/3208 on 229e1b466
   - `typecheck` (preload/main/renderer/storybook), `npm run lint`, `check-renderer-architecture --base upstream/main --strict-base`, `check:app-shell-hooks`, `check:e2e-budget` (34 tests, unchanged), `check:asf-headers`: all pass.
   - Full Electron suite on 229e1b466 + this fix: 34/34
   
   ## Review focus
   
   `sessionCount` in AppShell (`authoritativeSessionIds?.size ?? 0`) now stays 0 until the first list commits, even if a row patch lands first. Before, it briefly read 1. It feeds `isOnboardingLoading`, `showOnboardingHero` and `getOnboardingActivationCandidate`. The list normally lands tens of milliseconds later, and an unhydrated catalog is exactly the state these readers already handle.
   
   ## AI use
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: Claude Code (Claude Opus 5.5) measured the flake, wrote the diagnostic, wrote the fix and tests, and ran the verification. The commit carries `Generated-by: Claude Opus 5.5`.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — described under Summary above
   - [ ] No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): keep Workbar visibility until the first session list lands [maka]

["Astro-Han (via GitHub)" <gi...@apache.org>]

Astro-Han merged PR #5951:
URL: https://github.com/apache/maka/pull/5951


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org