From b85dd13a2215ea63cd770cd7fa91c529318b6283 Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 14:48:48 +0000 Subject: [PATCH 1/2] stewards: shadow assessment module and divergence log (BB-293) --- plugins/stewards/DESIGN.md | 25 ++ plugins/stewards/assessment.test.ts | 299 ++++++++++++++++++++++ plugins/stewards/assessment.ts | 275 ++++++++++++++++++++ plugins/stewards/migrations.snapshot.json | 6 +- plugins/stewards/reconcile.ts | 14 + plugins/stewards/server.test.ts | 49 ++++ plugins/stewards/server.ts | 93 ++++++- plugins/stewards/store.test.ts | 19 +- plugins/stewards/store.ts | 149 ++++++++++- 9 files changed, 913 insertions(+), 16 deletions(-) create mode 100644 plugins/stewards/assessment.test.ts create mode 100644 plugins/stewards/assessment.ts diff --git a/plugins/stewards/DESIGN.md b/plugins/stewards/DESIGN.md index e338cd17..db4523cf 100644 --- a/plugins/stewards/DESIGN.md +++ b/plugins/stewards/DESIGN.md @@ -658,6 +658,31 @@ At the project file's time, 01:00 US Eastern by default: All of it must finish before 03:00, when bb-gate's quiet window restarts servers to apply staged updates and would kill a running turn. +### The shadow assessment (BB-293) + +BB-291's direction is one assessment per binding: the merge gate, the worker +state and the wait liveness answered in one place instead of by the several +predicates that disagree today. This step adds only the derivation and the +evidence it is safe, in shadow: + +- `assessment.ts` is pure. `assess` reads the rows a reconcile already has + in hand — no extra GitHub or Linear call — and derives the gate (with the +decide-list mark and the recorded project approvals the `approvals.ts` path +omits), the worker state (an explicit `parked`, distinct from `running`) and +the liveness (wake source, idle, stall from the observable rows, not the +report-time `noWakeSource` flag). `compare` runs the predicates the +assessment will replace and returns every disagreement. +- `reconcile.ts` calls it on every binding inside a try/catch that can never + change the reconcile's own result, stores the latest assessment per binding + in `assessments`, and logs each distinct divergence to + `assessment_divergences` (one row per binding/surface/old/new, with a first + and last seen time and a count). The log is bounded by an age and a row + count, pruned on each reconcile. +- `bb stewards divergences [--binding] [--surface] [--json]` prints the log, + read-only. Nothing reads the assessment to decide or render anything: a + consumer is added only after the comparison has run clean in production for + several days. The log's `inputs` hold only ids, shas, states and timestamps. + ## 7. The ready gate No ticket reaches implementation until it is fully specified, and no diff --git a/plugins/stewards/assessment.test.ts b/plugins/stewards/assessment.test.ts new file mode 100644 index 00000000..5f948b5d --- /dev/null +++ b/plugins/stewards/assessment.test.ts @@ -0,0 +1,299 @@ +// BB-293 (child 2 of BB-291): the shadow assessment and its divergence log. +// The fixtures are the ones BB-292 characterized: the BB-266 shape (a gate-met +// own pull request, a parked `finished` worker and a second open own pull +// request) and bb-plugins#294 (a `pr` binding's own open pull request with a +// free-text wait). Each BB-292 divergence appears here as a fixture whose +// divergence `compare` returns, so the log records it. Every gate-met fixture +// carries a parked worker and a second open own pull request unless the test is +// about their absence (named in the title). +import Database from "better-sqlite3"; +import { describe, expect, it } from "vitest"; +import { ASSESSMENT_DIVERGENCE_MAX_AGE_MS, ASSESSMENT_DIVERGENCE_MAX_ROWS, assess, compare, ownPullEntities, type AssessmentInput } from "./assessment.js"; +import { parsePlaybook, type Playbook } from "./config.js"; +import { comment, issue, parkedWorker, pull } from "./fixtures.js"; +import { approvalMarkerLine } from "./gate.js"; +import { formatDivergences } from "./server.js"; +import { MIGRATIONS, createStore, readyStateSchema, type StatusRecord } from "./store.js"; +import { LINEAR_TICKETS } from "./templates.js"; +import type { EntitySnapshot } from "./source-contract.js"; +import type { WakeSourceInput } from "./wake-source.js"; + +const tickets: Playbook = (() => { + const p = parsePlaybook(LINEAR_TICKETS, "linear-tickets"); + if (!p.ok) throw new Error(p.error); + return p.value; +})(); + +const head = "aaaaaaa1"; +const green = { checks: { state: "success", failing: [] } }; +const noReviewMarker = comment({ id: "m1", author: "trotterdylan", body: ``, signedBy: "thr_reviewer", createdAt: "2026-10-14T16:00:00Z" }); +const ownerMarker = comment({ id: "m2", author: "trotterdylan", body: ``, signedBy: "thr_reviewer", createdAt: "2026-10-14T16:00:00Z" }); +const projectApprovalMarker = (id: string) => comment({ id, author: "trotterdylan", body: approvalMarkerLine({ sha: head, entry: null, verdict: "approved" }), signedBy: "thr_project", createdAt: "2026-10-14T16:10:00Z" }); + +const noSources: WakeSourceInput = { hold: null, holdBehind: [], mergeBehind: [], holdStack: [], deployments: {}, runningWorkers: 0, openEscalations: 0, openNeeds: 0, pullRequestWatch: false }; + +/** A gate-met own pull request (no-review, green) plus the reviewer marker. */ +const gateMetPull = (key: string, url: string) => pull({ key, url, comments: [noReviewMarker] }, { headSha: head, ...green }); +/** A second, not-gate-met own pull request, so the fixture is not about a lone PR. */ +const secondOpenPull = (key: string, url: string) => pull({ key, url }, { headSha: "bbbbbbb2" }); + +/** The lowercased keys and URLs of the binding's own pull requests (BB-52 rule 0). */ +const own = (...pulls: EntitySnapshot[]): Set => new Set(pulls.flatMap((p) => [p.key.toLowerCase(), (p.url ?? "").toLowerCase()]).filter((s) => s !== "")); + +const status = (waitingOn: string | null): StatusRecord => ({ phase: "In Progress", summary: "s", nextAction: "n", actor: "other", waitingOn, requeueAfterMs: null, conditions: [], resources: { workers: [], pullRequests: [], children: [] }, at: 1 }); + +const binding = (over: Partial = {}): AssessmentInput["binding"] => ({ + id: "b1", + entityKey: "ENG-12", + kind: "ticket", + ready: readyStateSchema.parse({}), + stage: "build", + ending: null, + hold: null, + deployments: {}, + requeueUntil: null, + noWakeSource: false, + status: null, + createdAt: 1, + advancedAt: 1, + phase: "In Progress", + ...over, +}); + +function input(over: Partial = {}): AssessmentInput { + return { + binding: binding(), + observed: { entity: issue({ state: "In Progress" }), pulls: [] }, + ownPulls: new Set(), + gateMetPulls: new Set(), + decisions: new Map(), + projectApprovals: [], + workers: [], + escalations: [], + needs: [], + sources: { ...noSources }, + playbook: tickets, + currentSpecHash: "", + now: 1_000_000, + ...over, + }; +} + +/** + * BB-266: a gate-met own pull request, a parked `finished` worker, and (unless + * `secondOpen` says the test is about its absence) a second open own pull + * request. `sources` is the report path's, with no tracked source, so the + * parked worker is not a wake source. + */ +function bb266(secondOpen: boolean): AssessmentInput { + const first = gateMetPull("acme/web#7", "https://github.com/acme/web/pull/7"); + const second = secondOpenPull("acme/web#8", "https://github.com/acme/web/pull/8"); + const pulls = secondOpen ? [first, second] : [first]; + return input({ + observed: { entity: issue({ state: "In Progress" }), pulls }, + ownPulls: own(...pulls), + gateMetPulls: new Set(["acme/web#7"]), + decisions: new Map(pulls.map((p) => [p.key.toLowerCase(), "user" as const])), + workers: [parkedWorker({ result: "Waiting on the owner's approval of acme/web#7." })], + sources: { ...noSources }, + }); +} + +describe("the shadow assessment: the worker state (BB-266)", () => { + it("names a parked finished worker `parked`, in flight but not running", () => { + const i = bb266(true); + const a = assess(i); + expect(a.worker).toMatchObject({ state: "parked", inFlight: true, running: false, parked: true, reason: null }); + expect(a.gate.pulls).toHaveLength(2); + expect(a.gate.pulls[0]).toMatchObject({ pull: "acme/web#7", met: true, lane: "no-review" }); + expect(a.gate.pulls[1]).toMatchObject({ pull: "acme/web#8", met: false }); + }); + + // DIVERGENCE (BB-292 #1): workerInFlight counts the parked worker in flight, + // runningWorkerReason says nobody is mid-turn, and hasLiveWakeSource says no + // tracked source is live; the assessment names the one state `parked`. + it("records the parked-worker divergences: in-flight, not mid-turn, no live source", () => { + const i = bb266(true); + const a = assess(i); + expect(a.liveness.idle).toBe(false); // the second open own PR is gate-unmet + const d = compare(i, a); + const by = new Map(d.map((x) => [x.surface, x])); + expect(by.get("workerInFlight")).toMatchObject({ old: "in-flight", new: "parked" }); + expect(by.get("runningWorkerReason")).toMatchObject({ old: "not-mid-turn", new: "parked" }); + expect(by.get("hasLiveWakeSource")).toMatchObject({ old: "no-live-source", new: "parked" }); + expect(by.has("idleInActiveStage")).toBe(false); + // The log's inputs hold only ids, states and timestamps. + expect(by.get("workerInFlight")!.inputs).toEqual({ binding: "b1", workers: 1, states: "finished" }); + }); + + // DIVERGENCE (BB-292 #1, the idle half): with only the gate-met own PR open, + // idleInActiveStage calls the binding idle while workerInFlight calls the + // parked worker in flight. The second own PR's absence is the test. + it("records idleInActiveStage when the second own PR is absent (BB-266)", () => { + const i = bb266(false); + const a = assess(i); + expect(a.liveness.idle).toBe(true); + const d = compare(i, a); + expect(d.find((x) => x.surface === "idleInActiveStage")).toMatchObject({ old: "idle", new: "parked" }); + }); + + it("derives the explicit states directly, including none and running", () => { + expect(assess(input()).worker.state).toBe("none"); + expect(assess(input({ workers: [{ ...parkedWorker(), status: "running" }] })).worker.state).toBe("running"); + expect(assess(input({ workers: [{ ...parkedWorker(), status: "failed" }] })).worker.state).toBe("failed"); + expect(assess(input({ workers: [{ ...parkedWorker(), status: "stopped" }] })).worker.state).toBe("stopped"); + }); +}); + +describe("the shadow assessment: the merge gate inputs (BB-292 #2)", () => { + // DIVERGENCE: approvals.ts builds the gate with `prGate(pull, + // productGateOptions(b, pull))`, omitting `decision` and the recorded project + // approvals; the full gate (server.ts, wake.ts, `bb stewards gate`) passes + // both. On a project entry whose recorded `bb-approval` is on the pull + // request the full gate is met and the approvals.ts path still asks the owner. + it("records the approvals.gate divergence: the full gate is met, the approvals.ts gate asks the owner", () => { + const entity = pull({ comments: [ownerMarker, projectApprovalMarker("pa1")] }, { headSha: head, ...green }); + const i = input({ + binding: binding({ kind: "review" }), + observed: { entity, pulls: [] }, + ownPulls: own(entity), + decisions: new Map([[entity.key.toLowerCase(), "project"]]), + projectApprovals: [{ commentId: "pa1", verdict: "approved" }], + // The BB-291 rule: a gate-met fixture carries a parked worker. + workers: [parkedWorker()], + }); + const a = assess(i); + expect(a.gate.pulls[0]).toMatchObject({ met: true, decision: "project", projectApproved: true, lane: "owner-reviewed" }); + const d = compare(i, a); + expect(d.find((x) => x.surface === "approvals.gate")).toMatchObject({ old: "owner approval needed", new: "met" }); + // The inputs are the ids and shas, never a comment body. + expect(d.find((x) => x.surface === "approvals.gate")!.inputs).toEqual({ binding: "b1", pull: "acme/web#7", headSha: head, decision: "project", recordedApprovals: 1 }); + }); + + it("agrees when the recorded project approval is absent from the gate inputs", () => { + const entity = pull({ comments: [ownerMarker] }, { headSha: head, ...green }); + const i = input({ binding: binding({ kind: "review" }), observed: { entity, pulls: [] }, ownPulls: own(entity), decisions: new Map([[entity.key.toLowerCase(), "project"]]) }); + const a = assess(i); + // No recorded approval, so the full gate is unmet too: no divergence. + expect(a.gate.pulls[0].met).toBe(false); + expect(compare(i, a).find((x) => x.surface === "approvals.gate")).toBeUndefined(); + }); +}); + +describe("the shadow assessment: the page refusal text (BB-292 #3)", () => { + // DIVERGENCE: the page action's refusal falls through to its `??` fallback on + // a met no-review gate, so it says "the pull request has not been observed + // yet" while the gate answers met. + it("records the approvals.pageRefusal divergence on a met no-review gate", () => { + const first = gateMetPull("acme/web#7", "https://github.com/acme/web/pull/7"); + const second = secondOpenPull("acme/web#8", "https://github.com/acme/web/pull/8"); + const i = input({ + observed: { entity: issue({ state: "In Progress" }), pulls: [first, second] }, + ownPulls: own(first, second), + gateMetPulls: new Set(["acme/web#7"]), + decisions: new Map([ + ["acme/web#7", "user"], + ["acme/web#8", "user"], + ]), + workers: [parkedWorker()], + }); + const a = assess(i); + expect(a.gate.pulls[0].met).toBe(true); + const d = compare(i, a); + expect(d.find((x) => x.surface === "approvals.pageRefusal")).toMatchObject({ old: "the pull request has not been observed yet", new: "met" }); + }); +}); + +describe("the shadow assessment: the liveness (bb-plugins#294 and noWakeSource)", () => { + it("derives the stall from the rows on the bb-plugins#294 shape, agreeing with the report-time flag", () => { + // #294: a `pr` binding's own open pull request with a free-text wait and no + // other source; the report path marks it noWakeSource, and the rows say the + // same thing. + const entity = pull({ key: "acme/web#7", url: "https://github.com/acme/web/pull/7" }); + const i = input({ + binding: binding({ kind: "pr", status: status("the dompurify fix on main"), noWakeSource: true }), + observed: { entity, pulls: [] }, + ownPulls: own(entity), + sources: { ...noSources, pullRequestWatch: false }, + }); + const a = assess(i); + expect(a.liveness.live).toBe(false); + expect(a.liveness.stalled).toBe(true); + expect(a.liveness.reportedNoWakeSource).toBe(true); + // Agreement, so nothing is logged. + expect(compare(i, a).find((x) => x.surface === "noWakeSource")).toBeUndefined(); + }); + + it("records a noWakeSource divergence when the report-time flag and the rows disagree", () => { + const i = input({ binding: binding({ noWakeSource: true, status: null }) }); + const a = assess(i); + expect(a.liveness.stalled).toBe(false); + expect(compare(i, a).find((x) => x.surface === "noWakeSource")).toMatchObject({ old: "no-wake-source", new: "source-or-no-wait" }); + }); + + it("does not call a dated wait a stall", () => { + const i = input({ binding: binding({ status: status("the vendor's reply"), requeueUntil: 2_000_000, noWakeSource: false }) }); + const a = assess(i); + expect(a.liveness.stalled).toBe(false); + }); + + it("selects the binding's own pull requests, not every observed one", () => { + const mine = pull({ key: "acme/web#7", url: "https://github.com/acme/web/pull/7" }); + const other = pull({ key: "acme/web#9", url: "https://github.com/acme/web/pull/9" }); + const entities = ownPullEntities({ entity: issue(), pulls: [mine, other] }, own(mine)); + expect(entities.map((e) => e.key)).toEqual(["acme/web#7"]); + }); +}); + +describe("the divergence store", () => { + function store() { + const db = new Database(":memory:"); + for (const sql of MIGRATIONS) db.exec(sql); + let now = 1_000; + const s = createStore(db, () => now); + return { s, advance: (ms: number) => (now += ms) }; + } + + it("keeps the latest assessment per binding and dedupes an unchanged divergence", () => { + const { s } = store(); + s.recordAssessment({ bindingId: "b1", at: 1_000, gate: { met: true }, worker: { state: "parked" }, liveness: { live: false } }); + s.recordAssessment({ bindingId: "b1", at: 2_000, gate: { met: false }, worker: { state: "running" }, liveness: { live: true } }); + expect(s.assessment("b1")).toMatchObject({ at: 2_000, gate: { met: false }, worker: { state: "running" } }); + expect(s.assessments()).toHaveLength(1); + s.recordDivergences([{ bindingId: "b1", surface: "workerInFlight", old: "in-flight", new: "parked", inputs: { workers: 1 } }], 1_000); + s.recordDivergences([{ bindingId: "b1", surface: "workerInFlight", old: "in-flight", new: "parked", inputs: { workers: 2 } }], 2_000); + const rows = s.divergences(); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ count: 2, firstSeenAt: 1_000, lastSeenAt: 2_000, inputs: { workers: 2 } }); + // Filters. + expect(s.divergences({ bindingId: "b1" })).toHaveLength(1); + expect(s.divergences({ surface: "workerInFlight" })).toHaveLength(1); + expect(s.divergences({ bindingId: "b1", surface: "workerInFlight" })).toHaveLength(1); + expect(s.divergences({ bindingId: "b2" })).toHaveLength(0); + expect(s.divergences({ surface: "approvals.gate" })).toHaveLength(0); + }); + + it("bounds the log by an age and a row count", () => { + const { s } = store(); + for (let i = 0; i < 5; i++) s.recordDivergences([{ bindingId: "b1", surface: `s${i}`, old: "a", new: "b", inputs: {} }], 1_000 + i); + expect(s.divergences()).toHaveLength(5); + // Neither bound bites: all rows are newer than the age and under the count. + expect(s.pruneDivergences(ASSESSMENT_DIVERGENCE_MAX_AGE_MS, ASSESSMENT_DIVERGENCE_MAX_ROWS, 2_000)).toBe(0); + // The row-count bound keeps the newest three. + expect(s.pruneDivergences(10_000, 3, 2_000)).toBe(2); + expect(s.divergences()).toHaveLength(3); + // The age bound drops the rest once they are old enough. + expect(s.pruneDivergences(1_000, 10, 20_000)).toBe(3); + expect(s.divergences()).toHaveLength(0); + }); + + it("formats the log for the CLI, one block per row", () => { + const rows = [{ bindingId: "b1", surface: "workerInFlight", old: "in-flight", new: "parked", inputs: { workers: 1 }, firstSeenAt: 1_000, lastSeenAt: 2_000, count: 2 }]; + const text = formatDivergences(rows); + expect(text).toContain("b1 workerInFlight x2"); + expect(text).toContain("old: in-flight"); + expect(text).toContain("new: parked"); + expect(text).toContain('inputs: {"workers":1}'); + expect(formatDivergences([])).toBe("No divergences recorded."); + }); +}); diff --git a/plugins/stewards/assessment.ts b/plugins/stewards/assessment.ts new file mode 100644 index 00000000..d408d005 --- /dev/null +++ b/plugins/stewards/assessment.ts @@ -0,0 +1,275 @@ +// BB-293 (child 2 of BB-291): the shadow assessment. One derived answer per +// binding for the merge gate (with the decide-list mark and the recorded +// project approvals the approvals.ts path omits), the worker state (an +// explicit `parked`, distinct from `running`) and the wait liveness (wake +// source, idle and stall derived from what the plugin observes, not the +// report-time `noWakeSource` flag). +// +// It is pure: it reads only the rows a reconcile already has in hand and +// decides nothing. `compare` runs the predicates the assessment will replace +// over the same rows and returns every place the old answer and the assessment +// disagree, so the divergence log can be read for several days before any +// consumer reads the assessment. +import { prGate, type ProjectApprovalRecord } from "./gate.js"; +import { gateState } from "./ready-gate.js"; +import { anyWorkerInFlight, idleInActiveStage, observedStall, productGateOptions, runningWorkerReason, stuckDue, type IdleBinding } from "./reconcile.js"; +import { hasLiveWakeSource, type WakeSourceInput } from "./wake-source.js"; +import type { Playbook } from "./config.js"; +import type { EntitySnapshot } from "./source-contract.js"; +import type { Binding, Escalation, Need, Observed, Worker } from "./store.js"; + +export type DecideMark = "project" | "user"; + +/** How long a divergence row is kept before the reconcile prunes it. */ +export const ASSESSMENT_DIVERGENCE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; +/** The most divergence rows kept, newest first; the reconcile prunes the rest. */ +export const ASSESSMENT_DIVERGENCE_MAX_ROWS = 500; + +/** The merge gate for one own pull request, derived with every input the plugin has. */ +export type AssessedGate = { + /** The pull request's key (owner/repo#N), an id, never free text. */ + pull: string; + met: boolean; + lane: string | null; + missing: string | null; + decision: DecideMark; + ownerApproved: boolean; + projectApproved: boolean; + checksGreen: boolean; + escalated: boolean; + reason: "product" | null; + needsOwnerApproval: boolean; + needsProjectApproval: boolean; +}; + +/** + * The binding's worker state. `parked` is a `finished` worker between turns: + * still in flight, not running, and the state the boolean predicates collapse + * to either "busy" (`workerInFlight`) or "not mid-turn" (`runningWorkerReason`). + */ +export type AssessedWorker = { + state: "running" | "parked" | "failed" | "stopped" | "none"; + inFlight: boolean; + running: boolean; + parked: boolean; + /** `runningWorkerReason`'s text for a mid-turn worker, else null. */ + reason: string | null; +}; + +export type AssessedLiveness = { + /** `hasLiveWakeSource` over the observable rows. */ + live: boolean; + /** `idleInActiveStage`. */ + idle: boolean; + /** `stuckDue`: the phase backstop. */ + stuck: boolean; + /** A free-text wait with no tracked source and no dated wait ahead, recomputed from the rows. */ + stalled: boolean; + /** The report-time flag the assessment replaces. */ + reportedNoWakeSource: boolean; +}; + +export type Assessment = { + bindingId: string; + entityKey: string; + gate: { + pulls: AssessedGate[]; + /** Every own pull's gate is met; true when the binding has none. */ + met: boolean; + decision: DecideMark; + missing: string | null; + /** The ticket ready gate's pass, or null when the playbook has none. */ + readyPassed: boolean | null; + }; + worker: AssessedWorker; + liveness: AssessedLiveness; +}; + +/** + * Everything the assessment reads. All of it is already in hand when the + * reconcile reaches the assessment, so no tick costs an extra GitHub or Linear + * call. + */ +export type AssessmentInput = { + binding: Pick< + Binding, + "id" | "entityKey" | "kind" | "ready" | "stage" | "ending" | "hold" | "deployments" | "requeueUntil" | "noWakeSource" | "status" | "createdAt" | "advancedAt" | "phase" + >; + /** The observation the reconcile just read. */ + observed: Observed; + /** The binding's own pull keys and URLs, lowercased (BB-52 rule 0). */ + ownPulls: ReadonlySet; + /** Own pull keys whose gate is met (BB-285). */ + gateMetPulls: ReadonlySet; + /** The decide-list mark for each own pull, from the files the snapshot already carries. */ + decisions: ReadonlyMap; + /** The plugin's recorded project approvals for the binding. */ + projectApprovals: readonly ProjectApprovalRecord[]; + workers: readonly Worker[]; + escalations: readonly Escalation[]; + needs: readonly Need[]; + /** The wake sources the report path already assembles (BB-154). */ + sources: WakeSourceInput; + playbook: Playbook; + /** The spec hash the ready gate compares its approval against. */ + currentSpecHash: string; + now: number; +}; + +const lc = (s: string | null | undefined): string => (s ?? "").toLowerCase(); + +/** Whether an observed entity is one of the binding's own pull requests (BB-52 rule 0). */ +const isOwn = (e: EntitySnapshot, own: ReadonlySet): boolean => + (e.key !== "" && own.has(lc(e.key))) || (e.url !== null && e.url !== "" && own.has(lc(e.url))); + +/** Every own pull request the binding's workers opened, the binding's own entity first. */ +export function ownPullEntities(observed: Observed, own: ReadonlySet): EntitySnapshot[] { + return [observed.entity, ...observed.pulls].filter((e) => e.pull !== null && isOwn(e, own)); +} + +/** The worker state, with a parked `finished` worker distinct from a running one. */ +export function assessWorker(workers: readonly Worker[]): AssessedWorker { + const running = workers.some((w) => w.status === "running"); + const parked = !running && workers.some((w) => w.status === "finished"); + const failed = !running && !parked && workers.some((w) => w.status === "failed"); + const state: AssessedWorker["state"] = running ? "running" : parked ? "parked" : workers.length === 0 ? "none" : failed ? "failed" : "stopped"; + return { state, inFlight: anyWorkerInFlight(workers), running, parked, reason: runningWorkerReason(workers) }; +} + +/** Derive the one assessment for a binding from the rows the reconcile already read. */ +export function assess(input: AssessmentInput): Assessment { + const { binding, observed, ownPulls, decisions, projectApprovals, workers, escalations, needs, sources, playbook, currentSpecHash, now } = input; + const pulls: AssessedGate[] = []; + for (const e of ownPullEntities(observed, ownPulls)) { + const g = prGate(e, { decision: decisions.get(lc(e.key)) ?? "user", projectApprovals, ...productGateOptions(binding, e) }); + pulls.push({ + pull: e.key, + met: g.gateMet, + lane: g.lane, + missing: g.missing, + decision: g.decision, + ownerApproved: g.ownerApproved, + projectApproved: g.projectApproved, + checksGreen: g.checksGreen, + escalated: g.escalated, + reason: g.reason, + needsOwnerApproval: g.needsOwnerApproval, + needsProjectApproval: g.needsProjectApproval, + }); + } + const idleBinding: IdleBinding = { + stage: binding.stage, + ending: binding.ending, + hold: binding.hold, + deployments: binding.deployments, + requeueUntil: binding.requeueUntil, + workers, + ownPulls, + gateMetPulls: input.gateMetPulls, + escalations, + needs, + }; + const live = hasLiveWakeSource(sources); + return { + bindingId: binding.id, + entityKey: binding.entityKey, + gate: { + pulls, + met: pulls.every((p) => p.met), + decision: pulls[0]?.decision ?? "user", + missing: pulls.find((p) => !p.met)?.missing ?? null, + readyPassed: gateState(playbook, binding.ready, currentSpecHash).passed, + }, + worker: assessWorker(workers), + liveness: { + live, + idle: idleInActiveStage(idleBinding, observed, now), + stuck: stuckDue(binding, playbook, [...escalations], now) !== null, + stalled: observedStall(binding, live, now), + reportedNoWakeSource: binding.noWakeSource, + }, + }; +} + +/** + * One place the old predicate and the shadow assessment disagree. `inputs` + * carries only ids, shas, states and timestamps: never a comment body or any + * free text, so the log cannot leak a transcript. + */ +export type Divergence = { + bindingId: string; + surface: string; + old: string; + new: string; + inputs: Record; +}; + +const verdict = (met: boolean, missing: string | null): string => (met ? "met" : (missing ?? "unmet")); + +/** + * Every place the predicates the assessment will retire answer differently + * from it, over the same rows. The surfaces: + * + * - `workerInFlight`, `runningWorkerReason`, `idleInActiveStage`, + * `hasLiveWakeSource`: a parked `finished` worker is in flight to the first + * and not mid-turn, idle and not a live source to the others (BB-292 #1). + * - `approvals.gate`: `approvals.ts` builds `prGate` without the decide mark + * or the recorded project approvals (BB-292 #2). + * - `approvals.pageRefusal`: the page action's refusal falls through to its + * `??` fallback on a met gate and says "not observed yet" (BB-292 #3). + * - `noWakeSource`: the report-time flag against the stall recomputed from + * the observable rows. + */ +export function compare(input: AssessmentInput, assessment: Assessment): Divergence[] { + const { binding, observed, ownPulls, projectApprovals, workers } = input; + const out: Divergence[] = []; + const add = (surface: string, old: string, next: string, inputs: Divergence["inputs"]): void => { + if (old !== next) out.push({ bindingId: binding.id, surface, old, new: next, inputs }); + }; + const workerInputs: Divergence["inputs"] = { binding: binding.id, workers: workers.length, states: workers.map((w) => w.status).join(",") }; + + // (1) A parked worker: the boolean predicates disagree about it; the + // assessment names the state `parked`. + if (assessment.worker.parked) { + if (anyWorkerInFlight(workers)) add("workerInFlight", "in-flight", "parked", workerInputs); + if (runningWorkerReason(workers) === null) add("runningWorkerReason", "not-mid-turn", "parked", workerInputs); + if (assessment.liveness.idle) add("idleInActiveStage", "idle", "parked", workerInputs); + if (!assessment.liveness.live) add("hasLiveWakeSource", "no-live-source", "parked", workerInputs); + } + + // (2) The gate inputs, and (3) the page's refusal text. + for (const e of ownPullEntities(observed, ownPulls)) { + const assessed = assessment.gate.pulls.find((p) => lc(p.pull) === lc(e.key)); + if (assessed === undefined) continue; + const oldGate = prGate(e, productGateOptions(binding, e)); + const gateInputs: Divergence["inputs"] = { + binding: binding.id, + pull: e.key, + headSha: e.pull?.headSha ?? null, + decision: assessed.decision, + recordedApprovals: projectApprovals.length, + }; + if (oldGate.gateMet !== assessed.met) add("approvals.gate", verdict(oldGate.gateMet, oldGate.missing), verdict(assessed.met, assessed.missing), gateInputs); + // The page acts only while `needsOwnerApproval` is true; on a met gate it + // falls to the `??` fallback, whose words this records. + const pageRefusal = oldGate.needsOwnerApproval ? null : (oldGate.missing ?? "the pull request has not been observed yet"); + if (pageRefusal !== null && assessed.met) add("approvals.pageRefusal", pageRefusal, "met", gateInputs); + } + + // (4) The report-time no-wake-source flag against the stall recomputed from + // the rows. `hasLiveWakeSource`, `idleInActiveStage` and `stuckDue` are the + // assessment's own derivation, so they are compared by construction; only + // the flag and the observed stall can disagree. + add("noWakeSource", binding.noWakeSource ? "no-wake-source" : "source-or-no-wait", assessment.liveness.stalled ? "no-wake-source" : "source-or-no-wait", { + binding: binding.id, + live: assessment.liveness.live, + idle: assessment.liveness.idle, + stuck: assessment.liveness.stuck, + readyPassed: assessment.gate.readyPassed, + requeueUntil: binding.requeueUntil, + phase: binding.phase, + // The row the predicates read, as a boolean, never the wait text. + hasWait: (binding.status?.waitingOn ?? "").trim() !== "", + }); + return out; +} diff --git a/plugins/stewards/migrations.snapshot.json b/plugins/stewards/migrations.snapshot.json index ea94ecc7..335ee690 100644 --- a/plugins/stewards/migrations.snapshot.json +++ b/plugins/stewards/migrations.snapshot.json @@ -118,5 +118,9 @@ "0cb5723b6d354beb4c2e6e60fabb5b5e3ddf0245bc755b3b4bcd3cbce6b6fd56", "a8f6be062abc53e51ea5b4acd27627cf915242aa1b0a60e7e32dc3184ab65b13", "cc5d181530cedb4b86b6a61a9cee4f19d141024e812a2e7b944c7796bb211b47", - "bc78b2041e26800eb88cc1de2043140d376b015e92af6105b18c84c5735b48a5" + "bc78b2041e26800eb88cc1de2043140d376b015e92af6105b18c84c5735b48a5", + "0458af01f85835c917014d4f6548b6f78c2f374e094574ccef6eed32018fd8af", + "b4bd28a48f934b472cd9eb37669b4e5c07fef9ba88b5f16f6eb8576562409bd4", + "3fce9f777b2ae7c333cb2bdb4f49578b842aab9be1ef7e8034750a0b3fa45e83", + "500d7556d3f6cb521e0e2b4dc88776f80daeae8f0dbb61024d766e69e849a55e" ] diff --git a/plugins/stewards/reconcile.ts b/plugins/stewards/reconcile.ts index 197f63c2..4c0fa328 100644 --- a/plugins/stewards/reconcile.ts +++ b/plugins/stewards/reconcile.ts @@ -916,3 +916,17 @@ export function stuckDue(binding: Pick before === null || before.phase !== after.phase || before.summary !== after.summary || before.nextAction !== after.nextAction; + +/** + * BB-293: the stall the shadow assessment derives from what the plugin can + * observe, rather than the `noWakeSource` flag stored at report time. A + * non-empty `waitingOn` with no live tracked source and no dated wait still + * ahead is a stall the rows show on their own; the flag is the report path's + * copy of the same fact. + */ +export function observedStall(binding: Pick, live: boolean, now: number): boolean { + if ((binding.status?.waitingOn ?? "").trim() === "") return false; + if (live) return false; + if (binding.requeueUntil !== null && binding.requeueUntil > now) return false; + return true; +} diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index 3b2769df..2655b1ae 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -15018,3 +15018,52 @@ describe("a gate-met pull request nobody acts on (BB-285)", () => { expect(t.hooks.store.find("ENG-12")!.noWakeSource).toBe(false); }); }); + +// BB-293: the shadow assessment is stored on every reconcile, and its +// divergence log is readable from `bb stewards divergences` (read-only). The +// pure derivations and the fixtures are in assessment.test.ts; this is the +// plugin path that writes and reads them. +describe("bb stewards divergences (BB-293)", () => { + it("stores an assessment on reconcile and prints the divergence log, with filters and --json", async () => { + const t = await setup(); + t.entities.set("uuid-12", issue()); + await t.ring("linear", "uuid-12"); + const b = t.hooks.store.find("ENG-12")!; + // The reconcile stored an assessment for the binding. It is shadow only: + // nothing reads it to decide or render. + expect(t.hooks.store.assessment(b.id)).not.toBeNull(); + expect(t.hooks.store.assessment(b.id)!.gate).toBeTruthy(); + // Seed the log the way the reconcile's compare would. + t.hooks.store.recordDivergences( + [ + { bindingId: b.id, surface: "workerInFlight", old: "in-flight", new: "parked", inputs: { binding: b.id, workers: 1, states: "finished" } }, + { bindingId: b.id, surface: "approvals.gate", old: "owner approval needed", new: "met", inputs: { binding: b.id } }, + { bindingId: "bnd_other", surface: "noWakeSource", old: "no-wake-source", new: "source-or-no-wait", inputs: {} }, + ], + 5_000, + ); + + const all = await t.harness.behavior.runCli(["divergences"]); + expect(all.exitCode).toBe(0); + expect(all.stdout).toContain("workerInFlight"); + expect(all.stdout).toContain("approvals.gate"); + expect(all.stdout).toContain("noWakeSource"); + + // --binding resolves an entity key to the binding and keeps only its rows. + const one = await t.harness.behavior.runCli(["divergences", "--binding", "ENG-12"]); + expect(one.stdout).toContain("workerInFlight"); + expect(one.stdout).toContain("approvals.gate"); + expect(one.stdout).not.toContain("noWakeSource"); + + // --surface keeps only that surface. + const surface = await t.harness.behavior.runCli(["divergences", "--surface", "approvals.gate"]); + expect(surface.stdout).toContain("approvals.gate"); + expect(surface.stdout).not.toContain("workerInFlight"); + + // --json emits the rows. + const json = await t.harness.behavior.runCli(["divergences", "--binding", "ENG-12", "--json"]); + const rows = JSON.parse(json.stdout) as Array<{ surface: string; old: string; new: string; count: number }>; + expect(rows.map((r) => r.surface).sort()).toEqual(["approvals.gate", "workerInFlight"]); + expect(rows.find((r) => r.surface === "workerInFlight")).toMatchObject({ old: "in-flight", new: "parked", count: 1 }); + }); +}); diff --git a/plugins/stewards/server.ts b/plugins/stewards/server.ts index d165a7b7..efed3302 100644 --- a/plugins/stewards/server.ts +++ b/plugins/stewards/server.ts @@ -86,7 +86,8 @@ import { type Role as StewardRole, type RolesFile, } from "./roles.js"; -import { explainDecision, formatDecisionListing, globMatches, type BigDecision } from "./decisions.js"; +import { decideForPull, explainDecision, formatDecisionListing, globMatches, type BigDecision } from "./decisions.js"; +import { ASSESSMENT_DIVERGENCE_MAX_AGE_MS, ASSESSMENT_DIVERGENCE_MAX_ROWS, assess, compare, ownPullEntities, type AssessmentInput } from "./assessment.js"; import { ADOPTION_CHOICES, adoptionChoices, adoptionHow, adoptionQuestion, alertClockAt, alertFingerprint, batchAlert, deadlineAt, depsLookSchema, dueDate, isBatchable, overdue, priorityFor, priorityLabel, priorityLabelForSeverity, reminderText, repoOfFingerprint, sightingOf, snoozeUntil, type DepsFinding, type DepsLook, type DepsTicket } from "./deps.js"; import { hostContract } from "./host-contract.js"; import { MULTI_REPO_ENVIRONMENT_PROVIDER_ID, MULTI_REPO_PLUGIN_ID, multiRepoEnvironmentFlags, multiRepoFreshEnvironment, multiRepoSource, nativeSource, projectSourceLabel, sourceRpcMissing, type FreshEnvironment, type NativeGitRead, type ProjectSource } from "./project-source.js"; @@ -119,7 +120,7 @@ import { } from "./source-contract.js"; import { DEPLOY_STATES, allDeployed, applyDeployment, applyDeploymentRead, applyWorkflowRun, awaitingOnRepo, deploymentLines, deploymentReportSchema, isFinalDeploymentState, matchesTarget, matchesWorkflow, mergedPulls, pickWatch, recordMerged, reportDeployment, targetKeyOf, timeOut, timeoutsDue } from "./deploy.js"; import { evaluateCarries, matchCarriesRule, noRuleNoted, type CarriesReader } from "./carries.js"; -import { ACTORS, MIGRATIONS, ESCALATED_NEED_KINDS, NEEDS_KINDS, RESOLUTION_KINDS, createStore, needChoices, type Assumption, type Binding, type DepsDecision, type Escalation, type Fingerprint, type Lesson, type Need, type Observed, type Question, type ReadyState, type Store, type WakeRetry, type Worker } from "./store.js"; +import { ACTORS, MIGRATIONS, ESCALATED_NEED_KINDS, NEEDS_KINDS, RESOLUTION_KINDS, createStore, needChoices, type Assumption, type Binding, type DepsDecision, type DivergenceRow, type Escalation, type Fingerprint, type Lesson, type Need, type Observed, type Question, type ReadyState, type Store, type WakeRetry, type Worker } from "./store.js"; import { hasLessonAnchor, landLessonInstruction, lessonAnchor, lessonLandingMark, planLessonTransitions, planValidations, retiredLessonAnchor, withLessonAnchor } from "./lessons.js"; import { AFTER_THE_FACT_RATIONALE_WHY, REPEAT_RATIONALE_WHY, ROUTINE_RATIONALE_WHY, clusterKey, routineRationaleWanted } from "./rationale.js"; import { GUIDANCE_UNIT_PREFIX, GUIDANCE_UNCITED_DAYS, escalationWeeks, parseGuidanceUnits, uncitedGuidanceUnits, type GuidanceUnit } from "./guidance.js"; @@ -335,6 +336,18 @@ const checkoutDirName = (path: string): string => path.replace(/[\\/]+$/u, "").s */ export const lessonBranchFor = (threadId: string): string => `refs/heads/guidance-${threadId.replace(/[^A-Za-z0-9._-]+/gu, "-").slice(0, 60)}`; +/** + * BB-293: the human-readable shadow-assessment divergence log, one block per + * distinct row (binding, surface, old, new, inputs, count, first/last seen). + */ +export function formatDivergences(rows: ReadonlyArray>): string { + if (rows.length === 0) return "No divergences recorded."; + const at = (ms: number): string => new Date(ms).toISOString().replace("T", " ").slice(0, 19); + return rows + .map((d) => `${d.bindingId} ${d.surface} x${d.count} ${at(d.firstSeenAt)} -> ${at(d.lastSeenAt)}\n old: ${d.old}\n new: ${d.new}\n inputs: ${JSON.stringify(d.inputs)}`) + .join("\n"); +} + /** Whitespace-normalised text, so a lesson matches however the playbook wraps it. */ const normalizeLessonText = (text: string) => text.replace(/\s+/gu, " ").trim(); @@ -2411,8 +2424,8 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } /** The structured sources that will wake a binding, as the report path reads them (BB-154). */ - function liveWakeSourceFor(b: Binding, ownPulls: ReadonlySet): boolean { - const sources: WakeSourceInput = { + function wakeSourcesFor(b: Binding, ownPulls: ReadonlySet): WakeSourceInput { + return { hold: b.hold, holdBehind: b.holdBehind, mergeBehind: b.mergeBehind, @@ -2423,7 +2436,10 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P openNeeds: store.openNeeds(b.id).length, pullRequestWatch: pullWatch(b, ownPulls), }; - return hasLiveWakeSource(sources); + } + + function liveWakeSourceFor(b: Binding, ownPulls: ReadonlySet): boolean { + return hasLiveWakeSource(wakeSourcesFor(b, ownPulls)); } /** @@ -3741,6 +3757,47 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P } } + /** + * BB-293: the shadow assessment. Computed from the rows this reconcile has + * already read, stored beside the predicates it will replace, and compared + * with them. It decides nothing; the caller wraps the call in a try/catch, + * so a failure here can never change the reconcile's own result. + */ + function recordShadowAssessment( + b: Binding, + observed: Observed, + workers: readonly Worker[], + ownPulls: ReadonlySet, + gateMetPulls: ReadonlySet, + playbook: Playbook, + currentSpecHash: string, + config: ProjectConfig, + ): void { + // The decide-list mark comes from the files the snapshot already carries; + // no extra GitHub read. Everything else is a row already in hand. + const decisions = new Map(); + for (const e of ownPullEntities(observed, ownPulls)) decisions.set(lc(e.key), decideForPull(e.pull?.files ?? null, config.file.bigDecisions)); + const input: AssessmentInput = { + binding: b, + observed, + ownPulls, + gateMetPulls, + decisions, + projectApprovals: projectApprovalsFor(b), + workers, + escalations: store.openEscalations(b.id), + needs: store.openNeeds(b.id), + sources: wakeSourcesFor(b, ownPulls), + playbook, + currentSpecHash, + now: now(), + }; + const assessment = assess(input); + store.recordAssessment({ bindingId: b.id, at: input.now, gate: assessment.gate, worker: assessment.worker, liveness: assessment.liveness }); + store.recordDivergences(compare(input, assessment)); + store.pruneDivergences(ASSESSMENT_DIVERGENCE_MAX_AGE_MS, ASSESSMENT_DIVERGENCE_MAX_ROWS, input.now); + } + /** * One reconcile of one binding: snapshot, lifecycle, compare, and wake * when something actionable changed (or `force` says this is a turn @@ -4009,6 +4066,14 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P if (!own) continue; if (prGate(e, { projectApprovals: projectApprovalsFor(b), ...productGateOptions(b, e) }).gateMet) gateMetPulls.add(e.key.toLowerCase()); } + // BB-293: the shadow assessment, computed from the rows above. It decides + // nothing; a failure is logged and swallowed so the reconcile's own + // result never changes. + try { + recordShadowAssessment(b, observed, workers, ownPulls, gateMetPulls, playbook, currentSpec, config); + } catch (error) { + bb.log.warn(`stewards: shadow assessment failed for ${b.entityKey}: ${errorMessage(error)}`); + } // BB-203: an open need or `user` escalation keeps the worker's own pull // request activity loud, so the steward can reconcile the user's item. const userWaiting = store.openNeeds(b.id).length > 0 || store.openEscalations(b.id).some((x) => x.status === "open" && x.route === "user"); @@ -11658,6 +11723,24 @@ export function createStewardsPlugin(deps: StewardsDeps): (bb: BbPluginApi) => P }, }), + divergences: cliCommand({ + summary: "The shadow assessment's divergence log (BB-293), read-only", + description: "Every place the shadow assessment disagrees with the predicate it will replace, one row per distinct (binding, surface, old, new) with a first/last seen time and a count. --binding filters by binding id or entity key, --surface by surface name. --json emits the rows. Nothing reads the log to decide or render.", + options: { + binding: { type: "string", description: "Only this binding's divergences (binding id or entity key)" }, + surface: { type: "string", description: "Only this surface (workerInFlight, runningWorkerReason, idleInActiveStage, hasLiveWakeSource, approvals.gate, approvals.pageRefusal, noWakeSource)" }, + project: PROJECT_OPTION, + json: JSON_OPTION, + }, + async run(input, ctx) { + const projectId = input.options.project ?? ctx.projectId; + const raw = input.options.binding; + const bindingId = raw === undefined ? undefined : (store.find(raw, projectId)?.id ?? raw); + const rows = store.divergences({ bindingId, surface: input.options.surface }); + return { exitCode: 0, stdout: input.options.json === true ? JSON.stringify(rows) : formatDivergences(rows) }; + }, + }), + merge: cliCommand({ summary: "Merge a pull request whose lane gate is met, through the same path as the Stewards page", description: "Re-reads the pull request, re-checks the gate with the project's approvals, and refuses with the gate's reason when it is unmet or a worker's turn is in progress. Otherwise squash-merges, posts the \"Merged:\" comment, and closes the binding's Merge item. Exit 1 on a refusal.", diff --git a/plugins/stewards/store.test.ts b/plugins/stewards/store.test.ts index 60bdfa4b..276e11ff 100644 --- a/plugins/stewards/store.test.ts +++ b/plugins/stewards/store.test.ts @@ -1,7 +1,7 @@ import Database from "better-sqlite3"; import { describe, expect, it } from "vitest"; import { z } from "zod"; -import { ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS, ASSUMPTION_MIGRATIONS, BASE_MIGRATIONS, CHAIN_MIGRATIONS, DATED_WAIT_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, GUIDANCE_MIGRATIONS, IDLE_MIGRATIONS, LESSON_COUNTER_MIGRATIONS, LESSON_PR_MIGRATIONS, LESSON_TIER_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, MIGRATIONS, PULL_WATCH_MIGRATIONS, RATIONALE_MIGRATIONS, RETARGET_PULLS_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, STACK_MIGRATIONS, STACKED_PULLS_MIGRATIONS, VALIDATION_MIGRATIONS, WAKE_RETRY_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, WORKER_CHECK_MIGRATIONS, WORKER_STOP_REASON_MIGRATIONS, createStore, lessonScope, statusRecordSchema, type Binding } from "./store.js"; +import { ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS, ASSESSMENT_MIGRATIONS, ASSUMPTION_MIGRATIONS, BASE_MIGRATIONS, CHAIN_MIGRATIONS, DATED_WAIT_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, GUIDANCE_MIGRATIONS, IDLE_MIGRATIONS, LESSON_COUNTER_MIGRATIONS, LESSON_PR_MIGRATIONS, LESSON_TIER_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, MIGRATIONS, PULL_WATCH_MIGRATIONS, RATIONALE_MIGRATIONS, RETARGET_PULLS_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, STACK_MIGRATIONS, STACKED_PULLS_MIGRATIONS, VALIDATION_MIGRATIONS, WAKE_RETRY_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, WORKER_CHECK_MIGRATIONS, WORKER_STOP_REASON_MIGRATIONS, createStore, lessonScope, statusRecordSchema, type Binding } from "./store.js"; /** MIGRATIONS without the groups appended after the ones these order tests assert. */ const beforeActorMigrations = MIGRATIONS.slice(0, MIGRATIONS.indexOf(ACTOR_MIGRATIONS[0]!)); @@ -486,9 +486,10 @@ describe("the actor rename migration (BB-97)", () => { // and BB-215's stack group follows it (migrations.test.ts pins the whole // list, this only checks the group order). // BB-256 appended the merge-hold group after BB-255's wake-retry group, - // BB-264 appended the retarget-pulls column after that, and BB-287 - // appended the stacked-pull columns after that. - const groups = [STACKED_PULLS_MIGRATIONS, RETARGET_PULLS_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, WAKE_RETRY_MIGRATIONS, DATED_WAIT_MIGRATIONS, ASSUMPTION_MIGRATIONS, WORKER_STOP_REASON_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, STACK_MIGRATIONS, PULL_WATCH_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, LESSON_PR_MIGRATIONS, ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS]; + // BB-264 appended the retarget-pulls column after that, BB-287 appended the + // stacked-pull columns after that, and BB-293 appended the shadow + // assessment and divergence-log tables after that. + const groups = [ASSESSMENT_MIGRATIONS, STACKED_PULLS_MIGRATIONS, RETARGET_PULLS_MIGRATIONS, MERGE_BEHIND_MIGRATIONS, WAKE_RETRY_MIGRATIONS, DATED_WAIT_MIGRATIONS, ASSUMPTION_MIGRATIONS, WORKER_STOP_REASON_MIGRATIONS, DEPS_DECISION_MIGRATIONS, ESCALATION_ROUTE_MIGRATIONS, STACK_MIGRATIONS, PULL_WATCH_MIGRATIONS, SERVER_SETTINGS_MIGRATIONS, WAKE_SOURCE_MIGRATIONS, REVIEWER_MODEL_MIGRATIONS, LESSON_PR_MIGRATIONS, ACTOR_MIGRATIONS, APPROVAL_MARKER_MIGRATIONS]; let end = MIGRATIONS.length; for (const group of groups) { expect(MIGRATIONS.slice(end - group.length, end)).toEqual([...group]); @@ -577,10 +578,12 @@ describe("the escalation route rename migration (BB-209)", () => { // worker-stop-reason column after that, BB-251's assumptions table after // that, BB-247's dated-wait columns after that, BB-255's wake-retry // columns after that, BB-256's merge-hold column after that, BB-264's - // retarget-pulls column after that, and BB-287's stacked-pull columns - // after that. - const withStacked = MIGRATIONS.slice(0, -STACKED_PULLS_MIGRATIONS.length); - expect(MIGRATIONS.slice(-STACKED_PULLS_MIGRATIONS.length)).toEqual([...STACKED_PULLS_MIGRATIONS]); + // retarget-pulls column after that, BB-287's stacked-pull columns after + // that, and BB-293's shadow-assessment tables after that. + const withAssessment = MIGRATIONS.slice(0, -ASSESSMENT_MIGRATIONS.length); + expect(MIGRATIONS.slice(-ASSESSMENT_MIGRATIONS.length)).toEqual([...ASSESSMENT_MIGRATIONS]); + const withStacked = withAssessment.slice(0, -STACKED_PULLS_MIGRATIONS.length); + expect(withAssessment.slice(-STACKED_PULLS_MIGRATIONS.length)).toEqual([...STACKED_PULLS_MIGRATIONS]); const withRetarget = withStacked.slice(0, -RETARGET_PULLS_MIGRATIONS.length); expect(withStacked.slice(-RETARGET_PULLS_MIGRATIONS.length)).toEqual([...RETARGET_PULLS_MIGRATIONS]); const withMergeBehind = withRetarget.slice(0, -MERGE_BEHIND_MIGRATIONS.length); diff --git a/plugins/stewards/store.ts b/plugins/stewards/store.ts index 754740ec..5e4604de 100644 --- a/plugins/stewards/store.ts +++ b/plugins/stewards/store.ts @@ -633,7 +633,38 @@ export const RETARGET_PULLS_MIGRATIONS = [`ALTER TABLE bindings ADD COLUMN retar // per pull request. Additive and defaulted: an old row reads as neither. export const STACKED_PULLS_MIGRATIONS = [`ALTER TABLE bindings ADD COLUMN stacked_pulls TEXT NOT NULL DEFAULT '[]'`, `ALTER TABLE bindings ADD COLUMN stacked_recovered TEXT NOT NULL DEFAULT '[]'`]; -export const MIGRATIONS = [...BASE_MIGRATIONS, ...CHAIN_MIGRATIONS, ...LESSON_COUNTER_MIGRATIONS, ...WORKER_CHECK_MIGRATIONS, ...DECLINED_PR_MIGRATIONS, ...DEPLOYMENT_MIGRATIONS, ...FINISHED_AND_DIGEST_MIGRATIONS, ...LESSON_TIER_MIGRATIONS, ...RATIONALE_MIGRATIONS, ...VALIDATION_MIGRATIONS, ...GUIDANCE_MIGRATIONS, ...IDLE_MIGRATIONS, ...APPROVAL_MARKER_MIGRATIONS, ...ACTOR_MIGRATIONS, ...LESSON_PR_MIGRATIONS, ...REVIEWER_MODEL_MIGRATIONS, ...WAKE_SOURCE_MIGRATIONS, ...SERVER_SETTINGS_MIGRATIONS, ...PULL_WATCH_MIGRATIONS, ...STACK_MIGRATIONS, ...ESCALATION_ROUTE_MIGRATIONS, ...DEPS_DECISION_MIGRATIONS, ...WORKER_STOP_REASON_MIGRATIONS, ...ASSUMPTION_MIGRATIONS, ...DATED_WAIT_MIGRATIONS, ...WAKE_RETRY_MIGRATIONS, ...MERGE_BEHIND_MIGRATIONS, ...RETARGET_PULLS_MIGRATIONS, ...STACKED_PULLS_MIGRATIONS]; +// BB-293 (child 2 of BB-291): the shadow assessment and its divergence log. +// `assessments` keeps the latest assessment per binding; `assessment_divergences` +// keeps one row per distinct (binding, surface, old, new), with a first and +// last seen time and a count, so an unchanged divergence is not re-logged +// every tick. Both are new tables, so nothing existing is altered and a store +// from before this step behaves exactly as before. `divergences` is bounded by +// the reconcile's prune (a row count and an age), never by the writer. +export const ASSESSMENT_MIGRATIONS = [ + `CREATE TABLE IF NOT EXISTS assessments ( + binding_id TEXT PRIMARY KEY, + at INTEGER NOT NULL, + gate TEXT NOT NULL, + worker TEXT NOT NULL, + liveness TEXT NOT NULL + )`, + `CREATE TABLE IF NOT EXISTS assessment_divergences ( + id TEXT PRIMARY KEY, + binding_id TEXT NOT NULL, + surface TEXT NOT NULL, + old TEXT NOT NULL, + new TEXT NOT NULL, + inputs TEXT NOT NULL DEFAULT '{}', + first_seen_at INTEGER NOT NULL, + last_seen_at INTEGER NOT NULL, + count INTEGER NOT NULL DEFAULT 1, + UNIQUE (binding_id, surface, old, new) + )`, + `CREATE INDEX IF NOT EXISTS assessment_divergences_binding ON assessment_divergences (binding_id, surface)`, + `CREATE INDEX IF NOT EXISTS assessment_divergences_seen ON assessment_divergences (last_seen_at)`, +]; + +export const MIGRATIONS = [...BASE_MIGRATIONS, ...CHAIN_MIGRATIONS, ...LESSON_COUNTER_MIGRATIONS, ...WORKER_CHECK_MIGRATIONS, ...DECLINED_PR_MIGRATIONS, ...DEPLOYMENT_MIGRATIONS, ...FINISHED_AND_DIGEST_MIGRATIONS, ...LESSON_TIER_MIGRATIONS, ...RATIONALE_MIGRATIONS, ...VALIDATION_MIGRATIONS, ...GUIDANCE_MIGRATIONS, ...IDLE_MIGRATIONS, ...APPROVAL_MARKER_MIGRATIONS, ...ACTOR_MIGRATIONS, ...LESSON_PR_MIGRATIONS, ...REVIEWER_MODEL_MIGRATIONS, ...WAKE_SOURCE_MIGRATIONS, ...SERVER_SETTINGS_MIGRATIONS, ...PULL_WATCH_MIGRATIONS, ...STACK_MIGRATIONS, ...ESCALATION_ROUTE_MIGRATIONS, ...DEPS_DECISION_MIGRATIONS, ...WORKER_STOP_REASON_MIGRATIONS, ...ASSUMPTION_MIGRATIONS, ...DATED_WAIT_MIGRATIONS, ...WAKE_RETRY_MIGRATIONS, ...MERGE_BEHIND_MIGRATIONS, ...RETARGET_PULLS_MIGRATIONS, ...STACKED_PULLS_MIGRATIONS, ...ASSESSMENT_MIGRATIONS]; // ---- values kept as JSON ------------------------------------------------------------ @@ -1335,6 +1366,26 @@ export type WorkerStatus = "running" | "finished" | "failed" | "stopped"; export type WorkerStopReason = "archived" | "stopped" | "runaway" | "deleted" | "misspawn"; export type Worker = { threadId: string; bindingId: string; title: string; status: WorkerStatus; result: string | null; startedAt: number; endedAt: number | null; modelCheckPending: boolean; stopReason: WorkerStopReason | null }; +/** + * BB-293: the latest shadow assessment stored for a binding. The three answers + * are kept as JSON so the shape can grow without another migration; nothing + * reads them to decide or render. + */ +export type AssessmentRow = { bindingId: string; at: number; gate: unknown; worker: unknown; liveness: unknown }; + +/** One place the shadow assessment and the predicate it will replace disagree. */ +export type DivergenceRow = { + id: string; + bindingId: string; + surface: string; + old: string; + new: string; + inputs: Record; + firstSeenAt: number; + lastSeenAt: number; + count: number; +}; + /** The requester thread the plugin spawned for an open provide-secret need (BB-22). */ export type SecretRequester = { threadId: string; needId: string; createdAt: number }; @@ -1513,6 +1564,25 @@ export function createStore(db: Database.Database, now: () => number, options: S // opened without the migration must still start (migrations.test.ts). const setTicket = lazy<[string, number, number, string | null, string, string], unknown>("UPDATE ops_fingerprints SET ticket_key = ?, filed_at = ?, last_commented_at = ?, filed_by_thread = ? WHERE project_id = ? AND fingerprint = ?"); + // BB-293: the shadow assessment and its divergence log, prepared lazily for + // the same reason: a store from before this group has no tables yet. + const upsertAssessmentStmt = lazy<[string, number, string, string, string], unknown>( + "INSERT INTO assessments (binding_id, at, gate, worker, liveness) VALUES (?, ?, ?, ?, ?) ON CONFLICT (binding_id) DO UPDATE SET at = excluded.at, gate = excluded.gate, worker = excluded.worker, liveness = excluded.liveness", + ); + const getAssessmentStmt = lazy<[string], Record>("SELECT * FROM assessments WHERE binding_id = ?"); + const allAssessmentsStmt = lazy<[], Record>("SELECT * FROM assessments ORDER BY at DESC"); + const insertDivergenceStmt = lazy<[string, string, string, string, string, string, number, number], unknown>( + "INSERT INTO assessment_divergences (id, binding_id, surface, old, new, inputs, first_seen_at, last_seen_at, count) VALUES (?, ?, ?, ?, ?, ?, ?, ?, 1) ON CONFLICT (binding_id, surface, old, new) DO UPDATE SET last_seen_at = excluded.last_seen_at, inputs = excluded.inputs, count = count + 1", + ); + const divergencesAllStmt = lazy<[], Record>("SELECT * FROM assessment_divergences ORDER BY last_seen_at DESC, surface"); + const divergencesByBindingStmt = lazy<[string], Record>("SELECT * FROM assessment_divergences WHERE binding_id = ? ORDER BY last_seen_at DESC, surface"); + const divergencesBySurfaceStmt = lazy<[string], Record>("SELECT * FROM assessment_divergences WHERE surface = ? ORDER BY last_seen_at DESC, binding_id"); + const divergencesByBothStmt = lazy<[string, string], Record>("SELECT * FROM assessment_divergences WHERE binding_id = ? AND surface = ? ORDER BY last_seen_at DESC"); + const pruneDivergencesByAgeStmt = lazy<[number], unknown>("DELETE FROM assessment_divergences WHERE last_seen_at < ?"); + const pruneDivergencesByCountStmt = lazy<[number], unknown>( + "DELETE FROM assessment_divergences WHERE id NOT IN (SELECT id FROM assessment_divergences ORDER BY last_seen_at DESC, id LIMIT ?)", + ); + // BB-251: assumptions, prepared lazily for the same reason: a store from // before this group has no `assumptions` table yet. const insertAssumptionStmt = lazy<[Record], unknown>( @@ -1712,6 +1782,34 @@ export function createStore(db: Database.Database, now: () => number, options: S needId: String(r.need_id), createdAt: Number(r.created_at), }); + // BB-293: the stored shadow assessment and its divergence rows. The three + // answers are JSON so the shape can grow without another migration. + const parseJson = (raw: unknown): unknown => { + if (raw === null || raw === undefined) return null; + try { + return JSON.parse(String(raw)); + } catch { + return null; + } + }; + const toAssessmentRow = (r: Record): AssessmentRow => ({ + bindingId: String(r.binding_id), + at: Number(r.at), + gate: parseJson(r.gate), + worker: parseJson(r.worker), + liveness: parseJson(r.liveness), + }); + const toDivergenceRow = (r: Record): DivergenceRow => ({ + id: String(r.id), + bindingId: String(r.binding_id), + surface: String(r.surface), + old: String(r.old), + new: String(r.new), + inputs: json(z.record(z.string(), z.unknown()), r.inputs === null || r.inputs === undefined ? null : String(r.inputs), {}), + firstSeenAt: Number(r.first_seen_at), + lastSeenAt: Number(r.last_seen_at), + count: Number(r.count), + }); const toFingerprint = (r: Record): Fingerprint => ({ projectId: String(r.project_id), fingerprint: String(r.fingerprint), @@ -2845,6 +2943,53 @@ export function createStore(db: Database.Database, now: () => number, options: S setServerSetting(key: string, value: string): void { db.prepare("INSERT INTO server_settings (key, value) VALUES (?, ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value").run(key, value); }, + + // BB-293: the shadow assessment and its divergence log. The reconcile + // writes them; the `bb stewards divergences` command reads them. + /** Store the latest shadow assessment for a binding, replacing any earlier one. */ + recordAssessment(fields: { bindingId: string; at: number; gate: unknown; worker: unknown; liveness: unknown }): void { + upsertAssessmentStmt().run(fields.bindingId, fields.at, JSON.stringify(fields.gate), JSON.stringify(fields.worker), JSON.stringify(fields.liveness)); + }, + /** The latest shadow assessment for a binding, or null. */ + assessment(bindingId: string): AssessmentRow | null { + const r = getAssessmentStmt().get(bindingId); + return r === undefined ? null : toAssessmentRow(r); + }, + /** Every binding's latest shadow assessment, newest first. */ + assessments(): AssessmentRow[] { + return allAssessmentsStmt().all().map(toAssessmentRow); + }, + /** + * Record the divergences a reconcile found. One row per distinct + * (binding, surface, old, new): an unchanged divergence bumps its count and + * last-seen time instead of adding a row, so a tick does not re-log it. + */ + recordDivergences(divergences: ReadonlyArray<{ bindingId: string; surface: string; old: string; new: string; inputs: Record }>, at = now()): number { + for (const d of divergences) insertDivergenceStmt().run(newId("dvg"), d.bindingId, d.surface, d.old, d.new, JSON.stringify(d.inputs), at, at); + return divergences.length; + }, + /** The divergence log, newest first, filtered by binding and/or surface. */ + divergences(opts: { bindingId?: string; surface?: string } = {}): DivergenceRow[] { + const rows = + opts.bindingId !== undefined && opts.surface !== undefined + ? divergencesByBothStmt().all(opts.bindingId, opts.surface) + : opts.bindingId !== undefined + ? divergencesByBindingStmt().all(opts.bindingId) + : opts.surface !== undefined + ? divergencesBySurfaceStmt().all(opts.surface) + : divergencesAllStmt().all(); + return rows.map(toDivergenceRow); + }, + /** + * Bound the divergence log: drop rows last seen before `maxAgeMs` and keep + * at most `maxRows` (newest first). Called by the reconcile, never by a + * reader; returns the number of rows removed. + */ + pruneDivergences(maxAgeMs: number, maxRows: number, at = now()): number { + let removed = pruneDivergencesByAgeStmt().run(at - maxAgeMs).changes; + removed += pruneDivergencesByCountStmt().run(maxRows).changes; + return removed; + }, /** * Forget a deleted project: its bindings and everything keyed by them * (escalations, workers, needs, approvals, fingerprints, queued answers), @@ -2858,7 +3003,7 @@ export function createStore(db: Database.Database, now: () => number, options: S const ids = db.prepare<[string], { id: string }>("SELECT id FROM bindings WHERE project_id = ?").all(projectId).map((r) => r.id); const byBinding = "SELECT id FROM bindings WHERE project_id = ?"; db.prepare(`DELETE FROM secret_requesters WHERE thread_id IN (SELECT thread_id FROM workers WHERE binding_id IN (${byBinding}))`).run(projectId); - for (const table of ["escalations", "workers", "needs", "project_approvals"]) db.prepare(`DELETE FROM ${table} WHERE binding_id IN (${byBinding})`).run(projectId); + for (const table of ["escalations", "workers", "needs", "project_approvals", "assessments", "assessment_divergences"]) db.prepare(`DELETE FROM ${table} WHERE binding_id IN (${byBinding})`).run(projectId); db.prepare(`DELETE FROM pending_deliveries WHERE origin_binding_id IN (${byBinding})`).run(projectId); for (const table of ["ops_fingerprints", "deps_decisions", "lessons", "guidance_citations", "assumptions", "bindings", "project_stewards"]) db.prepare(`DELETE FROM ${table} WHERE project_id = ?`).run(projectId); options.onProjectChange?.(projectId, ["purged"]); From bed18b819810bf5b78267414895f10b01938a66a Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Tue, 10 Nov 2026 15:04:43 +0000 Subject: [PATCH 2/2] stewards: test the shadow assessment's swallowed failure (BB-293) --- plugins/stewards/server.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/plugins/stewards/server.test.ts b/plugins/stewards/server.test.ts index 2655b1ae..82930b57 100644 --- a/plugins/stewards/server.test.ts +++ b/plugins/stewards/server.test.ts @@ -15066,4 +15066,18 @@ describe("bb stewards divergences (BB-293)", () => { expect(rows.map((r) => r.surface).sort()).toEqual(["approvals.gate", "workerInFlight"]); expect(rows.find((r) => r.surface === "workerInFlight")).toMatchObject({ old: "in-flight", new: "parked", count: 1 }); }); + + it("swallows a failure inside the shadow assessment and keeps reconciling", async () => { + const t = await setup(); + t.entities.set("uuid-12", issue()); + // Force the assessment write to fail: the lazy prepared statement throws + // "no such table: assessments", which the reconcile must swallow and log. + t.hooks.db.prepare("DROP TABLE assessments").run(); + await t.ring("linear", "uuid-12"); + await t.flush(); + // The reconcile completed past the failure: the binding's snapshot was + // stored and the warning names the shadow assessment. + expect(t.hooks.store.find("ENG-12")!.lastSnapshot).not.toBeNull(); + expect(t.harness.logEntries.some((e) => e.level === "warn" && e.message.includes("shadow assessment failed"))).toBe(true); + }); });