Skip to content

sync: port upstream background preview capture, PiP, source-control settings (#4204–#4975) - #192

Open
aaditagrawal wants to merge 9 commits into
sync/20260730-l2-server-perffrom
sync/20260730-l3-preview-scm
Open

sync: port upstream background preview capture, PiP, source-control settings (#4204–#4975)#192
aaditagrawal wants to merge 9 commits into
sync/20260730-l2-server-perffrom
sync/20260730-l3-preview-scm

Conversation

@aaditagrawal

Copy link
Copy Markdown
Owner

L3/7 of the 2026-07-30 upstream sync stack. Stacked on #191.

What lands

Fork deviations preserved

  • browserRecording.ts: upstream restructured single-recording → per-tab activeRecordings for PiP (376 changed lines vs the fork's 79). Took upstream's file and re-applied the fork's two surviving deltas — a rejectable startupSettled (so a real startup failure reaches pending stops instead of resolving them as success) and the narrowed isStartupWaitTimeout that distinguishes a genuine timeout from a propagated failure. These two are coupled. Upstream independently converged on the fork's removal of the "requires visible tab" guard.
  • Ported the fork's two regression tests to upstream's new flow; upstream deliberately changed stop-during-startup to yield an artifact, so the stale sequencing assertions were rewritten rather than kept.
  • PreviewAutomationBroker: kept the fork's recordingStop early return and evaluate carve-out. Upstream's contradicting "tracks the tab returned by a targeted recording stop" test is dropped in favour of the fork's "does not replace the default tab" test.
  • build-desktop-artifact.ts: upstream unpacks only on Windows. Kept the bundled Copilot CLI unpacked on every platform — resolveBundledCopilotCliPath probes app.asar.unpacked/node_modules/@github/copilot-* for a spawnable binary, so packing it would break the fork's Copilot driver on macOS/Linux.
  • CursorTextGeneration: kept the fork's TextGenerationShape typing while adopting upstream's policy / changeRequestTemplate plumbing.

Verification

vp run typecheck and vp check pass; browserRecording 15/15 and build-desktop-artifact 28/28 green.

juliusmarminge and others added 9 commits July 30, 2026 23:03
…tgg#4397)

Co-authored-by: codex <codex@users.noreply.github.com>
(cherry picked from commit f4c3943)
Co-authored-by: codex <codex@users.noreply.github.com>
(cherry picked from commit 32af2f0)
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: codex <codex@users.noreply.github.com>
(cherry picked from commit 10bca3f)
…ingdotgg#4975)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 50871eb)
…rk behavior

The fork's PreviewAutomationBroker returns early for `recordingStop` so a
stop artifact never re-targets the caller's browsing tab. Upstream pingdotgg#4397
removed that carve-out and added a test asserting the opposite routing.
Keep the fork behavior and its "does not replace the default tab" test,
and drop the contradicting upstream case.
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 105 files, which is 5 over the limit of 100.

To get a review, narrow the scope:
• coderabbit review --committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1a963c8c-4ba4-4f18-9979-c83540663d11

📥 Commits

Reviewing files that changed from the base of the PR and between 896ccca and 3463451.

📒 Files selected for processing (105)
  • .github/workflows/release.yml
  • apps/desktop/src/electron/ElectronWindow.test.ts
  • apps/desktop/src/electron/ElectronWindow.ts
  • apps/desktop/src/ipc/channels.ts
  • apps/desktop/src/ipc/methods/preview.ts
  • apps/desktop/src/preload.ts
  • apps/desktop/src/preview-pip-preload.ts
  • apps/desktop/src/preview/Manager.test.ts
  • apps/desktop/src/preview/Manager.ts
  • apps/desktop/src/window/DesktopWindow.test.ts
  • apps/desktop/src/window/DesktopWindow.ts
  • apps/desktop/vite.config.ts
  • apps/server/src/auth/EnvironmentAuth.test.ts
  • apps/server/src/auth/EnvironmentAuthPolicy.test.ts
  • apps/server/src/auth/EnvironmentAuthPolicy.ts
  • apps/server/src/auth/SessionStore.ts
  • apps/server/src/auth/utils.test.ts
  • apps/server/src/auth/utils.ts
  • apps/server/src/git/GitManager.test.ts
  • apps/server/src/git/GitManager.ts
  • apps/server/src/mcp/PreviewAutomationBroker.test.ts
  • apps/server/src/mcp/toolkits/preview/handlers.test.ts
  • apps/server/src/mcp/toolkits/preview/handlers.ts
  • apps/server/src/mcp/toolkits/preview/tools.ts
  • apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
  • apps/server/src/preview/Manager.test.ts
  • apps/server/src/preview/Manager.ts
  • apps/server/src/server.test.ts
  • apps/server/src/serverSettings.test.ts
  • apps/server/src/serverSettings.ts
  • apps/server/src/sourceControl/PrTemplateDetection.test.ts
  • apps/server/src/sourceControl/PrTemplateDetection.ts
  • apps/server/src/textGeneration/ClaudeTextGeneration.ts
  • apps/server/src/textGeneration/CodexTextGeneration.ts
  • apps/server/src/textGeneration/CursorTextGeneration.ts
  • apps/server/src/textGeneration/GrokTextGeneration.ts
  • apps/server/src/textGeneration/OpenCodeTextGeneration.ts
  • apps/server/src/textGeneration/TextGeneration.ts
  • apps/server/src/textGeneration/TextGenerationPrompts.test.ts
  • apps/server/src/textGeneration/TextGenerationPrompts.ts
  • apps/web/src/browser/BrowserSurfaceSlot.tsx
  • apps/web/src/browser/ElectronBrowserHost.tsx
  • apps/web/src/browser/HostedBrowserWebview.tsx
  • apps/web/src/browser/browserRecording.test.ts
  • apps/web/src/browser/browserRecording.ts
  • apps/web/src/browser/browserRecordingScope.test.ts
  • apps/web/src/browser/browserRecordingScope.ts
  • apps/web/src/browser/browserSurfaceStore.test.ts
  • apps/web/src/browser/browserSurfaceStore.ts
  • apps/web/src/browser/browserViewportActions.test.ts
  • apps/web/src/browser/browserViewportActions.ts
  • apps/web/src/browser/browserViewportLayout.test.ts
  • apps/web/src/browser/browserViewportLayout.ts
  • apps/web/src/browser/desktopTabLifetime.test.ts
  • apps/web/src/browser/desktopTabLifetime.ts
  • apps/web/src/browser/hostedBrowserWebviewStyle.test.ts
  • apps/web/src/browser/hostedBrowserWebviewStyle.ts
  • apps/web/src/browser/previewRuntimeTabId.test.ts
  • apps/web/src/browser/previewRuntimeTabId.ts
  • apps/web/src/browser/webviewCrashRecovery.test.ts
  • apps/web/src/browser/webviewCrashRecovery.ts
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/components/SidebarV2.tsx
  • apps/web/src/components/chat/ProviderModelPicker.tsx
  • apps/web/src/components/preview/PreviewAutomationHosts.tsx
  • apps/web/src/components/preview/PreviewChromeRow.tsx
  • apps/web/src/components/preview/PreviewMoreMenu.tsx
  • apps/web/src/components/preview/PreviewView.test.tsx
  • apps/web/src/components/preview/PreviewView.tsx
  • apps/web/src/components/preview/ThreadPreviewMiniPlayer.tsx
  • apps/web/src/components/preview/previewAutomationOpenReadiness.test.ts
  • apps/web/src/components/preview/previewAutomationOpenReadiness.ts
  • apps/web/src/components/preview/previewMiniPlayerLayout.test.ts
  • apps/web/src/components/preview/previewMiniPlayerLayout.ts
  • apps/web/src/components/preview/previewNavigationReadiness.test.ts
  • apps/web/src/components/preview/previewNavigationReadiness.ts
  • apps/web/src/components/preview/previewViewportRollback.test.ts
  • apps/web/src/components/preview/previewViewportRollback.ts
  • apps/web/src/components/preview/usePreviewBridge.ts
  • apps/web/src/components/preview/usePreviewSession.ts
  • apps/web/src/components/settings/SettingsPanels.tsx
  • apps/web/src/components/settings/SourceControlSettings.tsx
  • apps/web/src/components/settings/SourceControlWritingSettings.tsx
  • apps/web/src/components/ui/tooltip.tsx
  • apps/web/src/hooks/useSettings.ts
  • apps/web/src/modelSelection.ts
  • apps/web/src/previewMiniPlayerStore.test.ts
  • apps/web/src/previewMiniPlayerStore.ts
  • apps/web/src/previewStateStore.test.ts
  • apps/web/src/previewStateStore.ts
  • apps/web/src/routes/settings.tsx
  • apps/web/vercel.ts
  • package.json
  • packages/contracts/src/ipc.ts
  • packages/contracts/src/model.ts
  • packages/contracts/src/preview.test.ts
  • packages/contracts/src/preview.ts
  • packages/contracts/src/previewAutomation.ts
  • packages/contracts/src/settings.test.ts
  • packages/contracts/src/settings.ts
  • packages/shared/src/serverSettings.test.ts
  • packages/shared/src/serverSettings.ts
  • scripts/build-desktop-artifact.test.ts
  • scripts/build-desktop-artifact.ts
  • scripts/clean-tsgo-backups.mjs

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


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 vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Jul 31, 2026
@github-actions github-actions Bot added the size:XXL 1,000+ effective changed lines (test files excluded in mixed PRs). label Jul 31, 2026
@aaditagrawal

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

ℹ️ 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".

recentlySeenUrls,
};
if (current.serverEpoch !== null && event.serverEpoch !== current.serverEpoch) return current;
if (event.revision < current.serverRevision) return current;

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 Ignore events already represented by the latest list

When a preview.list response and its corresponding event race across the separate RPC streams, the list can be applied first with the same revision as the delayed event. Accepting equality here reapplies that event; notably, a delayed opened event then overwrites the active tab selected or preserved during reconciliation, even though the authoritative list already included the new tab. Treat revisions equal to current.serverRevision as already incorporated so only strictly newer events are applied.

Useful? React with 👍 / 👎.

Comment on lines 44 to +46
const reconcileSessions = (result: Atom.Type<typeof sessionsAtom>) => {
if (!AsyncResult.isSuccess(result)) return;
if (result.value.sessions.length > 0) {
recoveringUrl = null;
recoveryId += 1;
reconcilePreviewServerSessions(threadRef, result.value.sessions);
return;
}

const localSnapshot = readThreadPreviewState(threadRef).snapshot;
const recoverableUrl =
localSnapshot && localSnapshot.navStatus._tag !== "Idle"
? localSnapshot.navStatus.url
: null;
if (!recoverableUrl) {
applyPreviewServerSnapshot(threadRef, null);
return;
}
if (recoveringUrl === recoverableUrl) return;

recoveringUrl = recoverableUrl;
const currentRecoveryId = ++recoveryId;
void runAtomCommand(
get.registry,
previewEnvironment.open,
{
environmentId: threadRef.environmentId,
input: { threadId: threadRef.threadId, url: recoverableUrl },
},
{ reportDefect: false, reportFailure: false },
).then((openResult) => {
if (disposed || currentRecoveryId !== recoveryId) return;
recoveringUrl = null;
if (openResult._tag === "Failure") return;
applyPreviewServerSnapshot(threadRef, openResult.value);
get.refresh(sessionsAtom);
});
reconcilePreviewServerSessions(threadRef, result.value);

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 Recreate the active preview after a server restart

When the backend restarts, the new in-memory preview manager returns an empty list with a new serverEpoch; unconditionally reconciling that result now clears every local session and unmounts the still-running Electron webview. The removed code reopened the active recoverable URL with a fresh server tab, so ordinary backend restarts now discard the user's preview instead of safely remapping it to the new epoch. Restore that recovery without reusing the old raw tab ID.

AGENTS.md reference: AGENTS.md:L20-L25

Useful? React with 👍 / 👎.

Comment on lines +1599 to +1602
const changeRequestTemplate =
settings.style.followChangeRequestTemplates && provider.kind === "github"
? Option.getOrUndefined(yield* detectPrTemplate(cwd, baseRangeRef, gitCore.execute))
: undefined;

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 Honor change-request templates for GitLab repositories

When the detected provider is GitLab, this condition always suppresses template detection even though GitLab change requests are supported and the new setting is presented as the provider-neutral “Follow change request templates.” Consequently, repositories using GitLab merge-request templates always receive the generic generated body while the setting appears enabled. Dispatch to provider-specific template detection, including GitLab's merge-request template locations, rather than restricting the feature to GitHub.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ effective changed lines (test files excluded in mixed PRs). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants