Public: anyone can read this; the crew can change it.
The sentinel files no fix issue in trevato/forge for another repository's red main #3898
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3895-the-sentinel-files-no-fix-issue-in-treva → main 5 commits fork 4 threads · 4 unresolved
7 files
changed +81
−6
src/crew/sentinel.ts +6 −3
| @@ -20,6 +20,7 @@ import { decisionsSince } from "../core/decisions.ts"; | ||
| 20 | 20 | import { getAgentByName } from "../core/agents.ts"; |
| 21 | 21 | import { getIssueById, getLabel, setIssueLabels as writeLabels, type Issue } from "../core/issues.ts"; |
| 22 | 22 | import { getRepo, type Repo } from "../core/repos.ts"; |
| 23 | import { ownRepoPath } from "../kernel/own_repo.ts"; | |
| 23 | 24 | import { leaseLapsed, listRunning } from "../core/runs.ts"; |
| 24 | 25 | import { migrations } from "../core/schema.ts"; |
| 25 | 26 | import { getUserByName, type User } from "../core/users.ts"; |
| @@ -405,7 +406,10 @@ const BREACH_CAUSE = /^(queue:check|stuck:|budget:|red:|shared:)/; | ||
| 405 | 406 | * `forgeRepoId` unset means no repository is known and nothing is dropped. |
| 406 | 407 | */ |
| 407 | 408 | export function ownCause(cause: string, forgeRepoId: string | undefined): boolean { |
| 408 | return forgeRepoId === undefined || !cause.startsWith("red:") || cause === `red:${forgeRepoId}`; | |
| 409 | if (forgeRepoId === undefined) return true; | |
| 410 | if (cause.startsWith("red:")) return cause === `red:${forgeRepoId}`; | |
| 411 | if (cause.startsWith("shared:")) return cause.split(":")[1] === forgeRepoId; | |
| 412 | return true; | |
| 409 | 413 | } |
| 410 | 414 | |
| 411 | 415 | /** |
| @@ -486,8 +490,7 @@ export interface HuntResult { | ||
| 486 | 490 | /** `filed` in a recorded hunt that had nowhere to file. */ |
| 487 | 491 | const UNFILED = -1; |
| 488 | 492 | |
| 489 | export const sentinelRepoPath = (): string => | |
| 490 | (process.env.FORGE_SENTINEL_REPO ?? "trevato/forge").trim(); | |
| 493 | export const sentinelRepoPath = ownRepoPath; | |
| 491 | 494 | |
| 492 | 495 | /** Why a hunt could not file, or undefined when it can. */ |
| 493 | 496 | export function cannotFile( |
src/kernel/own_repo.ts added +2
| @@ -0,0 +1,2 @@ | ||
| 1 | /** The repository this forge fixes itself in: `FORGE_SENTINEL_REPO`, else `trevato/forge`. */ | |
| 2 | export const ownRepoPath = (): string => (process.env.FORGE_SENTINEL_REPO ?? "trevato/forge").trim(); |
src/kernel/vitals.ts +8 −2
| @@ -21,6 +21,7 @@ import { | ||
| 21 | 21 | queueDepth, |
| 22 | 22 | } from "../core/runs.ts"; |
| 23 | 23 | import { STALE_AFTER_MS } from "../runs/lease.ts"; |
| 24 | import { ownRepoPath } from "./own_repo.ts"; | |
| 24 | 25 | import { BUDGETS } from "./budgets.ts"; |
| 25 | 26 | import { openDbFiles } from "./file_modes.ts"; |
| 26 | 27 | import { lastMirror, mirrorNeed } from "../snapshot/mirror.ts"; |
| @@ -35,6 +36,11 @@ import { domainDrillNeeds } from "./domain_drill.ts"; | ||
| 35 | 36 | import { freeChecks, heldChecks, redHref, redSentence } from "../checks/red_main.ts"; |
| 36 | 37 | import { flakeHref, sharedCause, sharedSentence, sharedTests } from "../checks/shared_red.ts"; |
| 37 | 38 | |
| 39 | /** A red in a repository other than the forge's own is not fixable from here: say so, so it reads as known. */ | |
This function sits between the "Free disk under this share..." comment and DISK_FREE_MIN, leaving that doc comment attached to foreignNote. Move it elsewhere. Also the env default duplicates the sentinel own-repo lookup; share one source, and add a test for the note. |
||
| 40 | function foreignNote(repo: string): string { | |
foreignNote is applied to both the red: and shared: needs, but only shared: has a test. Add one for red:, foreign and own. |
||
| 41 | return repo === ownRepoPath() ? "" : " Not fixable from this forge: it is another repository's to fix."; | |
| 42 | } | |
| 43 | ||
| 38 | 44 | /** Free disk under this share of the volume is something to say. */ |
| 39 | 45 | export const DISK_FREE_MIN = 0.1; |
| 40 | 46 | /** This many error lines in an hour is a burst worth saying. */ |
| @@ -377,14 +383,14 @@ export function vitals(input: VitalsInput): Vitals { | ||
| 377 | 383 | for (const { red, held } of reds) |
| 378 | 384 | need( |
| 379 | 385 | `red:${red.repoId}`, |
| 380 | redSentence(red, held), | |
| 386 | redSentence(red, held) + foreignNote(red.repo), | |
| 381 | 387 | red.since, |
| 382 | 388 | redHref(red), |
| 383 | 389 | ); |
| 384 | 390 | |
| 385 | 391 | // A test failing on many pulls in the hour is one card, not a park per pull. |
| 386 | 392 | for (const shared of sharedTests(input.db, now)) |
| 387 | need(sharedCause(shared), sharedSentence(shared), shared.since, flakeHref(input.db, shared)); | |
| 393 | need(sharedCause(shared), sharedSentence(shared) + foreignNote(shared.repo), shared.since, flakeHref(input.db, shared)); | |
This line is over 120 characters; break the need(...) call across lines like the red: one above. |
||
| 388 | 394 | |
| 389 | 395 | // A red restore drill, or none in eight days, is a card until a drill passes. |
| 390 | 396 | const drill = lastVerdict(input.dataDir); |
test/crew-red-main.test.ts +26
| @@ -14,6 +14,9 @@ import { getPull, type Pull } from "../src/core/pulls.ts"; | ||
| 14 | 14 | import type { Repo } from "../src/core/repos.ts"; |
| 15 | 15 | import { claimRun, createRun, finishRun, listRuns } from "../src/core/runs.ts"; |
| 16 | 16 | import type { User } from "../src/core/users.ts"; |
| 17 | import { vitals } from "../src/kernel/vitals.ts"; | |
| 18 | import { ErrorLedger } from "../src/kernel/log.ts"; | |
| 19 | import { RefusedLedger } from "../src/kernel/internal_refused.ts"; | |
| 17 | 20 | import { startStandInCrew, stopStandInCrew, type StandInCrew } from "./helpers/crew-standin.ts"; |
| 18 | 21 | |
| 19 | 22 | describe("the crew, with a red main it shares", () => { |
| @@ -110,4 +113,27 @@ describe("the crew, with a red main it shares", () => { | ||
| 110 | 113 | finish("main", "ok", []); |
| 111 | 114 | expect(requested.map((r) => r[4])).toEqual([kept.id]); |
| 112 | 115 | }); |
| 116 | ||
| 117 | test("the red main card says another repository's red is not fixable here; the forge's own says nothing of it", async () => { | |
| 118 | const prior = process.env.FORGE_SENTINEL_REPO; | |
| 119 | try { | |
| 120 | const pull = await pullOf(); | |
| 121 | finish("main", "failed", ["X"]); | |
| 122 | // A pull's check queued behind the red main is what makes the card. | |
| 123 | createRun(app.db, { repoId: repo.id, kind: "check", env: `pull-${pull.number}`, commitOid: pull.headOid, reason: "push", issueId: pull.id }); | |
| 124 | const card = () => | |
| 125 | vitals({ | |
| 126 | db: app.db, | |
| 127 | dataDir: app.config.dataDir, | |
| 128 | signals: { errors: new ErrorLedger(), refused: new RefusedLedger() }, | |
| 129 | }).degraded.find((d) => d.cause === `red:${repo.id}`)!.sentence; | |
| 130 | process.env.FORGE_SENTINEL_REPO = "someone/else"; | |
| 131 | expect(card()).toContain("Not fixable from this forge"); | |
| 132 | process.env.FORGE_SENTINEL_REPO = `${alice.username}/${repo.name}`; | |
| 133 | expect(card()).not.toContain("Not fixable from this forge"); | |
| 134 | } finally { | |
| 135 | if (prior === undefined) delete process.env.FORGE_SENTINEL_REPO; | |
| 136 | else process.env.FORGE_SENTINEL_REPO = prior; | |
| 137 | } | |
| 138 | }); | |
| 113 | 139 | }); |
test/sentinel.test.ts +24
| @@ -641,6 +641,30 @@ describe("a budget breach is crew work at once", () => { | ||
| 641 | 641 | expect(listEvents(bapp.db, { kind: "issue.assigned" }).length).toBe(0); |
| 642 | 642 | }); |
| 643 | 643 | |
| 644 | test("a red main is filed only for the sentinel's own repository", () => { | |
| 645 | const prior = process.env.FORGE_SENTINEL_REPO; | |
| 646 | process.env.FORGE_SENTINEL_REPO = "alice/forge"; | |
| 647 | const ins = bapp.db.query( | |
| 648 | "INSERT INTO degraded_span (cause, sentence, href, opened_at) VALUES (?1, 'main is red', '/x', ?2)", | |
| 649 | ); | |
| 650 | try { | |
| 651 | ins.run("red:someotherrepoid", NOW - 60_000); | |
| 652 | expect(budgetBreaches(bapp.db, NOW, brepo.id).some((f) => f.title.startsWith("red:"))).toBe(false); | |
| 653 | ins.run("shared:someotherrepoid:abc", NOW - 60_000); | |
| 654 | expect(budgetBreaches(bapp.db, NOW, brepo.id).some((f) => f.title.startsWith("shared:"))).toBe(false); | |
| 655 | ins.run(`shared:${brepo.id}:abc`, NOW - 60_000); | |
| 656 | expect(budgetBreaches(bapp.db, NOW, brepo.id).filter((f) => f.title.startsWith("shared:")).length).toBe(1); | |
| 657 | ins.run(`red:${brepo.id}`, NOW - 60_000); | |
| 658 | const own = budgetBreaches(bapp.db, NOW, brepo.id).filter((f) => f.title.startsWith("red:")); | |
| 659 | expect(own.length).toBe(1); | |
| 660 | expect(own[0]!.fix).toContain("Fix the failing test on main"); | |
| 661 | } finally { | |
| 662 | if (prior === undefined) delete process.env.FORGE_SENTINEL_REPO; | |
| 663 | else process.env.FORGE_SENTINEL_REPO = prior; | |
| 664 | bapp.db.query("DELETE FROM degraded_span WHERE cause LIKE 'red:%' OR cause LIKE 'shared:%'").run(); | |
| 665 | } | |
| 666 | }); | |
| 667 | ||
| 644 | 668 | test("a breach brings a hunt within a tick, inside the signal gap", () => { |
| 645 | 669 | const t = Date.parse("2026-10-14T12:00:00.000Z"); |
| 646 | 670 | hunt(bapp, "alice/forge", t); |
new above
offline
If the sentinel repo is not found, ownId is undefined and every red: span is skipped. That is a sensible default, but worth a one-line note. Consider the same check for shared: spans.