fix(desktop): keep retrying preview snapshot captures on UnknownVizError for a time budget

[pujitha24]

What Changed

capturePageWithRetry (apps/desktop/src/preview/Manager.ts) now retries UnknownVizError every 120 ms for a 2.5 s budget instead of three fixed attempts. Other capture failures keep the three-attempt retry, and the 1 s per-attempt timeout is unchanged. Added a test with ten consecutive UnknownVizError rejections followed by success.

Why

While a Linux Wayland session is locked or monitors are off, the compositor (niri) sends frame callbacks to hidden windows only every ~1-2 s, so a guest rejects capturePage with UnknownVizError until then. The old retry gave up after ~240 ms, so some preview_snapshot calls failed. A time budget above the worst-case interval fixes that, while a guest that never paints still fails.

Validation: vp test run src/preview/Manager.test.ts passes (92 tests); the new case fails without the change and passes with it. Typecheck is clean. I could not reproduce a locked Wayland session here, so the compositor behavior relies on the issue's analysis. The tool error text is not changed.

Report: #13997

UI Changes

None. This change does not alter any UI; it only changes retry behavior for preview snapshot captures in the desktop app.

Checklist

• This PR is small and focused
• I explained what changed and why
• I included before/after screenshots for any UI changes
• I included a video for animation/interaction changes

Fixes #13997

Summary by CodeRabbit

• Bug Fixes
  • Improved preview screenshot capture reliability when temporary rendering errors occur. Capture attempts can now continue for longer in these cases, while other capture failures retain a limited retry period. This helps previews succeed more often when the browser encounters a brief rendering issue.

[macroscopeapp]

Approvability

Verdict: Approved at 36eb273

Macroscope's review found this PR approvable — This is a localized desktop reliability fix that extends retries only for the specific transient UnknownVizError condition, while preserving existing behavior for other failures. The production change is small, bounded, and accompanied by regression coverage.

Notes:

• Diff unchanged. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 364fae85-0918-4168-a290-fe909156d5a0

📥 Commits

Reviewing files that changed from the base of the PR and between ba79610 and 36eb273.

📒 Files selected for processing (2)

• apps/desktop/src/preview/Manager.test.ts
• apps/desktop/src/preview/Manager.ts

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

---

📝 Walkthrough

Walkthrough

Preview screenshot capture now retries UnknownVizError failures for up to 2,500 ms, while other capture errors retain a three-attempt limit. A test covers capture succeeding after ten consecutive UnknownVizError failures.

Changes

Preview capture retry policy

Layer / File(s)	Summary
Retry budget and capture validation apps/desktop/src/preview/Manager.ts, apps/desktop/src/preview/Manager.test.ts	capturePageWithRetry retries UnknownVizError at 120 ms intervals for up to 2,500 ms. Other PreviewOperationErrors retain the three-attempt limit. The test verifies success on the eleventh attempt after ten failures.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 36eb2

The preview capture retry change is mergeable after normal checks; the reported loss of ordinary retries does not occur.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 36eb2

The change adds no new access path and keeps the checks that prevent a capture from accepting a replaced preview guest. The 2.5-second retry budget is not a deadline for the whole capture, so automation control may be held longer than that figure suggests.

Retained concerns

• Low · reliability · inferred: The retry budget does not bound total snapshot duration. A capture attempt can continue under its separate timeout while the snapshot holds exclusive per-tab automation control.

Security review details

Security Blast Radius

• inferred — The changed retry policy affects captures for existing preview tabs, not a new external caller or privilege. Its longest relevant control effect is within an automation snapshot’s per-tab session.

Trust Boundaries and Controls

• observed — The current-guest checks are repeated inside the retried capture effect. Existing tests show that replacement during a retry delay or an in-flight capture prevents the stale image from being written as a screenshot artifact.

Resilience and Maintainability Implications

• inferred — The 2,500 ms schedule value is not an end-to-end capture deadline because a separately timed capture attempt can still be in progress. The exact persistent-failure termination behavior of the combined schedule remains unverified.

Hardening Proposals

• proposed — If exclusive-control duration must have a firm limit, apply and test an end-to-end capture deadline, including slow persistent UnknownVizError failures.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the desktop fix and the time-budget retry behavior for preview snapshot captures on UnknownVizError.
Description check	✅ Passed	The description includes What Changed, Why, UI Changes, and Checklist sections. It explains the retry behavior, rationale, validation, issue reference, and confirms that no UI changed. The unchecked s…
Linked Issues check	✅ Passed	Issue #13997 requires a longer retry window for UnknownVizError while preserving the per-attempt timeout. Manager.ts adds a 2,500 ms UnknownVizError budget with 120 ms retry spacing. It keeps th…
Out of Scope Changes check	✅ Passed	The pull request changes only preview capture retry logic and its automated test. The implementation directly supports issue #13997. The test validates the new delayed-frame behavior. No unrelated pro…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

Dismissing prior approval to re-evaluate 36eb273