Public: anyone can read this; the crew can change it.
The sentinel files no never-green-check issue in trevato/forge for another repository #5665
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3918-the-sentinel-files-no-never-green-check → main 5 commits fork 1 thread · 1 unresolved
3 files
changed +59
−4
README.md +2
| @@ -1402,6 +1402,8 @@ its evidence, filed in `FORGE_SENTINEL_REPO` (default `trevato/forge`) as the | ||
| 1402 | 1402 | credential-shaped before it is filed. Other findings (`held`, `ledger`, `red`) |
| 1403 | 1403 | are filed anew when they return. |
| 1404 | 1404 | |
| 1405 | A check that has never been green is filed only for the sentinel's own repository. In any other repository it stays on that repository's checks and the degraded list. | |
| 1406 | ||
| 1405 | 1407 | A capacity setting away from its default is not a finding: the sentinel files |
| 1406 | 1408 | nothing for it (#3269). On `/settings/capacity` the row of each such setting |
| 1407 | 1409 | carries one muted line, "changed from X N days ago by who", and its **Put back** |
src/crew/sentinel.ts +11
| @@ -254,9 +254,17 @@ interface SummaryRow { | ||
| 254 | 254 | summary: string; |
| 255 | 255 | } |
| 256 | 256 | |
| 257 | let warnedUnresolved = false; | |
| 258 | ||
| 257 | 259 | /** The last runs per repository: a check never green, or pushes that cannot land. */ |
| 258 | 260 | export function runStreaks(db: Database): Finding[] { |
| 259 | 261 | const out: Finding[] = []; |
| 262 | const [ownerName = "", repoName = ""] = sentinelRepoPath().split("/"); | |
| 263 | const ownId = getRepo(db, ownerName, repoName)?.id; | |
| 264 | if (ownId === undefined && !warnedUnresolved) { | |
| 265 | warnedUnresolved = true; | |
| 266 | log.warn("sentinel repository does not resolve; never-green checks are not filed", { repo: sentinelRepoPath() }); | |
| 267 | } | |
| 260 | 268 | const rows = db |
| 261 | 269 | .query<SummaryRow, [number]>( |
| 262 | 270 | `SELECT id, repo_id, kind, state, summary FROM ( |
| @@ -275,6 +283,9 @@ export function runStreaks(db: Database): Finding[] { | ||
| 275 | 283 | const [repoId = "", kind = ""] = k.split("\u0000"); |
| 276 | 284 | const failed = runs.filter((r) => r.state === "failed"); |
| 277 | 285 | if (kind === "check") { |
| 286 | // Another repository's gate is its owner's to fix: a builder here cannot | |
| 287 | // read or change it. It stays on that repository's checks and the degraded list. | |
| 288 | if (repoId !== ownId) continue; | |
| 278 | 289 | if (runs.some((r) => r.state === "ok")) continue; |
| 279 | 290 | const same = new Map<string, SummaryRow[]>(); |
| 280 | 291 | for (const r of failed) |
test/sentinel.test.ts +46 −4
| @@ -77,7 +77,10 @@ beforeAll(async () => { | ||
| 77 | 77 | if (!made.ok) throw new Error(String(made.error)); |
| 78 | 78 | repo = made.value; |
| 79 | 79 | }); |
| 80 | const keptSentinelRepo = process.env.FORGE_SENTINEL_REPO; | |
| 80 | 81 | afterAll(async () => { |
| 82 | if (keptSentinelRepo === undefined) delete process.env.FORGE_SENTINEL_REPO; | |
| 83 | else process.env.FORGE_SENTINEL_REPO = keptSentinelRepo; | |
| 81 | 84 | app.db.close(); |
| 82 | 85 | await rm(dataDir, { recursive: true, force: true }); |
| 83 | 86 | }); |
| @@ -165,6 +168,41 @@ describe("a push blocked on the network is still a push fault", () => { | ||
| 165 | 168 | }); |
| 166 | 169 | }); |
| 167 | 170 | |
| 171 | describe("a gate never green", () => { | |
| 172 | const gateKeys = (): string[] => | |
| 173 | findFaults(app.db, NOW).map((f) => f.key).filter((k) => k.startsWith("gate:")); | |
| 174 | const plant = (repoId: string): void => { | |
| 175 | for (let n = 0; n < 15; n++) { | |
| 176 | const run = createRun(app.db, { repoId, kind: "check" }); | |
| 177 | app.db | |
| 178 | .query("UPDATE run SET state = 'failed', summary = 'no check.run', created_at = ?2 WHERE id = ?1") | |
| 179 | .run(run.id, ago((40 + n) * HOUR)); | |
| 180 | } | |
| 181 | }; | |
| 182 | ||
| 183 | test("in another repository files nothing", async () => { | |
| 184 | const made = await createRepo(app, alice, { name: "elsewhere", visibility: "private" }); | |
| 185 | if (!made.ok) throw new Error(String(made.error)); | |
| 186 | process.env.FORGE_SENTINEL_REPO = "alice/forge"; | |
| 187 | try { | |
| 188 | plant(made.value.id); | |
| 189 | expect(gateKeys()).not.toContain(`gate:${made.value.id}`); | |
| 190 | } finally { | |
| 191 | app.db.query("DELETE FROM run WHERE repo_id = ?1").run(made.value.id); | |
| 192 | } | |
| 193 | }); | |
| 194 | ||
| 195 | test("in the sentinel's own repository still files", () => { | |
| 196 | process.env.FORGE_SENTINEL_REPO = "alice/forge"; | |
| 197 | try { | |
| 198 | plant(repo.id); | |
| 199 | expect(gateKeys()).toContain(`gate:${repo.id}`); | |
| 200 | } finally { | |
| 201 | app.db.query("DELETE FROM run WHERE repo_id = ?1 AND summary = 'no check.run'").run(repo.id); | |
| 202 | } | |
| 203 | }); | |
| 204 | }); | |
| 205 | ||
| 168 | 206 | describe("a failed push that pushed nothing", () => { |
| 169 | 207 | test("still raises the push fault", async () => { |
| 170 | 208 | const made = await createRepo(app, alice, { name: "blocked", visibility: "private" }); |
| @@ -184,6 +222,7 @@ describe("a failed push that pushed nothing", () => { | ||
| 184 | 222 | describe("the faults of the day", () => { |
| 185 | 223 | let queuedId = ""; |
| 186 | 224 | beforeAll(() => { |
| 225 | process.env.FORGE_SENTINEL_REPO = "alice/forge"; | |
| 187 | 226 | // 1. a slot held by a finished run: work waits, nothing runs. |
| 188 | 227 | plantRun("check", "cancelled", "cancelled", ago(3 * HOUR)); |
| 189 | 228 | queuedId = createRun(app.db, { repoId: repo.id, kind: "check" }).id; |
| @@ -267,13 +306,16 @@ describe("the faults of the day", () => { | ||
| 267 | 306 | describe("the schedule", () => { |
| 268 | 307 | test("a new fault is a signal inside the hour; a known one waits for the hour", () => { |
| 269 | 308 | const last = new Date(sentinelStanding(app.db).lastHuntAt!).getTime(); |
| 270 | expect(sentinelTick(app, last + 5 * 60_000)).toBeNull(); | |
| 271 | expect(sentinelTick(app, last + 20 * 60_000)).toBeNull(); | |
| 272 | process.env.FORGE_SENTINEL_REPO = "alice/forge"; | |
| 309 | const kept = process.env.FORGE_SENTINEL_REPO; | |
| 310 | delete process.env.FORGE_SENTINEL_REPO; | |
| 273 | 311 | try { |
| 312 | expect(sentinelTick(app, last + 5 * 60_000)).toBeNull(); | |
| 313 | expect(sentinelTick(app, last + 20 * 60_000)).toBeNull(); | |
| 314 | process.env.FORGE_SENTINEL_REPO = "alice/forge"; | |
| 274 | 315 | expect(sentinelTick(app, last + HOUR + 1)).not.toBeNull(); |
| 275 | 316 | } finally { |
| 276 | delete process.env.FORGE_SENTINEL_REPO; | |
| 317 | if (kept === undefined) delete process.env.FORGE_SENTINEL_REPO; | |
| 318 | else process.env.FORGE_SENTINEL_REPO = kept; | |
| 277 | 319 | } |
| 278 | 320 | }); |
| 279 | 321 | }); |
new above
offline
Module-level mutable flag: fine for a once-only warning, but it cannot be reset in tests, so the warning is untested. Consider reusing the existing not-found fault instead.