Repository navigation
feat(compliance): persist runner version in history - #7433
Conversation
3641f8e to
143a9b1
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean bounded operational change with no blocking or medium findings.
Checked:
- INSERT column↔value alignment verified (24→25 columns, trailing append) in compliance-db.ts.
- Migration 608 is safe additive nullable-TEXT; judged as production-facing (runs on startup) — no destructive DDL, no backfill risk.
- Changeset scope correct: touches server + registry OpenAPI only, no
static/schemas/source/**protocol surface, so no changeset required (operational-only). - schema↔OpenAPI↔route coherence confirmed; null no-substitution invariant on history reads preserved.
- No new I/O/timeout concerns on the operational-readiness path.
Gate checks:
- No no-auto-approve team match.
- gated_paths: false — row 2 does not apply.
- high_risk: true, but reasons are (modified)/(added) only with no medium-or-higher findings — per the high_risk flag reading, modifications with no concerns are presumed safe and new files are normal scaffolding. No (deleted) entries, so no escalation.
- No protocol/breaking-class change; migration-only server-side work.
No rows 1–8 fire → row 9 approve.
|
Thanks for picking this up. Three observations from outside, one of which may be time-sensitive. 1. There are now two migrations numbered Unique numbering looks like the convention — #7648's file was renumbered from 2. The registry API got slow at about the same time. 3. A question on scope, not an objection. Run provenance already carries the grader version: So is the persisted |
|
Thanks for the careful post-merge review, @EvgenyAndroid — all three observations are worth acting on. 1. Migration collision — needs immediate attention from someone with DB access. Fix: rename this PR's file to 2. API latency — flagged, can't diagnose externally. The timing correlation with a possibly-stuck migration or pending deploy is worth a log check from someone on the ops side. 3. The original answer was wrong in two ways. The field this PR adds is Corrected picture:
For #7429: Generated by Claude Code |
|
Withdrawing point 2 of my comment above — the latency observation was wrong on both counts and I'd rather retract it than leave anyone chasing it. It is not new, and it has nothing to do with this PR. It is #7630, filed 2026-09-21: Points 1 and 3 stand as written. On point 1, credit where due: the duplicate |
|
Two of these are already overtaken and the third is inverted. Everything below is checked against 1. The renumber already landed. #7678 ("fix(db): renumber compliance runner migration") merged at 13:31Z and did exactly what you prescribe. So nobody needs to push to this PR's head branch. 2. The latency point is withdrawn. I retracted it earlier in this thread. It is #7630, filed 09-21, and it has nothing to do with this PR. Worth saying explicitly so nobody re-opens that line here. 3. On
That last bullet is as far as your "confirm the migration table state" ask can be answered without DB access: the column is not observable in production right now. Either it is missing from the table, or the running build predates The useful part is that this is externally checkable from here on. If One consequence for #7429 either way: per 610's own |
|
Migration 610 has applied, and it can be confirmed without DB access after all.
Values behave exactly as the column's own So the field works going forward and is null for every run that predates it. That confirms the caveat from earlier in this thread: for #7429's retrospective question — which runner graded a run that already happened — No action needed; closing the loop since the migration state was left open pending DB access. |
Closes #7429.
Persists the runner capability version on each compliance run and exposes it in the history API, separately from the AdCP version under test.
result.summary.runner_capability_version, including partial results.LIBRARY_VERSIONwhen the heartbeat runner throws after target selection.null; history reads never substitute the currently deployed version.This is the bounded persistence slice proposed in the issue. Equal capability versions are not a guarantee of identical deployments or deterministic results; deployment provenance remains out of scope.
The branch was refreshed as one commit on current
main. The refresh composes the version field with newer compliance publication/provenance columns, preserves the current non-authoritative treatment of incomplete runner failures, and renumbers the migration to608.Validation on refreshed head:
npm run typecheck: passed.npm run test:openapi: passed; generated OpenAPI is current.npm run test:migrations: 3/3 passed.ECONNREFUSED). GitHub's PostgreSQL integration job is authoritative.git diff --check: passed.No protocol changeset is required because this only changes the application's operational history API.