Skip to content

fix(desktop): allow Windows guarded takeover to finish within one bounded budget - #6400

Merged
lidge-jun merged 3 commits into
devfrom
devin/1790860476-windows-takeover-budget
Oct 1, 2026
Merged

lidge-jun merged 3 commits into
devfrom
devin/1790860476-windows-takeover-budget

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A real Windows Tauri 2.76.0 app taking over the published npm 2.67.0 runtime failed twice after stopping the old proxy: the bundled CLI did not finish stopping before the deadline. The same bundled guarded-stop command completed successfully in 30.278 seconds (exitCode: 0, runtimeDown: true), already exceeding the entire desktop startup budget before resolve, claim, and replacement startup.

An approved Windows takeover extends the shared startup ceiling once from 30 to 90 seconds. The runner and deadline guard receive the same deadline atomically when consent is consumed. Ordinary startup, declined consent, and non-Windows behavior retain 30 seconds; all ownership, identity, receipt, and silence checks remain intact.

This fixes the reproduced timeout path. The original report's approved runtime or managing CLI changed message has not been reproduced in this baseline and is investigated separately.

Verification

  • Refreshed from dev at 06cc3815c06a10120d89ef986c46be95be37f28a without rewriting branch history, including the accepted upstream role-route and owner-registry ACL fixture corrections. Targeted verification: role routes 7 passed; owner-registry ACL 6 passed. Required CI passed at bd5c085c726ddf305ded802805efa2e7ee3591f7: zero failed and zero pending checks (platform/path-inapplicable checks remain skipped).

  • Rebuilt actual Windows Tauri app at 57056f444b96e32cc79c6849c91a4b2cd92f6d96: four npm 2.67.0 → bundled 2.76.0 takeovers passed, each with persisted desktop ownership. Approval click → healthy replacement plus recorded ownership: 46.278s, 47.511s, 46.545s, 47.609s (excludes pre-consent work and later dashboard rendering).

  • Native tray Quit/relaunch preserved install ID and consent generation without another prompt. Guest decline/Quit kept the npm PID alive and wrote no ownership. Baseline timeout reproduced twice before this fix.

  • cargo test --manifest-path desktop/src-tauri/Cargo.toml startup::tests: 33 passed.

  • cargo test --manifest-path desktop/src-tauri/Cargo.toml: 159 passed, doc tests passed.

  • cargo fmt --manifest-path desktop/src-tauri/Cargo.toml --check and cargo clippy --manifest-path desktop/src-tauri/Cargo.toml --all-targets -- -D warnings: passed.

  • Bun typecheck, changed-file GUI lint, privacy scan, structure check and file-size ratchet: passed.

  • Docs-site frozen install and build with Node 24: passed (545 pages). Full unrelated Bun suite not run for this Rust-only change.

  • Upstream provider requests, reboot persistence and native non-Windows behavior were not tested.

Baseline timeout Fixed native takeover
Baseline timeout Native takeover online

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Link to Devin session: https://app.devin.ai/sessions/a8795b27d4114aa6be37a88565e404b2
Open in Devin Desktop: https://app.devin.ai/desktop/session/a8795b27d4114aa6be37a88565e404b2?variant=devin

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 42f183e2-a3aa-408e-8b34-b6b6577ff9ca

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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 github-actions Bot added the bug Something isn't working label Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/0 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/0).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/0 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as draft October 1, 2026 15:00
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

CI is not green on 57056f444b96e32cc79c6849c91a4b2cd92f6d96: macos 2/2 failed at tests/server/codex-agent-role-routes.test.ts:163 (expected HTTP 500, received 200); the aggregate ci job failed downstream. All other checks settled successfully or were skipped.

Baseline evidence: isolated checkouts of this PR and dev at c94fcf564517109b0758f2812b12c4d40f148873, both on Windows/Bun 1.4.0, each produced six passes and the identical targeted failure. The test and role-writer files have identical object IDs on both revisions. This demonstrates a pre-existing failure; it does not make the required CI green.

The injected filesystem error is not reaching the writer. The named-import/namespace-spy interaction is the leading mechanism, not yet independently established. The proposed fix is limited to reliable error injection while preserving HTTP 500 and path-redaction assertions. No test or production workaround has been applied; approval for the fixture change is pending.

Written by Devin

@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review October 1, 2026 17:53
@github-actions
github-actions Bot marked this pull request as draft October 1, 2026 17:54
@lidge-jun
lidge-jun marked this pull request as ready for review October 1, 2026 23:51
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration record (MAINTAINERS.md, dev-only)

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 1, 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-01T23:55:14.748437Z bd5c085 Draft marked ready
ℹ️ 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.

@lidge-jun
lidge-jun merged commit 137164e into dev Oct 1, 2026
45 of 48 checks passed
@lidge-jun
lidge-jun deleted the devin/1790860476-windows-takeover-budget branch October 1, 2026 23:52

@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: bd5c085c72

ℹ️ 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 on lines +530 to +531
if approved && cfg!(target_os = "windows") {
deadline += Duration::from_secs(60);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Report the actual extended timeout budget

When an approved Windows takeover exhausts this new 90-second ceiling, the user-facing failures still interpolate the 30-second DEADLINE: the deadline guard reports that startup did not finish within 30 seconds, and the final waiting failure likewise says the runtime did not become healthy within 30 seconds. This makes a takeover that actually ran for about 90 seconds contradict the updated documentation and obscures where the extra minute was spent; carry the effective budget alongside the extended deadline or derive the reported duration from that deadline.

Useful? React with 👍 / 👎.

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