Public: anyone can read this; the crew can change it.
The map under churn: closing a blocker as dropped frees what waited on it, and nothing says an issue was split, parked or dropped #551
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/416-the-map-under-churn-closing-a-blocker-as → main 3 commits fork
13 files
changed +593
−21
docs/PLANNING.md +13
| @@ -22,6 +22,7 @@ _Holds up 3_, with each issue linked and a field to add one. Over the API: | ||
| 22 | 22 | GET /api/v1/repos/:owner/:name/issues/:n/dependencies → { blockedBy, blocks } |
| 23 | 23 | POST /api/v1/repos/:owner/:name/issues/:n/dependencies { "blockedBy": m } |
| 24 | 24 | DELETE /api/v1/repos/:owner/:name/issues/:n/dependencies/:m |
| 25 | POST /api/v1/repos/:owner/:name/issues/:n/dependencies/:m/confirm | |
| 25 | 26 | |
| 26 | 27 | |
| 27 | 28 | A refused link says why: `self`, `cycle`, `cross_repo`, `not_an_issue`. |
| @@ -263,3 +264,15 @@ material: whether the decider beats the rule is a query, not an opinion. | ||
| 263 | 264 | Every answer a decider gives is a log line — `decided by=typesafe ms=270 |
| 264 | 265 | [email protected] [email protected] …` — so what it said and how |
| 265 | 266 | long it took are always in the forge's log. |
| 267 | ||
| 268 | ## Closing takes a reason | |
| 269 | ||
| 270 | `PATCH …/issues/:n { "state": "closed", "reason": "done"|"dropped"|"split", "into": [m, …] }`. | |
| 271 | No reason means done. Done frees what waited. Dropped or split closes the | |
| 272 | issue but holds its dependents as _parked_, the map saying why ("#4 was | |
| 273 | dropped"), until a writer relinks them or confirms the edge | |
| 274 | (`POST …/dependencies/:m/confirm`, the page's "Still holds"); the crew may | |
| 275 | not confirm its own plan. Split also moves the parent's blocked-by edges to | |
| 276 | the issues named in `into` (a child that would make a loop is skipped), and | |
| 277 | the parent waits for nothing. Reopening clears the reason. Closes that | |
| 278 | predate reasons count as done. |
forge.json +1 −1
| @@ -6,7 +6,7 @@ | ||
| 6 | 6 | "ready": 90 |
| 7 | 7 | }, |
| 8 | 8 | "check": { |
| 9 | "run": "bun x tsc --noEmit && bun scripts/test-shards.ts --timeout 60000 -- crew crew-wrapup crew-limits crew-parks crew-sandbox steer session-settle landing decide ledger checks dependencies autonomy studio studio-words forks fork-flow cross-repo-pull pulls apps apps-adversarial apps-busy apps-token surfaces-adversarial orgs web git-trust helpers shards migrate vitals guard asks feed feed-page offline share cloud run-secrets forge-cloudflare usage credentials standing board-windows spawn-plan spawn web-apps fleet-ops meter told-by", | |
| 9 | "run": "bun x tsc --noEmit && bun scripts/test-shards.ts --timeout 60000 -- crew crew-wrapup crew-limits crew-parks crew-sandbox steer session-settle landing decide ledger checks dependencies map-closing autonomy studio studio-words forks fork-flow cross-repo-pull pulls apps apps-adversarial apps-busy apps-token surfaces-adversarial orgs web git-trust helpers shards migrate vitals guard asks feed feed-page offline share cloud run-secrets forge-cloudflare usage credentials standing board-windows spawn-plan spawn web-apps fleet-ops meter told-by", | |
| 10 | 10 | "timeout": 1800 |
| 11 | 11 | } |
| 12 | 12 | } |
src/actions/dependencies.ts +27
| @@ -9,6 +9,7 @@ import type { App } from "../app.ts"; | ||
| 9 | 9 | import { isCrew } from "../core/agents.ts"; |
| 10 | 10 | import { |
| 11 | 11 | addDependency, |
| 12 | confirmDependency, | |
| 12 | 13 | removeDependency, |
| 13 | 14 | type DependencyError, |
| 14 | 15 | } from "../core/dependencies.ts"; |
| @@ -81,3 +82,29 @@ export function unlinkIssue( | ||
| 81 | 82 | }); |
| 82 | 83 | return ok(undefined); |
| 83 | 84 | } |
| 85 | ||
| 86 | /** | |
| 87 | * A person says `issue` may go ahead although `blocker` closed as dropped or | |
| 88 | * split. Writers only: the crew parked it, and the crew may not unpark itself. | |
| 89 | */ | |
| 90 | export function confirmIssue( | |
| 91 | app: App, | |
| 92 | actor: User | undefined, | |
| 93 | repo: Repo, | |
| 94 | issue: Issue, | |
| 95 | blocker: Issue, | |
| 96 | ): Result<void, AccessError> { | |
| 97 | if (!canRead(app.db, repo, actor)) return err("not_found"); | |
| 98 | if (!actor) return err("unauthorized"); | |
| 99 | if (!canWrite(app.db, repo, actor) || isCrew(app.db, repo.id, actor.id)) | |
| 100 | return err("forbidden"); | |
| 101 | if (issue.repoId !== repo.id) return err("not_found"); | |
| 102 | if (!confirmDependency(app.db, issue.id, blocker.id)) return ok(undefined); | |
| 103 | app.bus.append({ | |
| 104 | kind: "issue.confirmed", | |
| 105 | repoId: repo.id, | |
| 106 | actorId: actor.id, | |
| 107 | payload: { number: issue.number, blockedBy: blocker.number }, | |
| 108 | }); | |
| 109 | return ok(undefined); | |
| 110 | } |
src/actions/issues.ts +44 −5
| @@ -20,9 +20,11 @@ | ||
| 20 | 20 | |
| 21 | 21 | import type { App } from "../app.ts"; |
| 22 | 22 | import { isCrew } from "../core/agents.ts"; |
| 23 | import { moveBlockersToChildren } from "../core/dependencies.ts"; | |
| 23 | 24 | import { |
| 24 | 25 | addComment as insertComment, |
| 25 | 26 | createIssue, |
| 27 | getIssue, | |
| 26 | 28 | getIssueById, |
| 27 | 29 | getLabel, |
| 28 | 30 | labelsFor, |
| @@ -30,6 +32,7 @@ import { | ||
| 30 | 32 | setAssignee, |
| 31 | 33 | setIssueLabels as writeLabels, |
| 32 | 34 | setIssueState as writeState, |
| 35 | type CloseReason, | |
| 33 | 36 | type Comment, |
| 34 | 37 | type Issue, |
| 35 | 38 | type IssueState, |
| @@ -170,7 +173,11 @@ export function setIssueState( | ||
| 170 | 173 | repo: Repo, |
| 171 | 174 | issue: Issue, |
| 172 | 175 | state: IssueState, |
| 173 | ): Result<Issue, AccessError | "already_merged" | MirrorError> { | |
| 176 | closing: Closing = {}, | |
| 177 | ): Result< | |
| 178 | Issue, | |
| 179 | AccessError | "already_merged" | MirrorError | "invalid_reason" | |
| 180 | > { | |
| 174 | 181 | const user = requireParticipant(app, actor, repo); |
| 175 | 182 | if (!user.ok) return user; |
| 176 | 183 | if (issue.repoId !== repo.id) return err("not_found"); |
| @@ -184,17 +191,46 @@ export function setIssueState( | ||
| 184 | 191 | getPull(app.db, repo.id, issue.number)?.mergedAt != null |
| 185 | 192 | ) |
| 186 | 193 | return err("already_merged"); |
| 187 | const changed = writeState(app.db, issue.id, state); | |
| 194 | const reason = state === "closed" ? (closing.reason ?? "done") : undefined; | |
| 195 | const children: Issue[] = []; | |
| 196 | if (closing.reason !== undefined || closing.into !== undefined) { | |
| 197 | // A reason is how an issue closes; a pull or a reopen has none. | |
| 198 | if (state !== "closed" || issue.kind !== "issue") | |
| 199 | return err("invalid_reason"); | |
| 200 | if (reason === "split") { | |
| 201 | for (const number of closing.into ?? []) { | |
| 202 | const child = getIssue(app.db, repo.id, number); | |
| 203 | if (!child || child.kind !== "issue" || child.id === issue.id) | |
| 204 | return err("invalid_reason"); | |
| 205 | if (!children.some((c) => c.id === child.id)) children.push(child); | |
| 206 | } | |
| 207 | if (children.length === 0) return err("invalid_reason"); | |
| 208 | } else if (closing.into !== undefined) return err("invalid_reason"); | |
| 209 | } | |
| 210 | const changed = writeState(app.db, issue.id, state, reason); | |
| 188 | 211 | if (!changed) return ok(getIssueById(app.db, issue.id) ?? issue); |
| 212 | if (children.length > 0) | |
| 213 | moveBlockersToChildren(app.db, issue, children, user.value.id); | |
| 189 | 214 | app.bus.append({ |
| 190 | 215 | kind: `${issue.kind}.${state === "closed" ? "closed" : "reopened"}`, |
| 191 | 216 | repoId: repo.id, |
| 192 | 217 | actorId: user.value.id, |
| 193 | payload: { number: issue.number }, | |
| 218 | payload: { | |
| 219 | number: issue.number, | |
| 220 | ...(issue.kind === "issue" && reason !== undefined ? { reason } : {}), | |
| 221 | ...(children.length > 0 ? { into: children.map((c) => c.number) } : {}), | |
| 222 | }, | |
| 194 | 223 | }); |
| 195 | 224 | return ok(changed); |
| 196 | 225 | } |
| 197 | 226 | |
| 227 | /** How an issue closes: done unless said otherwise; split names its children. */ | |
| 228 | export interface Closing { | |
| 229 | readonly reason?: CloseReason; | |
| 230 | /** Issue numbers a split issue became. */ | |
| 231 | readonly into?: readonly number[]; | |
| 232 | } | |
| 233 | ||
| 198 | 234 | /** |
| 199 | 235 | * Replace an issue's (or pull's) label set. Writers only — labels are the |
| 200 | 236 | * work queue. Labels may be named by id or by name; an unknown one rejects |
| @@ -333,14 +369,17 @@ export async function setIssueStateThrough( | ||
| 333 | 369 | repo: Repo, |
| 334 | 370 | issue: Issue, |
| 335 | 371 | state: IssueState, |
| 336 | ): Promise<Result<Issue, ThroughError<AccessError | "already_merged">>> { | |
| 372 | closing: Closing = {}, | |
| 373 | ): Promise< | |
| 374 | Result<Issue, ThroughError<AccessError | "already_merged" | "invalid_reason">> | |
| 375 | > { | |
| 337 | 376 | const user = requireParticipant(app, actor, repo); |
| 338 | 377 | if (!user.ok) return user; |
| 339 | 378 | if (issue.repoId !== repo.id) return err("not_found"); |
| 340 | 379 | const through = writeThroughFor(app, user.value, repo); |
| 341 | 380 | if (!through.ok) return through; |
| 342 | 381 | if (through.value === undefined) |
| 343 | return setIssueState(app, actor, repo, issue, state); | |
| 382 | return setIssueState(app, actor, repo, issue, state, closing); | |
| 344 | 383 | |
| 345 | 384 | // Same "author or writer" rule as a native issue: mirroring is not a |
| 346 | 385 | // permission bypass, and the origin will have its own opinion too. |
src/core/dependencies.ts +76 −2
| @@ -10,7 +10,7 @@ | ||
| 10 | 10 | |
| 11 | 11 | import type { Database } from "bun:sqlite"; |
| 12 | 12 | import { err, ok, type Result } from "../kernel/result.ts"; |
| 13 | import { getIssuesByIds, type Issue } from "./issues.ts"; | |
| 13 | import { getIssuesByIds, type CloseReason, type Issue } from "./issues.ts"; | |
| 14 | 14 | |
| 15 | 15 | export type DependencyError = "self" | "cross_repo" | "cycle" | "not_an_issue"; |
| 16 | 16 | |
| @@ -58,6 +58,45 @@ export function removeDependency( | ||
| 58 | 58 | ); |
| 59 | 59 | } |
| 60 | 60 | |
| 61 | /** | |
| 62 | * A person says the plan still holds: `issue` may go ahead although its | |
| 63 | * blocker closed as dropped or split. Returns whether an edge was changed. | |
| 64 | */ | |
| 65 | export function confirmDependency( | |
| 66 | db: Database, | |
| 67 | issueId: string, | |
| 68 | blockedById: string, | |
| 69 | ): boolean { | |
| 70 | return ( | |
| 71 | db | |
| 72 | .query( | |
| 73 | `UPDATE issue_dependency SET confirmed = 1 | |
| 74 | WHERE issue_id = ?1 AND blocked_by_id = ?2 AND confirmed = 0`, | |
| 75 | ) | |
| 76 | .run(issueId, blockedById).changes > 0 | |
| 77 | ); | |
| 78 | } | |
| 79 | ||
| 80 | /** | |
| 81 | * `parent` was split into `children`: what it waited for, they wait for, and | |
| 82 | * it waits for nothing. A child that would close a loop is left out of that | |
| 83 | * edge. Returns how many edges were moved. | |
| 84 | */ | |
| 85 | export function moveBlockersToChildren( | |
| 86 | db: Database, | |
| 87 | parent: Issue, | |
| 88 | children: readonly Issue[], | |
| 89 | actorId: string | undefined, | |
| 90 | ): number { | |
| 91 | const blockers = getIssuesByIds(db, blockerIds(db, parent.id)); | |
| 92 | let moved = 0; | |
| 93 | for (const blocker of blockers.values()) | |
| 94 | for (const child of children) | |
| 95 | if (addDependency(db, child, blocker, actorId).ok) moved++; | |
| 96 | db.query("DELETE FROM issue_dependency WHERE issue_id = ?1").run(parent.id); | |
| 97 | return moved; | |
| 98 | } | |
| 99 | ||
| 61 | 100 | /** Whether `from` is blocked, directly or through others, by `to`. */ |
| 62 | 101 | function reaches(db: Database, from: string, to: string): boolean { |
| 63 | 102 | const seen = new Set<string>([from]); |
| @@ -136,10 +175,22 @@ export function dependenciesOf( | ||
| 136 | 175 | |
| 137 | 176 | // ---- the graph of one repository ---- |
| 138 | 177 | |
| 178 | /** An open issue held by a blocker that closed without being done. */ | |
| 179 | export interface Held { | |
| 180 | readonly issueId: string; | |
| 181 | readonly blockedById: string; | |
| 182 | readonly reason: Exclude<CloseReason, "done">; | |
| 183 | } | |
| 184 | ||
| 139 | 185 | export interface Graph { |
| 140 | 186 | /** Every issue of kind `issue` that is open or on an edge. */ |
| 141 | 187 | readonly issues: Issue[]; |
| 142 | 188 | readonly edges: Edge[]; |
| 189 | /** | |
| 190 | * Edges whose blocker closed as dropped or split and that no person has | |
| 191 | * confirmed or relinked: closing frees only what was done. | |
| 192 | */ | |
| 193 | readonly held: Held[]; | |
| 143 | 194 | } |
| 144 | 195 | |
| 145 | 196 | const GRAPH_MAX = 400; |
| @@ -152,6 +203,20 @@ export function repoGraph(db: Database, repoId: string): Graph { | ||
| 152 | 203 | ) |
| 153 | 204 | .all(repoId) |
| 154 | 205 | .map((r) => ({ issueId: r.issue_id, blockedById: r.blocked_by_id })); |
| 206 | const heldRows = db | |
| 207 | .query< | |
| 208 | { issue_id: string; blocked_by_id: string; reason: Held["reason"] }, | |
| 209 | [string] | |
| 210 | >( | |
| 211 | `SELECT d.issue_id, d.blocked_by_id, b.close_reason AS reason | |
| 212 | FROM issue_dependency d | |
| 213 | JOIN issue i ON i.id = d.issue_id AND i.state = 'open' | |
| 214 | JOIN issue b ON b.id = d.blocked_by_id AND b.state = 'closed' | |
| 215 | WHERE i.repo_id = ?1 AND d.confirmed = 0 | |
| 216 | AND b.close_reason IN ('dropped','split') | |
| 217 | ORDER BY b.number`, | |
| 218 | ) | |
| 219 | .all(repoId); | |
| 155 | 220 | const open = db |
| 156 | 221 | .query<{ id: string }, [string, number]>( |
| 157 | 222 | `SELECT id FROM issue WHERE repo_id = ?1 AND kind = 'issue' AND state = 'open' |
| @@ -173,6 +238,11 @@ export function repoGraph(db: Database, repoId: string): Graph { | ||
| 173 | 238 | edges: edges.filter( |
| 174 | 239 | (e) => present.has(e.issueId) && present.has(e.blockedById), |
| 175 | 240 | ), |
| 241 | held: heldRows.map((r) => ({ | |
| 242 | issueId: r.issue_id, | |
| 243 | blockedById: r.blocked_by_id, | |
| 244 | reason: r.reason, | |
| 245 | })), | |
| 176 | 246 | }; |
| 177 | 247 | } |
| 178 | 248 | |
| @@ -259,8 +329,12 @@ export function readyNow(graph: Graph): Ranked[] { | ||
| 259 | 329 | const open = openIds(graph); |
| 260 | 330 | const holds = holdsUpMap(graph, open); |
| 261 | 331 | const waits = waitsForMap(graph, open); |
| 332 | const held = new Set(graph.held.map((h) => h.issueId)); | |
| 262 | 333 | return graph.issues |
| 263 | .filter((i) => open.has(i.id) && (waits.get(i.id)?.size ?? 0) === 0) | |
| 334 | .filter( | |
| 335 | (i) => | |
| 336 | open.has(i.id) && !held.has(i.id) && (waits.get(i.id)?.size ?? 0) === 0, | |
| 337 | ) | |
| 264 | 338 | .map((issue) => ({ |
| 265 | 339 | issue, |
| 266 | 340 | holdsUp: holds.get(issue.id)?.size ?? 0, |
src/core/issues.ts +15 −2
| @@ -12,6 +12,13 @@ import { repoVisible } from "./visibility.ts"; | ||
| 12 | 12 | |
| 13 | 13 | export type IssueKind = "issue" | "pull"; |
| 14 | 14 | export type IssueState = "open" | "closed"; |
| 15 | /** Why an issue closed: done frees what waited on it; the others hold it. */ | |
| 16 | export type CloseReason = "done" | "dropped" | "split"; | |
| 17 | export const CLOSE_REASONS: readonly CloseReason[] = [ | |
| 18 | "done", | |
| 19 | "dropped", | |
| 20 | "split", | |
| 21 | ]; | |
| 15 | 22 | |
| 16 | 23 | export interface Issue { |
| 17 | 24 | readonly id: string; |
| @@ -26,6 +33,8 @@ export interface Issue { | ||
| 26 | 33 | readonly createdAt: string; |
| 27 | 34 | readonly updatedAt: string; |
| 28 | 35 | readonly closedAt: string | null; |
| 36 | /** Why it closed; null while open, and for closes that predate reasons. */ | |
| 37 | readonly closeReason: CloseReason | null; | |
| 29 | 38 | /** |
| 30 | 39 | * For an imported conversation: the login of whoever actually wrote it on |
| 31 | 40 | * the origin. `authorId` stays the connecting user so every join and |
| @@ -62,6 +71,7 @@ interface IssueRow { | ||
| 62 | 71 | created_at: string; |
| 63 | 72 | updated_at: string; |
| 64 | 73 | closed_at: string | null; |
| 74 | close_reason: CloseReason | null; | |
| 65 | 75 | origin_author: string | null; |
| 66 | 76 | origin_html_url: string | null; |
| 67 | 77 | assignee_id: string | null; |
| @@ -90,6 +100,7 @@ function toIssue(row: IssueRow): Issue { | ||
| 90 | 100 | createdAt: row.created_at, |
| 91 | 101 | updatedAt: row.updated_at, |
| 92 | 102 | closedAt: row.closed_at, |
| 103 | closeReason: row.close_reason, | |
| 93 | 104 | originAuthor: row.origin_author, |
| 94 | 105 | originUrl: row.origin_html_url, |
| 95 | 106 | assigneeId: row.assignee_id, |
| @@ -396,14 +407,16 @@ export function setIssueState( | ||
| 396 | 407 | db: Database, |
| 397 | 408 | issueId: string, |
| 398 | 409 | state: IssueState, |
| 410 | reason: CloseReason = "done", | |
| 399 | 411 | ): Issue | undefined { |
| 400 | 412 | const now = new Date().toISOString(); |
| 401 | 413 | const closedAt = state === "closed" ? now : null; |
| 414 | const why = state === "closed" ? reason : null; | |
| 402 | 415 | const changed = db |
| 403 | 416 | .query( |
| 404 | "UPDATE issue SET state = ?1, closed_at = ?2, updated_at = ?3 WHERE id = ?4 AND state != ?1", | |
| 417 | "UPDATE issue SET state = ?1, closed_at = ?2, updated_at = ?3, close_reason = ?5 WHERE id = ?4 AND state != ?1", | |
| 405 | 418 | ) |
| 406 | .run(state, closedAt, now, issueId).changes; | |
| 419 | .run(state, closedAt, now, issueId, why).changes; | |
| 407 | 420 | return changed > 0 ? getIssueById(db, issueId) : undefined; |
| 408 | 421 | } |
| 409 | 422 |
src/core/schema.ts +7
| @@ -1172,4 +1172,11 @@ export const migrations: readonly string[] = [ | ||
| 1172 | 1172 | OR body LIKE '%without a pull: cancelled' |
| 1173 | 1173 | OR body LIKE '%interrupted by a restart%'); |
| 1174 | 1174 | `, |
| 1175 | // 44 — why an issue closed (done, dropped, split), and whether a person | |
| 1176 | // has confirmed an edge whose blocker closed for a reason other than done. | |
| 1177 | ` | |
| 1178 | ALTER TABLE issue ADD COLUMN close_reason TEXT | |
| 1179 | CHECK (close_reason IN ('done','dropped','split')); | |
| 1180 | ALTER TABLE issue_dependency ADD COLUMN confirmed INTEGER NOT NULL DEFAULT 0; | |
| 1181 | `, | |
| 1175 | 1182 | ]; |
src/http/api.ts +68 −1
| @@ -197,6 +197,7 @@ import { | ||
| 197 | 197 | dependencyWords, |
| 198 | 198 | linkIssue, |
| 199 | 199 | unlinkIssue, |
| 200 | confirmIssue, | |
| 200 | 201 | } from "../actions/dependencies.ts"; |
| 201 | 202 | import { dependenciesOf, repoGraph } from "../core/dependencies.ts"; |
| 202 | 203 | import { decisionStats, listDecisions } from "../core/decisions.ts"; |
| @@ -206,7 +207,12 @@ import { isEnvName } from "../apps/data.ts"; | ||
| 206 | 207 | import { getAppByRepo } from "../core/apps.ts"; |
| 207 | 208 | import type { Agent } from "../core/agents.ts"; |
| 208 | 209 | import { listApps, type AppRecord } from "../core/apps.ts"; |
| 209 | import { listAssignedTo } from "../core/issues.ts"; | |
| 210 | import { | |
| 211 | CLOSE_REASONS, | |
| 212 | listAssignedTo, | |
| 213 | type CloseReason, | |
| 214 | } from "../core/issues.ts"; | |
| 215 | import type { Closing } from "../actions/issues.ts"; | |
| 210 | 216 | import { |
| 211 | 217 | checksForHead, |
| 212 | 218 | getRun, |
| @@ -457,6 +463,12 @@ function sourceFailure(error: SourceError): Response { | ||
| 457 | 463 | * worth retrying against. |
| 458 | 464 | */ |
| 459 | 465 | function originAware(error: string, status = 422): Response { |
| 466 | if (error === "invalid_reason") | |
| 467 | return apiError( | |
| 468 | "invalid_reason", | |
| 469 | 422, | |
| 470 | "a reason is done, dropped, or split (with `into`: the issues it became); only an issue closing takes one", | |
| 471 | ); | |
| 460 | 472 | if (error === "mirrored") |
| 461 | 473 | return apiError( |
| 462 | 474 | "mirrored", |
| @@ -484,6 +496,30 @@ function originAware(error: string, status = 422): Response { | ||
| 484 | 496 | return respondAccess(error, status); |
| 485 | 497 | } |
| 486 | 498 | |
| 499 | /** | |
| 500 | * `reason` and `into` of a close request, or undefined when they are not | |
| 501 | * the shapes they must be (a bad reason word, `into` not a list of numbers). | |
| 502 | */ | |
| 503 | function closingFrom(input: Record<string, unknown>): Closing | undefined { | |
| 504 | const reason = input.reason; | |
| 505 | if (reason !== undefined && !CLOSE_REASONS.includes(reason as CloseReason)) | |
| 506 | return undefined; | |
| 507 | let into: number[] | undefined; | |
| 508 | if (input.into !== undefined) { | |
| 509 | if ( | |
| 510 | !Array.isArray(input.into) || | |
| 511 | input.into.length > 50 || | |
| 512 | !input.into.every((n) => Number.isInteger(n) && n > 0) | |
| 513 | ) | |
| 514 | return undefined; | |
| 515 | into = input.into as number[]; | |
| 516 | } | |
| 517 | return { | |
| 518 | ...(reason === undefined ? {} : { reason: reason as CloseReason }), | |
| 519 | ...(into === undefined ? {} : { into }), | |
| 520 | }; | |
| 521 | } | |
| 522 | ||
| 487 | 523 | /** |
| 488 | 524 | * Which status a branch refusal deserves. A conflict is something that |
| 489 | 525 | * changed under the caller (or was already there); everything else is the |
| @@ -1695,6 +1731,7 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 1695 | 1731 | number: i.number, |
| 1696 | 1732 | title: i.title, |
| 1697 | 1733 | state: i.state, |
| 1734 | ...(i.closeReason === null ? {} : { closeReason: i.closeReason }), | |
| 1698 | 1735 | }); |
| 1699 | 1736 | return json({ |
| 1700 | 1737 | blockedBy: deps.blockedBy.map(brief), |
| @@ -1766,6 +1803,33 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 1766 | 1803 | return new Response(null, { status: 204 }); |
| 1767 | 1804 | }, |
| 1768 | 1805 | ); |
| 1806 | // A person confirms an edge whose blocker closed dropped or split: the | |
| 1807 | // plan still holds, so the dependent is no longer parked on it. | |
| 1808 | router.add( | |
| 1809 | "POST", | |
| 1810 | "/api/v1/repos/:owner/:name/issues/:number/dependencies/:blocker/confirm", | |
| 1811 | (ctx) => { | |
| 1812 | const denied = requireScope(ctx, "rw"); | |
| 1813 | if (denied) return denied; | |
| 1814 | const found = loadIssue(ctx); | |
| 1815 | if ("error" in found) return found.error; | |
| 1816 | const number = positiveInt(ctx.params.blocker); | |
| 1817 | const blocker = | |
| 1818 | number === undefined | |
| 1819 | ? undefined | |
| 1820 | : getIssue(ctx.app.db, found.repo.id, number); | |
| 1821 | if (!blocker) return notFound(); | |
| 1822 | const confirmed = confirmIssue( | |
| 1823 | ctx.app, | |
| 1824 | ctx.auth.user, | |
| 1825 | found.repo, | |
| 1826 | found.issue, | |
| 1827 | blocker, | |
| 1828 | ); | |
| 1829 | if (!confirmed.ok) return respondAccess(confirmed.error, 422); | |
| 1830 | return new Response(null, { status: 204 }); | |
| 1831 | }, | |
| 1832 | ); | |
| 1769 | 1833 | // The map as data: ready now, blocked on, the critical path, every edge, |
| 1770 | 1834 | // and where the crew is. |
| 1771 | 1835 | router.add("GET", "/api/v1/repos/:owner/:name/map", (ctx) => { |
| @@ -1821,12 +1885,15 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 1821 | 1885 | const wanted = str(input.state); |
| 1822 | 1886 | if (wanted !== "open" && wanted !== "closed") |
| 1823 | 1887 | return json({ error: "invalid_state" }, 422); |
| 1888 | const closing = closingFrom(input); | |
| 1889 | if (closing === undefined) return json({ error: "invalid_reason" }, 422); | |
| 1824 | 1890 | const changed = await setIssueStateThrough( |
| 1825 | 1891 | ctx.app, |
| 1826 | 1892 | ctx.auth.user, |
| 1827 | 1893 | found.repo, |
| 1828 | 1894 | found.issue, |
| 1829 | 1895 | wanted, |
| 1896 | closing, | |
| 1830 | 1897 | ); |
| 1831 | 1898 | if (!changed.ok) return originAware(changed.error, 409); |
| 1832 | 1899 | return json(withLabels(ctx, changed.value)); |
src/web/pages/issues.ts +1
| @@ -550,6 +550,7 @@ export async function issuePage(ctx: Ctx): Promise<Response> { | ||
| 550 | 550 | stateAction: `${thread}/state`, |
| 551 | 551 | state: issue.state, |
| 552 | 552 | closeLabel: "Close issue", |
| 553 | reasons: true, | |
| 553 | 554 | })} |
| 554 | 555 | </div> |
| 555 | 556 | <aside> |
src/web/pages/map.ts +19 −3
| @@ -416,8 +416,10 @@ export interface Marks { | ||
| 416 | 416 | readonly crew: Map<string, RunState>; |
| 417 | 417 | /** Open issues an open pull would close: proposed, not landed. */ |
| 418 | 418 | readonly waiting: Set<string>; |
| 419 | /** Issues whose last word is the crew parking them. */ | |
| 419 | /** Issues whose last word is the crew parking them, or whose blocker closed unfinished. */ | |
| 420 | 420 | readonly parked: Set<string>; |
| 421 | /** Why an issue is parked on a closed blocker, by issue id: "#4 was dropped". */ | |
| 422 | readonly why: Map<string, string>; | |
| 421 | 423 | } |
| 422 | 424 | |
| 423 | 425 | /** |
| @@ -447,13 +449,23 @@ export function marksFor( | ||
| 447 | 449 | if (id !== undefined) waiting.add(id); |
| 448 | 450 | } |
| 449 | 451 | const open = graph.issues.filter((i) => i.state === "open").map((i) => i.id); |
| 450 | return { crew: crewMarks(runs), waiting, parked: struckOut(db, open) }; | |
| 452 | const parked = struckOut(db, open); | |
| 453 | const why = new Map<string, string>(); | |
| 454 | const numbers = new Map(graph.issues.map((i) => [i.id, i.number])); | |
| 455 | for (const h of graph.held) { | |
| 456 | parked.add(h.issueId); | |
| 457 | const note = `#${numbers.get(h.blockedById)} was ${h.reason === "split" ? "split" : "dropped"}`; | |
| 458 | const had = why.get(h.issueId); | |
| 459 | why.set(h.issueId, had === undefined ? note : `${had}; ${note}`); | |
| 460 | } | |
| 461 | return { crew: crewMarks(runs), waiting, parked, why }; | |
| 451 | 462 | } |
| 452 | 463 | |
| 453 | 464 | export const NO_MARKS: Marks = { |
| 454 | 465 | crew: new Map(), |
| 455 | 466 | waiting: new Set(), |
| 456 | 467 | parked: new Set(), |
| 468 | why: new Map(), | |
| 457 | 469 | }; |
| 458 | 470 | |
| 459 | 471 | /** |
| @@ -593,7 +605,7 @@ function node( | ||
| 593 | 605 | ${run ? html`data-run="${run}"` : null} |
| 594 | 606 | href="${issueHref(base, n.issue.number)}" |
| 595 | 607 | > |
| 596 | <title>${number} ${n.issue.title} — ${word}</title> | |
| 608 | <title>${number} ${n.issue.title} — ${word}${emphasis.marks.why.has(n.issue.id) ? `: ${emphasis.marks.why.get(n.issue.id)}` : ""}</title> | |
| 597 | 609 | ${word === "working" |
| 598 | 610 | ? html`<rect |
| 599 | 611 | class="map-breath" |
| @@ -1278,6 +1290,10 @@ export function mapJson( | ||
| 1278 | 1290 | state: i.state, |
| 1279 | 1291 | assignee: i.assigneeName, |
| 1280 | 1292 | word: wordFor(i, readySet, marks), |
| 1293 | ...(i.state === "closed" && i.closeReason !== null | |
| 1294 | ? { closeReason: i.closeReason } | |
| 1295 | : {}), | |
| 1296 | ...(marks.why.has(i.id) ? { parkedBecause: marks.why.get(i.id) } : {}), | |
| 1281 | 1297 | }); |
| 1282 | 1298 | return { |
| 1283 | 1299 | repo: { owner: repo.ownerName, name: repo.name }, |
src/web/pages/thread.ts +81 −7
| @@ -9,7 +9,11 @@ | ||
| 9 | 9 | */ |
| 10 | 10 | |
| 11 | 11 | import { listCrew } from "../../actions/crew.ts"; |
| 12 | import { linkIssue, unlinkIssue } from "../../actions/dependencies.ts"; | |
| 12 | import { | |
| 13 | confirmIssue, | |
| 14 | linkIssue, | |
| 15 | unlinkIssue, | |
| 16 | } from "../../actions/dependencies.ts"; | |
| 13 | 17 | import { dependenciesOf } from "../../core/dependencies.ts"; |
| 14 | 18 | import { isCrew } from "../../core/agents.ts"; |
| 15 | 19 | import { |
| @@ -18,7 +22,13 @@ import { | ||
| 18 | 22 | setIssueStateThrough, |
| 19 | 23 | } from "../../actions/issues.ts"; |
| 20 | 24 | import { listEvents } from "../../core/events.ts"; |
| 21 | import type { Issue, Label } from "../../core/issues.ts"; | |
| 25 | import { | |
| 26 | CLOSE_REASONS, | |
| 27 | type CloseReason, | |
| 28 | type Issue, | |
| 29 | type Label, | |
| 30 | } from "../../core/issues.ts"; | |
| 31 | import type { Closing } from "../../actions/issues.ts"; | |
| 22 | 32 | import { getIssue, labelsFor, listComments } from "../../core/issues.ts"; |
| 23 | 33 | import { canRead, canWrite, type Repo } from "../../core/repos.ts"; |
| 24 | 34 | import { inlineIcons, type IconName as Glyph } from "../icons.ts"; |
| @@ -311,6 +321,8 @@ export interface ComposerOptions { | ||
| 311 | 321 | /** Where close/reopen POSTs; omitted hides those buttons. */ |
| 312 | 322 | stateAction?: string; |
| 313 | 323 | state: "open" | "closed"; |
| 324 | /** Offer why an issue closes: done, dropped, or split into others. */ | |
| 325 | reasons?: boolean; | |
| 314 | 326 | /** A merged pull can never reopen. */ |
| 315 | 327 | locked?: boolean; |
| 316 | 328 | /** What the close button calls the thing here — dev's own noun |
| @@ -343,6 +355,22 @@ export function composer(ctx: Ctx, o: ComposerOptions): Html { | ||
| 343 | 355 | name="state" |
| 344 | 356 | value="${o.state === "open" ? "closed" : "open"}" |
| 345 | 357 | /> |
| 358 | ${ | |
| 359 | o.reasons && o.state === "open" | |
| 360 | ? html`<select name="reason" aria-label="Why it closes"> | |
| 361 | <option value="done">done</option> | |
| 362 | <option value="dropped">dropped</option> | |
| 363 | <option value="split">split into…</option> | |
| 364 | </select> | |
| 365 | <input | |
| 366 | class="input" | |
| 367 | name="into" | |
| 368 | inputmode="numeric" | |
| 369 | placeholder="split into #, #" | |
| 370 | aria-label="Issue numbers it was split into" | |
| 371 | />` | |
| 372 | : null | |
| 373 | } | |
| 346 | 374 | </form>` |
| 347 | 375 | : null; |
| 348 | 376 | const closeButton = |
| @@ -597,6 +625,28 @@ export async function stateSubmit( | ||
| 597 | 625 | |
| 598 | 626 | const wanted = field(form, "state") === "closed" ? "closed" : "open"; |
| 599 | 627 | const dest = `${repoBase(found.repo)}/${kind === "pull" ? "pulls" : "issues"}/${found.issue.number}`; |
| 628 | const why = field(form, "reason"); | |
| 629 | const intoText = field(form, "into").trim(); | |
| 630 | const into = intoText | |
| 631 | .split(/[\s,]+/) | |
| 632 | .filter((t) => t !== "") | |
| 633 | .map((t) => Number(t.replace(/^#/, ""))); | |
| 634 | if ( | |
| 635 | (why !== "" && !CLOSE_REASONS.includes(why as CloseReason)) || | |
| 636 | into.length > 50 || | |
| 637 | !into.every((n) => Number.isInteger(n) && n > 0) | |
| 638 | ) | |
| 639 | return flashOrJson(ctx, dest, { | |
| 640 | tone: "danger", | |
| 641 | message: "Close as done, dropped, or split into issue numbers.", | |
| 642 | }); | |
| 643 | const closing: Closing = | |
| 644 | wanted === "closed" && kind === "issue" && why !== "" | |
| 645 | ? { | |
| 646 | reason: why as CloseReason, | |
| 647 | ...(why === "split" ? { into } : {}), | |
| 648 | } | |
| 649 | : {}; | |
| 600 | 650 | // The `…Through` twin: identical on a native repo, and on a mirror it |
| 601 | 651 | // closes the issue at the origin first and reflects what came back. |
| 602 | 652 | const changed = await setIssueStateThrough( |
| @@ -605,6 +655,7 @@ export async function stateSubmit( | ||
| 605 | 655 | found.repo, |
| 606 | 656 | found.issue, |
| 607 | 657 | wanted, |
| 658 | closing, | |
| 608 | 659 | ); |
| 609 | 660 | if (!changed.ok) { |
| 610 | 661 | // The same noun the redirect target is about to render with |
| @@ -696,6 +747,19 @@ export function dependenciesCard( | ||
| 696 | 747 | ${icon(i.state === "closed" ? "issueClosed" : "issue")} |
| 697 | 748 | <a href="${base}/issues/${i.number}">${i.title}</a> |
| 698 | 749 | <span class="muted small">#${i.number}</span> |
| 750 | ${i.closeReason === "dropped" || i.closeReason === "split" | |
| 751 | ? html`<span class="muted small">${i.closeReason}</span>` | |
| 752 | : null} | |
| 753 | ${removable && | |
| 754 | mayEdit && | |
| 755 | (i.closeReason === "dropped" || i.closeReason === "split") | |
| 756 | ? html`<form method="post" action="${action}" class="inline"> | |
| 757 | <input type="hidden" name="confirm" value="${i.number}" /> | |
| 758 | <button class="btn btn-ghost btn-xs" type="submit"> | |
| 759 | Still holds | |
| 760 | </button> | |
| 761 | </form>` | |
| 762 | : null} | |
| 699 | 763 | ${removable && mayEdit |
| 700 | 764 | ? html`<form method="post" action="${action}" class="inline"> |
| 701 | 765 | <input type="hidden" name="unlink" value="${i.number}" /> |
| @@ -759,7 +823,13 @@ export async function dependencySubmit(ctx: Ctx): Promise<Response> { | ||
| 759 | 823 | if (form instanceof Response) return form; |
| 760 | 824 | const dest = `${repoBase(found.repo)}/issues/${found.issue.number}`; |
| 761 | 825 | const unlink = field(form, "unlink").trim(); |
| 762 | const raw = unlink !== "" ? unlink : field(form, "blockedBy").trim(); | |
| 826 | const confirm = field(form, "confirm").trim(); | |
| 827 | const raw = | |
| 828 | unlink !== "" | |
| 829 | ? unlink | |
| 830 | : confirm !== "" | |
| 831 | ? confirm | |
| 832 | : field(form, "blockedBy").trim(); | |
| 763 | 833 | const number = Number(raw.replace(/^#/, "")); |
| 764 | 834 | const other = |
| 765 | 835 | Number.isInteger(number) && number > 0 |
| @@ -773,7 +843,9 @@ export async function dependencySubmit(ctx: Ctx): Promise<Response> { | ||
| 773 | 843 | const result = |
| 774 | 844 | unlink !== "" |
| 775 | 845 | ? unlinkIssue(ctx.app, ctx.auth.user, found.repo, found.issue, other) |
| 776 | : linkIssue(ctx.app, ctx.auth.user, found.repo, found.issue, other); | |
| 846 | : confirm !== "" | |
| 847 | ? confirmIssue(ctx.app, ctx.auth.user, found.repo, found.issue, other) | |
| 848 | : linkIssue(ctx.app, ctx.auth.user, found.repo, found.issue, other); | |
| 777 | 849 | if (!result.ok) { |
| 778 | 850 | const words: Record<string, string> = { |
| 779 | 851 | self: "An issue cannot wait for itself.", |
| @@ -789,8 +861,10 @@ export async function dependencySubmit(ctx: Ctx): Promise<Response> { | ||
| 789 | 861 | } |
| 790 | 862 | return flashTo(ctx, dest, { |
| 791 | 863 | message: |
| 792 | unlink !== "" | |
| 793 | ? `#${found.issue.number} no longer waits for #${other.number}.` | |
| 794 | : `#${found.issue.number} waits for #${other.number}.`, | |
| 864 | confirm !== "" | |
| 865 | ? `#${found.issue.number} may go ahead without #${other.number}.` | |
| 866 | : unlink !== "" | |
| 867 | ? `#${found.issue.number} no longer waits for #${other.number}.` | |
| 868 | : `#${found.issue.number} waits for #${other.number}.`, | |
| 795 | 869 | }); |
| 796 | 870 | } |
test/map-closing.test.ts added +239
| @@ -0,0 +1,239 @@ | ||
| 1 | /** | |
| 2 | * Closing takes a reason. Done frees what waited; dropped and split hold it, | |
| 3 | * parked with the reason, until a person confirms or relinks; split moves the | |
| 4 | * parent's own blockers to its children. The map says why. | |
| 5 | */ | |
| 6 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 7 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 8 | import { tmpdir } from "node:os"; | |
| 9 | import { join } from "node:path"; | |
| 10 | import { createApp, type App } from "../src/app.ts"; | |
| 11 | import { confirmIssue } from "../src/actions/dependencies.ts"; | |
| 12 | import { openIssue, setIssueState } from "../src/actions/issues.ts"; | |
| 13 | import { createRepo } from "../src/actions/repos.ts"; | |
| 14 | import { | |
| 15 | addDependency, | |
| 16 | dependenciesOf, | |
| 17 | readyNow, | |
| 18 | removeDependency, | |
| 19 | repoGraph, | |
| 20 | } from "../src/core/dependencies.ts"; | |
| 21 | import { getIssue, type CloseReason, type Issue } from "../src/core/issues.ts"; | |
| 22 | import type { Repo } from "../src/core/repos.ts"; | |
| 23 | import { createToken } from "../src/core/tokens.ts"; | |
| 24 | import { createUser, type User } from "../src/core/users.ts"; | |
| 25 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 26 | import { mapJson, marksFor, wordFor } from "../src/web/pages/map.ts"; | |
| 27 | ||
| 28 | describe("closing with a reason", () => { | |
| 29 | let dataDir: string; | |
| 30 | let app: App; | |
| 31 | let servers: Servers; | |
| 32 | let alice: User; | |
| 33 | let repo: Repo; | |
| 34 | ||
| 35 | beforeAll(async () => { | |
| 36 | dataDir = await mkdtemp(join(tmpdir(), "forge-closing-")); | |
| 37 | app = createApp({ dataDir, port: 0 }); | |
| 38 | servers = await serve(app); | |
| 39 | const a = await createUser(app.db, { | |
| 40 | username: "alice", | |
| 41 | email: "[email protected]", | |
| 42 | password: "a-good-passphrase", | |
| 43 | adminIfFirst: true, | |
| 44 | }); | |
| 45 | if (!a.ok) throw new Error(a.error); | |
| 46 | alice = a.value; | |
| 47 | const made = await createRepo(app, alice, { | |
| 48 | name: "plan", | |
| 49 | visibility: "private", | |
| 50 | }); | |
| 51 | if (!made.ok) throw new Error(String(made.error)); | |
| 52 | repo = made.value; | |
| 53 | }); | |
| 54 | ||
| 55 | afterAll(async () => { | |
| 56 | servers.stop(); | |
| 57 | app.close(); | |
| 58 | await rm(dataDir, { recursive: true, force: true }); | |
| 59 | }); | |
| 60 | ||
| 61 | const make = (title: string): Issue => { | |
| 62 | const opened = openIssue(app, alice, repo, { title }); | |
| 63 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 64 | return opened.value; | |
| 65 | }; | |
| 66 | const fresh = (i: Issue): Issue => getIssue(app.db, repo.id, i.number)!; | |
| 67 | const close = ( | |
| 68 | i: Issue, | |
| 69 | reason?: CloseReason, | |
| 70 | into?: number[], | |
| 71 | ): ReturnType<typeof setIssueState> => | |
| 72 | setIssueState(app, alice, repo, fresh(i), "closed", { | |
| 73 | ...(reason ? { reason } : {}), | |
| 74 | ...(into ? { into } : {}), | |
| 75 | }); | |
| 76 | const readyNumbers = (): number[] => | |
| 77 | readyNow(repoGraph(app.db, repo.id)).map((r) => r.issue.number); | |
| 78 | const wordOf = (i: Issue): string => { | |
| 79 | const graph = repoGraph(app.db, repo.id); | |
| 80 | const ready = new Set(readyNow(graph).map((r) => r.issue.id)); | |
| 81 | const marks = marksFor(app.db, repo.id, graph, []); | |
| 82 | return wordFor(fresh(i), ready, marks); | |
| 83 | }; | |
| 84 | ||
| 85 | test("done, and a close with no reason, free what waited", () => { | |
| 86 | const blocker = make("Blocker"); | |
| 87 | const waiter = make("Waiter"); | |
| 88 | addDependency(app.db, waiter, blocker, alice.id); | |
| 89 | expect(readyNumbers()).not.toContain(waiter.number); | |
| 90 | expect(close(blocker).ok).toBe(true); | |
| 91 | expect(fresh(blocker).closeReason).toBe("done"); | |
| 92 | expect(readyNumbers()).toContain(waiter.number); | |
| 93 | expect(wordOf(waiter)).toBe("ready"); | |
| 94 | }); | |
| 95 | ||
| 96 | test("dropped holds dependents parked, with the reason, until confirmed", () => { | |
| 97 | const blocker = make("Replaced plan"); | |
| 98 | const waiter = make("Built on it"); | |
| 99 | addDependency(app.db, waiter, blocker, alice.id); | |
| 100 | expect(close(blocker, "dropped").ok).toBe(true); | |
| 101 | expect(fresh(blocker).closeReason).toBe("dropped"); | |
| 102 | expect(readyNumbers()).not.toContain(waiter.number); | |
| 103 | expect(wordOf(waiter)).toBe("parked"); | |
| 104 | ||
| 105 | const graph = repoGraph(app.db, repo.id); | |
| 106 | const json = mapJson(repo, graph, marksFor(app.db, repo.id, graph, [])) as { | |
| 107 | issues: { | |
| 108 | number: number; | |
| 109 | word: string; | |
| 110 | parkedBecause?: string; | |
| 111 | closeReason?: string; | |
| 112 | }[]; | |
| 113 | }; | |
| 114 | const card = json.issues.find((i) => i.number === waiter.number)!; | |
| 115 | expect(card.parkedBecause).toBe(`#${blocker.number} was dropped`); | |
| 116 | expect( | |
| 117 | json.issues.find((i) => i.number === blocker.number)!.closeReason, | |
| 118 | ).toBe("dropped"); | |
| 119 | ||
| 120 | const confirmed = confirmIssue(app, alice, repo, waiter, fresh(blocker)); | |
| 121 | expect(confirmed.ok).toBe(true); | |
| 122 | expect(readyNumbers()).toContain(waiter.number); | |
| 123 | expect(wordOf(waiter)).toBe("ready"); | |
| 124 | }); | |
| 125 | ||
| 126 | test("relinking frees a dependent held by a dropped blocker", () => { | |
| 127 | const blocker = make("Dropped too"); | |
| 128 | const waiter = make("Waits on dropped"); | |
| 129 | addDependency(app.db, waiter, blocker, alice.id); | |
| 130 | close(blocker, "dropped"); | |
| 131 | expect(wordOf(waiter)).toBe("parked"); | |
| 132 | removeDependency(app.db, waiter.id, blocker.id); | |
| 133 | expect(readyNumbers()).toContain(waiter.number); | |
| 134 | }); | |
| 135 | ||
| 136 | test("reopening clears the reason and the hold becomes a plain wait", () => { | |
| 137 | const blocker = make("Dropped then back"); | |
| 138 | const waiter = make("Waits again"); | |
| 139 | addDependency(app.db, waiter, blocker, alice.id); | |
| 140 | close(blocker, "dropped"); | |
| 141 | setIssueState(app, alice, repo, fresh(blocker), "open"); | |
| 142 | expect(fresh(blocker).closeReason).toBeNull(); | |
| 143 | expect(wordOf(waiter)).toBe("blocked"); | |
| 144 | }); | |
| 145 | ||
| 146 | test("split moves the parent's blockers to its children and parks its dependents", () => { | |
| 147 | const root = make("Root"); | |
| 148 | const parent = make("Too big"); | |
| 149 | const dependent = make("After the big one"); | |
| 150 | const a = make("Half A"); | |
| 151 | const b = make("Half B"); | |
| 152 | addDependency(app.db, parent, root, alice.id); | |
| 153 | addDependency(app.db, dependent, parent, alice.id); | |
| 154 | const split = close(parent, "split", [a.number, b.number]); | |
| 155 | expect(split.ok).toBe(true); | |
| 156 | expect(fresh(parent).closeReason).toBe("split"); | |
| 157 | expect(dependenciesOf(app.db, parent.id).blockedBy).toHaveLength(0); | |
| 158 | for (const child of [a, b]) | |
| 159 | expect( | |
| 160 | dependenciesOf(app.db, child.id).blockedBy.map((i) => i.number), | |
| 161 | ).toEqual([root.number]); | |
| 162 | expect(readyNumbers()).not.toContain(a.number); | |
| 163 | expect(wordOf(dependent)).toBe("parked"); | |
| 164 | const graph = repoGraph(app.db, repo.id); | |
| 165 | const why = marksFor(app.db, repo.id, graph, []).why.get(dependent.id); | |
| 166 | expect(why).toBe(`#${parent.number} was split`); | |
| 167 | // Once the root is done the halves are ready; the dependent stays held. | |
| 168 | close(root); | |
| 169 | expect(readyNumbers()).toEqual( | |
| 170 | expect.arrayContaining([a.number, b.number]), | |
| 171 | ); | |
| 172 | expect(readyNumbers()).not.toContain(dependent.number); | |
| 173 | }); | |
| 174 | ||
| 175 | test("a reason is only for an issue closing, and a split names real children", () => { | |
| 176 | const one = make("One"); | |
| 177 | const other = make("Other"); | |
| 178 | const noKids = close(one, "split"); | |
| 179 | expect(noKids.ok).toBe(false); | |
| 180 | if (!noKids.ok) expect(noKids.error).toBe("invalid_reason"); | |
| 181 | expect(close(one, "split", [one.number]).ok).toBe(false); | |
| 182 | expect(close(one, "split", [9999]).ok).toBe(false); | |
| 183 | expect(close(one, "dropped", [other.number]).ok).toBe(false); | |
| 184 | expect( | |
| 185 | setIssueState(app, alice, repo, fresh(one), "open", { reason: "done" }) | |
| 186 | .ok, | |
| 187 | ).toBe(false); | |
| 188 | expect(fresh(one).state).toBe("open"); | |
| 189 | }); | |
| 190 | ||
| 191 | test("the API takes a reason and refuses a bad one", async () => { | |
| 192 | const token = createToken(app.db, alice.id, "t", "rw").token; | |
| 193 | const headers = { | |
| 194 | authorization: `Bearer ${token}`, | |
| 195 | "content-type": "application/json", | |
| 196 | }; | |
| 197 | const url = (n: number) => | |
| 198 | `${servers.url}/api/v1/repos/alice/plan/issues/${n}`; | |
| 199 | const target = make("Via API"); | |
| 200 | const kid = make("Its child"); | |
| 201 | const bad = await fetch(url(target.number), { | |
| 202 | method: "PATCH", | |
| 203 | headers, | |
| 204 | body: JSON.stringify({ state: "closed", reason: "bored" }), | |
| 205 | }); | |
| 206 | expect(bad.status).toBe(422); | |
| 207 | const noKids = await fetch(url(target.number), { | |
| 208 | method: "PATCH", | |
| 209 | headers, | |
| 210 | body: JSON.stringify({ state: "closed", reason: "split", into: [] }), | |
| 211 | }); | |
| 212 | expect(noKids.status).toBe(422); | |
| 213 | const ok = await fetch(url(target.number), { | |
| 214 | method: "PATCH", | |
| 215 | headers, | |
| 216 | body: JSON.stringify({ | |
| 217 | state: "closed", | |
| 218 | reason: "split", | |
| 219 | into: [kid.number], | |
| 220 | }), | |
| 221 | }); | |
| 222 | expect(ok.status).toBe(200); | |
| 223 | expect(fresh(target).closeReason).toBe("split"); | |
| 224 | ||
| 225 | const waiter = make("API waiter"); | |
| 226 | addDependency(app.db, waiter, fresh(target), alice.id); | |
| 227 | const deps = await fetch(`${url(waiter.number)}/dependencies`, { headers }); | |
| 228 | const body = (await deps.json()) as { | |
| 229 | blockedBy: { closeReason?: string }[]; | |
| 230 | }; | |
| 231 | expect(body.blockedBy[0]!.closeReason).toBe("split"); | |
| 232 | const confirm = await fetch( | |
| 233 | `${url(waiter.number)}/dependencies/${target.number}/confirm`, | |
| 234 | { method: "POST", headers }, | |
| 235 | ); | |
| 236 | expect(confirm.status).toBe(204); | |
| 237 | expect(wordOf(waiter)).toBe("ready"); | |
| 238 | }); | |
| 239 | }); |
test/map.test.ts +2
| @@ -49,6 +49,7 @@ function paperIssue(n: number, state: "open" | "closed" = "open"): Issue { | ||
| 49 | 49 | createdAt: `2026-01-01T00:00:${String(n).padStart(2, "0")}Z`, |
| 50 | 50 | updatedAt: "2026-01-01T00:00:00Z", |
| 51 | 51 | closedAt: null, |
| 52 | closeReason: null, | |
| 52 | 53 | originAuthor: null, |
| 53 | 54 | originUrl: null, |
| 54 | 55 | assigneeId: null, |
| @@ -66,6 +67,7 @@ function paperGraph(n: number, edges: [number, number][]): Graph { | ||
| 66 | 67 | issueId: `i${issue}`, |
| 67 | 68 | blockedById: `i${blockedBy}`, |
| 68 | 69 | })), |
| 70 | held: [], | |
| 69 | 71 | }; |
| 70 | 72 | } |
| 71 | 73 |
new above
offline