Skip to content

fix(control-plane): store execution VC created_at in UTC on SQLite - #1170

Closed
santoshkumarradha wants to merge 1 commit into
mainfrom
fix/execution-vc-utc-timestamps
Closed

santoshkumarradha wants to merge 1 commit into
mainfrom
fix/execution-vc-utc-timestamps

Conversation

@santoshkumarradha

Copy link
Copy Markdown
Member

Summary

On SQLite, execution VC created_at was written in the host's local zone, while the VC filters bind a UTC bound and compare the stored text. On any host west of UTC, a VC created a moment ago sorts before a created_after bound in the past and is dropped; ORDER BY created_at also misorders rows written under different offsets (DST changes, a changed TZ).

StoreExecutionVC          ... status, time.Now())        -> "2026-11-09 09:19:24-05:00"
StoreExecutionVCRecord    created := vc.CreatedAt          (host zone)
buildExecutionVCFilterClauses
  evc.created_at >= ?     filters.CreatedAfter.UTC()     -> "2026-11-09 14:18:24+00:00"
  text compare: "09:19 -05:00" < "14:18 +00:00"  => fresh VC excluded

Fix:

StoreExecutionVC          time.Now().UTC()
StoreExecutionVCRecord    vc.CreatedAt.UTC()  (time.Now().UTC() when zero)
createSchema (SQLite)
  runMigrations()
  normalizeExecutionVCTimestamps()   // once, recorded in schema_migrations
    rewriteColumnToUTC("execution_vcs", "created_at")
      rows not LIKE '____-__-__ __:__:__%+00:00' -> parse -> .UTC() -> UPDATE (5000-row batches)

Same class of bug as #1040 (execution timestamps compared lexically on SQLite). Postgres stores timestamptz and is unaffected; the rewrite only runs on SQLite.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change
  • Documentation update

Evidence

Before (origin/main, time.Local = UTC-5, new test TestStoreExecutionVCWritesCreatedAtInUTC):

--- FAIL: TestStoreExecutionVCWritesCreatedAtInUTC
    Messages: vc-scalar created_at stored as "2026-11-09 09:19:24.660631371-05:00"

With the stored-text assertion removed, the user-visible effect on main:

--- FAIL: TestStoreExecutionVCWritesCreatedAtInUTC
    Error: "[]" should have 2 item(s), but has 0     // ListExecutionVCs(CreatedAfter: now-1m)

After:

--- PASS: TestStoreExecutionVCWritesCreatedAtInUTC
--- PASS: TestNormalizeExecutionVCTimestampsRewritesHostZoneRows
ok   github.com/Agent-Field/agentfield/control-plane/internal/storage

TestNormalizeExecutionVCTimestampsRewritesHostZoneRows seeds a row written in a UTC-5 zone that is newer than a UTC row but sorts first as text, runs the rewrite, and asserts the newest VC lists first, the instant is unchanged, and the migration is recorded once and is a no-op on the second run.

Test plan

  • cd control-plane && go build ./...
  • go vet ./internal/storage/
  • go test ./internal/storage/ -count=1
  • go test ./internal/services/ ./internal/handlers/... -count=1

Merge Danger

Door: one-way — on first start with this build, SQLite execution_vcs.created_at values not already in UTC are rewritten in place (same instant, UTC spelling); an older build reads them back fine, but the original offset text is not restored.

Blast Radius: low — one column of one table on SQLite only, batched, recorded in schema_migrations so it runs once; unparseable values are left untouched.

StoreExecutionVC and StoreExecutionVCRecord wrote created_at with
time.Now() in the host's zone. SQLite keeps it as text, and the
created_after/created_before filters bind a UTC bound, so on a host west of
UTC a VC created a moment ago sorted before a bound in the past and was
filtered out; ORDER BY created_at likewise mixed rows written under
different offsets.

Writers now store UTC, and a one-time startup pass on SQLite rewrites rows
an older build stored in the host's zone, keeping each row's instant.
Postgres columns are timestamptz and are unaffected.
@santoshkumarradha
santoshkumarradha requested review from a team and AbirAbbas as code owners November 9, 2026 14:22
@santoshkumarradha santoshkumarradha mentioned this pull request Nov 9, 2026
19 of 22 tasks
@santoshkumarradha

Copy link
Copy Markdown
Member Author

Folded into #1171 so all fixes get one review; commits unchanged.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 88.20% 87.40% ↑ +0.80 pp 🟡
sdk-go 93.80% 92.00% ↑ +1.80 pp 🟢
sdk-python 94.73% 93.73% ↑ +1.00 pp 🟢
sdk-typescript 92.44% 90.42% ↑ +2.02 pp 🟢
web-ui 84.90% 84.79% ↑ +0.11 pp 🟡
aggregate 86.12% 85.75% ↑ +0.37 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 81 70.00% ❌
sdk-go 0 — ➖ no changes
sdk-python 0 — ➖ no changes
sdk-typescript 0 — ➖ no changes
web-ui 0 — ➖ no changes

❌ Patch gate failed

control-plane — 70.00% on 81 touched lines (24 uncovered):

File Patch coverage Missing lines
control-plane/internal/storage/local.go 33.3% 641, 642
control-plane/internal/storage/local_vc.go 71.8% 64, 503, 504, 510, 511, 514, 515, 540, 541, 548 …(+12 more)

How to fix

  1. For each file listed above, add tests that exercise the missing line numbers in this same PR.
  2. Re-run locally: ./scripts/coverage-summary.sh && ./scripts/patch-coverage-gate.sh.
  3. Do not lower min_patch in .coverage-gate.toml to silence this — the floor is the contract.

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.

2 participants