Repository navigation
Review PR diffs in the app, with viewed marks and stacked-PR actions (#1527) - #1602
Conversation
Solenta-Message-Id: c9595604-04a9-4ada-8f93-f86c0dc45180
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Keep both link_pull_request and automation_create imports in orchServer. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f30a434c3e
ℹ️ 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".
| const diffArgs = ["pr", "diff", String(number)]; | ||
| if (only) diffArgs.push("--", only); | ||
| const diffed = await ghApiTryAsync(repo.cwd, diffArgs, ghOpts(other)); |
There was a problem hiding this comment.
When a file is omitted as lazy or large, the Code tab calls this path-specific branch, but gh pr diff does not accept a path after --. Its documented usage is gh pr diff [<number> | <url> | <branch>] [flags], and this invocation returns accepts at most 1 arg(s), received 2. Consequently, the files most likely to require lazy loading or “Show anyway” always fail to render; fetch the PR patch and extract the requested file, or use a supported API endpoint.
Useful? React with 👍 / 👎.
| const key = viewedKey(parseRepoSlug(input && input.repo), number); | ||
| const viewed = input && input.viewed === false | ||
| ? null | ||
| : { | ||
| headSha, | ||
| ...(input && input.blobSha ? { blobSha: String(input.blobSha) } : {}), | ||
| }; | ||
| store.setPrViewedMark(key, filePath, viewed); |
There was a problem hiding this comment.
For the primary PR, PrCodeTab omits repo, so this stores the mark under #<number>. A subsequent prDiff resolves the checkout's origin slug and reads from <owner>/<repo>#<number> instead. Thus clicking “Mark viewed” on the normal same-repository PR succeeds but immediately reloads as unseen; derive the repository key from projectPath here just as prDiff does.
Useful? React with 👍 / 👎.
| const merged = await mergePrAt( | ||
| projectPath, | ||
| { prNumber: current.number, method: input && input.method }, | ||
| opts, |
There was a problem hiding this comment.
When a linked PR belongs to another repository, prStack correctly discovers the chain using input.repo, but each mergePrAt call drops that repository. The merge therefore targets the checkout's origin and may either fail or merge an unrelated local PR with the same number, while the later retarget operation still targets the external repository. Include repo: other in every stack merge input.
Useful? React with 👍 / 👎.
| const repo = await requireGitHubRepo(projectPath); | ||
| if (!repo.ok) return repo; | ||
| const fetched = await gitTryAsync(repo.cwd, ["fetch", "origin"]); |
There was a problem hiding this comment.
For a stack selected from a linked repo, discovery is performed in that repository, but the rebase implementation fetches PR refs and force-pushes branches through the local project's origin. In the common case this fails because those refs or branches are absent; if matching names exist, it can rewrite the wrong repository. The rebase action must resolve and consistently use a remote for input.repo, or reject cross-repository stacks.
Useful? React with 👍 / 👎.
| if (!merged.ok) return merged; | ||
| const next = open[i + 1]; | ||
| if (next) { | ||
| const retarget = await retargetPrBase(repo.cwd, next.number, base, other); | ||
| if (!retarget.ok) return retarget; |
There was a problem hiding this comment.
In repositories configured to delete merged head branches, mergePrAt can delete current.headRefName before this code retargets the next PR whose base is that branch. GitHub may then close or otherwise invalidate the next PR before pr edit --base runs, leaving a partially merged stack. Retarget the next PR to the landing base before issuing the merge that can delete its current base.
Useful? React with 👍 / 👎.
| const links = upsertPrLink(normalizePrLinks(thread), link); | ||
| const primary = thread.prNumber == null ? link : null; | ||
| const patch = { | ||
| prLinks: links, | ||
| ...(primary | ||
| ? { | ||
| prNumber: primary.number, | ||
| prUrl: primary.url || thread.prUrl || null, | ||
| prState: primary.state || thread.prState || null, |
There was a problem hiding this comment.
The link_pull_request schema supplies no state, so newly persisted links normally have state unset, and the existing background refresher updates only the legacy prState fields rather than prLinks. Because effectiveSettled treats a nonempty prLinks list as authoritative, even a single linked PR that later becomes merged or closed will not settle from its terminal state and instead falls through to inactivity. Persist refreshed states into the matching link or merge the primary legacy state into normalized links.
Useful? React with 👍 / 👎.
| if (links.length > 0) { | ||
| if (allLinkedPrsSettled(links, settleOnMerge)) return true; | ||
| if (anyLinkedPrOpen(links)) return false; | ||
| } else if (changeRequestAutoSettles(thread.prState, settleOnMerge)) { |
There was a problem hiding this comment.
This all-linked terminal check only changes renderer-side folding. The background path in electron/worktrees-pr.js still calls prFlipSettles solely from the primary prState, then runs onSettle and may delete the merged worktree. Consequently, merging the bottom PR of a stack can execute settlement hooks and reclaim the checkout while another linked PR remains open. Apply the same all-linked predicate before background settlement and cleanup.
Useful? React with 👍 / 👎.
You can review a PR's files inside Solenta instead of leaving for GitHub. The workspace now has a Code tab (lazy per-file diffs + all-commits / single-commit picker), viewed marks that flip to Changed after a later push, several linked PRs per thread (including other repos), and rebase/merge for a stacked chain. Closes #1527.
Changes
FileHunksrenderer;git.prDiffbudgets large patches the same way local diffs do.repo#number+ head SHA (blob SHA when GitHub sends one). Same blob stays Viewed; a changed file becomes Changed.prLinks[]is additive;prNumberstays primary. Agents attach a PR with the always-onlink_pull_requestMCP tool.--force-with-lease.Verification
node --test electron/test/pr-workspace.test.js electron/test/pr-links.test.js— including secret-block (after unsetting inheritedCODER_GUARDRAILS_PATH) and merge-stack retarget onto main.test/prLinks.test.ts,test/prViewed.test.ts,test/prStack.test.ts,test/threadSettle.test.ts,test/prCodeTab.test.tsx.scripts/sync-ipc-preload.js --checkclean.Notes for reviewers
electron/prWorkspace.jsstackdefaultBasefirst: it must bestack[0].baseRefName, not the PR the user has open.services-projectsfromprWorkspace.js(pullsterminal.js/cross-spawnand breaks barenode --test).cursor-grok-4.6 via Cursor
Made with Cursor