FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(browser): harden desktop tool lifecycle by waleedlatif1 · Pull Request #7253 · simstudioai/sim · GitHub

fix(browser): harden desktop tool lifecycle - #7253

Merged
waleedlatif1 merged 5 commits into
stagingfrom
codex/desktop-browser-reliability
Aug 29, 2026
Merged

fix(browser): harden desktop tool lifecycle#7253
waleedlatif1 merged 5 commits into
stagingfrom
codex/desktop-browser-reliability

Conversation

Copy link
Copy Markdown
Collaborator

Summary

  • harden desktop browser authorization, exact cancellation, wait-budget consistency, and current-versus-retired tool boundaries
  • preserve known native outcomes across renderer unloads with compact beacon and keepalive delivery while marking genuinely indeterminate actions as unsafe to retry
  • add privacy-safe queue/execution observability and remove an extra screenshot viewport round trip
  • expand focused coverage for lifecycle races, authorization stalls, protocol drift, cross-frame effects, invalid input, and unload delivery

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

  • bun run lint and bun run lint:check
  • bun run check:audits (39 audits)
  • Sim focused Vitest suite: 8 files, 200 tests
  • Desktop focused Vitest suite: 3 files, 193 tests
  • TypeScript: @sim/app, @sim/desktop, @sim/browser-protocol, @sim/desktop-bridge
  • Desktop bridge compatibility audit
  • git diff --check

Review focus: native execution versus confirmation-delivery lifecycle, exact cancellation scoping, timeout hierarchy, and current/retired browser tool classification.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

N/A — no UI changes.

vercel Bot commented Aug 29, 2026
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Aug 29, 2026 4:48am

greptile-apps Bot commented Aug 29, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens the desktop browser and terminal tool lifecycle across authorization, cancellation, execution, confirmation delivery, and renderer unloads.

  • Adds bounded desktop authorization and exact scope-aware cancellation.
  • Distinguishes current browser tools from retired protocol entries.
  • Aligns renderer, native, and server timeout budgets.
  • Adds durable completion delivery and lifecycle-race coverage.
  • Strengthens screenshot coordinate validation and removes a redundant viewport query.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/desktop/src/main/ipc.ts Adds an authorization deadline, lifecycle diagnostics, and current-tool validation before native browser or terminal execution.
apps/sim/app/api/copilot/confirm/route.ts Extends confirmation handling for known pre-claim terminal outcomes and claimed indeterminate native exits.
apps/sim/lib/copilot/async-runs/repository.ts Adds state- and claimant-specific async tool completion transitions used to reconcile lifecycle races.
apps/sim/lib/copilot/tools/client/completion.ts Adds compact retry, keepalive, and beacon delivery paths for client tool completion.
apps/sim/lib/browser-agent/transport.ts Aligns browser authorization, queueing, execution, and bridge-delivery timeout behavior.
apps/desktop/src/main/browser-agent/driver.ts Normalizes wait budgets, strengthens screenshot coordinate verification, validates scroll direction, and improves scoped execution observability.
packages/browser-protocol/src/index.ts Separates current and retired browser tool classifications while centralizing wait-time normalization.

Sequence Diagram

sequenceDiagram
  participant R as Renderer
  participant M as Electron main
  participant A as Authorization API
  participant N as Native tool
  participant C as Confirmation API

  R->>M: Execute tool call
  M->>A: Claim pending call
  alt Authorization accepted
    A-->>M: Authorized tool, args, and scope
    M->>N: Execute authorized action
    N-->>M: Known result
    M-->>R: Native outcome
    R->>C: Deliver completion
  else Authorization rejected or times out
    A-->>M: Rejection or timeout
    M-->>R: Known pre-dispatch error
    R->>C: Deliver failed completion
  end
Loading

Reviews (7): Last reviewed commit: "fix(copilot): interrupt pending tool wai..." | Re-trigger Greptile

Copy link
Copy Markdown
Collaborator Author

@greptile

cubic-dev-ai 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

2 issues found across 25 files

Confidence score: 2/5

  • In apps/desktop/src/main/ipc.ts, the 8-second abort can race with the server marking an authorization request as running, causing a false authorization failure and skipped native execution; coordinate timeout handling with server-side claim state.
  • In apps/desktop/src/main/browser-agent/cdp.ts, successful CDP metrics can suppress the page URL and title in browser_screenshot; retain getViewportInfo for metadata while using CDP only for dimensions.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/main/browser-agent/cdp.ts">

<violation number="1" location="apps/desktop/src/main/browser-agent/cdp.ts:447">
P2: When CDP metrics succeed, `browser_screenshot` stops returning the page URL and title because this non-null viewport bypasses `getViewportInfo`. Preserve those fields while sourcing only dimensions from CDP.</violation>
</file>

<file name="apps/desktop/src/main/ipc.ts">

<violation number="1" location="apps/desktop/src/main/ipc.ts:516">
P1: The 8-second fetch abort can race with the server-side claim: if the authorization route moves the call to `running` just before the timeout, this handler returns an authorization failure and skips native execution, leaving the one-shot call stuck. Add claim reconciliation/cancellation for timed-out authorizations, or make the claim reversible until the client confirms receipt.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

waleedlatif1 force-pushed the codex/desktop-browser-reliability branch from 663c840 to e6e69db Compare August 29, 2026 03:10

Copy link
Copy Markdown
Collaborator Author

@greptile

Copy link
Copy Markdown
Collaborator Author

@cubic review

cubic-dev-ai Bot commented Aug 29, 2026

Copy link
Copy Markdown

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

4 issues found across 27 files

Confidence score: 2/5

  • apps/sim/lib/copilot/request/lifecycle/run.ts can let a hung tool run inherit a concurrent confirmation’s one-hour wait budget instead of its 60-second watchdog, delaying or stalling request completion; preserve per-tool watchdog enforcement.
  • apps/desktop/src/main/browser-agent/cdp.ts and apps/desktop/src/main/browser-agent/driver.ts can associate a captured image with stale viewport metadata when the browser resizes or navigates during capture, producing inconsistent screenshots for downstream consumers; revalidate and reject mismatched captures after capture.
  • apps/sim/app/api/copilot/confirm/route.ts may finalize a retired browser call when it reports error or cancelled, treating the result as an executable native outcome; restrict the preclaim exception to the current browser call.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/lib/copilot/request/lifecycle/run.ts">

<violation number="1" location="apps/sim/lib/copilot/request/lifecycle/run.ts:1033">
P1: When a confirmation wait and an unrelated tool run in parallel, this maximum budget makes the hung tool wait for the approval's one-hour budget instead of its 60-second watchdog. Preserve per-tool watchdog enforcement for bounded tools while allowing the approval promise to remain durable.</violation>
</file>

<file name="apps/desktop/src/main/browser-agent/cdp.ts">

<violation number="1" location="apps/desktop/src/main/browser-agent/cdp.ts:427">
P2: When the page resizes or navigates between `Page.getLayoutMetrics` and `Page.captureScreenshot`, `cssViewport` describes the pre-capture surface but the returned image can belong to the new surface. Revalidate the viewport after capture, or retain the post-capture consistency check for the CSS-metrics path, before exposing coordinates to the agent.</violation>
</file>

<file name="apps/desktop/src/main/browser-agent/driver.ts">

<violation number="1" location="apps/desktop/src/main/browser-agent/driver.ts:2147">
P2: When the visible browser resizes between CDP layout metrics and screenshot capture, `shot.viewport` describes the pre-capture viewport, but this branch skips any post-capture consistency check. Reject mismatched capture metadata or retain a post-capture viewport check before returning coordinates for `browser_click_at`.</violation>
</file>

<file name="apps/sim/app/api/copilot/confirm/route.ts">

<violation number="1" location="apps/sim/app/api/copilot/confirm/route.ts:272">
P2: When a pending retired browser call reports `error` or `cancelled`, this new preclaim exception treats it as an executable native outcome and finalizes the retired call. Restrict the preclaim exception to current browser tool names; retain the broader classification only for already-running legacy completions if that compatibility is required.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Copy link
Copy Markdown
Collaborator Author

@greptile

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

cubic-dev-ai Bot commented Aug 29, 2026

Copy link
Copy Markdown

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

1 issue found across 29 files

Confidence score: 3/5

  • In apps/sim/app/api/copilot/confirm/route.ts, a page-unload race can let desktop authorization claim the row after the request reads pending, causing a 404 while the row remains running and the native completion is suppressed; handle this race so the authorization flow can complete reliably.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/app/api/copilot/confirm/route.ts">

<violation number="1" location="apps/sim/app/api/copilot/confirm/route.ts:397">
P1: When page unload races desktop authorization, the authorization claim can win after this request reads `pending`; this branch returns 404 and leaves the row `running`. The unload path then suppresses the native completion because cancellation already occurred, so settle the claimed action as an indeterminate, non-retryable failure or coordinate cancellation so the row cannot remain running.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic

waleedlatif1 force-pushed the codex/desktop-browser-reliability branch from d2486c6 to 6029ab6 Compare August 29, 2026 04:24

Copy link
Copy Markdown
Collaborator Author

@greptile

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

cubic-dev-ai Bot commented Aug 29, 2026

Copy link
Copy Markdown

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

1 issue found across 32 files

Confidence score: 3/5

  • In apps/sim/lib/copilot/request/lifecycle/run.ts, an abort with an unresolved pending tool can remain blocked until the watchdog deadline, delaying lifecycle shutdown and potentially user-visible cancellation; race the lifecycle abort signal with watchdogController so the wait exits promptly.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/lib/copilot/request/lifecycle/run.ts">

<violation number="1" location="apps/sim/lib/copilot/request/lifecycle/run.ts:1120">
P1: When the lifecycle aborts with an unresolved pending tool, this race waits until the watchdog deadline because it listens only to `watchdogController`. Race the lifecycle abort signal and exit the wait so the existing cancellation path runs immediately.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic

Copy link
Copy Markdown
Collaborator Author

@greptile

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

cubic-dev-ai Bot commented Aug 29, 2026

Copy link
Copy Markdown

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No issues found across 32 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

waleedlatif1 force-pushed the codex/desktop-browser-reliability branch from 3724ef3 to b1aabef Compare August 29, 2026 04:48

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

Copy link
Copy Markdown
Collaborator Author

@greptile

cubic-dev-ai Bot commented Aug 29, 2026

Copy link
Copy Markdown

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No issues found across 32 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

waleedlatif1 merged commit 2cda264 into staging Aug 29, 2026
30 checks passed
waleedlatif1 deleted the codex/desktop-browser-reliability branch August 29, 2026 04:54
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL