safe_outputs: always use the same checkout layout as the agent job (not only for target-repo: "*")
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 46m
- Merged PRs (30d)
- 760
Description
## Summary
The `safe_outputs` job should **always** use the same checkout layout as the `agent` job. Today it only mirrors the agent layout when `safe-outputs.create-pull-request.target-repo` (or `push-to-pull-request-branch.target-repo`) is the wildcard `"*"`. For any other configuration it falls back to a divergent, single-repo layout. This divergence is unnecessary complexity and is a source of bugs.
## Current behavior
`buildSharedPRCheckoutSteps` in `pkg/workflow/compiler_safe_outputs_steps.go` branches on `targetRepoSlug`:
- **`target-repo: "*"`** → `buildMultiRepoCheckoutSteps`, which mirrors the agent job: default repo checked out to the workspace root, plus every `checkout:` entry checked out to its configured `path:`, then matching `Fetch additional refs` steps.
- **specific slug (e.g. `github/github`)** → single-repo path: checks out **only** that one target repo to the **workspace root** (no `path:`), with the base branch as `ref:`, followed by `Configure Git credentials` and a single fetch step.
- **no slug (same-repo)** → checks out the workspace repo to root.
So the layout an author gets depends on the `target-repo` value, and only the wildcard case matches what the `agent` job actually checked out.
## Why this is a problem
1. **Layout drift between agent and safe_outputs.** The agent operates against one on-disk layout (e.g. workflow repo at root, `github/github` at `./github`); the safe_outputs job applying the patch/creating the PR sees a *different* layout (e.g. `github/github` at root). Handlers then have to special-case which directory the repo lives in.
2. **The two-cross-repo case is unsupported.** Consider a workflow with two `checkout:` entries, e.g. `org/a` → `./a` and `org/b` → `./b`, and `create-pull-request.target-repo: "org/a"` (a *specific* slug, **not** `*`). The agent job checks out both repos at their paths. The safe_outputs single-repo path checks out only `org/a`, to the **root**, ignoring `org/b` and ignoring the `path:`. Any handler that expects the agent's layout (e.g. to read context from `./b`, or to find `org/a` at `./a`) breaks. There is no reason the specific-slug case should behave differently from the wildcard case here.
3. **It caused a real failure.** With a specific `target-repo`, the single-repo path checks out the target to root and then runs `Configure Git credentials` (which rewrites `origin` to an embedded-credential URL) *before* the fetch step, and omits the "Clear partial clone markers" reset that the agent path emits. The agent job's identical fetch — run against a clean `origin` in a subdirectory — succeeds, while the safe_outputs fetch fails with `fatal: couldn't find remote ref refs/heads/master`. Unifying on the agent layout removes this class of bug.
## Proposed change
Always generate the safe_outputs PR checkout steps using the same layout as the agent job, for **all** `target-repo` values (specific slug, `*`, or unset) — i.e. always take the multi-repo / agent-mirroring path. The handlers already locate the correct repo via `findRepoCheckout()` / the checkout manifest, so the wildcard machinery generalizes to the specific-slug case.
Considerations to preserve:
- safe_outputs must check out the **base branch** (not `github.sha`) for PR creation — keep the `extract-base-branch` step and base-branch `ref:` for the root checkout.
- Keep the run conditions (`create_pull_request` / `push_to_pull_request_branch`).
- Keep git-credential configuration for **all** checked-out repos so pushes work.
- Mirror the agent's partial-clone-marker reset and fetch-ref steps so cross-repo fetches behave identically.
## Acceptance criteria
- [ ] `safe_outputs` PR checkout layout is byte-for-byte equivalent (modulo base-branch `ref:` and run conditions) to the `agent` job's checkout layout for: same-repo, single specific cross-repo, two-or-more cross-repo, and wildcard `*` configurations.
- [ ] The two-cross-repo / specific-slug case checks out every `checkout:` entry to its `path:`.
- [ ] No regression in PR creation / push-to-branch for same-repo and cross-repo workflows.
- [ ] The `couldn't find remote ref` fetch failure no longer reproduces.
Contributor guide
Assessment
This issue has not been assessed yet.