fix(pr-merge): resolve repo from the project checkout, not process.cwd(), in PR-mode auto-merge#2281
Conversation
processPullRequestMergeTask already resolved prRepo from the per-project cwd (FN-7133) but only passed it to getPrMergeStatus. findPrForBranch/ createPr/mergePr fell back to GitHubClient.resolveRepo's process.cwd() resolution, which fails (or silently targets the wrong repo) in centrally-installed multi-project deployments. Co-Authored-By: Claude <noreply@anthropic.com>
…rCallback Co-Authored-By: Claude <noreply@anthropic.com>
…cess cwd Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
The CLI-injected respondOps.getCwd defaults to process.cwd() because no CLI composition site wires getTaskWorktree; in a centrally-installed multi-project server the review-response run's git ops and agent would run outside the project repo. The engine owns the store, so buildRespondCallback now prefers the task's recorded worktree and keeps ops.getCwd as the single-project fallback. Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughPR merge operations now receive explicit repository identity derived from task worktrees or callback cwd. PR response callbacks resolve their working directory from recorded task data with CLI fallback behavior, and tests cover group, per-task, and response flows. ChangesPR repository resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TaskWorktree
participant processPullRequestMergeTask
participant GitHubOperations
participant buildRespondCallback
TaskWorktree->>processPullRequestMergeTask: provide repository context
processPullRequestMergeTask->>GitHubOperations: find, create, and merge PR with owner/repo
buildRespondCallback->>TaskWorktree: resolve recorded worktree
buildRespondCallback->>GitHubOperations: construct git operations with resolved cwd
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR makes pull-request workflows use each project's checkout instead of the server process directory. The main changes are:
Confidence Score: 5/5The updated code looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(pr-merge): address review notes — fa..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/cli/src/commands/task-lifecycle.ts (1)
450-451: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the required FNXC format to the new requirement comments.
packages/cli/src/commands/task-lifecycle.ts#L450-L451: prefix the worktree rationale withFNXC:PrMergeAutoMerge yyyy-MM-dd-hh:mm:.packages/cli/src/commands/__tests__/task-lifecycle.test.ts#L162-L165: encode the central-install regression requirement with the same FNXC convention.As per coding guidelines, important implementation rationale must use dated
FNXC:Area-of-productcomments. Based on learnings, use the established FNXC convention without adding a literalFNXC_LOGtoken.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/task-lifecycle.ts` around lines 450 - 451, Update the rationale comment near the task worktree git operations in packages/cli/src/commands/task-lifecycle.ts:450-451 to begin with the dated convention “FNXC:PrMergeAutoMerge yyyy-MM-dd-hh:mm:”, without adding FNXC_LOG. Also update the central-install regression requirement comment in packages/cli/src/commands/__tests__/task-lifecycle.test.ts:162-165 with the same FNXC format and area prefix.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/gh-4-pr-merge-repo-resolution.md:
- Around line 5-7: Add a concise FNXC:Area-of-product Markdown comment near the
changeset summary, including a timestamp in yyyy-MM-dd-hh:mm format. Keep the
comment aligned with the documented multi-project deployment requirement and
preserve the existing summary and metadata.
In `@packages/cli/src/commands/__tests__/task-lifecycle.test.ts`:
- Around line 1909-1910: Update the push-call assertion in the task lifecycle
test to verify that the recorded Git push call’s cwd is
/projects/repo-a/.worktrees/fn-9601, while retaining the existing assertion that
the push call exists.
In `@packages/cli/src/commands/task-lifecycle.ts`:
- Around line 435-440: Update resolvePrSource to use the configured
options.getTaskWorktree resolver when task.worktree is absent, matching
createPr’s authoritative worktree behavior. Resolve the repository from the
resulting task worktree before falling back to process.cwd(), and preserve the
existing getCurrentRepo flow for valid worktree paths.
In `@packages/engine/src/pr-nodes.ts`:
- Around line 258-268: Update the responder setup around fullStore.getTask so it
first checks typeof fullStore.getTask === "function" before invoking it. When
the method is unavailable, use the existing cwd fallback and do not pass that
structural store to makePrResponseAgentRunner; only provide the store when
getTask exists.
---
Nitpick comments:
In `@packages/cli/src/commands/task-lifecycle.ts`:
- Around line 450-451: Update the rationale comment near the task worktree git
operations in packages/cli/src/commands/task-lifecycle.ts:450-451 to begin with
the dated convention “FNXC:PrMergeAutoMerge yyyy-MM-dd-hh:mm:”, without adding
FNXC_LOG. Also update the central-install regression requirement comment in
packages/cli/src/commands/__tests__/task-lifecycle.test.ts:162-165 with the same
FNXC format and area prefix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: be140fa7-fa2d-410c-bbaf-5ad3f939c691
📒 Files selected for processing (5)
.changeset/gh-4-pr-merge-repo-resolution.mdpackages/cli/src/commands/__tests__/task-lifecycle.test.tspackages/cli/src/commands/task-lifecycle.tspackages/engine/src/__tests__/pr-respond-cwd-resolution.test.tspackages/engine/src/pr-nodes.ts
…o/store Review-round fixes for the gh-4 repo-resolution PR: - resolvePrSource honors the configured getTaskWorktree resolver (same precedence as createPr) and throws instead of persisting entity.repo as "" when no repo resolves — the pr-create node maps the throw to a routable source-error, and "" could only ever succeed by silently targeting whatever repo process.cwd() sits in. - buildRespondCallback guards getTask with a typeof check: structural stores without getTask degrade to the ops.getCwd fallback and are withheld from the agent runner (whose optional store param assumes getTask); a real getTask rejection now propagates to the pr-respond node's routable respond-error instead of silently running a mutating agent + git push in a possibly-wrong cwd. - Tests: push-cwd assertion (execFile mock now records opts.cwd), getTaskWorktree-precedence + loud-throw coverage for resolvePrSource, store-withholding + rejection-propagation coverage for the respond run. - FNXC prefixes added to the comments flagged in review. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ChEa8SHFYNAzjCdFbwFMfh
|
Thank you! This is a big issue appreciate the fix |
Summary
In a centrally-installed, multi-project Fusion server (one process serving several repos,
process.cwd()= the install dir, not any repo), every task undermergeStrategy: "pull-request"fails at the auto-merge stage with:PR creation from the dashboard and status polling work; only the engine's automatic PR path fails. This is the non-workspace sibling of #1924 (FN-7610 routed workspace-mode tasks to direct merge but does not cover regular multi-project tasks) and the completion of #1797/FN-7133 (which fixed only the
getPrMergeStatusarguments).Root cause
GitHubClient.resolveRepo()(packages/dashboard/src/github.ts) falls back to a cwd-lessgetCurrentRepo()— i.e.git remote get-url origininprocess.cwd()— whenever a PR method is called without explicitowner/repo. The engine merge path already resolves the correct repo from the per-project cwd (prRepo = getCurrentRepo(cwd), FN-7133) but only threaded it intogetPrMergeStatus. Every other GitHub call omitted it:processPullRequestMergeTask:findPrForBranch/createPr/mergePron both the per-task and shared-branch-group pathscreateGroupPrCallback(group-PR promotion):findPrForBranch/createPrcreatePrNodeGithubOps(pr-create/pr-mergeworkflow nodes): cwd-lessgetCurrentRepo()persistedentity.repoas""(poisoning the downstreamsplitRepoSlugconsumers), and the git push/createPr/mergePrran againstprocess.cwd()buildRespondCallback):respondOps.getCwdcollapses toprocess.cwd()because no CLI composition site wiresgetTaskWorktree, so its git ops and response agent ran outside the project repoIn a central install the fallback throws; worse, if
process.cwd()happens to be inside some other git repo, it silently targets the wrong repository.What changed
fix(pr-merge): thread repo identity into PR auto-merge GitHub calls— widens the CLI-localGitHubOperationsinterface (optionalowner/repo, already accepted byGitHubClient'sFindPrParams/CreatePrParams/MergePrParams) and passesprRepoat all six call sites inprocessPullRequestMergeTask.fix(pr-merge): resolve group-PR repo from project cwd in createGroupPrCallback— resolves viagetCurrentRepo(cwd)from the callback input (same T4 pattern assyncGroupPrCallback) with a loud failure instead of a silent wrong-repo fallback.fix(pr-merge): resolve PR-node repo from task worktree instead of process cwd—resolvePrSourceresolves fromtask.worktree, git ops run ingetTaskWorktree(...) ?? task.worktree ?? process.cwd(), andcreatePr/mergePrpassowner/repoparsed fromentity.repo.fix(pr-merge): resolve review-response run cwd from the task worktree— the engine owns the store, sobuildRespondCallbackprefers the task's recordedworktreefor the response run's git ops + agent, keepingops.getCwdas the single-project fallback (defensive against structuralPrNodeStores withoutgetTask).@runfusion/fusionpatch, structured body) included.Deliberately not done: a constructor-scoped default repo on
GitHubClient— one client instance is shared across all projects in a central install (serve.ts/daemon.ts/dashboard.ts), so per-callowner/repois the only correct scope.Testing
getCurrentRepomocked as(cwd?) => cwd ? repo : null, exactly the failing environment) and drive the merge flow end-to-end on the per-task path, the shared-branch-group path,createGroupPrCallback, and all threecreatePrNodeGithubOpsops, asserting every GitHub call carries explicitowner/repo(45 tests inpackages/cli/src/commands/__tests__/task-lifecycle.test.ts, all green).packages/engine/src/__tests__/pr-respond-cwd-resolution.test.tscovers the respond-run cwd: worktree preferred,ops.getCwdfallback when the task has no worktree, when the lookup fails, and when a structural store has nogetTask.pnpm lint,pnpm typecheck, andpnpm buildgreen locally;pnpm test:gate's engine-core suite green (294/294) — its PostgreSQL-backend lane needs local PG credentials this environment lacks, so that lane defers to CI.pnpm verify:fast(scoped typecheck/build + CLI build + boot smoke) also passes.Repro
originandmergeStrategy: "pull-request".Could not determine repository …(tasks with a persisted PR poll fine but never merge). Merging the same task from the Pull Requests tab succeeds, because the dashboard route resolves the repo explicitly (parseBadgeUrl(...) ?? getCurrentRepo(rootDir)).Full analysis: Tchori-Labs#4
Developed with Claude (co-authored on all commits).
https://claude.ai/code/session_01ChEa8SHFYNAzjCdFbwFMfh
Summary by CodeRabbit
owner/repo) is used for pull request lookup, creation, and merging throughout the merge workflow.