Skip to content

Commit dc1136c

Browse files
author
JSONbored
committed
feat(orb): capture the decision-time wall clock so staleness rules are replayable (#9028)
gate.requireFreshRebaseWindow compared the base branch's tip against an inline Date.now() read inside maybeForceFreshRebase. Time is a decision INPUT, and nothing recorded which instant the comparison used — so re-deriving such a decision later could silently reach the opposite answer purely because the wall clock had moved, and report it as a match. The decision pass now takes ONE Date.now() reading, records it into decision_replay_inputs.replay_json as `clock`, and passes it to every clock-dependent rule instead of each calling the clock itself. Both staleness rules move to a pure, clock-injected module: isWithinFreshRebaseWindow takes the instant explicitly, and isBaseStaleByAheadBy is stated as the commit-count comparison it is — provably instant-independent, not merely assumed so. replayDecision gains a stage-0 `clock` check: replaying at the recorded instant (the CLI default) is bit-exact, while naming a different instant reports a `clock` divergence rather than silently certifying a re-derivation that never reproduced the original evaluation. Records written before this change carry no instant, so the stage is skipped rather than guessed. The CLI exposes it as `--at <epoch ms>`. Also corrects the decision_replay_inputs migration reference in two doc comments (0181 is alert_dedup_claims; the table is created in 0182).
1 parent b939913 commit dc1136c

5 files changed

Lines changed: 234 additions & 13 deletions

File tree

scripts/replay-decision.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
//
55
// node --experimental-strip-types scripts/replay-decision.ts <bundle.json>
66
// ... | node --experimental-strip-types scripts/replay-decision.ts -
7+
// node --experimental-strip-types scripts/replay-decision.ts <bundle.json> --at <epoch ms>
8+
//
9+
// #9028: `--at` names the instant to replay AT. Omit it to replay at the instant the decision itself recorded
10+
// (`replayInput.clock.nowMs`) — the bit-exact case. Passing a DIFFERENT instant exits 1 with a `clock`
11+
// divergence rather than reporting a match: time is a decision INPUT, and a clock-dependent rule
12+
// (`gate.requireFreshRebaseWindow`) can flip purely because the wall clock moved.
713
//
814
// The bundle is one JSON object: { record: {...decision_records row}, replayInput: {...replay_json} }.
915
// EXTRACT (operator, against the instance DB):
@@ -24,8 +30,13 @@
2430
import { readFileSync } from "node:fs";
2531
import { replayDecision, type DecisionReplayInput, type ReplayableRecord } from "../src/review/decision-replay";
2632

27-
/** Parse + normalize a bundle (snake_case SQL rows accepted) and replay it. Exported for tests. */
28-
export function runReplayBundle(raw: string): { outcome: ReturnType<typeof replayDecision> | null; error?: string } {
33+
/** Parse + normalize a bundle (snake_case SQL rows accepted) and replay it. Exported for tests.
34+
*
35+
* #9028: `atMs` names the instant to replay AT. Omitted (the default) replays at the instant the decision
36+
* recorded, which is the bit-exact case. Supplying a DIFFERENT instant is reported as a `clock` divergence,
37+
* never silently accepted — a clock-dependent rule can legitimately flip its answer as the wall clock moves,
38+
* so "it still matches at a different instant" is not a re-derivation of the original decision. */
39+
export function runReplayBundle(raw: string, atMs?: number): { outcome: ReturnType<typeof replayDecision> | null; error?: string } {
2940
let bundle: { record?: Record<string, unknown>; replayInput?: unknown };
3041
try {
3142
bundle = JSON.parse(raw) as never;
@@ -47,18 +58,25 @@ export function runReplayBundle(raw: string): { outcome: ReturnType<typeof repla
4758
if (!record || !replayInput || !Array.isArray(replayInput.findings) || typeof replayInput.evaluated !== "object") {
4859
return { outcome: null, error: "bundle must carry {record: {id, reason_code|reasonCode, action}, replayInput: {findings, policy, evaluated}}" };
4960
}
50-
return { outcome: replayDecision(record, replayInput) };
61+
return { outcome: replayDecision(record, replayInput, atMs === undefined ? {} : { nowMs: atMs }) };
5162
}
5263

5364
const invokedDirectly = process.argv[1]?.endsWith("replay-decision.ts") === true;
5465
if (invokedDirectly) {
55-
const source = process.argv[2];
66+
const argv = process.argv.slice(2);
67+
const atIndex = argv.indexOf("--at");
68+
const atRaw = atIndex === -1 ? undefined : argv[atIndex + 1];
69+
if (atIndex !== -1 && (atRaw === undefined || !Number.isFinite(Number(atRaw)))) {
70+
console.error("replay-decision: --at requires a Unix-epoch-milliseconds value");
71+
process.exit(2);
72+
}
73+
const source = argv.filter((arg, index) => index !== atIndex && index !== atIndex + 1)[0];
5674
if (!source) {
57-
console.error("usage: replay-decision.ts <bundle.json | ->");
75+
console.error("usage: replay-decision.ts <bundle.json | -> [--at <epoch ms>]");
5876
process.exit(2);
5977
}
6078
const raw = source === "-" ? readFileSync(0, "utf8") : readFileSync(source, "utf8");
61-
const { outcome, error } = runReplayBundle(raw);
79+
const { outcome, error } = runReplayBundle(raw, atRaw === undefined ? undefined : Number(atRaw));
6280
if (!outcome) {
6381
console.error(`replay-decision: ${error}`);
6482
process.exit(2);

src/queue/processors.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,7 @@ import {
663663
import { AI_JUDGMENT_BLOCKER_CODES } from "../rules/advisory";
664664
import { computeSalvageabilityForTarget } from "../review/salvageability-wire";
665665
import { deriveDecisionReasonCode, persistDecisionReplayInputForGate } from "../review/decision-replay";
666+
import { isWithinFreshRebaseWindow, type DecisionClockCapture } from "../review/staleness-clock";
666667
import { recordVerdictFlip } from "../review/verdict-flip-store";
667668
import { resolveAutomaticCloseConfidence } from "../review/risk-control-wire";
668669
import { maybeApplyCloseAuditHoldout } from "../review/close-audit-holdout";
@@ -3175,6 +3176,12 @@ async function runAgentMaintenancePlanAndExecute(
31753176
},
31763177
): Promise<void> {
31773178
const { installationId, repoFullName, pr, settings, otherOpenPullRequests, deliveryId, gate } = args;
3179+
// #9028: ONE wall-clock read for this whole decision pass, recorded into the replay input below and passed
3180+
// to every clock-dependent gate rule (today `gate.requireFreshRebaseWindow` via maybeForceFreshRebase)
3181+
// instead of each of them calling Date.now() independently. Two rules reading the clock at two different
3182+
// moments cannot both be replayed from one recorded instant — and an unrecorded instant is an unrecorded
3183+
// decision INPUT, which is exactly what makes a time-dependent decision unreplayable.
3184+
const decisionClock: DecisionClockCapture = { nowMs: Date.now() };
31783185

31793186
// Convergence safety: feed the planner the PR's changed paths + the repo's hard-guardrail globs so guarded
31803187
// paths force manual review, and flag owner-authored PRs so they are never auto-closed (standing rule).
@@ -3761,13 +3768,13 @@ async function runAgentMaintenancePlanAndExecute(
37613768
divertedByHoldout: closeAuditHoldout?.diverted ?? false,
37623769
});
37633770
const recordId = await persistDecisionRecord(env, record, recordDigest);
3764-
// #8838: persist the evaluation's own exact inputs beside the record (PRIVATE sibling, migration 0181)
3771+
// #8838: persist the evaluation's own exact inputs beside the record (PRIVATE sibling, migration 0182)
37653772
// so the replay harness can re-derive this decision bit-exactly. Best-effort, like the record itself;
37663773
// the no-replay no-op (synthetic content-lane/bridge evaluations) lives inside the helper. Keyed to the
37673774
// id persistDecisionRecord actually wrote (#9123: a supersession at the same head gets a revisioned id,
37683775
// not the base one this used to always recompute independently). #9135: the holdout outcome rides along
37693776
// so `holdout_consistency` has something to check the public record against.
3770-
if (recordId !== null) await persistDecisionReplayInputForGate(env, recordId, gate, policyCloseKind ?? null, closeAuditHoldout ?? null);
3777+
if (recordId !== null) await persistDecisionReplayInputForGate(env, recordId, gate, policyCloseKind ?? null, closeAuditHoldout ?? null, decisionClock);
37713778
}
37723779
// #2349 (PR 1): additive per-contributor calibration data, gated identically to recordNativeGateDecision
37733780
// above -- see src/review/contributor-calibration.ts's doc comment. Currently write-only; nothing reads
@@ -3897,6 +3904,7 @@ async function runAgentMaintenancePlanAndExecute(
38973904
token,
38983905
admissionKey,
38993906
deliveryId,
3907+
nowMs: decisionClock.nowMs,
39003908
}))
39013909
) {
39023910
return;
@@ -4806,17 +4814,20 @@ async function maybeForceFreshRebase(
48064814
token: string | undefined;
48074815
admissionKey: GitHubRateLimitAdmissionKey | undefined;
48084816
deliveryId: string;
4817+
// #9028: the decision pass's single captured instant — this rule reads it instead of the clock, so the
4818+
// window comparison is replayable from `decision_replay_inputs.replay_json`.
4819+
nowMs: number;
48094820
},
48104821
): Promise<boolean> {
4811-
const { installationId, repoFullName, pr, settings, windowMinutes, baseRef, token, admissionKey, deliveryId } = args;
4822+
const { installationId, repoFullName, pr, settings, windowMinutes, baseRef, token, admissionKey, deliveryId, nowMs } = args;
48124823
/* v8 ignore next -- structurally unreachable: the caller only invokes this after confirming
48134824
* (liveMergeState ?? pr.mergeableState) === "clean", which GitHub can never compute for a PR with no
48144825
* head commit; the null check is belt-and-suspenders against the field's optional TS type. */
48154826
if (!pr.headSha) return false;
48164827
const advancedAt = await fetchLiveBaseBranchAdvancedAt(env, repoFullName, baseRef, token, admissionKey);
48174828
if (!advancedAt) return false; // fail-open: unreadable base commit -> no forced rebase
48184829
const advancedAtMs = Date.parse(advancedAt);
4819-
if (!Number.isFinite(advancedAtMs) || Date.now() - advancedAtMs >= windowMinutes * 60_000) return false;
4830+
if (!isWithinFreshRebaseWindow({ baseAdvancedAtMs: advancedAtMs, windowMinutes, nowMs })) return false;
48204831

48214832
const countKey = freshRebaseForceCountKey(repoFullName, pr.number);
48224833
const storedCount = Number(await getTransientKey(env, countKey));

src/review/decision-replay.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
// guarantee is by construction, not by flag.
99
//
1010
// Stages, compared in pipeline order (the first mismatch wins — everything after it is downstream noise):
11+
// 0. `clock` — #9028: the instant the caller asked to replay AT vs the instant the live decision
12+
// recorded. Time is a decision input, so replaying a clock-dependent rule at a
13+
// DIFFERENT instant is never silently accepted as a match. Skipped when the caller
14+
// names no instant (replay at the recorded one) or the record predates #9028.
1115
// 1. `conclusion` — re-evaluated gate conclusion vs the decision-time snapshot.
1216
// 2. `blocker_codes` — ordered blocker code list (order IS meaning: blockerClass is the first code).
1317
// 3. `reason_code` — re-derived exactly as the finalize site derives it (blockerClass →
@@ -32,6 +36,7 @@
3236
// and file it; there is no "close enough" outcome.
3337
import { evaluateGateCheck, type GateCheckPolicy } from "../rules/advisory";
3438
import { neutralHoldReasonCode } from "./parity-wire";
39+
import type { DecisionClockCapture } from "./staleness-clock";
3540
import type { Advisory, AdvisoryFinding } from "../types";
3641
import { errorMessage, nowIso } from "../utils/json";
3742

@@ -60,6 +65,12 @@ export type DecisionReplayInput = {
6065
* this decision (ε absent/0, close autonomy not auto, or no eligible close) — the overwhelmingly common
6166
* case, and the SAME zero-I/O common path `maybeApplyCloseAuditHoldout` already guarantees. */
6267
holdout?: DecisionReplayHoldout | null | undefined;
68+
/** #9028: the decision-time wall clock — ONE `Date.now()` read per pass, which every clock-dependent gate
69+
* rule (today: `gate.requireFreshRebaseWindow`) reads instead of calling the clock itself. Recorded here so
70+
* a replay evaluates time-dependent rules at the instant the LIVE decision used, not at replay time.
71+
* undefined/null for a pre-#9028 record — such a record simply has no recorded instant, so the `clock`
72+
* stage cannot check anything and is skipped rather than guessed. */
73+
clock?: DecisionClockCapture | null | undefined;
6374
};
6475

6576
/** The slice of the PUBLIC decision record replay verifies against. */
@@ -79,11 +90,18 @@ export type ReplayOutcome =
7990
verdict: "divergence";
8091
recordId: string;
8192
/** The FIRST divergent stage — later stages are downstream of it and not reported. */
82-
stage: "conclusion" | "blocker_codes" | "reason_code" | "holdout_consistency";
93+
stage: "clock" | "conclusion" | "blocker_codes" | "reason_code" | "holdout_consistency";
8394
expected: string;
8495
actual: string;
8596
};
8697

98+
/** #9028: options for a replay run. `nowMs` names the instant the caller wants to replay AT — supply it only
99+
* to assert the recorded instant, or to deliberately probe a different one (which must FAIL, never silently
100+
* pass). Omitted (the default) means "replay at the recorded instant", which is the bit-exact case. */
101+
export type ReplayOptions = {
102+
nowMs?: number | undefined;
103+
};
104+
87105
/** The finalize site's reasonCode derivation, extracted verbatim so replay and live can never disagree
88106
* about the mapping itself (single source of truth — processors.ts finalize imports this too). */
89107
export function deriveDecisionReasonCode(blockerClass: string, policyCloseKind: string | null | undefined, conclusion: string): string {
@@ -97,7 +115,19 @@ export function deriveBlockerClass(evaluation: { blockers: Array<{ code: string
97115
}
98116

99117
/** PURE bit-exact replay. See the module doc for the stage contract. */
100-
export function replayDecision(record: ReplayableRecord, input: DecisionReplayInput): ReplayOutcome {
118+
export function replayDecision(record: ReplayableRecord, input: DecisionReplayInput, options: ReplayOptions = {}): ReplayOutcome {
119+
// #9028 stage 0: TIME IS AN INPUT. A caller that names the instant it is replaying at must be told when
120+
// that instant is not the one the live decision used — a clock-dependent rule (today
121+
// `gate.requireFreshRebaseWindow`) can legitimately flip its answer purely because the wall clock moved, so
122+
// silently replaying at "now" and reporting `match` would certify a re-derivation that never actually
123+
// reproduced the original evaluation. Checked FIRST: every later stage is evaluated as of this instant.
124+
// A caller that supplies no instant is replaying at the recorded one by definition, which is the bit-exact
125+
// path and the CLI's default. A pre-#9028 record has no recorded instant, so there is nothing to contradict
126+
// and the stage is skipped rather than guessed.
127+
const recordedNowMs = input.clock?.nowMs;
128+
if (typeof recordedNowMs === "number" && typeof options.nowMs === "number" && options.nowMs !== recordedNowMs) {
129+
return { verdict: "divergence", recordId: record.id, stage: "clock", expected: String(recordedNowMs), actual: String(options.nowMs) };
130+
}
101131
const advisory: Advisory = {
102132
id: `replay-${record.id}`,
103133
targetType: "pull_request",
@@ -141,7 +171,7 @@ export function replayDecision(record: ReplayableRecord, input: DecisionReplayIn
141171
return { verdict: "match", recordId: record.id, conclusion: evaluation.conclusion, blockerCodes, reasonCode, pinnedAction: record.action };
142172
}
143173

144-
/** Persist the replay input beside its record (PRIVATE sibling — see migration 0181). Best-effort: replay
174+
/** Persist the replay input beside its record (PRIVATE sibling — see migration 0182). Best-effort: replay
145175
* legibility must never break finalization, mirroring persistDecisionRecord's posture. Accepts the gate
146176
* EVALUATION and owns the no-replay no-op: content-lane/bridge evaluations are synthetic (their verdicts
147177
* come from their own deterministic pipelines, not the advisory evaluator) and carry no replay input —
@@ -154,6 +184,8 @@ export async function persistDecisionReplayInputForGate(
154184
// #9135: the close-audit holdout's decision-time outcome for this same decision, when it drew — threaded
155185
// straight through to the persisted replay input so `holdout_consistency` has something to check against.
156186
holdout?: DecisionReplayHoldout | null,
187+
// #9028: the decision pass's single captured wall-clock instant, so time-dependent rules are replayable.
188+
clock?: DecisionClockCapture | null,
157189
): Promise<void> {
158190
if (!gate.replay) return;
159191
await persistDecisionReplayInput(env, recordId, {
@@ -162,6 +194,7 @@ export async function persistDecisionReplayInputForGate(
162194
policyCloseKind,
163195
evaluated: { conclusion: gate.conclusion, blockerCodes: gate.blockers.map((blocker) => blocker.code) },
164196
holdout: holdout ?? null,
197+
clock: clock ?? null,
165198
});
166199
}
167200

src/review/staleness-clock.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Clock-injected staleness rule evaluation (#9028, epic #8828 Phase 4) — the two `gate.*` staleness rules
2+
// as PURE functions of their inputs plus an explicitly supplied instant.
3+
//
4+
// Why this module exists: `requireFreshRebaseWindowMinutes` was evaluated by reading `Date.now()` inline
5+
// inside an IO helper (processors.ts `maybeForceFreshRebase`). That made the rule structurally unreplayable
6+
// — nothing recorded WHICH instant the comparison used, so re-deriving the decision later could silently
7+
// reach the opposite answer purely because the wall clock had moved. Time is a decision INPUT; an input that
8+
// is not recorded is not replayable. The engine now reads one decision-time instant per pass, records it into
9+
// `decision_replay_inputs.replay_json`, and every clock-dependent rule reads THAT instant rather than the
10+
// clock (see `DecisionReplayInput.clock`).
11+
//
12+
// `staleBaseAheadByThreshold` is deliberately included here even though it reads NO clock: it is a commit-COUNT
13+
// comparison (`aheadBy >= threshold`), so it is instant-independent by construction. Stating that as a pure,
14+
// tested function is what makes the property provable rather than assumed — the replay suite pins it by
15+
// evaluating the same inputs at wildly different instants and asserting an identical answer.
16+
17+
/** Minutes → milliseconds, named so the fresh-rebase window's unit conversion has exactly one definition. */
18+
export const MS_PER_MINUTE = 60_000;
19+
20+
/** The decision-time wall clock, captured ONCE per evaluation pass and recorded with the replay input. Every
21+
* clock-dependent gate rule reads this instead of calling `Date.now()` itself, so a replay re-derives the
22+
* same answer the live pass reached instead of whatever the clock happens to say at replay time. */
23+
export type DecisionClockCapture = {
24+
/** Unix epoch milliseconds, from a single `Date.now()` read at the top of the decision pass. */
25+
nowMs: number;
26+
};
27+
28+
/**
29+
* `gate.requireFreshRebaseWindow` (#2552): true when the base branch's tip commit landed WITHIN
30+
* `windowMinutes` of `nowMs` — i.e. the base moved so recently that a `mergeable_state: clean` read may
31+
* predate it, so a merge should be preceded by a forced rebase + CI recheck.
32+
*
33+
* PURE and total: an unparseable/non-finite `baseAdvancedAtMs` returns false (fail-open to today's behavior —
34+
* an unreadable base commit must never manufacture a rebase), matching the live call site's own guard. A base
35+
* commit dated in the FUTURE relative to the captured instant (clock skew between GitHub and this engine)
36+
* yields a negative age, which is inside any positive window and therefore correctly reads as "just moved".
37+
*/
38+
export function isWithinFreshRebaseWindow(args: { baseAdvancedAtMs: number; windowMinutes: number; nowMs: number }): boolean {
39+
const { baseAdvancedAtMs, windowMinutes, nowMs } = args;
40+
if (!Number.isFinite(baseAdvancedAtMs)) return false;
41+
return nowMs - baseAdvancedAtMs < windowMinutes * MS_PER_MINUTE;
42+
}
43+
44+
/**
45+
* `gate.staleBaseAheadByThreshold` (#review-grounding stale-base fact): true when the repo's default branch
46+
* has advanced at least `threshold` commits beyond this PR's head, so the PR should be updated before review.
47+
*
48+
* PURE, total, and deliberately CLOCK-FREE — a commit count carries no notion of "now", so this rule's answer
49+
* is identical at every instant. A non-finite `aheadBy` (an unreadable compare API response) returns false,
50+
* fail-open, matching the live call site's `typeof aheadBy === "number"` guard.
51+
*/
52+
export function isBaseStaleByAheadBy(args: { aheadBy: number; threshold: number }): boolean {
53+
const { aheadBy, threshold } = args;
54+
if (!Number.isFinite(aheadBy)) return false;
55+
return aheadBy >= threshold;
56+
}

0 commit comments

Comments
 (0)