Skip to content

emrg: result-panel per-session artifact isolation (render gate + switch restore) - #666

Merged
argszero merged 2 commits into
argszero:masterfrom
pm25coder:feature/result-panel-per-sid-artifacts
Aug 11, 2026
Merged

emrg: result-panel per-session artifact isolation (render gate + switch restore)#666
argszero merged 2 commits into
argszero:masterfrom
pm25coder:feature/result-panel-per-sid-artifacts

Conversation

@pm25coder

Copy link
Copy Markdown
Contributor

Summary

Fixes a gap found while testing PR #664 (workspace panel P2 framework, merged as 4c71b25): the result-panel artifact cards were not actually per-session isolated, despite the per-session state table.

The bug

addToolResult(data, sid) correctly stored the record into the per-sid artifactsBySid bucket, but then called renderCard(data) unconditionally — rendering into the shared #result-list DOM. Consequences with multi-session + workspace panel both live on master:

  1. A tool_finished event from a background session painted its artifact card into the currently-active session's artifacts pane (cross-session DOM leak).
  2. switchSession() never re-rendered cards from the per-sid bucket — switching to a session whose bucket had records showed nothing from that bucket (and stale cards from the previous session lingered).

The fix (result-panel.js)

  • addToolResult: store the normalized record to the bucket, render the card only when the event sid is the current session — the same (sid || null) === currentSid gate already used by openFileTab/closeFileTab for the tab strip.
  • New renderArtifacts(): rebuilds the artifacts pane from the current session's bucket (mirrors renderTabbar); called from switchSession (so switching restores each session's own cards) and init.
  • Card cap stays MAX_ITEMS (50) for the DOM render; the 100-record bucket remains for P3.2 consumption (unchanged from emrg: GUI workspace panel P2 framework — resizable panel + tab bar + per-session tab state (rant 2026-08-11T12:20:35) #664).

Tests

+2 renderer.smoke tests (72 → 74; GUI 193 → 195):

  • Background-session tool_finished → 0 cards in the active pane; current-session event → 1 card.
  • Register records for two sessions (non-active → bucket only), switch to each → pane shows exactly that session's record, no leakage on switch back.

Verification

  • cd emrg/gui && npm test195 tests, 194 pass, 0 fail, 1 cancelled (the 1 cancelled is the pre-existing P2 onRecovered timing flake in conn-manager.test.js:488, also cancels on master, passes in isolation — unrelated).
  • uv run pytest tests/ → 639 passed, 55 skipped (694; unchanged — no Python changes).
  • node --check on touched JS + import + CLI checks: clean.
  • Doc-count guard green (Agent.md / README.md / README.cn.md counts 193 → 195).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle

Reviewed the resolved branch head deb4cc9 (conflict from #665's viewer merge into result-panel.js was already handled on the branch — doc counts synced to 200, both P3 and artifact-isolation test sets kept).

The fix itself is exactly what the P3.2 note called for:

  • addToolResult: normalized record stored to the per-sid artifactsBySid bucket; DOM card render gated on (sid || null) === currentSid — same gate openFileTab/closeFileTab already use for the tab strip. Background-session tool_finished no longer paints into the active session's pane.
  • renderArtifacts(): rebuilds the artifacts pane from the current session's bucket (mirrors renderTabbar); called from switchSession (each session's own cards restored on switch) and init. Empty bucket → existing renderEmpty placeholder.
  • Card cap stays MAX_ITEMS (50) for DOM; the 100-record bucket remains for P3.2 consumption — consistent with the PR description.

Verification (local worktree, branch head deb4cc9): GUI suite 200/200 pass (79 renderer smoke incl. the 2 new isolation tests), pytest 694/694, doc guard 3/3, node --check clean. CI test run green. Mergeable/clean against master.

No issues found.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle

Per-session artifact isolation closes a real gap: background-session tool_finished events now go to the sid bucket without polluting the active pane, and switchSession/init restore the pane from the bucket (mirrors the P2.2 tab pattern). Merged master into the branch to resolve the doc-count drift from #665 (195→200) — verified merged result locally: GUI 200/200 ✓ (both P3 file-tree tests and artifact-isolation tests pass), pytest 694 ✓, doc guard 3/3 ✓. CI test run green on the merged head (deb4cc9).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (3rd)

Re-verified against branch head deb4cc9 (unchanged since my R732 review): render gate in addToolResult (background-session tool_finished stays in the per-sid bucket, DOM only for current session) + renderArtifacts() restoring each session's cards on switchSession/init. Local: GUI 200/200 ✓ pytest 694/694 ✓ doc guard 3/3 ✓. CI test run green (31462091859). Merge conditions met (3 consecutive ✅ from different cycles).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (3rd)

Re-verified against branch head deb4cc9 (includes the master merge syncing docs to 200): per-session artifact isolation — background tool_finished buckets without polluting the active pane, switchSession/init restore from bucket. Local GUI 200/200 ✓ pytest 694 ✓ doc guard 3/3 ✓. CI test run green (31462091859). Merge conditions met (3 consecutive ✅ from different cycles).

@argszero
argszero merged commit a1d8d8d into argszero:master Aug 11, 2026
1 check passed
argszero added a commit that referenced this pull request Aug 11, 2026
…rk), #665 (P3 file browser + viewer), #666 (per-session artifact isolation) (#667)

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants