emrg: fix GUI session cross-wiring / empty history (session-cwd resolution, ghost-session rejection, cwd-scoped broadcasts) - #977
Conversation
…wd, reject ghost sessions, cwd-scoped broadcasts)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260825-182441 (1/3)
Fresh adversarial review of the full diff:
Root cause 1 (P0, main.js) — resolveSessionCwd(sessionId) correctly replaces the hardcoded DEFAULT_CWD in every session-scoped IPC handler (listHistory/rewindSession/deleteSession/renameSession/clearSession/compactSession/listMemories/readMemory + reconnect restore). Resolution order (openSessions bookkeeping → global sessions_index.json, 3 levels up for home-level sessions → null) is sensible; switchSession dropping the homedir fallback routes unknown sessions to the daemon's not_found → existing fallback to the most recent session (verified: conn-manager passes null cwd → daemon rejects with 'requires session_id and cwd' → sendCommandAndWait rejects → switch fallback path).
Root cause 2 (P1, daemon) — _handle_resume_session now rejects missing dir/meta.json AND 0-message sessions at a cwd that differs from the canonical sessions-index path. Positive state (canonical cwd → real message_count) and negative state (wrong cwd → rejected + warning log) both verified by TestWSGhostSessionGuard; the _canonical_session_cwd helper correctly handles the unpatched emrg.sessions_index.config_dir in tests (patched + restored in the test).
Root cause 3 (P1, daemon broadcast) — _session_subscribers → {ws: cwd} map + _session_task_cwds task-cwd registry; _broadcast filters by matching task cwd, falls back to all subscribers when no task is running (non-stream frames). Wrapper registers/clears task cwd around _run_tool_loop; the done frame is emitted inside the loop (before pop) so it is also filtered. TestWSBroadcast::test_broadcast_cwd_filtered proves symmetry both directions.
Verification (local, against PR head f87c528):
- Regression tests fail on master (no fix) and pass with the PR — discriminating signal reliable
- Full suite: 1063 passed / 1 skipped; e2e ws 44/44
- GUI: node --check OK, npm test 252 pass / 0 fail
- CI: test ✅ + test-windows ✅ (run 32836521662)
- Agent.md doc count bumped 1062 → 1064 (collected count, matches the guard's
--collect-onlysemantics)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260825-183339 (2/3)
Independent re-review of PR head f87c528 (unchanged since 1/3):
Test quality check — examined the two regression tests line-by-line:
test_broadcast_cwd_filtered: covers both filter directions (A's task reaches only A; B's task at the other cwd reaches only B), proving symmetry and that the negative-side socket is alive (not merely disconnected). The_drainhelper empties pre-task frames so the negative asserts are unambiguous; fresh_make_fake_chat_stream()per task keeps the round counter deterministic.test_resume_rejects_ghost_at_wrong_cwd: three-state coverage — wrong-cwd ghost rejected, canonical cwd resumes with the real message_count (2), unknown session still not_found. Correctly patchesemrg.sessions_index.config_dir(the module the daemon's_canonical_session_cwdimports) with restore, and writes the index AFTER_boot_serverso the daemon's startup rebuild cannot prune it.
main.js memory handlers: listMemories/readMemory use resolveSessionCwd(sessionId || currentSessionId) with DEFAULT_CWD only as the final fallback for project-scope memory — session-history-critical paths never fall back to homedir.
Verification (this cycle, against f87c528):
- Regression tests: 2/2 PASSED
- Full suite: 1063 passed / 1 skipped
- GUI: node --check OK, npm test 252 pass / 0 fail
- CI: test ✅ + test-windows ✅ (run 32836521662), MERGEABLE/CLEAN
The three root causes are fixed with correct scope separation; the daemon-side broadcast filter and ghost-session rejection behave as documented in both positive and negative states.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260825-184143 (3/3)
Third independent review — merge gate satisfied (3 consecutive ✅ from cycles 182441/183339/184143, no ❌ in between).
This cycle's fresh verification:
- Head f87c528 unchanged since the 2/3 review (diff vs f87c528 empty)
- Regression tests: 2/2 PASSED (cwd-filtered broadcast + ghost-session rejection)
- Full suite: 1063 passed / 1 skipped
- GUI: node --check OK
- CI: test ✅ + test-windows ✅ (run 32836521662), MERGEABLE/CLEAN
The fix addresses all three root causes of the GUI session cross-wiring / empty-history bug with proper scope separation and regression coverage. Merging.
Uh oh!
There was an error while loading. Please reload this page.
Fix GUI session cross-wiring / empty history (three root causes)
Closes the GUI session bug where opening a session showed another session's content and many sessions loaded 0 messages (rant 2026-08-25T17:38:56).
Root cause 1 (P0) — hardcoded homedir cwd in session-scoped IPC handlers
emrg:listHistory(and related handlers) hardcodedcwd: DEFAULT_CWD(=os.homedir()). Every GUI session read its history from~/.emrg/sessions/<sid>/history.jsonlinstead of the real project dir → 0 messages / wrong session content.Fix: new
resolveSessionCwd(sessionId)helper resolves a session's canonical project cwd (openSessions bookkeeping first, then the globalsessions_index.json— session dir 3 levels up, covering home-level sessions). All session-scoped handlers (listHistory,rewindSession,deleteSession,renameSession,clearSession,compactSession,listMemories,readMemory, reconnect restore) now use it.switchSessiondrops theDEFAULT_CWDfallback entirely — unknown sessions go to the daemon'snot_foundand fall back to the most recent session, never silently re-creating ghost sessions at homedir.Root cause 2 (P1) — resume blessed 0-message ghost sessions
A wrong-cwd client caused a 0-message ghost session dir at the wrong project;
resume_sessionreported success (0 messages), masking the real session.Fix:
_handle_resume_sessionnow rejects when the session dir ormeta.jsonis missing, and whenmessage_count == 0at a cwd that differs from the session's canonical project cwd (global sessions index) — with a warning log.Root cause 3 (P1) — broadcasts keyed by session_id only
Subscribers were grouped by
session_idalone; a GUI ghost connection (wrong cwd) landed in the same group as the real TUI connection and received the real session's live stream.Fix:
_session_subscribersis nowsession_id → {ws: cwd}and a new_session_task_cwdsrecords each running task's cwd._broadcastfilters recipients by matching task cwd; non-task broadcasts (no running task) still reach all subscribers.Tests
TestWSBroadcast::test_broadcast_cwd_filtered— same session_id, two cwds: A's task stream reaches only A; B's later task (other cwd) reaches only B (symmetric, proves B is alive).TestWSGhostSessionGuard::test_resume_rejects_ghost_at_wrong_cwd— ghost (0-message) session at the wrong cwd is rejected; the canonical cwd still resumes with the real message count; unknown sessions still return not_found.npm test252 pass / 0 fail;node --check main.jsOK.