fix(trust): unauthenticate ledger verify, catch tail truncation, publish records, record every close (#9120 #9122 #9123 #9134) - #9172
Merged
Conversation
…tion, publish decision records, and record every completed merge/close (#9120, #9122, #9123, #9134) Four linked issues on the decision-ledger/decision-record trust surface: - #9120: /v1/public/decision-ledger/verify was missing from the requiresApiToken exemption list and 401'd in prod despite its own doc comment always claiming otherwise. Added the exemption, table-driven tests over every /v1/public/* exemption, and audited every other public route (all already exempted). - #9122: verifyDecisionLedger now returns tipSeq/tipHash/totalCount so a third party can checkpoint, and catches tail truncation (DELETE ... WHERE seq > N) by cross-checking decision_ledger against decision_records — deleting ledger rows never touches the records they describe, so an orphaned record past the verified tip is the tell. Propagated the tamper-evident-not-tamper-proof honest limit into decision-record.ts's module header and corrected the overclaiming route comment. Added buildLedgerAnchorPayload as the documented, not-yet-wired shape a future external-anchoring job would publish. - #9123: added GET /v1/public/decision-records/:owner/:repo/:pull, publishing the full record + digest (DecisionRecord is public-safe by construction). Full 64-hex digests now render in the review panel instead of 12-char prefixes. persistDecisionRecord no longer UPDATEs a record body in place — a re-persist at the same (repo, pull, head) gets its own revisioned row, so a chained digest always keeps a live preimage. - #9134: hoisted decision-record + ledger-row emission into executeAgentMaintenanceActions itself, so every completed merge/close gets one by construction instead of depending on the call site remembering to. Six of seven call sites previously wrote nothing at all. Deferred (noted in the PR): self-hosted tunnel exposure (#9120, ops decision); real external anchoring beyond the documented payload shape (#9122); backfilling the 51 already-superseded live records (#9123, undoable); recomputing the risk- control label population (#9134, operational follow-up).
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | fddfc0a | Commit Preview URL Branch Preview URL |
Jul 27 2026, 05:32 AM |
Bundle ReportChanges will increase total bundle size by 2.24kB (0.03%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
21 tasks
❌ 6 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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
Four linked issues on the decision-ledger/decision-record trust surface, fixed together since they touch the same small set of files (
src/review/decision-record.ts,src/api/routes.ts,src/queue/processors.ts,src/services/agent-action-executor.ts,src/services/agent-approval-queue.ts).Closes #9120
Closes #9122
Closes #9123
Closes #9134
#9120 — public decision-ledger verify endpoint required an API token (401 in prod)
/v1/public/decision-ledger/verify's own doc comment always claimed "safe unauthenticated," but the route was missing fromrequiresApiToken's exemption list insrc/api/routes.ts, so it 401'd in prod while its siblings (/v1/public/subnet-interface,/v1/public/stats,.../quality) answered 200.test/integration/public-decision-ledger-routes.test.ts) over every currently-exempt/v1/public/*route, asserting each answers without credentials, plus a negative control proving the table isn't vacuously passing (a genuinely protected route still 401s)./v1/public/*route against its own doc comment vs. the exemption list: all others were already correctly exempted (subnet-interface,stats,github/repos/.../stats,repos/.../badge.{svg,json},repos/.../quality) — this route was the only mismatch.#9122 — no external anchor, verify returns no hash, tail truncation passes clean
verifyDecisionLedgerproved only self-consistency and returned nothing an observer could persist/re-compare, and aDELETE FROM decision_ledger WHERE seq > N(dropping the newest rows) verified perfectly clean since nothing in the remaining window was disturbed.verifyDecisionLedgernow returnstipSeq,tipHash, andtotalCounton every call, so a third party can checkpoint independent of pagination position.decision_ledgeranddecision_recordsare written together in the same call (persistDecisionRecordappends its ledger row immediately after inserting the record), so deleting ledger rows can never touch the records they describe. On the last page of a verify pass, adecision_recordsrow created strictly after the verified tip with no chain entry covering it is exactly the signature of a truncated tail — now reported as a newshort_tailbreak, notok: true. Regression test included (decision-record.test.ts), using fake system time to keep the timestamp ordering deterministic rather than relying on real wall-clock speed.buildLedgerAnchorPayload({seq, rowHash}, at?)— the exact, documented shape a future periodic external-anchoring job (git commit / transparency log / on-chain commitment) would publish. Not wired to a scheduler; this repo doesn't call it from anywhere yet.migrations/0180_decision_ledger.sql's header) intodecision-record.ts's own module header, and corrected the overclaiming route comment inroutes.ts(it previously implied the endpoint could rule out a wholesale chain rewrite, which it cannot without an external anchor).buildLedgerAnchorPayload) is included.#9123 — decision record never published; 12-char digest prefixes; re-gate at the same head destroyed the prior record
The record was persisted (
decision_records) but never exposed anywhere beyond a bounded, truncated review-comment summary, andpersistDecisionRecordusedON CONFLICT(id) DO UPDATE, so a re-gate at the same head SHA silently overwrote the earlier record body while the ledger still held a chain row pointing at the now-destroyed digest.GET /v1/public/decision-records/:owner/:repo/:pull, returning the latestrecord_jsonverbatim + itsrecord_digest.DecisionRecordis public-safe by construction (its own type doc: counts/digests/enums only, no diffs, no private config contents, no wallet/hotkey/trust-score/reward fields) — no field-level redaction was needed before exposing it. Added to the samerequiresApiTokenexemption list as orb(trust): the 'public' decision-ledger verify endpoint requires an API token — 401 in prod, so the chain is verifiable by nobody #9120's fix, in this same PR.renderDecisionRecordSectionnow prints full 64-hex digests instead of 12-char prefixes (the head SHA keeps its conventional 7-char git abbreviation — a display convention, not a digest commitment).persistDecisionRecordno longer overwrites: the first record for a(repo, pull, head)keeps the plainrecord:<repo>#<pr>@<head>id every existing consumer expects; a supersession at the exact same head gets its own row at<baseId>:rev<N>instead, so a digest the ledger already chained always keeps a live preimage. Returns the id it actually wrote sopersistDecisionReplayInputForGate(the private replay-input sibling) targets the correct row, including on a supersession.UPDATEbefore this fix landed. This is a documentation note, not a data migration.#9134 — six of seven actuation sites wrote no decision record
buildDecisionRecord/persistDecisionRecordran at exactly one ofexecuteAgentMaintenanceActions's seven call sites. The other six — a contributor-cap close on PR open, two review-nag closes (ping-count and monitored-mention), an approval-queue accept (merge/close), and two forced-rebaseupdate_branchcalls — performed real GitHub mutations with no record and no ledger row, biasingloadCalibrationPairs's label population away from exactly the closes a contributor is most likely to dispute.executeAgentMaintenanceActionsitself: every completedmerge/closeaction now emits a decision record + chained ledger row by construction, regardless of which of the seven call sites produced it.decisionRecord: DecisionRecordContextfield toAgentActionExecutionContext, made required (not optional) so a new call site cannot compile without deciding what its record should carry. The one call site that already built its own record unconditionally at plan time (coveringmerge/close/holddispositions, since aholdnever even reaches this executor) opts out via an explicit{ managedByCaller: true }— a deliberate, self-documenting override, not a silent omission, to avoid double-recording the same decision.agent-action-executor.test.ts) that exercises the executor directly across every action class and asserts the decision-record/ledger-row invariant holds for all of them at once — a future call site that adds a new action class without accounting for this would fail here, not silently ship unrecorded.queue-3.test.ts), both review-nag variants (queue-5.test.ts), and the approval-queue's staged merge and staged close (agent-approval-queue.test.ts).processors.ts:3322-3410for the two review-nag sites) had landed —git logon rebase showed no such merge, so this fix composes at the shared executor level regardless of landing order.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not touchsite/,CNAME, or**/lovable/**.Closes #9120,Closes #9122,Closes #9123,Closes #9134(all currently open).Validation
git diff --checknpm run typecheck(clean, full repo)npm run actionlint— not run (no workflow files touched); skipped per explicit instruction to skip the full local gate for this PR and rely on CInpm run test:coverage— not run as the full/unsharded suite; instead ran targetedvitest run --coveragescoped to every touched file (decision-record.ts100% stmts/branches/functions/lines;agent-action-executor.ts,agent-approval-queue.ts,processors.ts,routes.tsdiffs all 100% line+branch covered when run against their existing + new test files) — see "If any required check was skipped" belownpm run test:workers— not run, per the same instructionnpm run build:mcp/npm run test:mcp-pack— not run; this PR does not touchpackages/loopover-mcpnpm run ui:openapi:check(regeneratedapps/loopover-ui/public/openapi.jsonvianpm run ui:openapi, then verified clean)npm run ui:lint/npm run ui:typecheck/npm run ui:build— not run; this PR touches noapps/loopover-ui/**filesnpm audit --audit-level=moderate— not run, per the same instructionIf any required check was skipped, explain why:
npm run test:ci,npm audit, etc.) before pushing — CI runs the gates. I instead rannpm run typecheck(clean) and targetedvitest run/vitest run --coverageagainst every touched test file and every file I changed, confirming 100% line+branch coverage on my actual diff in each case. I also discovered, and separately flagged (not fixed here, out of scope), one pre-existing test failure onorigin/main(test/unit/queue-4.test.ts's "debounces noisy PR events" test) introduced by an unrelated recent commit (fix(webhook): catch a base retarget with no new commit, tighten event health, and stop label/issue churn from going stale #9169) — reproduced identically with this PR's changes fully reverted, so it predates and is unrelated to this PR.Safety
DecisionRecord(now published verbatim via the new route) was verified public-safe against its own type doc before exposing it.requiresApiTokenexemption list) include negative-path tests (the genuinely-protected-route 401 control, plus per-route exemption assertions).src/openapi/spec.ts, spec regenerated and checked.CHANGELOG.mdnot touched.UI Evidence
Not applicable — this PR is backend/API-only (no
apps/loopover-ui/**changes).Notes