merge: sync upstream through 0a7c662d3 (10 commits) - #392
Merged
Conversation
…ing (pingdotgg#5659) Co-authored-by: Rodrigo Brechard <rodrigo@clubtidy.fr> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: codex <codex@users.noreply.github.com>
…g#4849) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: maria <maria@kuuro.net> Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com> Co-authored-by: codex <codex@users.noreply.github.com>
…dotgg#5878) Co-authored-by: tsouth89 <tsouth89@users.noreply.github.com>
Adopts the 10 upstream commits that landed after 3d74474, headlined by the multi-provider pull requests page with in-app reviews (pingdotgg#4849), themed confirmation dialogs (pingdotgg#5624), built-in theme contrast (pingdotgg#6000) and the v0.0.33 release prep. Co-authored-by: Patrick Roza <42661+patroza@users.noreply.github.com>
pingdotgg#5624 replaced the desktop's native confirmation with a themed React dialog: it removed `confirm` from DesktopBridge, the preload API and the IPC channel and handler. The auto-merge had dropped the fork's `confirm` declaration while keeping its implementation, and restoring it faithfully — as I first did — left a service whose only remaining callers were its own tests, competing with the themed dialog that actually runs. The whole surface goes: declaration, implementation, error class and union entry, input type, button-index constant and the tests. All confirmations now go through LocalApi.dialogs.confirm and ConfirmDialogHost, which is upstream's intent. The fork's flat/recency sidebar list never received pingdotgg#4849's right-panel PR opening. Threading the required prop into the project rows was enough to make the merge typecheck, so the gap was invisible: the recency rows called openPrLink without a thread ref and always fell out to the browser. They now take openPullRequestsInRightPanel and use the same contract as the project rows — open in the panel, and navigate to the thread so the panel has something to sit beside. Found by the gpt-5.6-sol and grok-4.5 adversarial reviews respectively. Co-authored-by: Patrick Roza <42661+patroza@users.noreply.github.com>
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopts upstream through
0a7c662d3— the 10 commits after3d74474f6, headlined by pingdotgg#4849, themulti-provider pull requests page with in-app reviews, plus themed confirmation dialogs (pingdotgg#5624),
built-in theme contrast (pingdotgg#6000) and the v0.0.33 release prep.
Important
Merge with a merge commit, not squash. Squashing collapses the second parent and discards
upstream's commits as ancestors — the thing that keeps "commits behind upstream" honest.
Upstream commits entering the product
0a7c662d3b91a000a18de0aa24dcad2c936178f462c4e3b72d17cbd440442db67ef189d8fbcae59ef9821bca1cResolutions
12 conflicted files. The two that actually mattered:
ElectronDialog.tsconfirm(Option<BrowserWindow>); pingdotgg#5624 addedshowMessageBoxnext to it. The auto-merge kept the fork'sconfirmimplementation while dropping its interface declaration, itsElectronDialogConfirmErrorclass, itsElectronDialogConfirmInput, theCONFIRM_BUTTON_INDEXconstant and its entry in theElectronDialogErrorunion — leaving a method that existed but could not be calledHEAD^1. Review then showed the whole surface was dead — pingdotgg#5624 removed the bridge, preload API and IPC channel — so it is removed instead, and confirmations run through upstream's themed dialog. See the review sectionLegacySidebar.tsxshowProjectGroupsand puts a flat/recency list above it. Upstream's copy would have rendered the group twice, since the shared region below already completes the fork'sopenPullRequestsInRightPanelprop threaded to the two fork-only call sites, and pingdotgg#6000's contrast change (text-muted-foreground/60→text-icon-muted) applied by hand rather than lostThe rest were additive unions — the fork's
identity.*RPCs beside upstream's 13 newpullRequests.*ones, fork props beside
onOpenPullRequest, fork-owned release workflow and desktop dialog testskept whole.
routeTree.gen.tswas regenerated with the router plugin, not hand-merged.Two consequences of upstream's new feature that needed fixing rather than resolving: fork test
helpers gained upstream's new
ProcessRunOutputinvalid-UTF-8 fields, and upstream's newStyledDiffCodeView.test.tsxjoined the fork'sisolatedUnitTestFiles— it usesvi.mockand failsunder the fork's
isolate: false, which is the documented fork-only case that list exists for.Checks I ran on the two riskiest unions
Rather than trust the unions:
(
projects.add/list/remove) are identical onHEAD^1and onupstream/main, so theyare pre-existing and not something this merge introduced. Method count 88 (fork) → 101 confirms
upstream's 13 arrived intact.
onOpenPullRequestis genuinely wired, not a prop nothing supplies:ChatView.tsx:6697→
ChatHeader.tsx:494→GitActionsControl.tsx:1240invokes it. Upstream's in-app PR openingworks on the fork.
Adversarial review
Both reviewers went through all 12 resolutions. Neither found a HIGH. Two findings were real and
are fixed in
46200ae7d; the rest confirmed sound.ElectronDialog.confirmis dead production code. pingdotgg#5624 removedconfirmfromDesktopBridge, the preload API and the IPC channel/handler, so the fork's owner-aware implementation had no caller left but its own tests, competing with the themed dialog that actually runsHEAD^1that its one production caller (ipc/methods/window.ts:231) is gone. Declaration, implementation, error class and union entry, input type, button-index constant and tests all removed; confirmations run throughLocalApi.dialogs.confirm→ConfirmDialogHostopenPrLinkwithout a thread ref and always fell out to the browseropenPullRequestsInRightPaneland use the same contract as the project rowsWS_METHODS.projects{List,Add,Remove}have no scope entryRpc.make, no handler and no caller — dead legacy names, identical on both parents. The real invariant (registeredWsRpcGrouprequests vsRPC_REQUIRED_SCOPES) is complete and test-enforcedtext-muted-foreground/60chrome elsewhere in the legacy sidebarConfirmed sound by both: the
LegacySidebarProjects group renders once with the prop threaded androuteThreadRef !== nullpreserved;onOpenPullRequestis live end-to-end; the route tree matchesall 22 route files with
/board,/jumpand/pull-requestspresent; the invalid-UTF-8 fixtureadditions track a real upstream decode change rather than masking one; isolating
StyledDiffCodeView.test.tsxpreserves its assertions rather than weakening them; and the fork's PRbadges and upstream's PR page are distinct surfaces — branch-scoped VCS status versus
repository-wide
PullRequestService— with no competing state or dead fetching path.Verification
vp buildof the web app passes — it caught a duplicateuseComposerDraftStoreimport my unionleft behind, which typecheck did not.
CodexTextGenerationlaunch-args one,which reproduces on unmerged
fork/dev.pnpm-lock.yamlregenerated rather than hand-merged.Co-authored by @patroza
opened by Patrick Roza in chat thread Discord · Discord · T3