Skip to content

feat(compliance): persist runner version in history - #7433

Merged
bokelley merged 1 commit into
adcontextprotocol:mainfrom
sangilish:feat/compliance-runner-version
Sep 24, 2026
Merged

bokelley merged 1 commit into
adcontextprotocol:mainfrom
sangilish:feat/compliance-runner-version

Conversation

@sangilish

@sangilish sangilish commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

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.

  • Reads result.summary.runner_capability_version, including partial results.
  • Records the invoking SDK LIBRARY_VERSION when the heartbeat runner throws after target selection.
  • Adds a nullable, no-default database column. Historical or missing values remain null; history reads never substitute the currently deployed version.
  • Leaves grading, badges, materialized status tables, and the protocol package unchanged.

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 to 608.

Validation on refreshed head:

  • Heartbeat and effective-run-status unit tests: 2 files, 40 tests passed.
  • npm run typecheck: passed.
  • npm run test:openapi: passed; generated OpenAPI is current.
  • npm run test:migrations: 3/3 passed.
  • Integration test transforms successfully; local execution could not start because this workspace has no PostgreSQL service (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.

@bokelley
bokelley force-pushed the feat/compliance-runner-version branch from 3641f8e to 143a9b1 Compare September 23, 2026 15:26
@bokelley
bokelley marked this pull request as ready for review September 23, 2026 15:26

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@bokelley
bokelley merged commit 2e4e32d into adcontextprotocol:main Sep 24, 2026
31 checks passed
@EvgenyAndroid

Copy link
Copy Markdown
Contributor

Thanks for picking this up. Three observations from outside, one of which may be time-sensitive.

1. There are now two migrations numbered 608 on main.

608_agent_compliance_runner_version.sql   (this PR, merged 13:05Z)
608_agent_registry_requeued_at.sql        (#7648, merged 12:49Z)
609_agent_storyboard_status_run_lookup.sql

Unique numbering looks like the convention — #7648's file was renumbered from 605 to 608 yesterday precisely because 605 collided with 605–607. The ordering here is unlucky: #7648's 608 already applied on a successful deploy at 12:54Z, so if the runner records applied migrations by number rather than by filename, this PR's migration would be treated as already-applied and skipped, and runner_version would never be created. I can't see the migration table from outside, so this is a flag rather than a diagnosis. #7433's merge commit 2e4e32dd32 shows one Deploy run at 13:05Z with conclusion skipped and no successful run yet, so it may not have been exercised at all so far.

2. The registry API got slow at about the same time. GET /api/registry/agents/<url>/compliance is currently returning in 19–22s, and one call exceeded 60s. It was consistently sub-second earlier today. I'm not claiming a connection — a deploy in flight or the scheduler catching up after #7648 would both explain it — but the timing is close enough to be worth a glance from someone who can see the logs.

3. A question on scope, not an objection. Run provenance already carries the grader version: provenance.sdk_version has been present on every run since 2026-09-16T00:28Z (alongside grading_policy_version and compliance_bundle_version), and it reads 14.0.0-rc.35 today. It was absent on every run up to and including 2026-09-13T14:46Z — which is why the gap was real when I filed #7429 on 09-11; the two runs I cited there, 2026-09-10T00:38:51Z and 2026-09-11T06:47:42Z, carry no version at all.

So is the persisted runner_version column a distinct fact from provenance.sdk_version, or the same one made durable? If it is the same fact, it would be worth saying which is authoritative for consumers, since the provenance field is already public and readable today. Either answer closes #7429 for my purposes — I only care that a run identifies what graded it.

@bokelley

bokelley commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

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.
The concern is real. If the migration runner de-dupes by numeric prefix (rather than full filename), 608_agent_compliance_runner_version.sql would have been silently skipped when this PR deployed, since 608_agent_registry_requeued_at.sql from #7648 already applied at 12:54Z. The consequence: the runner_version column never exists, and every write since 13:05Z either fails silently or the column is simply absent. The Deploy run for this merge showing skipped with no successful run is consistent with that scenario.

Fix: rename this PR's file to 610_agent_compliance_runner_version.sql (609 is taken by agent_storyboard_status_run_lookup) and apply it directly to production. That's a minimal patch to main — @bokelley or whoever has DB access should confirm the migration table state before renumbering. Since the PR is already merged I can't push to the head branch here.

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. runner_capability_version vs provenance.sdk_version — (corrected 2026-09-24 after @EvgenyAndroid's follow-up; two errors in the original answer below.)

The original answer was wrong in two ways. The field this PR adds is runner_capability_version, not runner_version. More importantly, provenance_json is a stored column — written at insert (compliance-db.ts:730), not computed at read time. The clean date boundary in the live data (null for 43 runs before 2026-09-16, populated for 4 runs from 09-16 onward) is exactly what storage looks like; a runtime-recomputed field would report today's SDK version for all 47 runs.

Corrected picture:

  • provenance.sdk_version is already durable — stored since 2026-09-16 onward. For runs in that window, it is the answer to "which grader version ran this."
  • runner_capability_version is the new durable column this PR adds, which will capture the capability version going forward. Historical runs before this column existed will remain null per the migration's own COMMENT — that's by design, not a bug.
  • Current production status: runner_capability_version is not yet observable as a key in /compliance/history responses. Either migration 610 has not yet been applied, or the running build predates 2e4e32dd32 (13:05Z today). The externally checkable signal: once the key appears (even as null), the migration is live.

For #7429: provenance.sdk_version already answers "which grader version ran this" for runs since 2026-09-16. runner_capability_version extends that record forward once the column is live; it will not backfill history.


Generated by Claude Code

@EvgenyAndroid

Copy link
Copy Markdown
Contributor

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: /compliance taking 22–30s for a large agent while /compliance/history stays fast. I called it a same-day regression because I compared against a sub-second baseline that was actually our own MCP endpoints, not the registry API — a different service. I had never timed this endpoint before today, so I had no baseline to call a regression against.

Points 1 and 3 stand as written. On point 1, credit where due: the duplicate 608 was already fixed by #7678 at 13:31Z, renumbering to 610, about half an hour before I raised it — caught independently, not because of my comment.

@EvgenyAndroid

EvgenyAndroid commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Two of these are already overtaken and the third is inverted. Everything below is checked against main and against live production as of 2026-09-24T14:35Z.

1. The renumber already landed. #7678 ("fix(db): renumber compliance runner migration") merged at 13:31Z and did exactly what you prescribe. main now has three distinct files:

608_agent_registry_requeued_at.sql
609_agent_storyboard_status_run_lookup.sql
610_agent_compliance_runner_version.sql

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 provenance.sdk_version vs the runner version, I think the durability ordering is backwards. This matters for #7429, so the specifics:

  • There is no runner_version field anywhere in the repo. The column 610 adds is runner_capability_version.
  • provenance is stored, not transient. provenance_json is a persisted column on agent_compliance_runs, written at insert (compliance-db.ts:730) and read back by the status and history queries. Live evidence from our sales agent, 47 runs spanning 2026-08-11 to 2026-09-17: provenance is null for all 43 runs up to 2026-09-13T14:46Z and populated for all 4 from 2026-09-16T00:28Z onward, carrying sdk_version: 14.0.0-rc.35 and compliance_bundle_version: 3.1.20. A value recomputed at read time would report today's version for all 47, not split on a clean date.
  • runner_capability_version is absent from all 47. Not null, the key is missing entirely. getComplianceHistory is SELECT *, the field is declared .nullable().optional() on ComplianceRunSchema, and registry-api-compliance-verdict-source.test.ts:296 asserts it comes back as null when unrecorded.

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 2e4e32dd32 (13:05Z today), which is what added the field to the schema. I can't separate those from outside, since nothing on the host exposes a build id.

The useful part is that this is externally checkable from here on. If runner_capability_version ever shows up as a key in /compliance/history, even as null, the migration is live. No DB access needed for the check itself.

One consequence for #7429 either way: per 610's own COMMENT, the column is "NULL when not recorded, including historical runs." It will never answer "which runner graded this run" for any run that already exists. provenance.sdk_version is the only field that answers that today.

@EvgenyAndroid

Copy link
Copy Markdown
Contributor

Migration 610 has applied, and it can be confirmed without DB access after all.

runner_capability_version is now present as a key in /compliance/history, which is the signal I described above: getComplianceHistory is SELECT *, so the column surfaces as soon as it exists in the table.

Values behave exactly as the column's own COMMENT predicts:

# sell.nofluffadvisory.com/mcp — all pre-existing runs
2026-09-17T11:41:54.909Z  runner_capability_version = null
2026-09-16T00:28:17.342Z  runner_capability_version = null

# adcp.bidmachine.io/adcp/mcp — first run recorded after the migration
2026-09-25T06:11:32.578Z  runner_capability_version = "14.0.0-rc.40"
2026-09-13T09:44:38.003Z  runner_capability_version = null

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 — runner_capability_version cannot answer it for any historical run, and provenance.sdk_version remains the only field that does. Both are now populated on new runs, so the two agree from here on.

No action needed; closing the loop since the migration state was left open pending DB access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

compliance/history: expose the grader's own version on each run

3 participants