Skip to content

fix(desktop): serialize shim publication with production process creation (ETXTBSY) - #7106

Merged
lidge-jun merged 7 commits into
devfrom
fix/desktop-etxtbsy-shim-exec
Oct 15, 2026
Merged

lidge-jun merged 7 commits into
devfrom
fix/desktop-etxtbsy-shim-exec

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fix the Unix ETXTBSY race when a newly installed desktop shim is executed. Atomic replacement flushes and closes its writer before publishing by rename. A production process-wide gate now excludes executable writes from shell-owned process creation, so a fork cannot retain a writable duplicate after the parent closes its descriptor. Ordinary record writes and concurrent spawners remain independent.

The spawn inventory covers bundled runtime start, resolve/claim/stop output futures, launchctl/plutil login probes, browser and Login Items launchers, updater installation helpers, and app restart. The configured LaunchAgent and Linux autostart registration write files without spawning. Windows uses the same entry points without synchronization. Lazy output futures acquire the gate during each poll, releasing before yielding; no lock spans an output await. The standard-library gate lets launchers join existing launchers while a writer is queued, avoiding a deadlock if an updater waits for a main-thread callback that opens a URL. Only an active writer excludes spawns.

Rust fixtures use the production gate instead of a cfg(test) copy. Tests cover both exclusion directions, release before child exit, spawn errors, async Pending release and the updater callback ordering. The login-command deadline regression retains its child-process isolation and original timing/slot assertions. Current dev (7640a942b, including #7107) remains merged without rebasing or force-pushing.

Verification

  • cargo test --locked --manifest-path .tmp/rust-cli-harness/Cargo.toml: 57 passed, 0 failed, 2 existing opt-in tests ignored. The isolated harness imports the real record, POSIX, Windows, login-registration and process-spawn modules. cargo check --locked on that harness also passes without cfg(test).
  • Removed executable-write exclusion and async polling exclusion separately: both exact regression tests failed, then passed after restoration.
  • bun test tests/clients/desktop-cli-contracts.test.ts tests/clients/desktop-cli-command-surface.test.ts tests/clients/desktop-startup-surface.test.ts tests/clients/desktop-exit-ownership.test.ts: 58 passed, 0 failed.
  • bun test tests/clients/desktop-update-surface.test.ts tests/clients/desktop-start-at-login-default.test.ts tests/clients/desktop-tray-availability.test.ts: 25 passed, 0 failed.
  • bun run typecheck, bun run structure:check, bun run privacy:scan, cargo fmt --manifest-path desktop/src-tauri/Cargo.toml --check, git diff --check: pass.
  • Full desktop Cargo tests cannot build on this macOS host because its SwiftUI macro toolchain is unavailable. Hosted Linux desktop shell supplies the complete crate build and tests.
  • Full local Bun suite deferred because of concurrent worktree contention and the Rust-only behavior change. The seven explicit read-as-data contract files cover the affected shell surfaces; hosted lane=all supplies the broader Linux/macOS/Windows coverage. Pending, skipped and cancelled checks are not passing evidence.
  • Final-head cross-platform lane=all on e504c23cc: run 38741932609 dispatched; Linux desktop shell and all platforms are pending. Results will be recorded here when complete. Earlier run 38698251320 is green on 6a12ee3 and is not evidence for this new head.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed (structure/desktop-terminal-command.md and its shell cross-reference).
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (atomic close-before-rename ordering preserved; no credential or permission policy changes, dependencies, or unsafe code added).

Summary by CodeRabbit

  • Bug Fixes
    • Improved coordination between process launches and executable updates on Unix, preventing them from interfering with each other.
    • Process launches can continue while an executable update is waiting to begin.
    • Asynchronous process operations release coordination while waiting for results.

jun added 3 commits October 14, 2026 23:19
Share a Unix test read/write lock between executable fixture writes and every synchronous subprocess spawn. Close-on-exec does not prevent a forked child from temporarily retaining a writer, which made direct shim execution return ETXTBSY during the CLI record lock stress test. Keep non-executable record writes and all four stress spawners concurrent, and assert that executable writes wait until spawn protection ends.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T15:46:19.836746Z 17c60c4 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added the bug Something isn't working label Oct 14, 2026
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 4dc328cf-3491-472f-b4db-fc404500d4c5
📥 Commits

Reviewing files that changed from the base of the PR and between 6a12ee3 and e504c23.

📒 Files selected for processing: 19
  • desktop/src-tauri/src/claim.rs
  • desktop/src-tauri/src/cli_command.rs
  • desktop/src-tauri/src/cli_command_posix.rs
  • desktop/src-tauri/src/cli_command_record.rs
  • desktop/src-tauri/src/exit.rs
  • desktop/src-tauri/src/lib.rs
  • desktop/src-tauri/src/login_registration.rs
  • desktop/src-tauri/src/process_spawn.rs
  • desktop/src-tauri/src/resolve.rs
  • desktop/src-tauri/src/runtime_stop.rs
  • desktop/src-tauri/src/sidecar.rs
  • desktop/src-tauri/src/tray.rs
  • desktop/src-tauri/src/tray_login.rs
  • desktop/src-tauri/src/updater.rs
  • desktop/src-tauri/src/window.rs
  • structure/desktop-shell.md
  • structure/desktop-terminal-command.md
  • tests/clients/desktop-cli-contracts.test.ts
  • tests/clients/desktop-startup-surface.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Unix executable writes and process creation now coordinate through a shared gate. Desktop command paths and tests use wrappers that hold shared access during process creation or each asynchronous poll. Executable replacement uses exclusive access.

Changes: Process Spawn Coordination

Layer / File(s) Summary
Coordinate executable writes and spawns
desktop/src-tauri/src/process_spawn.rs, desktop/src-tauri/src/cli_command_record.rs
The Unix gate permits concurrent spawns and waits for active spawns before executable writes. Executable atomic writes acquire exclusive access and close the temporary file before replacement.
Apply spawn guards to desktop operations
desktop/src-tauri/src/lib.rs, desktop/src-tauri/src/claim.rs, desktop/src-tauri/src/resolve.rs, desktop/src-tauri/src/runtime_stop.rs, desktop/src-tauri/src/sidecar.rs, desktop/src-tauri/src/login_registration.rs, desktop/src-tauri/src/exit.rs, desktop/src-tauri/src/tray.rs, desktop/src-tauri/src/tray_login.rs, desktop/src-tauri/src/updater.rs, desktop/src-tauri/src/window.rs
Command execution, sidecar creation, restarts, opener calls, and updater installation use the spawn wrappers. Existing deadline, result, and error handling remains in place.
Validate and document guarded operations
desktop/src-tauri/src/process_spawn.rs, desktop/src-tauri/src/cli_command.rs, desktop/src-tauri/src/cli_command_posix.rs, desktop/src-tauri/src/cli_command_record.rs, desktop/src-tauri/src/login_registration.rs, structure/desktop-terminal-command.md, structure/desktop-shell.md, tests/clients/desktop-cli-contracts.test.ts, tests/clients/desktop-startup-surface.test.ts
Tests cover spawn exclusion, queued-writer behavior, async poll boundaries, and guard release after errors. Subprocess tests use the wrappers. Documentation describes guarded operations, and client assertions check the updated call patterns.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant SpawnCaller
  participant process_spawn
  participant ExecutableWriter
  SpawnCaller->>process_spawn: request shared access
  process_spawn->>SpawnCaller: run spawn closure with shared access
  ExecutableWriter->>process_spawn: request exclusive access
  process_spawn->>ExecutableWriter: grant access after active spawns finish
  ExecutableWriter->>ExecutableWriter: replace executable
Loading
Merge Risk: 🔵 Low · up to e504c

A CLI repair started during a Debian desktop update may wait for the installer to finish. This limited contention does not prevent merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 58.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 17 files. (2 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the desktop fix: serializing shim publication with production process creation to prevent Linux ETXTBSY races. It is specific, concise, and directly matches the pull reque…
Full details: Docstring Coverage

Explanation

Docstring coverage is 58.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 17 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17c60c4128

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread desktop/src-tauri/src/cli_command_record.rs Outdated
@lidge-jun lidge-jun changed the title fix(desktop): close the shim writer before publish and keep test forks off executable writes (ETXTBSY) fix(desktop): serialize shim publication with production process creation (ETXTBSY) Oct 15, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev (MAINTAINERS.md dev maintainer-integration policy; batch integration authorized by @lidge-jun on 2026-10-15).

  • Head: 18d4c8400de4faf17df8747d1be9ff7d73f87a1a (tree c7e0440ee5978b0a6c0b1635347e76e6c2eafcca) = this branch merged with origin/dev ade27f4ebd.
  • Exact-head evidence: GitHub-hosted Actions for this account have not been assigning runners since 2026-10-15 13:30 UTC, so by owner decision the evidence is a full bun run test (Bun 1.4.3 test runner, root and GUI dependencies installed, GUI built) of this exact tree on a maintainer host (macOS arm64): exit 1, 9 min, 6 failing test(s), each also failing in the same host's baseline run of dev 60bf4c7 (environment-only): a secondary ensure parent preserves shared Grok, Codex and Claude agent bytes, connected Desktop uses hub IDs across a cold restart (stored profile=false), live audio readiness and bounded closure > bad native frame fails and later normal closure, live audio readiness and bounded closure > native open and non-readiness events cannot cla, npm cache access pre-flight > runs the real worker protocol against npm's configured cache, update stops the running proxy before replacing files > npm launcher restarts the stopped .
  • Local gates on the same tree: bun run typecheck, file-size ratchet, test-layout and structure:check.
  • Not counted as a regression: npm cache access pre-flight > runs the real worker protocol against npm's configured cache path hit its 5 s timeout under full-suite parallel load on macOS; the same file on this exact tree passed 20/20 three times in isolation, and this PR does not touch the update/npm code.
  • Hosted CI checks for this head may still be queued; they are not claimed as evidence.

@lidge-jun
lidge-jun merged commit d73e9ab into dev Oct 15, 2026
2 checks passed
@lidge-jun
lidge-jun deleted the fix/desktop-etxtbsy-shim-exec branch October 15, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant