Public: anyone can read this; the crew can change it.
A refused New app leaves a repository behind #331
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/329-a-refused-new-app-leaves-a-repository-be → main 1 commit fork
2 files
changed +28
−4
src/actions/apps.ts +18 −3
| @@ -20,6 +20,7 @@ import { | ||
| 20 | 20 | createApp as insertApp, |
| 21 | 21 | deleteApp as removeApp, |
| 22 | 22 | getAppByRepo, |
| 23 | getAppBySlug, | |
| 23 | 24 | slugFor, |
| 24 | 25 | updateApp as patchApp, |
| 25 | 26 | type AppAccess, |
| @@ -41,7 +42,7 @@ import { listAllAgents } from "./crew.ts"; | ||
| 41 | 42 | import { plan, planIssueBody, planningAvailable } from "../crew/plan.ts"; |
| 42 | 43 | import type { Issue } from "../core/issues.ts"; |
| 43 | 44 | import { getPull } from "../core/pulls.ts"; |
| 44 | import { canWrite, type Repo } from "../core/repos.ts"; | |
| 45 | import { canWrite, getRepo, type Repo } from "../core/repos.ts"; | |
| 45 | 46 | import type { Run } from "../core/runs.ts"; |
| 46 | 47 | import type { User } from "../core/users.ts"; |
| 47 | 48 | import { resolveOid } from "../git/merge.ts"; |
| @@ -59,6 +60,7 @@ import { assignIssue, openIssue } from "./issues.ts"; | ||
| 59 | 60 | import { |
| 60 | 61 | commitToBranch, |
| 61 | 62 | createRepo, |
| 63 | deleteRepo, | |
| 62 | 64 | type CreateRepoActionError, |
| 63 | 65 | } from "./repos.ts"; |
| 64 | 66 | |
| @@ -284,6 +286,14 @@ export async function newApp( | ||
| 284 | 286 | : getAgentByName(app.db, input.agent); |
| 285 | 287 | if (input.agent && !agent) return err("no_such_agent"); |
| 286 | 288 | |
| 289 | // Refuse a taken slug before anything exists to take back. | |
| 290 | // A name the owner already has stays `name_taken`, which createRepo says. | |
| 291 | if ( | |
| 292 | getAppBySlug(app.db, slugFor(input.name)) && | |
| 293 | !getRepo(app.db, input.owner || user.value.username, input.name) | |
| 294 | ) | |
| 295 | return err("slug_taken"); | |
| 296 | ||
| 287 | 297 | const repo = await createRepo(app, user.value, { |
| 288 | 298 | name: input.name, |
| 289 | 299 | visibility: input.visibility ?? "private", |
| @@ -298,12 +308,17 @@ export async function newApp( | ||
| 298 | 308 | description: input.description ?? "", |
| 299 | 309 | }), |
| 300 | 310 | }); |
| 301 | if (!committed.ok) return err("scaffold_failed"); | |
| 311 | if (!committed.ok) { | |
| 312 | await deleteRepo(app, user.value, repo.value); | |
| 313 | return err("scaffold_failed"); | |
| 314 | } | |
| 302 | 315 | const record = enableApp(app, user.value, repo.value, {}); |
| 303 | 316 | // The creator administers the repository they just made, so `not_found` |
| 304 | 317 | // here can only mean the row vanished under us — a failed scaffold. |
| 305 | if (!record.ok) | |
| 318 | if (!record.ok) { | |
| 319 | await deleteRepo(app, user.value, repo.value); | |
| 306 | 320 | return err(record.error === "not_found" ? "scaffold_failed" : record.error); |
| 321 | } | |
| 307 | 322 | |
| 308 | 323 | const description = (input.description ?? "").trim(); |
| 309 | 324 | let chosen = agent; |
test/apps.test.ts +10 −1
| @@ -39,7 +39,7 @@ import { | ||
| 39 | 39 | import type { AppRecord } from "../src/core/apps.ts"; |
| 40 | 40 | import { listEvents } from "../src/core/events.ts"; |
| 41 | 41 | import { getPull, type Pull } from "../src/core/pulls.ts"; |
| 42 | import { getRepoById, type Repo } from "../src/core/repos.ts"; | |
| 42 | import { getRepo, getRepoById, type Repo } from "../src/core/repos.ts"; | |
| 43 | 43 | import { resolveOid } from "../src/git/merge.ts"; |
| 44 | 44 | import { repoDir } from "../src/git/repos.ts"; |
| 45 | 45 | import { |
| @@ -1373,6 +1373,15 @@ printf '%s\\n' '{"is_error":false,"result":"{\\"summary\\":\\"Build a notes app\ | ||
| 1373 | 1373 | expect(made.value.issue?.body).toContain("- [ ] Add a form"); |
| 1374 | 1374 | }); |
| 1375 | 1375 | |
| 1376 | test("a slug already in use is refused before any repository exists", async () => { | |
| 1377 | const first = await newApp(app, alice, { name: "held-x" }); | |
| 1378 | expect(first.ok).toBe(true); | |
| 1379 | const second = await newApp(app, alice, { name: "held_x" }); | |
| 1380 | expect(second.ok).toBe(false); | |
| 1381 | if (!second.ok) expect(second.error).toBe("slug_taken"); | |
| 1382 | expect(getRepo(app.db, alice.username, "held_x")).toBeUndefined(); | |
| 1383 | }); | |
| 1384 | ||
| 1376 | 1385 | test("a plan naming an agent not on the forge is refused; the sentence still opens unassigned", async () => { |
| 1377 | 1386 | const made = await newApp(app, alice, { |
| 1378 | 1387 | name: "notes-b", |
new above
offline