Repository navigation
Conversation
ce363b9 to
22de4bc
Compare
0f5fd1d to
bbd72ea
Compare
nwparker
left a comment
There was a problem hiding this comment.
Compared with OrcaWin’s #21175 and dogfooter8750’s #9514. The review covers custody, queued writes, restart fences, archive/database failures, divergent drafts, and shared scrollback references. It reproduced truncated stored scrollback, unsafe fallback/removal, and partial file reads. Archives now use the existing bounded reader for the complete available buffer; unreadable/oversized sources survive. Archive failure preserves active state; it does not restore a pairing already removed by the GUI. Merge #25974 first, then retarget/rebase and rerun CI. Preserve retired-runtime-sessions/ with the profile; database-only exports omit recovery files.
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.
There was a problem hiding this comment.
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 1be5ed9b40c8. References and coauthor credit for #21175 (OrcaWin) and #9514 (dogfooter8750) remain in the PR; close originals after replacements merge.
Merge #25974 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.
Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Co-authored-by: KimYoungHwan8750 <142006989+KimYoungHwan8750@users.noreply.github.com>
6326097 to
1371f3e
Compare
1be5ed9 to
1b6f7dd
Compare
nwparker
left a comment
There was a problem hiding this comment.
Adversarial self-review of rebased head 1b6f7ddb3c6a.
Rebased onto the rebased #25974 and main 00984ccb2504. Main's managed SSH refactor (#24863) added deletion and migration paths; seven real-SQLite regressions failed after the initial rebase. GUI removal, startup reconciliation, and managed-server cleanup now use the existing custody-checked archive path. Renderer writes and shutdown checkpoints retain main's migration fence as well as the retirement admission guard. Startup repair extends main's existing reconciliation module; the duplicate implementation was removed. Cleanup joins asynchronous archive work while releasing transports. A new failing regression also showed that archive failure after managed-server unlink could strand its SSH claim; that callback now retains the session, logs the failure, and lets shutdown finish releasing the claim. Generic cleanup still reports archive failure. Server test selection uses the same dependency-free unit patterns as #25969. These regressions pass. Moderate risk because this changes persisted session custody and archives. After #25974 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.
ELI5
Removing a paired host should release its old client-side session without losing unsaved text. Orca now saves a private recovery archive before removing that session, and keeps delayed saves from bringing its tabs back.
What Changed
GUI removal, observed CLI removal, managed-server/VM cleanup, and conservative startup repair share one retirement path. Startup repair extends the existing reconciliation module. The path checks current workspace ownership again inside the database mutation, securely archives the complete session and available scrollback, and only then removes active state. A durable fence rejects later session and PTY recreation. Newer unsaved text in a delayed save goes into a separate, host-scoped recovery archive instead of being discarded.
Why
Clearing stale state alone would lose dirty editor buffers; rejecting only renderer writes would leave background writers and restarts exposed. This central persistence fence covers all writers. Archive failures preserve the active state and ordinary saves; database failures restore the partition. Local, direct SSH, and namespaces still owned by the catalog are protected. Missing, corrupt, unsupported, or empty historical registries are treated as unknown, not proof of removal.
Linked Issue
Part of #21489. Stacked on #25974; merge transport retirement first.
Adapts the custody and write-admission work in #21175 (OrcaWin) and historical/VM cleanup ideas in #9514 (dogfooter8750 / KimYoungHwan8750), with contributor credit in the commit. Those originals should close only after this replacement merges. #21113's empty-tab persistence root is already covered by #22929 and #22955; this PR adds a close/restart regression rather than a second close API.
Visual Proof
N/A — no rendered UI change. Native hidden Electron tests exercise both the real GUI removal handler and a separate built CLI removing the last paired host, inspect the durable archive, and verify the standing authenticated socket closes.
Testing
1b6f7ddb3c6a; wait for current-head checks before merging.Review
Compared with OrcaWin’s #21175 and dogfooter8750’s #9514. The review covers custody, queued writes, restart fences, archive/database failures, divergent drafts, and shared scrollback references. It reproduced truncated stored scrollback, unsafe fallback/removal, and partial file reads. Archives now use the existing bounded reader for the complete available buffer; unreadable/oversized sources survive. Archive failure preserves active state; it does not restore a pairing already removed by the GUI. Merge #25974 first, then rebase onto its actual main result and rerun CI. Preserve retired-runtime-sessions/ with the profile; database-only exports omit recovery files.
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 #25974 and main
00984ccb2504. Main's managed SSH refactor (#24863) added deletion and migration paths; seven real-SQLite regressions failed after the initial rebase. GUI removal, startup reconciliation, and managed-server cleanup now use the existing custody-checked archive path. Renderer writes and shutdown checkpoints retain main's migration fence as well as the retirement admission guard. Startup repair extends main's existing reconciliation module; the duplicate implementation was removed. Cleanup joins asynchronous archive work while releasing transports. A new failing regression also showed that archive failure after managed-server unlink could strand its SSH claim; that callback now retains the session, logs the failure, and lets shutdown finish releasing the claim. Generic cleanup still reports archive failure. Server test selection uses the same dependency-free unit patterns as #25969. These regressions pass. Moderate risk because this changes persisted session custody and archives. After #25974 merges, rebase onto its actual main result and rerun CI.Agent skill upstream boundary
Checklist