Skip to content

Cancel retired combined diff reads at their shared owner - #35048

Closed
OrcaWin wants to merge 1 commit into
mainfrom
OrcaWin/git-diff-owner-cancellation
Closed

OrcaWin wants to merge 1 commit into
mainfrom
OrcaWin/git-diff-owner-cancellation

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​432 $\color{#cf222e}{\Huge{\mathbf{−}}}$​21 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​411
Prod 25 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​461 $\color{#cf222e}{\Huge{\mathbf{−}}}$​290 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​171

Closing or replacing a combined diff view, or reaching its existing row deadline, used to leave issued Git reads queued. Once admission reopened, those retired reads still started physical commands before the next live viewer. Propagate cancellation from the section owner through local IPC and the existing runtime subscription signal, and reuse the existing shared read lease owner so one canceled reader cannot terminate a surviving same-key read.

Retire issued controllers on unmount, generation replacement (including cached restores), and timeout. Cancel queued admission and supported active Git/file I/O after the final lease retires. Aborts propagate as errors instead of becoming empty successful diffs. Optional request tokens and signal arguments preserve old call shapes; older preload/hosts can still retire the renderer caller even when physical cancellation is unavailable. Existing WSL routing keys, provider options, result budgets and the row timeout message remain in place.

Validation:

  • Focused production admission, renderer ownership, local/web cancellation, provider and diff transport suites: 11 suites / 189 tests passed under Node 24.22.0; pnpm tc passed. Five additional unchanged routing/setup-cancellation/sender suites passed 26/26 controls.
  • Exact pinned-baseline substitution: all 4 new hook ownership regressions fail, while 30 existing controls pass. Both backend abort-semantics regressions also fail on baseline. The backend red proof alone does not establish the missing renderer-to-host cancellation.
  • Actual system Git reproduction through production hooks → renderer client → IPC handler or runtime method → runtime/provider → backend → executor/capture → admission, at 5 and 50 retired viewers: baseline starts 5/50 stale children, then admits the live successor at index 5/50. Fixed starts zero stale children, leaves zero retired queued reads, and admits the live successor first. Shared-reader controls retain one queued physical read after one viewer closes, then return exact content from one child. Baseline queued work also survives the real 30.1-second renderer/dedupe expiry.
  • Electron transport and remote transport are represented by in-process dispatch in this reproduction; actual Git, admission, capture, backend, hook lifecycle and provider code execute. Evidence proves abandoned command counts and queue ownership, not measured heap/RSS or latency improvement. Native Windows/macOS/WSL, live SSH/network, mobile UI and rendered Electron flows were not exercised.

Required gate: ORCA_BACKGROUND_LAUNCH=1 ORCA_CODE_QUALITY_BASE=2d3e7e452000c26433650d75422e79a06be80684 MISE_NODE_VERSION=24.22.0 pnpm_config_verify_deps_before_run=false pnpm check:changed --maxWorkers=2 passed 25/27 checks. Formatting, lint, changed-code quality, typechecking and ratchets passed. The aggregate remains non-green: untouched tr.json has three baseline extra keys (English/Turkish files are byte-identical to base); the broad affected-test selection was deliberately stopped after 228.8 seconds without results, so its coverage is unverified. Exact focused tests above provide the completed test evidence.

Reliability invariant (git.diff.read-ownership): a retired diff caller cannot start queued physical work; a surviving same-key reader keeps its owner's read. Oracle: production queue/spawn/termination counts, exact surviving content, and no successor publication from retired generations. No matching reliability-manifest gate exists at this pinned base; the deterministic focused tests are the explicit accepted gap. Local/daemon PTY lifecycle and persistence are unaffected. Linux/system Git and semantic WSL option isolation are covered; native platforms, live SSH and mobile/relay execution remain accepted validation gaps. Runtime environments execute on their own server via its local provider; direct unserved ssh:/runtime: targets keep refusing local fallback. Existing subscription teardown carries cancellation through setup and socket close; older-host physical cancellation is best effort.

Integration: this independently targets pinned 2d3e7e452000c26433650d75422e79a06be80684. Resolve overlapping blob-reader edits with #34767 (bounded reads), #34778 (literal blob paths), and #34967 (incomplete-read errors), preserving their semantics and this signal forwarding/abort rethrow behavior. Reviewed related drafts include #34873 (projection cancellation), #34961/#35017 (publication echoes), #17506 (registry timing), #11287 (file selection), #9312 (native blobs) and #9597/#9598 (history publication); none of the inspected changes supplies this shared diff-read cancellation.

@OrcaWin

OrcaWin commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

The combined-diff section scheduler runs one load at a time, so a retired viewer leaves at most one already-deduped diff read. That gain does not justify about 900 lines across renderer, IPC, RPC and git, a swapped dedupe primitive behind every diff read, and a cached-restore generation bump that remounts editors. Per-request remote cancellation is also not delivered, because ctx.signal is socket-scoped.

@OrcaWin OrcaWin closed this Oct 30, 2026
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