Public: anyone can read this; the crew can change it.
A merge through the API or MCP is refused while the green is on an older base, and the docs stop saying it is not #9937
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9929-a-merge-through-the-api-or-mcp-is-refuse → main 1 commit fork
2 files
changed +115
−1
docs/OPERATING.md +1 −1
Diff not shown — a line 2,622 characters wide (limit 2,000). +1 −1
View filetest/merge-stale-green.test.ts +114
| @@ -7,6 +7,12 @@ import { serve, type Servers } from "../src/http/server.ts"; | ||
| 7 | 7 | import { gitClientEnv } from "./helpers/git-client.ts"; |
| 8 | 8 | import { createRun, setRunBase } from "../src/core/runs.ts"; |
| 9 | 9 | import { seedAdmin } from "./helpers/admin.ts"; |
| 10 | import { mergePull } from "../src/actions/pulls.ts"; | |
| 11 | import { getPull } from "../src/core/pulls.ts"; | |
| 12 | import { getRepo } from "../src/core/repos.ts"; | |
| 13 | import { getUserByName } from "../src/core/users.ts"; | |
| 14 | import { mergeable } from "../src/git/merge.ts"; | |
| 15 | import { repoDir } from "../src/git/repos.ts"; | |
| 10 | 16 | |
| 11 | 17 | const gitEnv = gitClientEnv(); |
| 12 | 18 | |
| @@ -134,3 +140,111 @@ test("the API merge refuses a green checked against an older base when checks ar | ||
| 134 | 140 | const landed = await call("/pulls/1/merge", { method: "POST", body: "{}" }); |
| 135 | 141 | expect(landed.status).toBe(200); |
| 136 | 142 | }); |
| 143 | ||
| 144 | /** A pull from `branch` off main, green against main as it is now. */ | |
| 145 | async function greenPull(branch: string): Promise<{ number: number; runId: string }> { | |
| 146 | await gitOk(["checkout", "main"]); | |
| 147 | await gitOk(["pull", "origin", "main"]); | |
| 148 | await gitOk(["checkout", "-b", branch]); | |
| 149 | await Bun.write(join(dir, `${branch}.txt`), `${branch}\n`); | |
| 150 | await gitOk(["add", "."]); | |
| 151 | await gitOk(["commit", "-m", branch]); | |
| 152 | await gitOk(["push", "origin", branch]); | |
| 153 | const made = await call("/pulls", { | |
| 154 | method: "POST", | |
| 155 | body: JSON.stringify({ base: "main", head: branch, title: branch }), | |
| 156 | }); | |
| 157 | expect(made.status).toBe(201); | |
| 158 | const number = ((await made.json()) as { number: number }).number; | |
| 159 | const row = app.db | |
| 160 | .query<{ issue_id: string; repo_id: string; head_oid: string }, [number]>( | |
| 161 | "SELECT pull.issue_id, issue.repo_id, pull.head_oid FROM pull JOIN issue ON issue.id = pull.issue_id WHERE issue.number = ?1", | |
| 162 | ) | |
| 163 | .get(number)!; | |
| 164 | const run = createRun(app.db, { | |
| 165 | repoId: row.repo_id, | |
| 166 | kind: "check", | |
| 167 | issueId: row.issue_id, | |
| 168 | env: `pull-${number}`, | |
| 169 | commitOid: row.head_oid, | |
| 170 | }); | |
| 171 | app.db.query("UPDATE run SET state = 'ok' WHERE id = ?1").run(run.id); | |
| 172 | setRunBase(app.db, run.id, await gitOk(["rev-parse", "origin/main"])); | |
| 173 | return { number, runId: run.id }; | |
| 174 | } | |
| 175 | ||
| 176 | /** Main moves on with a commit of its own. */ | |
| 177 | async function moveMain(path: string): Promise<string> { | |
| 178 | await gitOk(["checkout", "main"]); | |
| 179 | await gitOk(["pull", "origin", "main"]); | |
| 180 | await Bun.write(join(dir, path), "moved\n"); | |
| 181 | await gitOk(["add", "."]); | |
| 182 | await gitOk(["commit", "-m", `main moves: ${path}`]); | |
| 183 | await gitOk(["push", "origin", "main"]); | |
| 184 | return gitOk(["rev-parse", "main"]); | |
| 185 | } | |
| 186 | ||
| 187 | let mcpId = 1; | |
| 188 | async function mcpMerge(number: number): Promise<{ error?: string; value?: any }> { | |
| 189 | const res = await fetch(`${base}/mcp`, { | |
| 190 | method: "POST", | |
| 191 | headers: { | |
| 192 | ...auth, | |
| 193 | "content-type": "application/json", | |
| 194 | accept: "application/json, text/event-stream", | |
| 195 | }, | |
| 196 | body: JSON.stringify({ | |
| 197 | jsonrpc: "2.0", | |
| 198 | id: mcpId++, | |
| 199 | method: "tools/call", | |
| 200 | params: { name: "merge", arguments: { repo: "dev/app", number, dryRun: false } }, | |
| 201 | }), | |
| 202 | }); | |
| 203 | const body = (await res.json()) as any; | |
| 204 | const text: string = body.result.content[0].text; | |
| 205 | return body.result.isError === true ? { error: text } : { value: JSON.parse(text) }; | |
| 206 | } | |
| 207 | ||
| 208 | const merged = (number: number) => | |
| 209 | app.db | |
| 210 | .query<{ merged_at: string | null }, [number]>( | |
| 211 | "SELECT pull.merged_at FROM pull JOIN issue ON issue.id = pull.issue_id WHERE issue.number = ?1", | |
| 212 | ) | |
| 213 | .get(number)!.merged_at; | |
| 214 | ||
| 215 | test("the MCP merge tool refuses a green checked against an older base, and lands a fresh one", async () => { | |
| 216 | const pull = await greenPull("by-mcp"); | |
| 217 | const tip = await moveMain("before-mcp.txt"); | |
| 218 | ||
| 219 | const refused = await mcpMerge(pull.number); | |
| 220 | expect(refused.error).toStartWith("checks_required"); | |
| 221 | expect(merged(pull.number)).toBeNull(); | |
| 222 | expect(await gitOk(["ls-remote", "origin", "refs/heads/main"])).toStartWith(tip); | |
| 223 | ||
| 224 | setRunBase(app.db, pull.runId, tip); | |
| 225 | const landed = await mcpMerge(pull.number); | |
| 226 | expect(landed.error).toBeUndefined(); | |
| 227 | expect(merged(pull.number)).not.toBeNull(); | |
| 228 | }); | |
| 229 | ||
| 230 | test("the merge queue's gated landing (expect) is not held by a green on an older base", async () => { | |
| 231 | const pull = await greenPull("by-queue"); | |
| 232 | const tip = await moveMain("before-queue.txt"); | |
| 233 | const repo = getRepo(app.db, "dev", "app")!; | |
| 234 | const p = getPull(app.db, repo.id, pull.number)!; | |
| 235 | const bare = repoDir(app.reposRoot, "dev", "app"); | |
| 236 | const tree = await mergeable(bare, tip, p.headOid); | |
| 237 | if (!tree.ok) throw new Error(JSON.stringify(tree.error)); | |
| 238 | const dev = getUserByName(app.db, "dev")!; | |
| 239 | ||
| 240 | // The same pull, without the queue's guard, is still refused. | |
| 241 | const held = await mergePull(app, dev, repo, p, "merge"); | |
| 242 | expect(held.ok).toBe(false); | |
| 243 | if (!held.ok) expect(held.error).toBe("checks_required"); | |
| 244 | ||
| 245 | const landed = await mergePull(app, dev, repo, p, "merge", { | |
| 246 | baseOid: tip, | |
| 247 | treeOid: tree.value.treeOid, | |
| 248 | }); | |
| 249 | expect(landed.ok).toBe(true); | |
| 250 | }); |