Public: anyone can read this; the crew can change it.
The sentinel does not refile an incident whose issue the planner closed as split or dropped #5154
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5149-the-sentinel-does-not-refile-an-incident → main 2 commits fork 2 threads · 2 unresolved
2 files
changed +38
−7
src/crew/sentinel.ts +8 −5
| @@ -557,9 +557,10 @@ function labelBreach(app: App, repo: Repo, actor: User, issue: Issue): void { | ||
| 557 | 557 | } |
| 558 | 558 | |
| 559 | 559 | /** |
| 560 | * A degraded incident whose issue was closed as done and which goes on (same | |
| 561 | * cause, same first-open time) is told to that issue, not filed again: the | |
| 562 | * fix is likely merged but not live. At most one comment a day. True when | |
| 560 | * A degraded incident whose issue was closed as done or split and which goes | |
| 561 | * on (same cause, same first-open time) is told to that issue, not filed | |
| 562 | * again: the fix is likely merged but not live. At most one comment a day. | |
| 563 | * One closed as dropped stays silent. True when | |
| 563 | 564 | * the incident is that one: the comments made (0 or 1), else undefined. |
| 564 | 565 | */ |
| 565 | 566 | function continuesDone(app: App, repo: Repo, actor: User, f: Finding, evidence: string, at: string, now: number): 0 | 1 | undefined { |
| @@ -572,7 +573,9 @@ function continuesDone(app: App, repo: Repo, actor: User, f: Finding, evidence: | ||
| 572 | 573 | .get(f.key, f.incident ?? ""); |
| 573 | 574 | if (prior === null) return undefined; |
| 574 | 575 | const issue = getIssueById(app.db, prior.issue_id); |
| 575 | if (issue === undefined || issue.closeReason !== "done") return undefined; | |
| 576 | if (issue === undefined) return undefined; | |
| 577 | if (issue.closeReason === "dropped") return 0; | |
| 578 | if (issue.closeReason !== "done" && issue.closeReason !== "split") return undefined; | |
| 576 | 579 | if (prior.commented_at !== null && now - Date.parse(prior.commented_at) < DAY_MS) return 0; |
| 577 | 580 | const minutes = f.evidence.find((e) => e.startsWith("total: ")); |
| 578 | 581 | const said = addComment( |
| @@ -580,7 +583,7 @@ function continuesDone(app: App, repo: Repo, actor: User, f: Finding, evidence: | ||
| 580 | 583 | actor, |
| 581 | 584 | repo, |
| 582 | 585 | issue, |
| 583 | `Still degraded after the fix; the fix may not be live yet. At ${at}${minutes === undefined ? "" : `, ${minutes.slice(7)}`}; evidence now:\n${f.evidence.map((e) => `- ${e}`).join("\n")}`, | |
| 586 | `Still degraded${issue.closeReason === "split" ? "; the work was split into other issues" : " after the fix; the fix may not be live yet"}. At ${at}${minutes === undefined ? "" : `, ${minutes.slice(7)}`}; evidence now:\n${f.evidence.map((e) => `- ${e}`).join("\n")}`, | |
| 584 | 587 | ); |
| 585 | 588 | if (!said.ok) { |
| 586 | 589 | log.warn("sentinel could not comment", { key: f.key, error: String(said.error) }); |
test/sentinel.test.ts +30 −2
| @@ -747,14 +747,42 @@ describe("a degraded incident whose issue closed as done", () => { | ||
| 747 | 747 | expect(issues("open").length).toBe(1); |
| 748 | 748 | }); |
| 749 | 749 | |
| 750 | test("a finding closed as dropped that returns is refiled", () => { | |
| 750 | test("a finding closed as dropped stays silent while the incident goes on", () => { | |
| 751 | 751 | const t = T0 + 10 * DAY; |
| 752 | 752 | span("disk", t - 2 * HOUR, 30); |
| 753 | 753 | expect(hunt(iapp, "alice/forge", t).filed).toBe(1); |
| 754 | 754 | iapp.db.query("UPDATE issue SET state = 'closed', close_reason = 'dropped', closed_at = ?1 WHERE state = 'open'").run(new Date(t).toISOString()); |
| 755 | 755 | const r = hunt(iapp, "alice/forge", t + 30 * D_MIN); |
| 756 | expect(r.filed).toBe(1); | |
| 756 | expect(r.filed).toBe(0); | |
| 757 | 757 | expect(r.commented).toBe(0); |
| 758 | expect(issues("open").length).toBe(0); | |
| 759 | }); | |
| 760 | ||
| 761 | test("a finding closed as split is commented once a day, not refiled", () => { | |
| 762 | const t = T0 + 20 * DAY; | |
| 763 | span("queue", t - 2 * HOUR, 30); | |
| 764 | expect(hunt(iapp, "alice/forge", t).filed).toBe(1); | |
| 765 | iapp.db.query("UPDATE issue SET state = 'closed', close_reason = 'split', closed_at = ?1 WHERE state = 'open'").run(new Date(t).toISOString()); | |
| 766 | const again = hunt(iapp, "alice/forge", t + 30 * D_MIN); | |
| 767 | expect(again.filed).toBe(0); | |
| 768 | expect(again.commented).toBe(1); | |
| 769 | const third = hunt(iapp, "alice/forge", t + 2 * HOUR); | |
| 770 | expect(third.filed).toBe(0); | |
| 771 | expect(third.commented).toBe(0); | |
| 772 | expect(issues("open").length).toBe(0); | |
| 773 | }); | |
| 774 | ||
| 775 | test("a new incident after a split close is refiled", () => { | |
| 776 | const t = T0 + 30 * DAY; | |
This test never closes an issue as split, so it passes without the change. Replace it with: file, close as split, let the incident end, start a new span with a different first-open time, assert filed is 1. |
||
| 777 | span("cache", t - 2 * HOUR, 30); | |
| 778 | expect(hunt(iapp, "alice/forge", t).filed).toBe(1); | |
| 779 | iapp.db.query("UPDATE issue SET state = 'closed', close_reason = 'split', closed_at = ?1 WHERE state = 'open'").run(new Date(t).toISOString()); | |
| 780 | // The incident ends; a fresh one opens later, with another first-open time. | |
| 781 | const later = t + 2 * DAY; | |
| 782 | span("cache", later - HOUR, 20); | |
| 783 | const r = hunt(iapp, "alice/forge", later); | |
| 784 | expect(r.filed).toBe(1); | |
| 785 | expect(issues("open").length).toBe(1); | |
| 758 | 786 | }); |
| 759 | 787 | }); |
| 760 | 788 |
new above
offline
Returning 0 for dropped is right, but add a one-line comment here that silence is deliberate, so a later reader does not take it for a missed refile.