Public: anyone can read this; the crew can change it.
Live triage closes an open issue whose title repeats an older open one, pointing at the survivor #3438
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3267-live-triage-closes-an-open-issue-whose-t → main 5 commits fork 4 threads · 4 unresolved
3 files
changed +246
−0
README.md +9
| @@ -1074,6 +1074,15 @@ so its comment says "Closed by the crew's live triage". Reopening the issue | ||
| 1074 | 1074 | undoes it and the reopen stands: an issue that already has a close note is |
| 1075 | 1075 | never closed by triage again. The tick line counts them as `triaged=N`. |
| 1076 | 1076 | |
| 1077 | Live triage also folds duplicates: of two open issues whose titles match after | |
| 1078 | case and punctuation, the newer closes (reason dropped) with the comment | |
| 1079 | "duplicate of #N", and whatever waited on it waits on the older. A person's | |
| 1080 | newer issue is never closed into a crew member's; the older closes instead. A | |
| 1081 | fold a person reopened stands, the plan's own issues are never folded, a | |
| 1082 | dependency that would make a loop stays where it was, and folds share the | |
| 1083 | 25-per-tick cap with the already-done closes. Exact match only, no fuzzy | |
| 1084 | matching. | |
| 1085 | ||
| 1077 | 1086 | **Live triage files issues.** On the same tick, an open issue an hour old |
| 1078 | 1087 | that serves no pillar is filed: under the pillar its title, else its body, |
| 1079 | 1088 | names by its short form (`P0`) or whole label (`P0 Trust`) — a bare word like |
src/crew/livetriage.ts +114
| @@ -33,6 +33,7 @@ import { linkIssue } from "../actions/dependencies.ts"; | ||
| 33 | 33 | import { addComment, setIssueState } from "../actions/issues.ts"; |
| 34 | 34 | import { alreadyDone, CLOSED_NOTE, DONE_LIKE } from "../core/done_on_main.ts"; |
| 35 | 35 | import { getIssueById, getIssuesByIds, type Issue } from "../core/issues.ts"; |
| 36 | import { addDependency, removeDependency } from "../core/dependencies.ts"; | |
| 36 | 37 | import { isStructural, pillarsOf, servedBy, type Pillar, type Pillars } from "../core/pillars.ts"; |
| 37 | 38 | import { canWrite, type Repo } from "../core/repos.ts"; |
| 38 | 39 | import { getUserById, type User } from "../core/users.ts"; |
| @@ -272,6 +273,118 @@ function candidates(app: App, repo: Repo): string[] { | ||
| 272 | 273 | .map((r) => r.id); |
| 273 | 274 | } |
| 274 | 275 | |
| 276 | /** The comment a folded duplicate carries; `#N` is the survivor. */ | |
| 277 | export const DUPLICATE_NOTE = "duplicate of #"; | |
| 278 | ||
| 279 | /** A title as the fold compares it: case and punctuation do not count. */ | |
| 280 | export function sameTitle(title: string): string { | |
| 281 | return title | |
| 282 | .toLowerCase() | |
| 283 | .replace(/[^\p{L}\p{N}]+/gu, " ") | |
| 284 | .trim(); | |
| 285 | } | |
| 286 | ||
| 287 | /** | |
| 288 | * Open issues whose titles match an older open one, as [loser, survivor]. | |
| 289 | * The newer closes into the older, except that a person's newer issue is | |
| 290 | * never closed into a crew member's: then the older closes into it. A loser | |
| 291 | * the triage already folded once (a person reopened it) stays open, and the | |
| 292 | * plan's own issues are neither folded nor chosen as survivor. The note is | |
| 293 | * matched exactly and by the triage's author, so a person's comment that | |
| 294 | * merely starts the same way does not count. | |
| 295 | */ | |
| 296 | function duplicates(app: App, repo: Repo, by: User): Array<[Issue, Issue]> { | |
| 297 | const plan = pillarsOf(app.db, repo); | |
| 298 | const rows = app.db | |
| 299 | .query<{ id: string; title: string; agent: number; folded: number }, [string, string]>( | |
| 300 | `SELECT i.id AS id, i.title AS title, u.is_agent AS agent, | |
| 301 | EXISTS (SELECT 1 FROM comment c WHERE c.issue_id = i.id | |
| 302 | AND c.author_id = ?2 | |
| 303 | AND c.body GLOB '${DUPLICATE_NOTE}[0-9]*' | |
| 304 | AND substr(c.body, ${DUPLICATE_NOTE.length + 1}) NOT GLOB '*[^0-9]*') AS folded | |
| 305 | FROM issue i JOIN user u ON u.id = i.author_id | |
| 306 | WHERE i.repo_id = ?1 AND i.kind = 'issue' AND i.state = 'open' | |
| 307 | ORDER BY i.number`, | |
| 308 | ) | |
| 309 | .all(repo.id, by.id); | |
| 310 | type Entry = { id: string; agent: boolean; folded: boolean }; | |
| 311 | const first = new Map<string, Entry>(); | |
| 312 | const pairs: Array<[string, string]> = []; | |
| 313 | for (const r of rows) { | |
| 314 | if (isStructural(plan, r.id)) continue; | |
| 315 | const key = sameTitle(r.title); | |
| 316 | if (key === "") continue; | |
| 317 | const older = first.get(key); | |
| 318 | const mine: Entry = { id: r.id, agent: r.agent === 1, folded: r.folded === 1 }; | |
| 319 | if (older === undefined) { | |
| 320 | first.set(key, mine); | |
| 321 | } else if (!mine.agent && older.agent) { | |
| 322 | first.set(key, mine); | |
| 323 | if (!older.folded) pairs.push([older.id, mine.id]); | |
| 324 | } else if (!mine.folded) pairs.push([mine.id, older.id]); | |
| 325 | } | |
| 326 | const out: Array<[Issue, Issue]> = []; | |
| 327 | for (const [a, b] of pairs) { | |
| 328 | const loser = getIssueById(app.db, a); | |
| 329 | const survivor = getIssueById(app.db, b); | |
| 330 | if (loser && survivor) out.push([loser, survivor]); | |
| 331 | } | |
| 332 | return out; | |
| 333 | } | |
| 334 | ||
| 335 | /** | |
| 336 | * Whatever waited on the folded issue waits on the survivor. A link that | |
| 337 | * would close a loop is kept as it was, so no dependency is dropped silently. | |
| 338 | */ | |
| 339 | function moveDependents(app: App, loser: Issue, survivor: Issue, actorId: string) { | |
| 340 | const waiting = app.db | |
| 341 | .query<{ issue_id: string }, [string]>( | |
| 342 | "SELECT issue_id FROM issue_dependency WHERE blocked_by_id = ?1", | |
| 343 | ) | |
| 344 | .all(loser.id); | |
| 345 | for (const w of waiting) { | |
| 346 | const issue = getIssueById(app.db, w.issue_id); | |
| 347 | if (!issue) continue; | |
| 348 | if (issue.id === survivor.id || addDependency(app.db, issue, survivor, actorId).ok) | |
| 349 | removeDependency(app.db, issue.id, loser.id); | |
| 350 | } | |
| 351 | // What the folded issue waited on, the survivor now waits on too. | |
| 352 | const blockers = app.db | |
| 353 | .query<{ blocked_by_id: string }, [string]>( | |
| 354 | "SELECT blocked_by_id FROM issue_dependency WHERE issue_id = ?1", | |
| 355 | ) | |
| 356 | .all(loser.id); | |
| 357 | for (const b of blockers) { | |
| 358 | const blocker = getIssueById(app.db, b.blocked_by_id); | |
| 359 | if (blocker && blocker.id !== survivor.id) addDependency(app.db, survivor, blocker, actorId); | |
| 360 | } | |
| 361 | } | |
| 362 | ||
| 363 | /** Fold repeated titles; returns how many issues closed. */ | |
| 364 | function foldDuplicates(app: App, repo: Repo, by: User, room: number): number { | |
| 365 | let closed = 0; | |
| 366 | for (const [loser, survivor] of duplicates(app, repo, by)) { | |
| 367 | if (closed >= room) break; | |
| 368 | try { | |
| 369 | const done = setIssueState(app, by, repo, loser, "closed", { | |
| 370 | reason: "dropped", | |
| 371 | triage: true, | |
| 372 | }); | |
| 373 | if (!done.ok || done.value.state !== "closed") continue; | |
| 374 | closed++; | |
| 375 | addComment(app, by, repo, loser, `${DUPLICATE_NOTE}${survivor.number}`); | |
| 376 | moveDependents(app, loser, survivor, by.id); | |
| 377 | } catch (error) { | |
| 378 | log.error("fold failed", { | |
| 379 | repo: `${repo.ownerName}/${repo.name}`, | |
| 380 | issue: loser.number, | |
| 381 | error: String(error), | |
| 382 | }); | |
| 383 | } | |
| 384 | } | |
| 385 | return closed; | |
| 386 | } | |
| 387 | ||
| 275 | 388 | /** One repository's pass. */ |
| 276 | 389 | export async function liveTriage( |
| 277 | 390 | app: App, |
| @@ -283,6 +396,7 @@ export async function liveTriage( | ||
| 283 | 396 | if (isMirror(app, repo)) return did; |
| 284 | 397 | const by = getUserById(app.db, repo.autonomyBy); |
| 285 | 398 | if (!by) return did; |
| 399 | did.closed += foldDuplicates(app, repo, by, MOST_CHANGES); | |
| 286 | 400 | try { |
| 287 | 401 | [did.labelled, did.missed] = file(app, repo, by, now); |
| 288 | 402 | } catch (error) { |
test/livetriage.test.ts +123
| @@ -7,8 +7,10 @@ import { mkdtemp, rm } from "node:fs/promises"; | ||
| 7 | 7 | import { tmpdir } from "node:os"; |
| 8 | 8 | import { join } from "node:path"; |
| 9 | 9 | import { createApp, type App } from "../src/app.ts"; |
| 10 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; | |
| 10 | 11 | import { addComment, openIssue, setIssueState } from "../src/actions/issues.ts"; |
| 11 | 12 | import { commitToBranch, createRepo, updateRepoSettings } from "../src/actions/repos.ts"; |
| 13 | import { addDependency, blockerNumbersFor } from "../src/core/dependencies.ts"; | |
| 12 | 14 | import { getIssueById, labelsFor, listComments, type Issue } from "../src/core/issues.ts"; |
| 13 | 15 | import { getRepoById, type Repo } from "../src/core/repos.ts"; |
| 14 | 16 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; |
| @@ -150,6 +152,127 @@ describe("live triage", () => { | ||
| 150 | 152 | expect((await liveTriage(app, fresh())).closed).toBe(3); |
| 151 | 153 | }); |
| 152 | 154 | |
| 155 | describe("duplicate titles", () => { | |
| 156 | let bot: User; | |
| 157 | beforeAll(async () => { | |
| 158 | const made = await createAgent(app, alice, { username: "dupbot" }); | |
| 159 | if (!made.ok) throw new Error(String(made.error)); | |
| 160 | bot = made.value.user; | |
| 161 | expect(addToCrew(app, alice, fresh(), "dupbot").ok).toBe(true); | |
| 162 | }); | |
| 163 | const by = (who: User, title: string): Issue => { | |
| 164 | const opened = openIssue(app, who, fresh(), { title }); | |
| 165 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 166 | return opened.value; | |
| 167 | }; | |
| 168 | ||
| 169 | test("the newer closes into the older, dependents follow, case and punctuation ignored", async () => { | |
| 170 | const older = by(alice, "Dup: Add the Thing!"); | |
| 171 | const newer = by(alice, "dup add the thing"); | |
| 172 | const waiter = issue("Waits on the newer"); | |
| 173 | addDependency(app.db, waiter, newer, alice.id); | |
| 174 | const did = await liveTriage(app, fresh()); | |
| 175 | expect(did.closed).toBe(1); | |
| 176 | expect(state(older).state).toBe("open"); | |
| 177 | expect(state(newer).state).toBe("closed"); | |
| 178 | expect(listComments(app.db, newer.id).some((c) => c.body === `duplicate of #${older.number}`)).toBe(true); | |
| 179 | expect(blockerNumbersFor(app.db, [waiter.id]).get(waiter.id)).toEqual([older.number]); | |
| 180 | expect((await liveTriage(app, fresh())).closed).toBe(0); | |
| 181 | }); | |
| 182 | ||
| 183 | test("a person's newer issue is never closed into a crew member's: the older closes", async () => { | |
| 184 | const older = by(bot, "Crew wrote this first"); | |
| 185 | const newer = by(alice, "crew wrote this FIRST"); | |
| 186 | expect((await liveTriage(app, fresh())).closed).toBe(1); | |
| 187 | expect(state(older).state).toBe("closed"); | |
| 188 | expect(state(newer).state).toBe("open"); | |
| 189 | expect(listComments(app.db, older.id).some((c) => c.body === `duplicate of #${newer.number}`)).toBe(true); | |
| 190 | }); | |
| 191 | ||
| 192 | test("a crew member's newer issue closes into a person's older one", async () => { | |
| 193 | const older = by(alice, "Person wrote this first"); | |
| 194 | const newer = by(bot, "Person wrote this first"); | |
| 195 | expect((await liveTriage(app, fresh())).closed).toBe(1); | |
| 196 | expect(state(older).state).toBe("open"); | |
| 197 | expect(state(newer).state).toBe("closed"); | |
| 198 | expect(listComments(app.db, newer.id).some((c) => c.body === `duplicate of #${older.number}`)).toBe(true); | |
| 199 | }); | |
| 200 | ||
| 201 | test("the folded issue's own blockers move to the survivor", async () => { | |
| 202 | const older = by(alice, "Blockers move"); | |
| 203 | const newer = by(alice, "blockers MOVE"); | |
| 204 | const gate = issue("Blocks the newer"); | |
| 205 | addDependency(app.db, newer, gate, alice.id); | |
| 206 | expect((await liveTriage(app, fresh())).closed).toBe(1); | |
| 207 | expect(blockerNumbersFor(app.db, [older.id]).get(older.id)).toEqual([gate.number]); | |
| 208 | }); | |
| 209 | ||
| 210 | test("a near-miss title stays open", async () => { | |
| 211 | const a = by(alice, "Make the parser faster"); | |
| 212 | const b = by(alice, "Make the parsers faster"); | |
| 213 | expect((await liveTriage(app, fresh())).closed).toBe(0); | |
| 214 | expect(state(a).state).toBe("open"); | |
| 215 | expect(state(b).state).toBe("open"); | |
| 216 | }); | |
| 217 | ||
| 218 | test("a person's reopen of a folded issue stands, in both orders", async () => { | |
| 219 | const older = by(alice, "Reopen me please"); | |
| 220 | const newer = by(alice, "reopen me please"); | |
| 221 | expect((await liveTriage(app, fresh())).closed).toBe(1); | |
| 222 | expect(setIssueState(app, alice, fresh(), state(newer), "open").ok).toBe(true); | |
| 223 | expect((await liveTriage(app, fresh())).closed).toBe(0); | |
| 224 | expect(state(newer).state).toBe("open"); | |
| 225 | expect(state(older).state).toBe("open"); | |
| 226 | ||
| 227 | const crewOlder = by(bot, "Reopen the crew one"); | |
| 228 | const personNewer = by(alice, "Reopen the crew one"); | |
| 229 | expect((await liveTriage(app, fresh())).closed).toBe(1); | |
| 230 | expect(state(crewOlder).state).toBe("closed"); | |
| 231 | expect(setIssueState(app, alice, fresh(), state(crewOlder), "open").ok).toBe(true); | |
| 232 | expect((await liveTriage(app, fresh())).closed).toBe(0); | |
| 233 | expect(state(crewOlder).state).toBe("open"); | |
| 234 | expect(state(personNewer).state).toBe("open"); | |
| 235 | }); | |
| 236 | ||
| 237 | test("a person's comment that starts like the note does not protect a duplicate", async () => { | |
| 238 | by(alice, "Typed note title"); | |
| 239 | const newer = by(alice, "Typed note title"); | |
| 240 | addComment(app, alice, fresh(), newer, "duplicate of #1 is what I thought, but no"); | |
| 241 | expect((await liveTriage(app, fresh())).closed).toBe(1); | |
| 242 | expect(state(newer).state).toBe("closed"); | |
| 243 | }); | |
| 244 | ||
| 245 | test("a link that would close a loop is kept, not dropped", async () => { | |
| 246 | const older = by(alice, "Loop survivor"); | |
| 247 | const newer = by(alice, "loop survivor"); | |
| 248 | const mid = issue("Loop middle"); | |
| 249 | addDependency(app.db, mid, newer, alice.id); | |
| 250 | addDependency(app.db, older, mid, alice.id); | |
| 251 | expect((await liveTriage(app, fresh())).closed).toBe(1); | |
| 252 | expect(state(newer).state).toBe("closed"); | |
| 253 | expect(blockerNumbersFor(app.db, [mid.id]).get(mid.id)).toEqual([newer.number]); | |
| 254 | }); | |
| 255 | ||
| 256 | test("the plan's own issues are neither folded nor chosen as survivor", async () => { | |
| 257 | const umbrella = by(alice, "Later: park these"); | |
| 258 | const twin = by(alice, "later: park these"); | |
| 259 | expect((await liveTriage(app, fresh())).closed).toBe(0); | |
| 260 | expect(state(umbrella).state).toBe("open"); | |
| 261 | expect(state(twin).state).toBe("open"); | |
| 262 | }); | |
| 263 | ||
| 264 | test("folds plus already-done closes stay within 25 per tick", async () => { | |
| 265 | for (let n = 0; n < MOST_CHANGES + 3; n++) { | |
| 266 | by(alice, `Twin ${n}`); | |
| 267 | by(alice, `twin ${n}`); | |
| 268 | } | |
| 269 | const extra = issue("Done too"); | |
| 270 | note(extra, oid); | |
| 271 | expect((await liveTriage(app, fresh())).closed).toBe(MOST_CHANGES); | |
| 272 | expect((await liveTriage(app, fresh())).closed).toBe(4); | |
| 273 | }); | |
| 274 | }); | |
| 275 | ||
| 153 | 276 | describe("questions", () => { |
| 154 | 277 | beforeAll(() => { |
| 155 | 278 | ensureDirectionIssue(app, alice, fresh()); |
new above
offline
state(on).numberis the issue's own number, so this checks for "duplicate of #<self>", which is never written.noted(newer)).toBe(false)passes whatever the code does. Compare against the survivor's number and assert true on the folded issue.