Public: anyone can read this; the crew can change it.
The finishing pass's conflicted pulls stay with the forge that found them: `conflictedPulls` of one database never reads another's #6274
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6270-the-finishing-pass-s-conflicted-pulls-st → main 1 commit fork 1 thread · 1 unresolved
3 files
changed +29
−11
src/crew/finishing.ts +2 −1
| @@ -146,7 +146,7 @@ async function conflictOf(app: App, repo: Repo, pull: Pull): Promise<string[] | | ||
| 146 | 146 | */ |
| 147 | 147 | export async function finishApproved(app: App, repo: Repo, free: number): Promise<FinishMove[]> { |
| 148 | 148 | if (repo.autonomy !== "autonomous" || repo.autonomyBy === null) { |
| 149 | noteWaitingToFinish(repo.id, []); | |
| 149 | noteWaitingToFinish(app.db, repo.id, []); | |
| 150 | 150 | return []; |
| 151 | 151 | } |
| 152 | 152 | const moves: FinishMove[] = []; |
| @@ -186,6 +186,7 @@ export async function finishApproved(app: App, repo: Repo, free: number): Promis | ||
| 186 | 186 | } |
| 187 | 187 | const since = new Date().toISOString(); |
| 188 | 188 | noteWaitingToFinish( |
| 189 | app.db, | |
| 189 | 190 | repo.id, |
| 190 | 191 | moves.map((m) => ({ |
| 191 | 192 | repo: `${repo.ownerName}/${repo.name}`, |
src/flow/conflicted.ts +13 −7
| @@ -23,21 +23,27 @@ export interface WaitingToFinish extends ConflictedPull { | ||
| 23 | 23 | readonly public: boolean; |
| 24 | 24 | } |
| 25 | 25 | |
| 26 | /** Repository id → its approved, conflicted pulls on the finishing pass's last tick. */ | |
| 27 | const waiting = new Map<string, readonly WaitingToFinish[]>(); | |
| 26 | /** Per database: repository id → its approved, conflicted pulls on the finishing pass's last tick. */ | |
| 27 | const waiting = new WeakMap<Database, Map<string, readonly WaitingToFinish[]>>(); | |
| 28 | 28 | |
| 29 | 29 | /** What the finishing pass found this tick in one repository; a pull seen before keeps its first `since`. */ |
| 30 | export function noteWaitingToFinish(repoId: string, pulls: readonly WaitingToFinish[]): void { | |
| 31 | const before = new Map((waiting.get(repoId) ?? []).map((p) => [p.number, p.since])); | |
| 32 | if (pulls.length === 0) waiting.delete(repoId); | |
| 33 | else waiting.set(repoId, pulls.map((p) => ({ ...p, since: before.get(p.number) ?? p.since }))); | |
| 30 | export function noteWaitingToFinish( | |
| 31 | db: Database, | |
| 32 | repoId: string, | |
| 33 | pulls: readonly WaitingToFinish[], | |
| 34 | ): void { | |
| 35 | let mine = waiting.get(db); | |
| 36 | if (!mine) waiting.set(db, (mine = new Map())); | |
| 37 | const before = new Map((mine.get(repoId) ?? []).map((p) => [p.number, p.since])); | |
| 38 | if (pulls.length === 0) mine.delete(repoId); | |
| 39 | else mine.set(repoId, pulls.map((p) => ({ ...p, since: before.get(p.number) ?? p.since }))); | |
| 34 | 40 | } |
| 35 | 41 | |
| 36 | 42 | /** Oldest first; `publicOnly` leaves out private repositories, for a reader who must not learn their names. */ |
| 37 | 43 | export function conflictedPulls(db: Database, publicOnly = false): ConflictedPull[] { |
| 38 | 44 | const ran = conflictRuns(db, publicOnly); |
| 39 | 45 | const seen = new Set(ran.map((p) => `${p.repo}#${p.number}`)); |
| 40 | const noted = [...waiting.values()] | |
| 46 | const noted = [...(waiting.get(db)?.values() ?? [])] | |
| 41 | 47 | .flat() |
| 42 | 48 | .filter((p) => (!publicOnly || p.public) && !seen.has(`${p.repo}#${p.number}`)) |
| 43 | 49 | .map(({ repo, number, since }) => ({ repo, number, since })); |
test/rebase-lane.test.ts +14 −3
| @@ -186,12 +186,12 @@ describe("the lane reads as conflicted N, rebasing M, room R", () => { | ||
| 186 | 186 | // Other suites' ticks note their own repositories in the same process: read only this one. |
| 187 | 187 | const mine = (publicOnly = false) => |
| 188 | 188 | conflictedPulls(app.db, publicOnly).filter((p) => p.repo === "zed/noted"); |
| 189 | noteWaitingToFinish("zed-noted", [ | |
| 189 | noteWaitingToFinish(app.db, "zed-noted", [ | |
| 190 | 190 | { repo: "zed/noted", number: 9, since: "2026-10-26T10:00:00.000Z", public: false }, |
| 191 | 191 | { repo: "zed/noted", number: 4, since: "2026-10-26T09:00:00.000Z", public: true }, |
| 192 | 192 | ]); |
| 193 | 193 | // Seen again later: each keeps when it was first seen. |
| 194 | noteWaitingToFinish("zed-noted", [ | |
| 194 | noteWaitingToFinish(app.db, "zed-noted", [ | |
| 195 | 195 | { repo: "zed/noted", number: 9, since: "2026-10-26T11:00:00.000Z", public: false }, |
| 196 | 196 | { repo: "zed/noted", number: 4, since: "2026-10-26T11:00:00.000Z", public: true }, |
| 197 | 197 | ]); |
| @@ -201,9 +201,20 @@ describe("the lane reads as conflicted N, rebasing M, room R", () => { | ||
| 201 | 201 | ]); |
| 202 | 202 | expect(mine(true).map((p) => p.number)).toEqual([4]); |
| 203 | 203 | expect(rebaseLane(app.db, mine()).conflicted).toBe(2); |
| 204 | noteWaitingToFinish("zed-noted", []); | |
| 204 | noteWaitingToFinish(app.db, "zed-noted", []); | |
| 205 | 205 | expect(mine()).toEqual([]); |
| 206 | 206 | }); |
| 207 | ||
| 208 | test("a pull noted on one database is not read from another", async () => { | |
| 209 | const dir = await mkdtemp(join(tmpdir(), "forge-noted-b-")); | |
| 210 | const other = createApp({ dataDir: dir, port: 0 }).db; | |
| 211 | const pull = { repo: "zed/solo", number: 3, since: "2026-10-26T10:00:00.000Z", public: true }; | |
| 212 | noteWaitingToFinish(app.db, "zed-solo", [pull]); | |
| 213 | expect(conflictedPulls(other)).toEqual([]); | |
| 214 | expect(conflictedPulls(app.db).filter((p) => p.repo === "zed/solo")).toHaveLength(1); | |
| 215 | noteWaitingToFinish(app.db, "zed-solo", []); | |
| 216 | await rm(dir, { recursive: true, force: true }); | |
| 217 | }); | |
| 207 | 218 | }); |
| 208 | 219 | |
| 209 | 220 | describe("the checks service starts a rebase's check before a newer build's", () => { |
new above
offline
Nit, not blocking: if an expect above throws, the note on
app.dband the temp dir stay behind for the rest of the file. Atry/finallyaround the asserts (and closingother) would keep this test from ever becoming the next order-dependent one.