ci: validate upstream PR 8480 - #2
Closed
bbingz wants to merge 1 commit into
Closed
Conversation
Owner
Author
|
All executable code gates passed for the exact upstream base/head pair. Closing this temporary CI mirror; results remain linked from upstream PR stablyai#8480. |
5 tasks
bbingz
pushed a commit
that referenced
this pull request
Jul 22, 2026
…ablyai#9506) * fix(editor): don't flag editor-initiated moves as changed-on-disk An in-app move/rename (explorer drag-drop, inline rename, tab rename) re-homes the open tab to the new path and carries its unsaved draft forward. The move also physically relocates the file, which the worktree watcher reports as delete(old)+create(new) a few ms later. Because the tab already lives at the new path by then, that create echo was treated as an external write landing on a dirty tab and raised a spurious "changed on disk" banner. Add a short-lived self-move registry (the move analog of the existing self-write registry) stamped at the single remap choke point, and have the external-watch handler recognize the move's own watcher echo: suppress the changed-on-disk mark on the re-homed dirty tab and the tombstone on the source path. Genuine external edits are unaffected. Covered by unit tests for the registry, the remap recorder, and the watch-hook suppression (plus a no-over-suppression guard). Co-authored-by: Orca <help@stably.ai> * refactor(editor): harden move-echo suppression per adversarial review Addresses review findings on the self-move suppression: - Stamp the self-move at the call sites BEFORE the on-disk rename (recordSelfMoveForOpenTabs), not after the tab re-home, so the watcher echo can't win the race. This makes the source-side delete guard actually effective and removes a possible one-frame flash. - Suppress only the move's own create echo, not update events, so a genuine external write to the moved path within the TTL still raises the changed-on-disk banner (closes an over-suppression gap). - Track source/target roles independently per path so an immediate undo can't clobber the original move's still-in-flight stamp. - Raise the registry cap above realistic bulk-move sizes so a large directory move never self-evicts its own not-yet-echoed stamps. Updated + added tests: registry undo/role + cap, the new call-site helper (incl. directory move), and a real-update-still-marks case. Co-authored-by: Orca <help@stably.ai> * fix(editor): make move-echo suppression watcher- and TTL-robust Round-3 hardening after adversarial review: - Suppress the move's own watcher echo regardless of event kind. The main-process watcher coalesces a create+attr-change burst into a lone update, so a create-only gate let the echo through on some hosts and re-exposed the false banner. Suppression is now bounded by the self-move TTL; a genuine write to the exact path within that short window is the documented trade-off (draft is preserved regardless). - Bracket the on-disk rename with the self-move stamp via a single renameOpenTabsPathOnDisk wrapper: stamp before (to beat the watcher), re-stamp on success (a slow SSH/runtime rename can outlive the TTL, so the fresh window must start when the file actually moved), and clear on failure (a rename that never happened must not suppress real events). All move entry points (explorer move, inline/tab rename incl. undo/redo, untitled rename) route through it. - Treat a tab as remote for TTL purposes when it has a runtime owner OR an SSH worktree connection (an SSH tab can carry a null runtime owner). - Registry tracks source/target roles independently per path so an immediate undo can't clobber the original move's in-flight stamp; cap raised above realistic bulk-move sizes. Tests: registry role/clear/cap, the rename wrapper (success re-stamp + failure clear), the call-site helper (dir move + clear), and watch-hook coalescing-robust suppression + post-TTL surfacing. Co-authored-by: Orca <help@stably.ai> * fix(editor): refcount self-move roles so a failed move can't clear a live one Two concurrent moves onto the same destination both stamp that path as a target; if the second rename fails and clears, it must not erase the first (successful) move's still-live target stamp. Reference count each role's registrations and clear only the failed move's own contribution. Adds a regression test for the shared-destination case. Co-authored-by: Orca <help@stably.ai> * fix(editor): give self-move stamps per-registration expiries + retract tokens The refcount model used a single shared expiry scalar per role that only grew via max() and reset at refs=0, so releasing the max-contributing registration left survivors inheriting an over-extended window (and an expired registration could be resurrected by a later stamp on a key the opposite role kept resident). Both over-suppress genuine changes. Model each stamp as an independent registration carrying its own expiry (a list per role). recordSelfMove returns a ticket; clearSelfMove retracts exactly that registration. A role is live while any of its registrations is unexpired, so concurrent stamps, failed-move clears, and expiry are all precise. Wrapper/helper thread the tickets through. Adds regressions for the over-extension and resurrection cases. Co-authored-by: Orca <help@stably.ai> * test(editor): cover source-side self-move guard in the pre-remap ordering Adds the case where the watcher's delete(old) arrives while the tab is still at the old path (before remap re-homes it): with a live self-move source stamp the tombstone must be suppressed. Pairs with the existing naked-delete control (no stamp → deleted) to pin the guard's behavior. Co-authored-by: Orca <help@stably.ai> * docs(editor): document the failed-move suppression window as a bounded trade-off A self-move stamp is placed before the rename and retracted if it fails, so an event consumed during the pre-failure window is swallowed. Note that this only matters for the rare unrelated-dirty-tab-at-destination case and the recoverable missed-source-tombstone case, and that a move has no bytes to echo-verify the way self-writes do. Co-authored-by: Orca <help@stably.ai> * feat(editor): decide move echo vs external write by content identity Replaces the time-bounded self-move suppression heuristic with a correct-by-construction identity check, so a genuine external write to a just-moved path is never swallowed and the move's own echo is never a false conflict — regardless of watcher event-kind coalescing or timing. - Remap now carries the edit-session identity (lastKnownDiskSignature, externalMutation, pendingDiskBaselineVerification) onto the re-homed tab. Previously the close+reopen dropped it, so a moved tab lost its disk baseline (and any pre-existing changed-on-disk conflict silently vanished on move). - On a live self-move-target dirty event the watch hook now reads the destination and compares getDiskBaselineSignature(disk) to the tab's carried baseline: equal => move echo (suppress), differ/binary => genuine write (banner). Autosave is suspended synchronously before the read so a write landing mid-read can't be overwritten; a generation token makes overlapping reads safe. Fails CLOSED (marks changed) on a missing baseline or read error — never blind-suppresses. - The self-move registry now only scopes WHEN to verify. The source-side delete still can't be content-verified (nothing to read), so it stays a bounded, documented suppression. - Trim the verbose Why-comments across these files to 1-2 lines. Adds content-identity verification tests (echo/differ/no-baseline/read- error/binary/autosave-gate) and a remap test for the carried identity; splits the watch-hook suite to stay under the max-lines limit. Co-authored-by: Orca <help@stably.ai> * fix(editor): give live move-verification its own autosave gate The live self-move echo verification reused pendingDiskBaselineVerification as its autosave gate, but that field is also the always-mounted restored-tab conflict scanner's work queue: the scanner scans any pending dirty tab, launches its own read, and clears the flag without checking the live generation — so it could lift the gate mid-read and let autosave overwrite a genuine external write. Give live verification a dedicated pendingLiveDiskVerification field (both suspend autosave; each cleared only by its owner). Transient, not persisted, not carried across a re-home. Co-authored-by: Orca <help@stably.ai> * feat(editor): atomic rekeyOpenFilesForPathChange store action (move restructure stage 1) Foundation for treating an Orca-owned move as an in-place retarget of the open edit session (not close+reopen), per the locked design. One commit-only store update migrates every path-derived id + all id-keyed state (openFiles full-spread, the 6 file-id maps, activeFileId(+byWorktree), tabBarOrder, unified tabs/groups via the now editor-family-widened migrateHydratedEditorTabsAndGroups, pendingEditorReveal, untitled consume). Preflight fails closed on collision (never merges two live sessions) or stale with zero mutation. Not yet wired to a coordinator (stage 4). Stage 1 of 5; behind the shipped content-identity fix. Co-authored-by: Orca <help@stably.ai> * feat(editor): op-scoped in-flight move registry + source integration (stage 2) editor-path-move-inflight.ts tracks Orca-owned moves for the exact duration of the rename+rekey (no TTL): source paths suppress the delete tombstone, target paths latch a destination event seen before the rekey (never suppress). Wired into the watcher's delete filter alongside the old TTL registry (OR fallback) so suppression keeps working until the stage-4 coordinator drives every move through beginEditorPathMove. Stage 2 of 5. Co-authored-by: Orca <help@stably.ai> * feat(editor): move-echo provenance + autosave gate on OpenFile (stage 3 store) Adds pendingSelfMoveEcho {operationId,targetPath} to OpenFile and has the rekey action install it + pendingLiveDiskVerification on dirty autosave-capable destinations (moveOperationId arg), atomically in the same commit that re-homes the tab — so the verify gate survives the rekey and its op-id token supersedes a stale in-flight verification. Replaces the module-scoped generation map (which broke under rekey). Verification-reader wiring + coordinator follow. Co-authored-by: Orca <help@stably.ai> * refactor(editor): remap moves via atomic in-place rekey, not close+reopen (stage 4a) remapOpenEditorTabsForPathChange now builds an owner-aware rekey plan (plain-path id to the first owner, owner-qualified to the rest; previews resolve their source to the moved edit's new id) and applies it via rekeyOpenFilesForPathChange in one commit — preserving the full OpenFile + cursor/view/group/MRU state and closing the close/reopen watcher-race window. Passes moveOperationId through so dirty destinations get the content-verify gate. 4545 tests green. Co-authored-by: Orca <help@stably.ai> * feat(editor): move coordinator drives rename/drag/undo/redo/untitled (stage 4b) executeOpenEditorPathMove is the single transaction for every in-app move: quiesce affected saves -> op-scoped begin (per runtime owner) -> on-disk rename -> atomic in-place rekey (installs the content-verify gate/provenance) -> settle -> re-verify any destination echo latched before the rekey. On failure the store is untouched. Verification now triggers off the on-OpenFile provenance (consumed on resolve) and the watcher latches pre-rekey destination events. Wired into all five call sites; old renameOpenTabsPathOnDisk + separate remap removed from them. 2751 tests green. (TTL self-move registry now dead; removed next.) Co-authored-by: Orca <help@stably.ai> * refactor(editor): remove the dead TTL self-move registry (stage 4c) The coordinator + op-scoped in-flight suppression + on-OpenFile provenance fully replace the time-bounded self-move registry, so delete it and its two helper modules (record-self-move-for-open-tabs, rename-open-editor-tabs-path). The watcher source filter now uses only isActiveMoveSourcePath and the verification trigger only the tab's pendingSelfMoveEcho. Rewrote the self-move test suite onto the new primitives. 7225 tests green. Co-authored-by: Orca <help@stably.ai> * test(editor): end-to-end coordinator move test (stage 4 done) executeOpenEditorPathMove renames on disk, retargets the session in place with draft/dirty/baseline preserved + gate/provenance installed, settles the in-flight transaction; on rename failure the store is byte-identical and the transaction is released. Co-authored-by: Orca <help@stably.ai> * feat(editor): mirror-safe move — detach moved mirrored tab + close-notify host (stage 5) The atomic rekey changes a tab's id, so a moved mirrored tab would be culled by the host snapshot (losing its draft) or resurrect the old path. Ship the safe minimum: the rekey detaches a moved tab from the host mirror (mirroredFromRuntimeSession cleared) and the coordinator close-notifies the host's old-path tab (close intent suppresses re-mirroring). Prevents the data-loss/resurrection; the moved tab becomes companion-local. The full host-rekey path-change protocol (preserving mirror ownership) is a documented follow-up. Co-authored-by: Orca <help@stably.ai> * fix(editor): address review round 1 (4 majors) - Coordinator now propagates the rekey result: a collision/stale AFTER a successful on-disk rename triggers an inverse rename + throws, instead of reporting success with the source tab stranded at a vanished path (#1). - Cross-worktree: affected set spans all worktrees at the source path, sub-ops scoped per (worktree, owner), and the rekey partitions tab/group/tab-bar migration by each file's own worktree (was applied under one scope) (#2). - Diff tabs: single-file unstaged diff tabs are now retargeted on a directory move (rebuild the diff id + relative path) instead of stranding (#3). - Mirror close-notify moved to AFTER a successful rename so a failed rename can't desync the host by closing its authoritative tab (#4). Adds tests: collision->inverse-rename, diff-tab retarget. 6698 tests green. Co-authored-by: Orca <help@stably.ai> * fix(editor): review round 2 (mirror ordering, rollback error, diff sources) - Close the host mirror tab only AFTER the rekey commits (capture pre-rekey resolution first): a rekey collision after a successful rename no longer desyncs the host by closing its authoritative tab (high). - Surface a failed inverse rename instead of swallowing it: the thrown error now states the on-disk move may remain at the new path (high). - Restrict diff-tab retargeting to staged/unstaged (purely path-derived ids); branch/commit diffs carry compare metadata and combined 'Changes' is worktree-rooted, so rebuilding them from path would produce a wrong id (med). Co-authored-by: Orca <help@stably.ai> * fix(editor): resolve the move verify gate proactively (review round 3) The rekey gates every dirty moved tab pending a destination content check, but verification only ran when a watcher event arrived for that path. If the watcher was down, throttled, or the event coalesced away, the gate never cleared and autosave stayed suspended for the tab. The coordinator now drives verification for every tab it gated once the rename has committed, so the gate resolves on its own. That makes the destination-side event latch redundant, so the in-flight tracker is source-only again. Co-authored-by: Orca <help@stably.ai> * fix(editor): review round 4 (gate strand, cross-worktree verify path, leak) - Don't install the move-echo verify gate on a tab already showing the changed-on-disk banner: it's autosave-suspended via externalMutation and verification skips a 'changed' tab, so the gate would strand forever. - Content-verify reads each moved tab's own filePath. A cross-worktree/ floating tab's relativePath is relative to its own root ('../…') and must not be joined onto the initiating worktree path (would read the wrong file and raise a false conflict banner on unsaved work). - Coordinator settles the in-flight source suppression in a finally so a throw between rename and commit can't leak it, and only after the rollback rename so a late forward-rename delete stays suppressed. - Migrate pendingEditorReveal.fileId across the rekey (matcher prefers it). Co-authored-by: Orca <help@stably.ai> * fix(editor): don't consume move-echo provenance in the safety-net verify (round 5) The round-3 proactive post-commit verify ran resolveLiveMoveVerification, which consumed pendingSelfMoveEcho. On FSEvents/SSH the real destination watcher event reliably lands AFTER the fast local read, so it then found no provenance, took the immediate changed-on-disk mark (no baseline check when there is no recent self-write), and raised a false conflict banner on the just-moved dirty tab — the exact data-loss this change removes. The proactive verify is a safety net: it now releases the autosave gate but leaves the provenance, so a later destination event is still recognized as the move's own echo and content-verified. Only a real watcher event consumes the provenance. Keeping it is safe — every watcher consumer verifies by content, which is strictly safer than the immediate mark. Co-authored-by: Orca <help@stably.ai> * fix(editor): scope move rekey to the initiating execution host (round 6) Co-authored-by: Orca <help@stably.ai> * fix(editor): prefix-suppress the move root so late tabs under a dir move aren't flagged (round 8) Co-authored-by: Orca <help@stably.ai> * chore(editor): trim move-fix comments to the why; drop redundant rename quiesce Co-authored-by: Orca <help@stably.ai> * fix(editor): record mirrored-close intent synchronously to close the ghost-tab window Co-authored-by: Orca <help@stably.ai> * fix(editor): use flavor-aware path containment for move selection (Windows/UNC case) Co-authored-by: Orca <help@stably.ai> * fix(editor): reconstruct moved path by segment count (WSL alias / duplicate-separator safe) Co-authored-by: Orca <help@stably.ai> * fix(editor): infer moved-path flavor by syntax, preserving legal POSIX backslashes Co-authored-by: Orca <help@stably.ai> * test(editor): lock POSIX ancestor-backslash destination flavor Co-authored-by: Orca <help@stably.ai> * fix(editor): flavor-aware trailing-separator strip; preserve POSIX literal backslashes Co-authored-by: Orca <help@stably.ai> * fix(editor): flavor-aware separator folding in relative-path recompute (POSIX backslash) Co-authored-by: Orca <help@stably.ai> * perf(editor): keep the fs-watcher delete path O(deletes) when no move is in flight Co-authored-by: Orca <help@stably.ai> * docs(editor): tighten move-fix comments to one-line why-only Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
bbingz
pushed a commit
that referenced
this pull request
Jul 28, 2026
…persist a watermark past them (stablyai#10816) * fix(mobile): keep the reconnect watermark alive across the app's own teardown The catch-up added in stablyai#8690 could never run. app/index.tsx unsubscribes the notification stream on every non-'connected' state and builds a fresh subscription on reconnect, so the closure holding the ready-counter, the delivered watermark and the seen-set is destroyed exactly when a reconnect needs them. Every reconnect looked like a cold open, `reconnectReadyCount` was always 1, and notifications dispatched while the socket was down were never fetched. Move that state to a per-host module-scope session so it survives the teardown. Refs stablyai#8591 Co-authored-by: Orca <help@stably.ai> * fix(mobile): tag the notification watermark with a counter epoch so a desktop restart can't kill catch-up The desktop's notification `seq` is a per-process in-memory counter that starts at 0 on every launch. The mobile client's watermark is persisted in AsyncStorage and monotonic. After a desktop restart the two index different counters, so a client holding seq 57 meets a fresh counter at 2, `57 >= 2` cuts everything, and reconnect catch-up dies silently until the new process out-dispatches the old watermark — 57 notifications later. Users see nothing and get no error (stablyai#8591). Stamp every dispatched notification with an epoch identifying the counter lifetime, ride it on the `ready` frame and the getMissedSince response, and persist it beside the watermark. A watermark whose epoch doesn't match the live counter is void: the client resets to 0 and the desktop returns its retained buffer instead of nothing. The epoch param is optional on the wire in both directions, so a client or daemon that predates it degrades to today's seq-only cut rather than erroring. Also extracts the OS-permission helpers to notification-permissions.ts (re- exported, so no importer changes) to keep mobile-notifications.ts under its max-lines budget. Mutation-tested: 3 mutations applied to the epoch logic, 3 killed — including the storage-seed race guard, whose first mutant survived until the deferred-read test was added. * fix(mobile): make the notification watermark atomic and counter-scoped Round-1 review found four ways the epoch fix could still lose notifications. All four are addressed here. 1. Seen-set survived an epoch change. Seen-keys are seq-derived, and terminal bells carry no notificationId (they key on `seq:N` alone). After a restart the fresh counter re-issues low seqs, so a replayed post-restart bell was dropped as a duplicate of a bell from the previous counter. The dedup window belongs to one counter lifetime, so it is cleared on epoch change. 2. Legacy watermarks were trusted. Pre-upgrade installs stored a bare seq with no epoch. Adopting the first observed epoch as "nothing changed" left that unprovenanced seq cutting a counter it was never measured against — stablyai#8591 through the upgrade path. An epoch-less seq no longer survives adoption. 3. seq and epoch were separate storage keys. A process death between the two writes left epoch-B beside seq-57-from-A: a pair that looks internally valid on the next launch and is therefore trusted. They are now one JSON value, which cannot tear, with a read-only migration from the legacy key. 4. Sessions were never retired. They live at module scope so they survive the subscription teardown a reconnect performs, so host removal is the only thing that can drop them. Removal now retires the session and its watermark. Mutation-tested: 3 mutations, 3 killed. The first version of the bell test passed with the fix removed — it exercised the live path, which only adds to the seen-set; only the replay path consults it. Rewritten against the replay path, it fails with `expected 1 to be 2`: the literal lost notification. Mobile notifications + transport: 355 passed. Desktop replay: 11/11. * fix(mobile): catch up on the first connection after a cold open Catch-up hung off 'has this process connected before', which is false on the first ready of a fresh launch — exactly the post-upgrade / post-eviction case that loses everything between the stored watermark and the next live seq. Wait for the persisted read, then catch up whenever this device has delivered for the host before; a first-ever pairing still gets no replay. Co-authored-by: Orca <help@stably.ai> * fix(mobile): serialize live delivery behind the watermark seed, and key catch-up on the record Co-authored-by: Orca <help@stably.ai> * test(mobile): pin the two catch-up mechanisms mutation testing found unguarded Mutating each mechanism of the stablyai#8591 fix in turn showed two survived with the suite still green: the seed's epoch-provenance check, and the host session outliving the subscription teardown. Both are load-bearing, so pin them. - seen-set survives teardown: the desktop's retained buffer replays a notification already delivered live, and only the session-scoped seen-set stops a duplicate banner. - a seed resolving after a live epoch was adopted must not reinstate the dead watermark. Not reachable through subscribeToDesktopNotifications today ('ready' awaits the seed first), so it asserts on the exported pair and says so. Co-authored-by: Orca <help@stably.ai> * fix(mobile): serialize notification delivery per host so the watermark can't outrun what was shown Addresses two MAJOR findings from review of this branch. MAJOR #1 — the watermark could be persisted past a notification the user never saw. `deliverLive` advanced `lastDeliveredSeq` before awaiting the local show, and replay + live delivery ran concurrently, so a live seq 11 handled while catch-up was still showing seq 6 persisted 11. A process death before 7..10 were shown lost them permanently: the next launch asks the desktop for seq > 11. This predates the branch — `origin/main` advances the watermark at the same point — so it is a residual this fix closes, not a regression the branch introduced. It is fixed here because the branch is what makes the watermark load-bearing. Three changes: - the advance moves AFTER the show/dismiss await, so the watermark means "everything up to here reached the user" rather than "was dispatched" - a per-host `deliveryTail` promise chain (`enqueueHostDelivery`) serializes deliveries, so a monotonic advance is also an in-order one - the catch-up batch is ONE queue entry, not one per event. Awaiting per event returns to the event loop between replays and let a live event slot in between seq 6 and 7 — which is exactly the interleave being fixed. The RPC stays outside the queue: `sendRequest` waits up to 30s and holding the chain for that would stall live delivery on a slow link. MAJOR #2 — every delivery awaits the persisted read, so an AsyncStorage read that never settled disabled the host's notifications for the whole app lifetime, with no error and nothing to see. The seed is now bounded at 3s; a late seed still applies when it lands. Proceeding unseeded is strictly better: the watermark stays 0, so catch-up over-fetches and the seen-set de-duplicates. Serializing removed an overlap the duplicate-suppression relied on: `showLocalNotification` deduped two same-id events by observing the first still pending when the second arrived. With deliveries serialized the first completes first, so the second saw no pending state and scheduled a second banner for the same notification. The claim moves to enqueue time, where the overlap is still observable. Dismisses are deliberately not claimed — a dismiss for a shown id is what retires it. Evidence — each mechanism disabled individually against the unchanged suite: - batch-as-one-entry -> reverted to per-item enqueue: ordering test fails - watermark advance -> moved back before the await: ordering test fails - seed timeout -> removed: wedged-read test fails - live-path claim -> removed: concurrent-dedup test fails - replay-path claim -> removed: cross-path dedup test fails Each kills exactly one test, so no mechanism is unguarded and none is redundant. `mobile-notifications.test.ts`'s local `flushAsync` drained 10 microtask ticks. Deliveries are now several awaits deeper, so a fixed tick count under-drains; it yields to the macrotask queue instead. Verified with real timers that the behavior it asserts is unchanged — only the drain depth was wrong. Full mobile suite: 344 files, 2499 passed, 2 skipped. tsc clean, oxlint clean. --------- Co-authored-by: Orca <help@stably.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI mirror for upstream PR stablyai#8480.
Base is pinned to the exact upstream main commit a5faf19. Head is the exact upstream PR commit a1f8a8a. This PR exists only to run fork-authorized repository checks and will be closed after results are recorded.