Skip to content

Recover damaged editor owners without losing divergent drafts - #25969

Open
nwparker wants to merge 6 commits into
nwparker/fix-editor-document-retentionfrom
nwparker/fix-durable-editor-owner-recovery
Open

nwparker wants to merge 6 commits into
nwparker/fix-editor-document-retentionfrom
nwparker/fix-durable-editor-owner-recovery

Conversation

@nwparker

@nwparker nwparker commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1558 $\color{#cf222e}{\Huge{\mathbf{−}}}$​4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1554
Prod 26 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1069 $\color{#cf222e}{\Huge{\mathbf{−}}}$​91 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​978

ELI5

A damaged saved session can give one editor document several identities or the wrong host. Restore now repairs records only when the workspace has a clear recorded owner. Conflicting unsaved versions remain recoverable, including after quitting Orca.

What Changed

Builds on #25966. Hydration merges equivalent document records, redirects their tabs and selections, and corrects contradictory host stamps using catalog/session ownership rather than the currently focused host. Ambiguous owners, external SSH files, and read-only logs are preserved.

When two records contain different unsaved text, neither version is discarded or written over the other. Recovery copies keep their exact text, owner and disk baseline in a client-local optional session field. Ordinary close-history limits apply only to ordinary history. Recovery remains reachable through Reopen Closed Tab after cross-type history expires; blocked or failed opens retain the copy. Autosave waits for baseline verification, or user resolution when no baseline exists.

Why

Adapted from BearMett’s #21125, with 김영민 credited as coauthor. Adversarial review reproduced four failures in its capped recovery stack: overflow, deferred drafts, and repeated hydration could lose text or duplicate copies. The original recovery stack also disappeared across restart. Durable recovery is preferable to selecting an arbitrary winner or retaining duplicate live document IDs.

Linked Issue

Refs #21489. This repairs damaged editor records; terminal partition retirement and transport invalidation remain separate fixes. Replacement for #21125 after this and #25966 merge; leave the contributor PR open until then.

Visual Proof

Same damaged session in a hidden Electron renderer, on a disposable workspace created through orca worktree create:

Before: two rendered tabs, local and deleted-host records.
Before

After: one rendered document with the recorded local owner.
After

42 recovery buffers were saved through real session IPC, flushed, and retained on a fresh Electron process. Reopening the first displayed its exact text and the user-resolution banner; 41 remained parked.
After restart

Testing

  • Relevant failures reproduced before their fixes; automated regressions retained. Rebase-specific failures and fixes are described below.
  • Rebased assembled application tree: 6,117 tests passed (four skips); full typecheck and changed-code quality passed, with zero findings across 151 changed files.
  • Fresh app and CLI builds; four hidden macOS flows passed: open/edit/save, first-mount agent/Setup PTY identity, GUI last-host archive, and separate CLI last-host archive. The final main refresh changes only CI files; the application tree is identical to this tested tree.
  • The new ten-shard CI planner from main passes all 82 focused tests.
  • Earlier-base isolated native Windows and Linux runs passed the same four flows: Windows, Linux. These are historical evidence, not verification of this rebased head.
  • Required CI for rebased head 0fadf6ef9314; wait for current-head checks before merging.

Review

Compared with BearMett’s #21125. The capped recovery history could lose dirty overflow and deferred drafts, and did not survive restart. Durable client-local recovery retains those copies separately, with an evidence-based owner repair and disk-baseline guard before autosave. Main’s bulk-close placeholder protection remains intact. Ambiguous ownership is preserved rather than guessed. Merge #25966 first, then rebase this PR onto its actual main result and rerun CI.

The series includes local/direct SSH isolation, folder workspaces, and remote authority coverage. Real Windows+WSL reproduction of the original damaged profile and a native nested SSH recovery/save journey remain unverified. #25938 and the independent CI memory fix #26049 are merged.

Rebased onto the rebased #25966, which includes main 00984ccb2504 and the restored captured-generation close guard. Server test discovery now imports the existing unit-file patterns from a dependency-free module, preserving classification before dependencies are installed; 108 detector/runner/qualification regressions pass. The runner and detector still share one selection implementation. Low–medium risk. After #25966 merges, rebase onto its actual main result and rerun CI.

Agent skill upstream boundary

  • Not applicable.

Checklist

  • Explained user-facing behavior, mechanism, and tradeoffs.
  • Visual proof attached above, or N/A with reason.
  • Self-reviewed correctness, data preservation, and performance.
  • Cross-platform, SSH, folder-workspace, and compatibility impact considered.
  • Current-head CI is green before merge.

@nwparker
nwparker force-pushed the nwparker/fix-durable-editor-owner-recovery branch from 4aab741 to 8391b3d Compare October 6, 2026 22:01
@nwparker
nwparker force-pushed the nwparker/fix-editor-document-retention branch from 58c0e59 to 792739c Compare October 6, 2026 22:41
@nwparker
nwparker force-pushed the nwparker/fix-durable-editor-owner-recovery branch 2 times, most recently from fb0fe95 to 3be6472 Compare October 6, 2026 22:53
@nwparker
nwparker marked this pull request as ready for review October 6, 2026 23:30
@nwparker
nwparker requested a review from brennanb2025 as a code owner October 6, 2026 23:30

@nwparker nwparker left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Compared with BearMett’s #21125. The capped recovery history could lose dirty overflow and deferred drafts, and did not survive restart. Durable client-local recovery retains those copies separately, with an evidence-based owner repair and disk-baseline guard before autosave. Main’s bulk-close placeholder protection remains intact. Ambiguous ownership is preserved rather than guessed. Merge #25966 first, then retarget/rebase this PR onto main and rerun CI.

Final assembled verification: 6,313 tests pass (five skips) in 563 suites; 48 additional upstream authority tests pass; typecheck and changed-code quality pass (143 files, zero findings).. Fresh macOS and isolated Windows each pass all four native flows. Windows proof: https://github.com/stablyai/orca/actions/runs/37556420437. Final isolated Linux verification is running.. Current-head required CI is still running; merge recommendation remains conditional on all required checks passing. Contributor originals remain open until their replacements merge.

@nwparker nwparker left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Follow-up adversarial review of the headless CI selection: the existing --exclude flag was ignored by the Vitest project config, so compatibility containers tried to launch a desktop Electron probe. Reused the existing test discovery to select concrete server files before invoking Vitest. The original detector fails the new regression; 106 selection/qualification tests and real Vitest discovery of 97 intended server suites now pass. Desktop validation still executes the Electron probe. Typecheck and code quality pass on the complete assembly (147 files, zero findings). Native macOS, Windows, and Linux each passed the same four application flows; the application tree is unchanged by this follow-up. Required CI is green on c2d5935c4f50. References and coauthor credit for #21125 (BearMett) remain in the PR; close originals after replacements merge.

Merge #25966 first, then rebase this child onto the actual main result and rerun CI. The original damaged Windows+WSL profile and a native nested SSH recovery/save journey remain unverified.

The repeated ARM runner shutdowns were traced to duplicated audit bundles by module and resource logging. A separate build-time fix is under verification in #26049; it preserves application behavior. Contributor originals should close after their replacements merge.

@nwparker nwparker left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Clean-code and adversarial self-review: removed 40 lines, including duplicate recovery-state types, repeated deferral calls, and two temporary sets. The reopen fallback reads the captured pre-open snapshot; openFile and draft setters replace state immutably, so a reused alias still compares against its prior unsaved text and dirty flag. Deferral, notification, and activation order remains unchanged. Equal drafts keep newer baselines; missing baselines block autosave; read-only, other-owner, and failed-open paths preserve the parked draft.

All 117 focused recovery tests pass on this branch. The final combined cleanup tree passes 448 editor/terminal tests across 49 suites, renderer typecheck, and changed-code quality with zero findings. Compared again with BearMett’s #21125; the durable uncapped recovery and restart protections remain, and contributor credit is preserved. #21124 is covered by parent #25966. No new blocker found; low–medium merge risk, conditional on current-head CI. Merge #25966 first, rebase onto its actual merged main result, then rerun CI. The original damaged Windows+WSL profile and native nested SSH recovery/save remain unverified.

@nwparker
nwparker force-pushed the nwparker/fix-editor-document-retention branch from 5253faa to 4979062 Compare October 7, 2026 20:26
@nwparker
nwparker force-pushed the nwparker/fix-durable-editor-owner-recovery branch from f4a4aaf to 0fadf6e Compare October 7, 2026 20:26

@nwparker nwparker left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adversarial self-review of rebased head 0fadf6ef9314.

Rebased onto the rebased #25966, which includes main 00984ccb2504 and the restored captured-generation close guard. Server test discovery now imports the existing unit-file patterns from a dependency-free module, preserving classification before dependencies are installed; 108 detector/runner/qualification regressions pass. The runner and detector still share one selection implementation. Low–medium risk. After #25966 merges, rebase onto its actual main result and rerun CI.

Validation of the assembled rebased application tree: 6,117 tests passed (four skips), full typecheck, zero changed-code quality findings across 151 files, fresh app/CLI builds, and all four hidden macOS flows. The final main refresh is CI-only and its 82 planner tests pass. Earlier Windows/Linux native runs were on the previous base; current-head CI must pass before merge.

Contributor references and coauthor trailers are preserved. The original damaged Windows+WSL profile and a native nested SSH recovery/save journey remain unverified; this does not justify closing #21489. No PRs were merged or superseded contributor PRs closed during this rebase.

This branch has not been deployed

No deployments
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.

1 participant