-
Notifications
You must be signed in to change notification settings - Fork 296
Description
Summary
Today the docs state that Push to PR Branch is "same-repo only" (safe-outputs.md line 41). The handler already supports target-repo and allowed-repos in config and resolves the target repo for the GitHub API; the missing piece is patch generation and conclusion git operations both assume a single workspace root. When the workflow checks out the target repo under a path (e.g. ./proxy-frontend for caido/proxy-frontend), patch generation runs in the workspace root and finds no commits, and the conclusion job runs git fetch/checkout/apply in the wrong directory. This plan makes push_to_pull_request_branch work end-to-end for cross-repo and multi-checkout.
Analysis
Current behavior
- Config: compiler_types.go and safe_outputs_config_generation.go already pass
target-repo(and allowed-repos) into the handler config. cross-repository.md already showspush-to-pull-request-branch: target-repo: "org/target-repo". - MCP handler (safe_outputs_handlers.cjs
pushToPullRequestBranchHandler): Resolves target repo viaresolveAndValidateRepo(entry, defaultTargetRepo, allowedRepos)and getsbaseBranchfor that repo, but then callsgetCurrentBranch()with no args andgenerateGitPatch(entry.branch, baseBranch, pushPatchOptions)withoutcwdorrepoSlug. So patch generation always runs inGITHUB_WORKSPACE/process.cwd()(e.g. ai-ops root). If the agent committed in./proxy-frontend, that repo’s branch/commits are invisible there → "No commits were found to push." - Conclusion handler (push_to_pull_request_branch.cjs): Uses
resolveAndValidateRepoand runsgit fetch,git checkout,git applyviaexec.execwith nocwd. So git runs in the job’s default cwd (workspace root), not in the target repo’s checkout.
Existing pattern to reuse
- create_pull_request in safe_outputs_handlers.cjs: When
entry.repois set, it callsfindRepoCheckout(repoSlug), usesrepoCwd = checkoutResult.path, passesgetCurrentBranch(repoCwd)andpatchOptions.cwd/patchOptions.repoSlugintogenerateGitPatch. So patch is generated from the correct repo directory. - generate_git_patch.cjs already supports
options.cwdandoptions.repoSlug(lines 91–95). - get_current_branch.cjs accepts
customCwd. - find_repo_checkout.cjs returns
{ success, path, repoSlug }for a givenowner/reposlug.
Gap
- MCP handler: For push_to_pull_request_branch, after resolving
repoResult(so we haveitemRepo), we never callfindRepoCheckout(itemRepo). We should: if we have a resolved target repo, find its checkout path; use that forgetCurrentBranch(cwd)and forgenerateGitPatch(..., { cwd, repoSlug, ... }). If target repo is configured but not found in workspace, return a clear error (e.g. "Repository 'org/repo' not found in workspace. Check out the target repo with a path in checkout."). - Conclusion handler: Before running any git commands, resolve the target repo (already done) then resolve the path to that repo’s checkout (e.g. via
findRepoCheckout(itemRepo)). Run all git operations (fetch, checkout, apply, push) with that directory as the working directory (e.g.exec.exec(..., { cwd: repoPath })or a singleprocess.chdir(repoPath)at the start of the push logic for that message). - Docs: Remove "same-repo only" for push-to-pull-request-branch and document cross-repo (target-repo, allowed-repos, checkout path requirement).
- Tool schema (optional): Add optional
repoparameter to safe_outputs_tools.json for push_to_pull_request_branch so whenallowed-reposhas multiple repos the agent can specify which one; align with create_pull_request. If scope is kept minimal, this can be a follow-up.
Implementation plan
1. MCP handler – patch generation in target repo directory
File: actions/setup/js/safe_outputs_handlers.cjs
- In
pushToPullRequestBranchHandler, afterresolveAndValidateRepoandgetBaseBranch(repoParts):- Set
itemRepo = repoResult.repo(the resolved slug, e.g.caido/proxy-frontend). - Call
findRepoCheckout(itemRepo, undefined, { allowedRepos }). If!checkoutResult.success, return a JSON error with a clear message that the target repo must be checked out with a path (point to checkout path docs or cross-repository.md). - Set
repoCwd = checkoutResult.path,repoSlug = itemRepo.
- Set
- When branch is not provided or equals base branch: call
getCurrentBranch(repoCwd)instead ofgetCurrentBranch(). - When building
pushPatchOptions: ifrepoCwdis set, addpushPatchOptions.cwd = repoCwdandpushPatchOptions.repoSlug = repoSlug. - Pass
pushPatchOptionsintogenerateGitPatch(entry.branch, baseBranch, pushPatchOptions)(already done; only the contents of pushPatchOptions change).
This mirrors the create_pull_request branch/patch flow (lines 253–331) for push_to_pull_request_branch.
2. Conclusion handler – run git in target repo directory
File: actions/setup/js/push_to_pull_request_branch.cjs
- Require
findRepoCheckoutfrom./find_repo_checkout.cjs. - After resolving the target repo (
repoResult/itemRepo) and before running any git commands (fetch, checkout, apply, push), resolve the checkout path:- If
itemRepodiffers fromprocess.env.GITHUB_REPOSITORY(or wheneverdefaultTargetRepois set), callfindRepoCheckout(itemRepo, process.env.GITHUB_WORKSPACE, { allowedRepos }). If not found, return{ success: false, error: "..." }with a message that the target repo must be checked out with a path. - Set
repoCwd = checkoutResult.path.
- If
- Run all git operations for this message in
repoCwd: useexec.exec("git", [...], { cwd: repoCwd, env: { ...process.env, ...gitAuthEnv } })(and equivalent for any other git calls) so that fetch, checkout, apply, and push run in the target repo. Alternatively,process.chdir(repoCwd)at the start of the push block and restore cwd in a finally block; prefer explicitcwdin exec to avoid global state. - Ensure
getGitAuthEnvis still applied for fetch/push in the target repo.
3. Tests
Files: actions/setup/js/safe_outputs_handlers.cjs (or a dedicated test file if handlers are tested elsewhere), actions/setup/js/push_to_pull_request_branch.test.cjs
- Add or extend tests for push_to_pull_request_branch when
target-repois set and the workspace contains the target repo at a subdirectory: mock or fixture with two checkouts (e.g. root = workflow repo, subdir = target repo); assert patch generation is called withcwdandrepoSlugpointing at the target repo. - Add a test for conclusion handler: when target repo is different from GITHUB_REPOSITORY, assert git commands are run with
cwdset to the target repo path (or that findRepoCheckout is invoked and its path is used). - Add a test for error path: target-repo configured but that repo is not found in workspace; assert a clear error is returned (MCP handler and, if feasible, conclusion handler).
4. Documentation
File: docs/src/content/docs/reference/safe-outputs.md
- Remove "same-repo only" from the Push to PR Branch bullet (line 41). Replace with wording that cross-repo is supported when
target-repois set and the target repository is checked out (e.g. with apathincheckout).
File: docs/src/content/docs/reference/safe-outputs-pull-requests.md (and cross-repository.md if needed)
- In the Push to PR Branch section, add a short "Cross-repo" subsection: when using
target-repo(and optionallyallowed-repos), the target repository must be checked out in the workspace (e.g.checkout: - repository: org/target-repo; path: ./target-repo). Link to cross-repository.md and the existing example that showspush-to-pull-request-branch: target-repo: "org/target-repo".
5. Go / validation (optional)
- pkg/workflow/safe_outputs_validation_config.go:
push_to_pull_request_branchcurrently has"branch": {Required: true}. The tool schema treats branch as optional (defaulting to current branch). If the implementing agent finds that Required: true causes validation failures when branch is omitted in cross-repo flows, relax to optional or align with create_pull_request. - No change to compiler or config generation is strictly required for the above; target-repo and allowed-repos are already passed through.
6. Tool schema (optional follow-up)
- In pkg/workflow/js/safe_outputs_tools.json, add an optional
repoproperty to the push_to_pull_request_branch tool (type string, description: target repository inowner/repoformat; when omitted, use the configured target repository; must be in allowed-repos if specified). This allows the agent to disambiguate when multiple repos are allowed. Implement only if the first iteration supports multiple allowed-repos and the agent needs to pass repo explicitly.
Guidelines for the implementing agent
- Follow scratchpad/code-organization.md and scratchpad/testing.md.
- Use existing helpers:
findRepoCheckout,getCurrentBranch(customCwd),generateGitPatch(..., { cwd, repoSlug }),resolveAndValidateRepo,resolveTargetRepoConfig. - Error messages: use the template [what's wrong]. [what's expected]. [example] per CONTRIBUTING.md.
- Run
make agent-finishbefore completing (build, test, recompile, format, lint). - After implementation, a workflow that runs in repo A with
checkout: - repository: B; path: ./Bandpush-to-pull-request-branch: target-repo: "B"should generate the patch from./Band apply/push in./B, so commits made in./Bare pushed correctly.
Out of scope (for this issue)
- Adding an optional
repoargument to the tool (can be a follow-up issue). - Changing how the activation job checks out repos (no change to checkout_manager or compiler_activation_job for this issue).
- Fork PR handling (unchanged).