Repository navigation
fix(web): clear off-branch inspector selection - #333
Conversation
Fixes mikamikasuki#111. Preserve node drafts while reconciling branch and selected-node URL state. Assisted-by: OpenAI Codex
There was a problem hiding this comment.
🟢 Approval recommended
The change is a correct, narrowly scoped frontend selection fix that fully aligns inspector edit/run actions with the displayed branch, preserves unsaved drafts, and is backed by comprehensive real-backend browser regression coverage with no backend or data-integrity impact.
No open issues 🎉
What changed in this PR
This PR fixes a FOREST Workspace defect (issue #111) where changing the branch filter left the node inspector editing a node from the previous branch — a node absent from the displayed graph — while still exposing Save changes/edit_node for that off-branch node ID. The fix keeps the inspector and its edit/run actions aligned with the branch shown on the canvas, while preserving the user's unsaved per-node draft.
The change is well-scoped and correct. I traced the full selection/branch state path:
setBranchnow clears an off-branch selection in the same history push as the branch change and closes the inspector, while retaining the selection when switching to the node's own branch or to "All research paths" (graph.nodes.some(...)guard).- A new derivation makes the editable
noderesolve only whenbranch === "all"or the selected node is on the current branch; a guarduseEffectreconciles off-branch URLs that arrive via direct links/Focus/reload/history by removing thenodeparam withupdateQuery(..., replace=true). The effect is idempotent and cannot loop (clearingnodemakesselectedNodeundefined on the next render). - All edit/run entry points consistently use the guarded
node(inspector save at L1738, Focus editor at L1647, Run buttondisabled={!node}at L1303,run()if (!node) returnat L1097), so off-branch editing is fully closed off, not just hidden. - Draft restoration correctly reuses the existing per-node sessionStorage key (
forest-node-editor:${projectId}:${node.id}) withkey={node.id}remounting, so returning to the original node restores the unsaved draft. - The
setInspector(Boolean(selected))change (L766) also closes the inspector when selection is cleared; I verified Escape (L1169 closes inspector first) and the toggle/SSE-refresh paths do not regress.
The new Playwright spec exercises real temporary projects/forked branches against an isolated backend (no response mocks) across 1440/800/390px, verifying same-branch/All retention, off-branch removal, draft restoration, Back/Forward, mismatched Focus links, reload, and that the persisted graph is unchanged with zero browser-issued graph mutations.
Changes:
- Clear off-branch node selection and close the inspector on branch change; retain selection for same-branch/All.
- Guard the editable
nodeto on-branch selections and reconcile off-branch URLs in place via history replacement. - Add a cross-width Playwright regression covering selection retention, off-branch removal, draft restoration, and history/reload/Focus paths.
| File | Description |
|---|---|
| apps/web/src/Workspace.tsx | Adds replace support to updateQuery, clears off-branch selection in setBranch, derives the editable node and a reconciliation effect, and closes the inspector when selection is empty. |
| apps/web/tests/branch-inspector.spec.ts | New Playwright regression validating branch/inspector alignment, draft retention, and history/reload/Focus behavior against a real isolated backend. |
Review effort: Balanced
Changing the Workspace branch filter could leave the inspector editing a node from the previous branch, even though that node was absent from the displayed graph. Clear an off-branch node selection together with the branch change and close its inspector. Keep the selection when switching to its own branch or All research paths.
Guard the editor against inconsistent branch/node URLs immediately, then remove the off-branch node query parameter using history replacement. This also covers direct links, Focus view, reload and browser history. Reuse the existing per-node draft storage so returning to the original node restores its unsaved draft.
Fixes #111.
Validation at
67c842e87f4a4087cde41ff11523a7f36e4b062e:git diff --checkpass. Desktop and mobile screenshots inspected.Limits: this is a frontend selection correction. No backend, dependencies, schemas, authentication, workflow or execution-isolation settings change. No model or paid calls were made. Worker execution, PostgreSQL, the full backend suite, non-Chrome browsers, dark mode and Chinese visual rendering were not validated. Existing npm audit reports four dependency advisories (two moderate, two high), and the existing large-chunk build warning remains; neither is suppressed. This does not make all hidden-node cases (such as archive/collapse) follow a new editing policy.
Implemented and tested with OpenAI Codex assistance. No independent human review is claimed.