Skip to content

feat: EditorHost warn before leaving unsaved edits (#4962) - #4967

Merged
natechadwick-intsof merged 7 commits into
mainfrom
fix/issue-4962-editor-leave-unsaved
Sep 27, 2026
Merged

natechadwick-intsof merged 7 commits into
mainfrom
fix/issue-4962-editor-leave-unsaved

Conversation

@natechadwick-intsof

@natechadwick-intsof natechadwick-intsof commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Parent: #4532. Slice #4962.

EditorHost asks before leaving with unsaved text or file edits: Open folder, opening another item, switching Edit / View, or Close. Cancel stays on the dirty form. Confirm continues and does not PUT. A successful Save clears the prompt. Confirming discard also drops pending file picks and pending binary clears so a later save cannot upload or delete that binary.

Operator: Grok: night-issue-prs (model grok-4.7)

Test plan

  • Vitest EditorHost.leaveDirty.test.tsx (14 tests): cancel keeps values, confirm does not save, save clears the prompt, pending file, switch item, confirm drops a pending upload, confirm drops a pending clear
  • Surface Playwright tests/editor-host-leave-unsaved.spec.js (1 passed)
  • WebUI rtk mvn clean install BUILD SUCCESS

Product documentation

  • Updated product-docs/8.2/getting-started/index.md

C3 evidence

  • modules_built: WebUI
  • cd WebUI && rtk mvn clean install — BUILD SUCCESS. JUnit Tests run: 69, Failures: 0. Vitest Test Files 573 passed, Tests 5263 passed.
  • downstream_checked: none (no public Java signature change)

C5 UI proof

  • python3 docker/scripts/perc-devctl.py qa-up — TEST_CMS_URL=http://127.0.0.1:9993 QA_CONTAINER=perc-matrix-cms-h2
  • qa-health RESULT:OK HEALTH:healthy
  • python3 docker/scripts/perc-devctl.py qa-deploy-webui then qa-health again RESULT:OK
  • npm run test:surface -- --path tests/editor-host-leave-unsaved.spec.js — 1 passed
  • console-clean=yes (spec asserts no pageerror/console error)
  • server.log-clean=yes (no ERROR/FATAL lines in the QA server.log tail for the run)

Pre-push local code review

Erlang review — PR 4967

Re-review of head ac69171f108d82b93f621b9616b2ed0a84e4926a (discarded binary picks cleared before a later save). CLI: mkd-code-review 0.1.18, --pack percussion --format markdown --gate advisory --git-base origin/main --models models.ollama-dev-coder.toml.

Summary

Machine analysis found 1 finding(s), 1 bug(s).

Scope

  • Base: origin/main
  • Head: HEAD
  • Files: 7 analyzed
  • Persona: erlang 0.1.1
  • Persona source: /home/nate/.local/share/mkd/agents/erlang

Recommendation

request-changes

Gate

  • Blocking bugs: 1
  • May commit/push: yes

Issues

Issue 1 -- Severity: bug

  • File: WebUI/src/main/ts/editor/EditorHost.tsx:1865
  • Rule: llm.ollama-dev-coder
  • Tool: llm
  • Description: The allowLeave function is called before the recycleItem, copyItem, and createItem functions, which can lead to unintended behavior if these functions modify the state in a way that affects the unsaved edits check.
  • Suggestion: Ensure that the allowLeave function is called after any state modifications made by recycleItem, copyItem, and createItem. This will ensure that the unsaved edits check is accurate.
  • Status: open

Erlang (re-review)

Machine issue 1 is a false positive. EditorHost.tsx:1865 is copyErrorKeyFor, not a leave check. allowLeave() before recycleItem (handleRecycle), copyItem (handleCopy), and createItem (handleCreate) is the correct order: Cancel must not delete, copy, or create. Calling the prompt after those calls would reintroduce the bug fixed in 160e939fe9.

The prior blocking bug is fixed. allowLeave (EditorHost.tsx:1938) calls discardUnsavedEdits (:1925) only after confirm, which clears pendingFiles and pendingClears, resets draft from the loaded payload, and bumps discardEpoch so file widgets remount. The field-load effect (:818) also clears both maps when contentId or readOnly changes, so a confirmed mode change or item switch cannot leave a binary for a later save. EditorHost.leaveDirty.test.tsx covers confirm-on-mode-change for a pending upload and a pending clear; a following save does not call uploadBinary or clearBinary.

Host gate: approve. No in-diff bug remains. Do not merge until required checks on this head are green.

Recommendation: approve.

Fixes #4962

Co-Authored by Grok Build 1.0.41 using grok-4.6 with agent night-issue-prs-erlang.

Prompt on open folder, another item, Edit/View, and Close when field
edits are dirty. Cancel stays. Confirm does not save. A successful save
clears the prompt.

> Co-Authored by Grok Build 1.0.41 using grok-4.7 with agent night-issue-prs.
Record the host mkd-code-review report. Host gate is request-changes: recycle prompts only after the item is deleted.

> Co-Authored by Grok Build 1.0.41 using grok-4.7 with agent night-issue-prs-erlang.
@natechadwick-intsof

Copy link
Copy Markdown
Collaborator Author

request-changes — do not merge.

Bug WebUI/src/main/ts/editor/EditorHost.tsx:2275 (handleRecycle): the unsaved-edits confirm runs only after recycleItem has already deleted the item. Cancel is supposed to keep the draft and stay; it cannot restore the item, and the editor stays open on a recycled id. Run allowLeave() before recycleItem.

Same late prompt: switchOpenItem after a successful New item or Copy (EditorHost.tsx around the create/copy success paths) asks to discard only after the server write. If cancel should keep the dirty editor and not commit the other action, prompt first.

Machine mkd-code-review reported 0 in-diff bugs; this is a host gate. Report: docs/ai-generated/code-reviews/pr-4967-erlang.md.

Co-Authored by Grok Build 1.0.41 using grok-4.7 with agent night-issue-prs-erlang.

Ask to discard unsaved edits before deleting, creating, or copying so
Cancel keeps the open item. Confirm still does not save.

> Co-Authored by Grok Build 1.0.41 using grok-4.7 with agent night-issue-prs-erlang-fix.
Record the post-fix machine report and the remaining pending-file discard bug.

> Co-Authored by Grok Build 1.0.41 using grok-4.7 with agent night-issue-prs-erlang.
@natechadwick-intsof

Copy link
Copy Markdown
Collaborator Author

Erlang re-review (head 160e939fe9, report b08dfaf6e7): request-changes. Not merged.

Machine mkd-code-review 0.1.18: Persona erlang 0.1.1, 0 in-diff bugs, advisory approve. The earlier leave-after-recycle/copy/create bug is fixed: allowLeave() runs before those mutations.

Bug: confirming discard does not clear pendingFiles / pendingClears. Edit/View calls allowLeave() then only flips mode. The load effect setDrafts server fields when readOnly changes but never clears pending binaries (cleared on successful save, and pending files on restore). The dialog says edits will be discarded; a picked file stays and a later save can still upload it. EditorHost.leaveDirty.test.tsx asserts cancel keeps the file name, not that confirm drops it.

Clear both maps on the confirmed leave path and add that assertion. Do not merge until that is in.

PRs #4965 and #4966 were already MERGED before this pass.

Co-Authored by Grok Build 1.0.41 using grok-4.7 with agent night-issue-prs-erlang.

Confirming leave still keeps pendingFiles and pendingClears, so a later
save can upload a binary the dialog said was discarded.

> Co-Authored by Grok Build 1.0.41 using grok-4.6 with agent night-issue-prs-erlang.
@natechadwick-intsof

Copy link
Copy Markdown
Collaborator Author

request-changes (head 6bf296ba24). Do not merge.

The Ollama finding that allowLeave() must run after recycle/copy/create is a false positive. Cancel has to return before recycleItem (EditorHost.tsx:2269), copyItem (:1888), and createItem (:2355).

Bug — confirming discard does not drop a pending file or clear.

  • WebUI/src/main/ts/editor/EditorHost.tsx:1919 (allowLeave), :1945 (handleModeChange), :873 and :944 (field-load effect)
  • editorDraftIsDirty treats pendingFiles and pendingClears as dirty, but confirm only updates the query string. The load effect resets draft from the server and never calls setPendingFiles({}) or setPendingClears({}) (cleared on save at :1318, files only on restore at :2699).
  • Confirm on View keeps the picked file. A later Save uploads or clears that binary. The same stale file can follow an item switch.
  • EditorHost.leaveDirty.test.tsx:185 only asserts cancel keeps the file. Add a confirm case that the name is gone and the next save does not upload it.

Co-Authored by Grok Build 1.0.41 using grok-4.6 with agent night-issue-prs-erlang.

Confirming leave now clears pending files and clears, and reloading an item drops them too, so a later save cannot upload or delete a binary the dialog already discarded.

> Co-Authored by Grok Build 1.0.41 using grok-4.6 with agent night-issue-prs-erlang-fix.
Confirming leave now clears pending files and clears. The remaining machine finding still asks to prompt after recycle, copy, and create, which is a false positive.

> Co-Authored by Grok Build 1.0.41 using grok-4.6 with agent night-issue-prs-erlang.
@natechadwick-intsof

Copy link
Copy Markdown
Collaborator Author

Erlang re-review

Head ac69171f (review doc aebf8ec) fixes the pending-file discard bug. Confirming leave clears pendingFiles and pendingClears in discardUnsavedEdits (EditorHost.tsx:1925) and the field-load effect (:818) drops them on contentId / readOnly change. EditorHost.leaveDirty.test.tsx asserts a later save does not upload or clear that binary.

Machine issue llm.ollama-dev-coder at EditorHost.tsx:1865 is a false positive (copyErrorKeyFor). Prompting after recycle, copy, or create would delete or create before Cancel.

Recommendation: approve. Not merged this pass: the check snapshot on ac69171f still had CodeQL javascript, product-docs, and QA wiring in progress (mergeStateStatus BLOCKED, reviewDecision REVIEW_REQUIRED).

Co-Authored by Grok Build 1.0.41 using grok-4.6 with agent night-issue-prs-erlang.

@natechadwick-intsof
natechadwick-intsof merged commit 07ab976 into main Sep 27, 2026
8 checks passed
@natechadwick-intsof
natechadwick-intsof deleted the fix/issue-4962-editor-leave-unsaved branch September 27, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model:grok-4.7 Session model grok-4.7 operator:grok Changes authored by Grok operator:night-issue-prs night-issue-prs workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

issue 4532 slice 46: EditorHost warn before leaving unsaved edits

1 participant