Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/MAINTAINERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,10 @@ lists, and approve
([runbook](../apps/cli/docs/release-process.md#claude-resolved-develop-into-next)).
Otherwise merge the target into the sync branch, resolve, and push; then
**approve** the PR. Approval fast-forwards the
target and deletes the branch. Never use the merge button. If the target or the
source moved, the bot merges the latest target and source into the approved head and lands
it directly if the merge is clean, as for a clean sync (the merge commit is
target and deletes the branch. Never use the merge button. A moved source does
not block landing; the next sync run brings its newer commits in. If the target
moved, the bot merges the latest target into the approved head and lands it
directly if the merge is clean, as for a clean sync (the merge commit is
untested); only a new conflict, or a target that keeps moving, sends the PR
back for a new approval. Only two sync pairs
exist (`sync/main-into-develop` into `develop`, `sync/develop-into-next` into
Expand Down
49 changes: 9 additions & 40 deletions .github/scripts/fast-forward.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -733,8 +733,11 @@ describe("runFastForward sync", () => {
expect(() => repo.remoteTip("sync/main-into-develop")).toThrow();
});

test("lands the approved head merged with a non-conflicting target move", async () => {
test("lands the approved head merged with a target move without pulling in the moved source", async () => {
const { repo, head, io, comments } = resolvedSyncScenario();
git(repo.seed, "switch", "main");
const movedMain = repo.commit(repo.seed, "late-main.txt", "late\n", "fix(cli): late main");
git(repo.seed, "push", "origin", "main");
git(repo.seed, "switch", "develop");
const movedDevelop = repo.commit(repo.seed, "late.txt", "late\n", "feat(cli): late");
git(repo.seed, "push", "origin", "develop");
Expand All @@ -745,6 +748,7 @@ describe("runFastForward sync", () => {
expect(outcome).toEqual({ status: "merged-after-resync", sha: landed });
expect(repo.isAncestor(head, landed)).toBe(true);
expect(repo.isAncestor(movedDevelop, landed)).toBe(true);
expect(repo.isAncestor(movedMain, landed)).toBe(false);
expect(() => repo.remoteTip("sync/main-into-develop")).toThrow();
expect(comments).toHaveLength(1);
expect(comments[0]).toContain("moved since approval");
Expand Down Expand Up @@ -846,52 +850,17 @@ describe("runFastForward sync", () => {
expect(comments[0]).toContain("- `shared.txt`");
});

test("lands the approved head merged with a non-conflicting source move", async () => {
const { repo, head, io, comments } = resolvedSyncScenario();
git(repo.seed, "switch", "main");
const movedMain = repo.commit(repo.seed, "late-main.txt", "late\n", "fix(cli): late main");
git(repo.seed, "push", "origin", "main");

const outcome = await runFastForward(io, { reviewCommitId: head });

const landed = repo.remoteTip("develop");
expect(outcome).toEqual({ status: "merged-after-resync", sha: landed });
expect(landed).not.toBe(head);
expect(repo.isAncestor(head, landed)).toBe(true);
expect(repo.isAncestor(movedMain, landed)).toBe(true);
expect(() => repo.remoteTip("sync/main-into-develop")).toThrow();
expect(comments).toHaveLength(1);
expect(comments[0]).toContain("`main` moved since approval");
});

test("refuses and leaves the branch alone when a moved source conflicts", async () => {
test("fast-forwards the approved head even when the source moved with a conflicting change", async () => {
const { repo, head, io, comments } = resolvedSyncScenario();
const developBefore = repo.remoteTip("develop");
git(repo.seed, "switch", "main");
repo.commit(repo.seed, "shared.txt", "conflicting main\n", "fix(cli): conflicting main");
git(repo.seed, "push", "origin", "main");

const outcome = await runFastForward(io, { reviewCommitId: head });

expect(outcome.status).toBe("refused");
expect(repo.remoteTip("sync/main-into-develop")).toBe(head);
expect(repo.remoteTip("develop")).toBe(developBefore);
expect(comments[0]).toContain("`main` moved since approval");
expect(comments[0]).toContain("- `shared.txt`");
});

test("dry run pushes nothing when the source moved cleanly", async () => {
const { repo, head, io, comments } = resolvedSyncScenario();
const developBefore = repo.remoteTip("develop");
git(repo.seed, "switch", "main");
repo.commit(repo.seed, "late-main.txt", "late\n", "fix(cli): late main");
git(repo.seed, "push", "origin", "main");

const outcome = await runFastForward(io, { reviewCommitId: head, dryRun: true });

expect(outcome.status).toBe("dry-run");
expect(repo.remoteTip("develop")).toBe(developBefore);
expect(repo.remoteTip("sync/main-into-develop")).toBe(head);
expect(outcome).toEqual({ status: "fast-forwarded", sha: head });
expect(repo.remoteTip("develop")).toBe(head);
expect(() => repo.remoteTip("sync/main-into-develop")).toThrow();
expect(comments).toEqual([]);
});
});
71 changes: 25 additions & 46 deletions .github/scripts/fast-forward.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,40 +371,31 @@ async function resyncSyncBranch(
promotion: Promotion,
headSha: string,
syncBranch: string,
moved: string,
): Promise<FastForwardOutcome> {
const { git } = io;
const { target, source } = promotion;
const { target } = promotion;
const targetRef = `refs/remotes/origin/${target}`;
let resyncedSha = headSha;

for (let attempt = 1; attempt <= MAX_PUSH_ATTEMPTS; attempt += 1) {
gitOrThrow(git, [
"fetch",
"--no-tags",
"origin",
`+refs/heads/${target}:${targetRef}`,
`+refs/heads/${source}:refs/remotes/origin/${source}`,
]);
gitOrThrow(git, ["fetch", "--no-tags", "origin", `+refs/heads/${target}:${targetRef}`]);
const targetSha = gitOrThrow(git, ["rev-parse", targetRef]);
gitOrThrow(git, ["checkout", "--detach", headSha]);
for (const ref of [target, source]) {
const merge = git(["merge", "--no-edit", `refs/remotes/origin/${ref}`]);
if (merge.status !== 0) {
const files = git(["diff", "--name-only", "--diff-filter=U"]).stdout;
git(["merge", "--abort"]);
const list = files
.split("\n")
.filter(Boolean)
.map((file) => `- \`${file}\``)
.join("\n");
return refuse(
io,
input,
`Merging ${ref} into ${syncBranch} conflicts`,
`\`${moved}\` moved since approval and merging \`${ref}\` into \`${syncBranch}\` conflicts:\n\n${list}\n\nResolve on the branch and re-approve.`,
);
}
const merge = git(["merge", "--no-edit", targetRef]);
if (merge.status !== 0) {
const files = git(["diff", "--name-only", "--diff-filter=U"]).stdout;
git(["merge", "--abort"]);
const list = files
.split("\n")
.filter(Boolean)
.map((file) => `- \`${file}\``)
.join("\n");
return refuse(
io,
input,
`Merging ${target} into ${syncBranch} conflicts`,
`\`${target}\` moved since approval and merging it into \`${syncBranch}\` conflicts:\n\n${list}\n\nResolve on the branch and re-approve.`,
);
}
resyncedSha = gitOrThrow(git, ["rev-parse", "HEAD"]);

Expand All @@ -416,15 +407,15 @@ async function resyncSyncBranch(
if (pushMergedTarget(git, target, targetSha) === "pushed") {
git(["push", "origin", "--delete", syncBranch]);
await io.comment(
`\`${moved}\` moved since approval, so I merged the latest \`${target}\` and \`${source}\` into the approved head and landed it as ${short(resyncedSha)}.`,
`\`${target}\` moved since approval, so I merged the latest \`${target}\` into the approved head and landed it as ${short(resyncedSha)}.`,
);
return { status: "merged-after-resync", sha: resyncedSha };
}
}

gitOrThrow(git, ["push", "origin", `${resyncedSha}:refs/heads/${syncBranch}`]);
await io.comment(
`\`${target}\` kept moving, so I merged the latest \`${target}\` and \`${source}\` into \`${syncBranch}\` (${short(resyncedSha)}). Re-approve once checks pass.`,
`\`${target}\` kept moving, so I merged the latest \`${target}\` into \`${syncBranch}\` (${short(resyncedSha)}). Re-approve once checks pass.`,
);
return { status: "resynced", sha: resyncedSha };
}
Expand Down Expand Up @@ -482,9 +473,6 @@ export async function runFastForward(
const fetchRefs = [
`+refs/heads/${promotion.target}:refs/remotes/origin/${promotion.target}`,
`+refs/pull/${pullRequest.number}/head:refs/ff/pr-head`,
...(promotion.kind === "sync"
? [`+refs/heads/${promotion.source}:refs/remotes/origin/${promotion.source}`]
: []),
];
gitOrThrow(git, ["fetch", "--no-tags", "origin", ...fetchRefs]);

Expand Down Expand Up @@ -515,18 +503,9 @@ export async function runFastForward(
const canFastForward = (): boolean =>
git(["merge-base", "--is-ancestor", targetRef, headSha]).status === 0;

const sourceCurrent = (): boolean =>
promotion.kind !== "sync" ||
git(["merge-base", "--is-ancestor", `refs/remotes/origin/${promotion.source}`, headSha])
.status === 0;

let movedBranch: string | null = null;
if (!canFastForward()) {
movedBranch = promotion.target;
} else if (!sourceCurrent()) {
movedBranch = promotion.source;
}
if (movedBranch === null) {
// Source commits newer than the approved head reach the target through the next sync run.
let targetMoved = !canFastForward();
if (!targetMoved) {
const push = pushSha(io, input, headSha, promotion.target);
if (!push.ok) {
gitOrThrow(git, [
Expand All @@ -538,13 +517,13 @@ export async function runFastForward(
if (canFastForward()) {
throw new Error(`git push to ${promotion.target} failed: ${push.stderr}`);
}
movedBranch = promotion.target;
targetMoved = true;
}
}

if (movedBranch !== null) {
if (targetMoved) {
if (promotion.kind === "sync") {
return resyncSyncBranch(io, input, promotion, headSha, pullRequest.head.ref, movedBranch);
return resyncSyncBranch(io, input, promotion, headSha, pullRequest.head.ref);
}
const recovery =
promotion.kind === "deploy"
Expand Down
2 changes: 1 addition & 1 deletion apps/cli/docs/release-process.md
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ Before pushing, the job checks, in order:
2. **Approved commit is the head.** The approval's commit must equal the current PR head, and the PR must be open and not a draft. A stale approval is rejected with a comment; re-approve the new head.
3. **Required checks are green on the head.** `Check code quality`, `Run unit and integration tests`, `Run end-to-end tests`, and `Lint Pull Request` must all have succeeded. They are read from the PR's own check rollup (GraphQL), counting only GitHub Actions runs attached to this PR, so runs of another PR on the same commit cannot satisfy or mask a check. Deploy PRs and hotfix PRs into `main` run the full suite (`test.yml` also triggers on PRs into `main`), so these checks exist on the deploy PR head.
4. **Major guard (any promotion into `main`).** If the commits since the last stable tag include a breaking `!` title, the PR needs the `release-major` label. Without it the job comments and fails.
5. **Target has not moved.** The push is a plain, non-force push of the approved commit, so the server refuses anything that is not a fast-forward. For deploy and cut PRs the job comments and fails; update the PR and re-approve. When the deploy PR is refused because `main` has commits `develop` lacks, the comment names the open `sync/main-into-develop` pull request, or the sync dispatch if none is open. A sync PR also counts as stale when the current source tip (`main` or `develop`) is not in the approved head, even if the target did not move. For sync PRs the bot merges the latest target and source into the approved head and, when that merge is clean, pushes it straight to the target and comments, as for a clean sync (the merge commit is untested). A new conflict, or a target that keeps moving, sends the PR back for re-approval instead: on a conflict the bot comments and fails; if the target keeps moving it pushes the re-synced head to the sync branch and comments, and you re-approve once checks pass.
5. **Target has not moved.** The push is a plain, non-force push of the approved commit, so the server refuses anything that is not a fast-forward. For deploy and cut PRs the job comments and fails; update the PR and re-approve. When the deploy PR is refused because `main` has commits `develop` lacks, the comment names the open `sync/main-into-develop` pull request, or the sync dispatch if none is open. A sync PR lands its approved head even when its source (`main` or `develop`) has moved on; the next push to the source runs `Sync branches` again and brings the newer commits in. When the target moved, the bot merges the latest target into the approved head and, when that merge is clean, pushes it straight to the target and comments, as for a clean sync (the merge commit is untested). A new conflict, or a target that keeps moving, sends the PR back for re-approval instead: on a conflict the bot comments and fails; if the target keeps moving it pushes the re-synced head to the sync branch and comments, and you re-approve once checks pass.

When all checks pass and no major is involved, a deploy PR behaves as before: the App pushes the approved commit to `main` and `push: main` triggers the stable release.

Expand Down
Loading