fix(trust): commit the decision record to what actually decided, record the holdout draw, and fix the attestation envelope's width and freshness - #9178
Merged
JSONbored merged 1 commit intoJul 27, 2026
Conversation
…rd the holdout draw, and fix the attestation envelope's width and freshness Three linked issues on the decision-record/replay/attestation trust surface: - #9124: configDigest hashed raw settings instead of the resolved gate policy (missing the live-calibrated close-confidence floor and the untrustworthy-rule-code set); promptDigest hashed the base template instead of the actual sent system prompt; modelId was hardcoded null. configDigest now digests {resolved policy, untrustworthy rule codes} with the raw settings digest kept separately as settingsDigest; promptDigest is sha256 of buildSystemPrompt's real output, threaded through ai-review-orchestration.ts's finding construction; modelIds carries the real parsed-reviewer set. ciState is populated from the live CI aggregate instead of hardcoded null. Schema bumped to v4. - #9135: the close-audit holdout's Math.random() draw was unrecorded, so two decisions with identical findings/policy could silently diverge to close vs hold with no trace. The draw is now HMAC(instance secret, seed) — reproducible from the record without storing raw entropy, unpredictable without the secret — and the outcome is threaded into DecisionRecord.divertedByHoldout (public) and DecisionReplayInput.holdout (private), with a new replay stage that reports the divergence if the two ever disagree instead of silently matching. - #9140: the attestation envelope's reportData was 32 bytes for TEE hardware's 64-byte REPORT_DATA/REPORTDATA field, with no freshness binding (a genuine attested run's report could be replayed for any later run with the same corpus/shas). reportData is now sha256(binding) || sha256(runId), with the binding inputs constrained to hex shapes to close the colon-injection ambiguity, and the base64 validator now rejects lengths that aren't multiples of 4. #9141 (research-only, no implementation): posted a written scope decision on the issue and closed it as answered — attesting the live gate-decision path is not worth pursuing now, given the fail-open/ fail-closed dilemma a KBS outage would force and that the anchoring + complete-records work already closes the practical gap more cheaply. verify-this-review.mdx documents the boundary permanently.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 2ac63b7 | Commit Preview URL Branch Preview URL |
Jul 27 2026, 05:48 AM |
Contributor
Logic backtestReplayed 0 historical case(s) for Backtest comparison:
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
❌ 7 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three linked issues on the decision-record / decision-replay / attestation trust surface:
orb(trust): the decision record's commitments don't commit — modelId is always null, promptDigest ignores ten prompt suffixes, configDigest misses the calibrated close floor #9124 —
DecisionRecord's three commitments (configDigest,promptDigest,modelId) did not commit to what actually decided the PR.configDigestnow digests{ policy: gate.replay?.policy ?? settings, untrustworthyRuleCodes }— the RESOLVEDgateCheckPolicy(...)objectevaluateGateCheckactually ran against (including the live-calibrated AI close-confidence floor and the cron-refreshed untrustworthy-rule-code set the precision breaker consults), not rawsettings. The raw settings digest survives as a new, separatesettingsDigestfield.promptDigestis nowsha256HexofbuildSystemPrompt's ACTUAL output (base template + whichever of its suffixes resolved for this call — grounding/enrichment/profile/security-focus/path-instructions/review.instructions/screenshot-evidence/inline/category/improvement-signal), computed inai-review.tsand threaded onto theai_consensus_defect/ai_review_splitfinding inai-review-orchestration.ts.modelIdis renamedmodelIds: string[] | nulland carries the real parsed-reviewer identities (parsedReviewModelIds), read straight off the finding at the decision-record call site instead of hardcodingnull.ciStateis populated from the live CI aggregate already in scope, instead of hardcodednull.DECISION_RECORD_SCHEMA_VERSIONbumped"3"→"4", documented in the module header and each field's own doc comment.orb(replay): an unrecorded Math.random() draw turns closes into holds — two identical records, different actions, and replay cannot see it #9135 — the randomized close-audit holdout's
Math.random()draw was recorded nowhere, so two decisions with identical findings/policy/configDigestcould diverge toaction: "close"vsaction: "hold"with no trace of why, and replay could not see it. The draw is nowHMAC(instance secret, seed)(src/review/close-audit-holdout.ts) — the seed is the decision's own eventual record id (record:<repo>#<pr>@<head sha>), and the secret is a dedicated 256-bit value persisted insystem_flags(mirroringorb-collector.ts'sgetOrCreateAnonSecretpattern, key-separated from every other secret in the codebase). This makes the draw reproducible from the record (for anyone holding the instance secret) without storing raw entropy, and unpredictable to a contributor without that secret trying to time a PR to dodge the holdout. The outcome ({ epsilonPct, draw, diverted }) is threaded intoDecisionRecord.divertedByHoldout(public — rendered on the record's own face in the PR comment) andDecisionReplayInput.holdout(private), andreplayDecisiongained a newholdout_consistencystage that reports a divergence between the two rather than silently matching. The replay contract's module header now states explicitly what is (and, honestly, is not yet) captured: breaker-flag state, the live CI aggregate, and the global pause/freeze switches remain outside this input, documented as a narrower claim rather than an unqualified broad one.calibration: the attestation envelope's reportData is 32 bytes for a 64-byte TEE field, and its binding has no freshness component — a valid report replays #9140 —
packages/loopover-engine's attestation envelope (attestation-envelope.ts, schema seam for a future attested-evaluation epic, no hardware involved) had two gaps:reportDatawas a 32-byte SHA-256 digest for a TEE hardware field that is 64 bytes wide (SEV-SNPREPORT_DATA/ TDXREPORTDATA), and the binding (corpusChecksum:headSha:baseSha) had no freshness component, so one genuine attested run's report could be replayed for any later run with the same corpus/shas.reportDatais nowsha256(binding) || sha256(runId)— 128 hex chars — with a newrunIdenvelope field carrying the plaintext freshness token, published as a worked test vector inbuildAttestationReportData's own doc comment. The three binding inputs are now constrained to their expected hex shapes (64-hex checksum, 40/64-hex git shas), closing the colon-injection ambiguity the old raw string-join allowed. The base64 validator (attestationReport) now rejects a length that isn't a multiple of 4.#9141 (research-only — "do NOT start implementation") — posted the requested written scope decision as an issue comment (in the #8136/#8137 two-spike format: cost analysis, comparison against anchoring+complete-records, explicit answer on the partial-attestation question) and closed the issue as answered, matching the precedent set by #8136/#8137 themselves. Verdict: not worth pursuing live-path attestation now — a KBS-outage fail-open/fail-closed dilemma is a worse availability tradeoff than the marginal proof it would buy over the deterministic replay harness (#8832/#8838) already in place, and the anchoring + complete-records work (this same batch, and the parallel #9120/#9122/#9123/#9134 PR already merged to
main) closes the practical gap a network verifier cares about more cheaply.apps/loopover-ui/content/docs/verify-this-review.mdxdocuments this boundary permanently in its existing "what this proves — and what it does not" callout.Rebase note: this branch was rebased twice against a fast-moving
main(most recently onto70b60e729) to reconcile with the parallel #9120/#9122/#9123/#9134 PR, which also toucheddecision-record.ts(revisioned persist ids, full-digest rendering) andprocessors.ts(unconditional record-building context). Both sets of changes are compatible and merged cleanly;agent-action-executor.ts's newDecisionRecordContext.modelIdwas renamed tomodelIdsto match, withsettingsDigestwired through for its call sites too (theirconfigDigestalready IS the raw settings digest, so the same value is honest there).Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.md; nosite/,CNAME, orlovable/changes.gh issue close, since it required no code change).Validation
git diff --checknpm run actionlint— not run this pass (no workflow files touched).npm run typecheck— clean, re-run after each rebase.npm run test:coverage(full/unsharded) — not run this pass, per explicit instruction to skip the full local gate and let CI run it. Instead ran targetedvitest runacross every touched/added test file plus every test file the parallel fix(trust): unauthenticate ledger verify, catch tail truncation, publish records, record every close (#9120 #9122 #9123 #9134) #9172 PR touched (to verify the rebase):ai-review.test.ts,close-audit-holdout.test.ts,crypto.test.ts,decision-record.test.ts,decision-replay.test.ts,gate-check-policy.test.ts,queue-2.test.ts,backfill-decision-labels-core.test.ts,agent-action-executor.test.ts,agent-approval-queue.test.ts,public-decision-ledger-routes.test.ts,held-lock-registry.test.ts,surface-disposition-reconciler.test.ts— 872 passing in the final combined run, plus a scoped--coveragepass showing 100% branch coverage ondecision-record.ts,close-audit-holdout.ts, anddecision-replay.tsspecifically.packages/loopover-engine's ownnpm run test(builds + node:test) passes all 734 tests including the new/updated attestation-envelope suite.npm run test:workers— not run this pass (no Worker-specific surface touched; see note above).npm run build:mcp/npm run test:mcp-pack— not run this pass (nopackages/loopover-mcpchanges).npm run ui:openapi:check— not run; no API/OpenAPI schema changed (DecisionRecordis not exposed through any route).npm run ui:lint/npm run ui:typecheck/npm run ui:build— not run; only change underapps/loopover-uiis a prose addition to one existing.mdxdoc page (no component/route/nav change).npm audit --audit-level=moderate— not run this pass, per the same skip-the-full-gate instruction.Per-issue regression tests added:
gate-check-policy.test.tspins that a changed calibrated close floor moves the resolved-policy digest while raw settings (settingsDigest) stays byte-identical;ai-review.test.tspins thatsystemPromptDigestissha256Hexof the real sent prompt and moves whenrepoInstructionschanges;queue-2.test.tsadds an end-to-end webhook test assertingmodelIds/promptDigest/ciState/both digests are populated (never the old hardcoded nulls) when an AI judgment shapes the decision.close-audit-holdout.test.tscovers both arms of the holdout (draw under/at-or-above ε) plus HMAC-determinism (same repo/PR/head sha reproduces the same draw; a different head sha does not) and the dedicated-secret persistence/race-safety;decision-replay.test.tsadds the newholdout_consistencystage (both a clean match and each direction of a claimed-vs-recorded divergence);queue-2.test.tsadds an end-to-end test with a pre-seeded deterministic secret proving a real webhook-driven close is diverted to a hold withdivertedByHoldout: trueon both the public record and the private replay input.packages/loopover-engine/test/attestation-envelope.test.tsand its vitest-coverage mirrortest/unit/attestation-envelope-engine.test.tsboth assert the published test vector verbatim, injectivity over the (corpusChecksum, headSha, baseSha, runId) quadruple, that only the relevant half ofreportDatamoves for a binding-only vs. runId-only change, malformed-input rejection (colon injection, wrong hex shapes), and the base64 length-multiple-of-4 fix.If any required check was skipped, explain why:
npm run test:ci,npm audit) and let GitHub Actions CI run it; targeted checks above (typecheck + scoped test runs + scoped coverage) were run instead as the fast local signal.Safety
divertedByHoldout) and a model-id join, no new private terms.apps/loopover-uiis a prose addition to one existing.mdxdoc page's existing<Callout>(no new component, layout, or visual difference to screenshot).verify-this-review.mdx's existing "what this proves — and what it does not" callout now states the live-path-attestation boundary permanently (per Research: the attested-evaluation epic covers offline backtests only — decide whether the LIVE decision path needs attestation, and in what order #9141's own written decision). NoCHANGELOG.mdedit.Notes
gate.replay?.policy ?? settingsfallback's FALSE arm (a synthetic content-lane/bridge evaluation with no.replay) is not exercised by a dedicated test in this PR — that path is allowlist-scoped to registry-submission repos and reaching it end-to-end through the full agent-maintenance pipeline would need a more involved fixture than the time budget for this pass allowed. Ifcodecov/patchflags it, it is a one-line, low-risk branch to cover in a fast follow-up.