Skip to content

dx(platform): bake synthetic preview seed fixture as rolling release - #13575

Merged
ntindle merged 3 commits into
devfrom
dx/preview-seed-fixture-bake
Aug 6, 2026
Merged

dx(platform): bake synthetic preview seed fixture as rolling release#13575
ntindle merged 3 commits into
devfrom
dx/preview-seed-fixture-bake

Conversation

@ntindle

@ntindle ntindle commented Jul 14, 2026

Copy link
Copy Markdown
Member

Why / What / How

Why. Preview environments today boot with an empty database, so a PR's own migrations run against zero rows. That hides the failures that actually bite in production: NOT NULL columns added without a default, new UNIQUE constraints that collide on real data, and backfill/UPDATE migrations that only misbehave when there are rows to touch. We want previews to restore a populated database before the PR's migrations run, so those migrations are exercised against realistic data and fail in the preview instead of in prod. Batch/rollup previews (many approved PRs deployed together) benefit the most, since they stack multiple untested migrations onto one database.

This PR builds the bake half: a workflow that produces the fixture. The restore half (wiring previews to download and load it) is a follow-up infra-repo PR.

What. A new workflow .github/workflows/platform-preview-seed-fixture.yml that:

  • Triggers on pushes to dev touching backend/migrations/**, backend/test/**, or the workflow file; a weekly schedule backstop; and workflow_dispatch. A concurrency group coalesces overlapping bakes.
  • Spins up a throwaway pgvector/pgvector:pg15 service container (matches the platform's real Postgres major version — supabase/postgres:15.8.x — and provides the vector extension the docs-embedding migrations need plus the pg_trgm contrib module).
  • Sets up Python 3.12 + Poetry mirroring backend CI, runs prisma generate + prisma migrate deploy against the container using ?schema=platform.
  • Runs the four seeders in order, then pg_dumps the platform schema (custom format, gzipped) and writes a manifest.json, and publishes both as assets on a rolling release tagged preview-seed-fixture.

How.

  • Seeders (in order). test_data_creator.py (required — a failure fails the bake), then load-store-agents, e2e_test_data.py, and test_data_updater.py (best-effort — each failure emits a loud ::warning:: but does not abort the bake, per "tolerate soft-failures but require test_data_creator").
  • Dummy Supabase env (why it exists). ⚠️ Corrected — an earlier revision of this description described a GoTrue fallback that no longer applies, and it misled three automated reviewers. e2e_test_data.py on dev does not touch Supabase at all (grep -ci supabase autogpt_platform/backend/test/e2e_test_data.py0). Users are created by _ensure_auth_user() (e2e_test_data.py:219-244) through raw Better Auth inserts — prisma.authuser.create + prisma.authaccount.create with a bcrypt password hash — and the resulting id is handed to get_or_create_user(). SUPABASE_URL / SUPABASE_SERVICE_ROLE_KEY are set only because backend settings read them at import time and reject empty values; nothing in this job ever connects to them, which is why the URL is a deliberately unreachable http://localhost:54321. The inline comment at .github/workflows/platform-preview-seed-fixture.yml:83-86 already states this accurately.
  • Dump. pg_dump --format=custom -Z0 --schema=platform | gzip — the platform schema only, schema and data, including platform._prisma_migrations so the restore side only replays a PR's delta migrations. -Z0 disables pg_dump's internal compression so the outer gzip is the single compressor (restore = gunzip -c fixture.dump.gz | pg_restore). pg_dump/psql run inside the service container via docker exec so the client version always matches the server (the runner's bundled client can lag the server major and refuse the dump — confirmed: the local host client is pg14 and aborts against a pg15 server).
  • Manifest. manifest.json = { baked_at, dev_sha, migration_head }, where baked_at comes from git log of the checked-out commit (not wall-clock) and migration_head is the last applied migration name read from _prisma_migrations.
  • Publish. Create-or-update the rolling preview-seed-fixture release (gh release create / edit + upload --clobber); GITHUB_TOKEN with contents: write suffices on this public repo. Release notes are one paragraph including the security invariant.

🔒 Security invariant

This workflow holds NO cloud credentials and has NO read path to any real database. It runs entirely against a disposable Postgres service container. The fixture contains only Faker-generated synthetic rows plus the already-public marketplace agent exports already checked into autogpt_platform/backend/agents/. The Supabase env it sets is a dummy, unreachable http://localhost:54321 with a non-secret placeholder key — it cannot reach any real auth backend. No production data is ever touched, read, or dumped.

How the restore side consumes it (follow-up infra PR)

A preview's DB provisioning will: (1) ensure the vector extension exists (create the extensions schema + CREATE EXTENSION vector, since the fixture is platform-only), (2) gunzip -c fixture.dump.gz | pg_restore into a fresh DB, (3) then run the PR's prisma migrate deploy — which, because the restored _prisma_migrations already records the baked migrations, applies only the PR's new migrations against the populated tables.

Changes 🏗️

  • Add .github/workflows/platform-preview-seed-fixture.yml (bake + publish workflow). No application code, .env.default, or docker-compose.yml changes.

Checklist 📋

For configuration changes:

  • .env.default is updated or already compatible with my changes (no change needed — the workflow sets its own throwaway env)
  • docker-compose.yml is updated or already compatible with my changes (no change — uses a GHA service container)
  • I have included a list of my configuration changes in the PR description (under Changes)

Test plan

  • actionlint (with embedded shellcheck) passes on the workflow.
  • Local dry-run against a pgvector/pgvector:pg15 container: prisma migrate deploy applied all migrations; test_data_creator succeeded (116 users); load-store-agents loaded 17 agents; e2e_test_data completed with the GoTrue-less raw-UUID fallback firing for all 15 users; test_data_updater applied its mutations (it then errors in a pre-existing report-only KeyError('agentGraphId'), which is tolerated as a soft failure — pre-existing on dev, unrelated to this workflow).
  • pg_dump produced a valid custom-format archive (67 TABLE DATA + 6 materialized views, _prisma_migrations included), ~4.9 MB gzipped; manifest.json populated with baked_at / dev_sha / migration_head.
  • First real run on dev publishes/updates the preview-seed-fixture release (verified post-merge).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wj2E4V37tKjiGsXafYKDuk


Note

Low Risk
Changes are limited to a new synthetic-only CI workflow and a one-line test seeder column rename; no production app paths or real database access.

Overview
Adds platform-preview-seed-fixture.yml, a CI job that builds a fully synthetic platform schema dump for preview databases. It runs on eligible dev pushes, a weekly schedule, and manual dispatch; uses a throwaway pgvector/pg15 Postgres, prisma migrate deploy, then seeders (test_data_creator required; others best-effort with timeouts/warnings). It publishes fixture.dump.gz, restore-preamble.sql (vector/pg_trgm + schema before restore), and manifest.json to the rolling preview-seed-fixture GitHub release so previews can restore populated data before PR migrations run.

Also fixes test_data_updater.py sample output to read graph_id from mv_agent_run_counts (replacing agentGraphId) so the seeder’s verification step matches the current view definition.

Reviewed by Cursor Bugbot for commit eeb4f88. Bugbot is set up for automated code reviews on this repo. Configure here.

Add a GitHub Actions workflow that generates a fully synthetic preview-DB
seed fixture against a throwaway pgvector Postgres container and publishes
it as a rolling release asset (tag: preview-seed-fixture) with a manifest.

The workflow holds no cloud credentials and has no read path to any real
database: data is Faker-synthetic plus the already-public marketplace agent
exports checked into autogpt_platform/backend/agents/. Runs prisma migrate
deploy, then the four seeders (test_data_creator required; load-store-agents,
e2e_test_data GoTrue-less fallback, and test_data_updater best-effort with
loud warnings), then pg_dumps the platform schema (incl. _prisma_migrations)
as a gzipped custom-format archive alongside a manifest.json.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wj2E4V37tKjiGsXafYKDuk
@ntindle
ntindle requested a review from a team as a code owner July 14, 2026 20:07
@github-project-automation github-project-automation Bot moved this to 🆕 Needs initial review in AutoGPT development kanban Jul 14, 2026
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The workflow builds a synthetic Platform preview database, seeds and validates it, creates reproducible dump artifacts, and publishes them to a rolling GitHub release. A test expectation now uses the graph_id column.

Changes

Preview seed fixture bake

Layer / File(s) Summary
Workflow runtime and database setup
.github/workflows/platform-preview-seed-fixture.yml
Adds eligible triggers, concurrency, write permissions, a disposable PostgreSQL service, and synthetic environment settings.
Database migration and deterministic seeding
.github/workflows/platform-preview-seed-fixture.yml, autogpt_platform/backend/test/test_data_updater.py
Installs dependencies, generates Prisma, applies migrations, runs required and best-effort seeders, and updates the sample output to use graph_id.
Fixture export and release publishing
.github/workflows/platform-preview-seed-fixture.yml
Validates seeded users, creates the compressed schema dump, writes restoration metadata, and publishes the assets to the rolling preview-seed-fixture release.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant PostgresService
  participant SeedScripts
  participant GitHubRelease
  GitHubActions->>PostgresService: Apply Prisma migrations
  GitHubActions->>SeedScripts: Run required and best-effort seeders
  SeedScripts->>PostgresService: Insert preview data
  GitHubActions->>PostgresService: Validate users and export fixture dump
  GitHubActions->>GitHubRelease: Upload dump, manifest, and preamble
Loading

Suggested reviewers: bentlybro, kcze

Poem

A rabbit watched the workflow run,
Seeds filled tables one by one.
A dump and manifest crossed the gate,
The rolling release updated its state.
“Fresh preview data!” cried the rabbit.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main workflow change: baking a synthetic preview seed fixture as a rolling release.
Description check ✅ Passed The description is directly related to the workflow and test-data changes and explains their purpose, implementation, security, and testing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dx/preview-seed-fixture-bake

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (3)
.github/workflows/platform-preview-seed-fixture.yml (3)

15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Trigger paths omit agent-export and dependency changes.

The push filter covers migrations/** and test/** but not autogpt_platform/backend/agents/** (referenced by the load-store-agents seeder/dump comments) or pyproject.toml/poetry.lock. Changes there won't rebake the fixture until the weekly cron fires, so the published fixture can lag behind marketplace-export or dependency updates for up to a week.

Suggested path additions
     paths:
       - "autogpt_platform/backend/migrations/**"
       - "autogpt_platform/backend/test/**"
+      - "autogpt_platform/backend/agents/**"
+      - "autogpt_platform/backend/pyproject.toml"
+      - "autogpt_platform/backend/poetry.lock"
       - ".github/workflows/platform-preview-seed-fixture.yml"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/platform-preview-seed-fixture.yml around lines 15 - 21,
Update the push path filters in the workflow trigger to include
autogpt_platform/backend/agents/** and the repository dependency manifests
pyproject.toml and poetry.lock, while preserving the existing migration, test,
and workflow paths.

53-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Pin the Postgres service image to a specific tag/digest.

pgvector/pgvector:pg15 is a floating tag; an upstream update could silently change the pgvector/pg_trgm build baked into every future run, undermining the reproducibility goal of a "baked" fixture (manifest ties baked_at/dev_sha/migration_head to a commit, but not to the exact Postgres image).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/platform-preview-seed-fixture.yml around lines 53 - 54,
Update the postgres service image configuration to use an immutable, specific
image tag or digest instead of the floating pgvector/pgvector:pg15 tag. Preserve
the PostgreSQL 15 and required pgvector/pg_trgm extensions while ensuring future
workflow runs resolve the exact same image.

123-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Repeated best-effort-seeder boilerplate.

The || { echo "::warning..."; exit 0; } pattern is duplicated across the three best-effort seeder steps. Since this is static YAML with only three repeats and the explicit warning message is intentional, this is optional — but a step-level continue-on-error: true plus a single follow-up "report failures" step would avoid the copy-pasted block if you want less duplication.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/platform-preview-seed-fixture.yml around lines 123 - 142,
The three seeder steps—“Seed: load-store-agents,” “Seed: e2e_test_data,” and
“Seed: test_data_updater”—duplicate best-effort failure handling. If reducing
duplication, replace each inline `|| { ... }` block with step-level
`continue-on-error: true`, then add one follow-up step that reports which
seeders failed while preserving workflow continuation and warning behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/platform-preview-seed-fixture.yml:
- Around line 86-87: Update the actions/checkout step in the platform preview
seed fixture workflow to set persist-credentials to false. Keep the existing
checkout behavior and explicitly supplied GH_TOKEN for the release step
unchanged.

---

Nitpick comments:
In @.github/workflows/platform-preview-seed-fixture.yml:
- Around line 15-21: Update the push path filters in the workflow trigger to
include autogpt_platform/backend/agents/** and the repository dependency
manifests pyproject.toml and poetry.lock, while preserving the existing
migration, test, and workflow paths.
- Around line 53-54: Update the postgres service image configuration to use an
immutable, specific image tag or digest instead of the floating
pgvector/pgvector:pg15 tag. Preserve the PostgreSQL 15 and required
pgvector/pg_trgm extensions while ensuring future workflow runs resolve the
exact same image.
- Around line 123-142: The three seeder steps—“Seed: load-store-agents,” “Seed:
e2e_test_data,” and “Seed: test_data_updater”—duplicate best-effort failure
handling. If reducing duplication, replace each inline `|| { ... }` block with
step-level `continue-on-error: true`, then add one follow-up step that reports
which seeders failed while preserving workflow continuation and warning
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4db50c9f-b8d1-48eb-9a61-3e39caa11e00

📥 Commits

Reviewing files that changed from the base of the PR and between d709943 and c7f6de4.

📒 Files selected for processing (1)
  • .github/workflows/platform-preview-seed-fixture.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Check PR Status
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (typescript)
🧰 Additional context used
🪛 Betterleaks (1.6.1)
.github/workflows/platform-preview-seed-fixture.yml

[high] 83-83: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 Checkov (3.3.8)
.github/workflows/platform-preview-seed-fixture.yml

[medium] 72-73: Basic Auth Credentials

(CKV_SECRET_4)

🪛 zizmor (1.26.1)
.github/workflows/platform-preview-seed-fixture.yml

[warning] 86-87: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🔇 Additional comments (1)
.github/workflows/platform-preview-seed-fixture.yml (1)

89-121: LGTM!

Also applies to: 150-160, 182-204, 207-228

Comment thread .github/workflows/platform-preview-seed-fixture.yml
Bentlybro
Bentlybro previously approved these changes Jul 17, 2026
@github-project-automation github-project-automation Bot moved this from 🆕 Needs initial review to 👍🏼 Mergeable in AutoGPT development kanban Jul 17, 2026
…ling tag

Review findings on #13575: (1) schedule/dispatch runs execute from the
default branch (master) — checkout now pins ref: dev and DEV_SHA comes
from git rev-parse of the checkout, not GITHUB_SHA, so the fixture can
never silently bake master's schema; (2) persist-credentials: false —
only the release step needs a token, via env; (3) the rolling release
tag is retargeted at the baked dev commit on every update so release
metadata matches the published assets.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wj2E4V37tKjiGsXafYKDuk
Bentlybro
Bentlybro previously approved these changes Jul 17, 2026
@ntindle

ntindle commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

/review

@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #13575 at 0646c6c.

autogpt-pr-reviewer[bot]
autogpt-pr-reviewer Bot previously approved these changes Aug 4, 2026

@autogpt-pr-reviewer autogpt-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📋 Automated Review — PR #13575

PR #13575 — dx(platform): bake synthetic preview seed fixture as rolling release
Author: ntindle | Files: 1

🎯 Verdict: APPROVE

PR Description Quality

✅ Has Why + What + How — the description explains the goal (populated synthetic fixture so preview migrations run against realistic data), the scope (bake half only; restore wiring is a follow-up infra PR), and the mechanics (seeders → pg_dump → manifest → rolling release). It also honestly flags one intentionally-deferred, post-merge-only verification item.

What This PR Does

Adds a single new GitHub Actions workflow (.github/workflows/platform-preview-seed-fixture.yml) that spins up a throwaway pgvector/pgvector:pg15 container, runs prisma migrate deploy and four synthetic-data seeders, dumps the platform schema to a gzipped custom-format archive, writes a manifest.json, and publishes both as a rolling preview-seed-fixture GitHub release. The point is to give preview environments a realistic, populated database so schema migrations get exercised against actual rows (unique collisions, backfills) instead of silently passing against empty tables. No application code, API, or UI is touched.

Specialist Findings

🛡️ Security ✅ — Verified the core invariant: seeders are Faker-driven and the one external call (supabase.auth.admin.create_user) falls back to a synthetic UUID, so there is no real-DB read path. No pull_request_target/untrusted-input injection surface, persist-credentials: false, and contents: write is least-privilege. The hardcoded ENCRYPTION_KEY (:82) is a documented, already-public non-prod test key with # pragma: allowlist secret.
🔵 Key-shaped literal at :82 and curl | python3 poetry install at :111 are informational hardening notes only.

🏗️ Architecture ✅/⚠️ — Clean, well-scoped, reuses existing CI conventions (ref: dev pin, docker exec for pg-version parity). Main gaps are in the bake↔restore contract: soft-fail seeders can publish a silently-incomplete fixture (:176), the manifest carries no integrity hash of fixture.dump.gz (:207), and the release update is non-atomic under cancel-in-progress (:230).
🟠 Completeness guard only checks User >= 1; other critical tables can be empty and still publish.

Performance ✅ — No runtime/hot-path surface; all cost is CI wall-clock. Two optional efficiency notes: the poetry cache covers only ~/.cache/pypoetry and not the resolved venv (:108, ~1–3 min/run), and gzip -9 on a ~5 MB artifact (:200) trades CPU for negligible size gain.

🧪 Testing ⚠️ — No restore round-trip is exercised before publish (:170), so a truncated/corrupt archive would ship and only fail in every preview env. The User >= 1 gate (:176) is near-tautological (creator seeds ~116 users). Soft-fail seeders (:131, :138) can publish a degraded fixture indistinguishable from a healthy one, and test_data_updater reportedly always partially fails on a known KeyError('agentGraphId') (:147).

📖 Quality ✅ — Rated A-; unusually well-documented (every non-obvious flag explained). Minor: three duplicated soft-fail seeder blocks (:131), a ~600-char single-line NOTES string (:226), and the image tag hardcoded in two places (:54, :161).

📦 Product ✅/⚠️ — Matches its stated scope exactly, no scope creep. Two signal-quality gaps: partial fixtures publish with no consumer-visible signal beyond a buried ::warning:: (:176), and marketplace-agent exports under backend/agents/** aren't in the trigger paths (:18), so the fixture can lag up to a week.

📬 Discussion ⚠️ — Could not fetch live CI/thread state (invalid reviewer token). The recoverable Cursor Bugbot review rated the change Low Risk with no blocking findings, and its stated risk (bad/empty fixture) is already guarded in-code. Note the new workflow only runs on pushes to dev, so it will not execute on this PR branch. One cosmetic doc note: the (master) parenthetical at :73/:91 names a branch that may not be the actual default.

🔎 QA ✅ — Independently reproduced every mechanic against a live pg15 server: user-count gate → 15, migration-head query resolves, the exact pg_dump --format=custom -Z0 | gzip produced a valid 419K custom-format archive (538 TOC entries, 69 TABLE DATA, 6 materialized views) with _prisma_migrations included, migrations self-provision the extensions schema on plain pgvector, and the empty-fixture negative gate correctly refuses to publish at 0 users. QA PASS.

🟠 Should Fix

  1. No restore round-trip before publish (.github/workflows/platform-preview-seed-fixture.yml:170) — the fixture is published without ever being restored, so a corrupt/truncated archive fails only downstream in every preview. Add a gunzip -c fixture.dump.gz | pg_restore into a throwaway DB as a publish gate. (Flagged by: testing)
  2. Silent partial-fixture publish (.github/workflows/platform-preview-seed-fixture.yml:176) — soft-fail seeders + a User >= 1-only guard let a fixture missing store/agent/e2e rows ship as the rolling "good" asset, defeating the exact false-negative this PR exists to remove. Assert minimum row counts on other critical tables (e.g. StoreListing, AgentGraph) and/or record per-seeder status + row counts in manifest.json. (Flagged by: architect, testing, product, ui-reviewer — 4 specialists)
  3. Manifest has no fixture integrity hash (.github/workflows/platform-preview-seed-fixture.yml:207) — the restore side pipes a network-downloaded asset straight into pg_restore with no way to detect a truncated upload. Add sha256sum fixture.dump.gz to the manifest. (Flagged by: architect)

🟡 Nice to Have

  1. Non-atomic release update (.github/workflows/platform-preview-seed-fixture.yml:230) — under cancel-in-progress, a cancel between gh release edit and asset upload can leave notes/tag advertising a SHA whose bytes weren't replaced. Upload assets before editing notes/retargeting the tag. (architect, ui-reviewer)
  2. Trigger on marketplace exports (.github/workflows/platform-preview-seed-fixture.yml:18) — add autogpt_platform/backend/agents/** to push paths so store-agent changes rebake before the weekly cron. (product)
  3. Cache the poetry venv (.github/workflows/platform-preview-seed-fixture.yml:108) and drop gzip -9-6/pigz (:200) — optional CI wall-clock savings. (performance)
  4. Track/fix the known test_data_updater KeyError('agentGraphId') (.github/workflows/platform-preview-seed-fixture.yml:147) — a permanently-half-failing updater under-exercises backfill/UPDATE migrations. (testing)

🔵 Nits

  1. Misleading (master) comment (.github/workflows/platform-preview-seed-fixture.yml:73/:91) — name the actual default branch or say "the default branch". (discussion, architect)
  2. Duplicated soft-fail seeder blocks (.github/workflows/platform-preview-seed-fixture.yml:131) and single-line NOTES string (:226) — collapse to a shell function / heredoc for readability. (quality)
  3. Image tag hardcoded twice (.github/workflows/platform-preview-seed-fixture.yml:54, :161) — hoist to a job-level env var. (quality)

Human Review Needed

NO — this is a self-contained CI workflow, not a change to the application's authentication/authorization, secret storage, or a service trust boundary; the security specialist verified least-privilege token scope, no real-DB read path, and no new production exposure.

Risk Assessment

Merge risk: LOW | Rollback: EASY (delete one workflow file; no runtime/app surface)

CI Status

Local harness: ✅ 5/5 checks pass (frontend lint, backend lint, frontend typecheck, frontend unit tests, frontend build). GitHub CI: UNVERIFIED — live check status could not be fetched (invalid reviewer token). Note: the new workflow only triggers on pushes to dev, so it will not run on this PR branch itself; its first real execution is post-merge, as the author's checklist states.


UI Testing — Variant Results

✅ local: CI-only workflow; every DB/pg_dump/manifest/gate mechanic reproduces correctly against a matching pg15 server and the archive is valid and restorable with _prisma_migrations included — QA PASS with two low-severity robustness notes.

  • low: The empty-fixture guard only checks platform."User" > 0. Because load-store-agents, e2e_test_data, and test_data_updater are soft-fail (::warning:: then exit 0), a fixture with users but no store agents / graphs can still publish, so restore-side migrations touching those tables wouldn't be exercised against real rows.
  • low: concurrency: cancel-in-progress: true can cancel a bake mid-publish. Since fixture.dump.gz and manifest.json are uploaded in separate gh release upload operations, a cancellation between them can leave the rolling release with a new fixture but a stale manifest (mismatched migration_head).

✅ hosted: CI-only workflow whose every DB/dump/manifest/guard command I independently reproduced against the identical pg15.8 environment — all pass, both publish guards fail-closed correctly, no defects found.

Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
…ore failures

Fixes validated by a full local bake + restore of this workflow:

- Cap e2e_test_data.py and test_data_updater.py with GNU `timeout 300`:
  this job runs no RabbitMQ/Redis services, so the backend's conn_retry
  (max_retry=100, 30s-capped backoff) hangs ~48 min acquiring queue/cache
  connections and the 45-min job timeout kills the run before dump/publish.
  The soft-failure `|| { ...; exit 0; }` catches nonzero exits, not hangs;
  timeout converts a hang into exit 124.
- Publish restore-preamble.sql as a third release asset: pg_dump
  --schema=platform omits CREATE EXTENSION for extensions installed into
  the platform schema (vector, per 20260605120000_fix_vector_extension_
  search_path, and pg_trgm), so a fresh-database restore fails with
  'type "platform.vector" does not exist'. Consumers apply the preamble
  before pg_restore and must not use --exit-on-error (the dump's own
  CREATE SCHEMA collides benignly). Documented in the header comment,
  manifest.json (restore_preamble) and release notes.
- test_data_updater.py: mv_agent_run_counts exposes graph_id (since
  20260304123456_update_store_views), not agentGraphId — the verification
  print raised KeyError on every run.
- Refresh the stale SUPABASE_URL env comment: e2e_test_data.py now creates
  users via Better Auth (raw Prisma), not supabase-py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ntindle
ntindle requested a review from a team as a code owner August 5, 2026 01:23
@ntindle
ntindle requested review from Bentlybro and kcze and removed request for a team August 5, 2026 01:23
@ntindle

ntindle commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions github-actions Bot added the platform/backend AutoGPT Platform - Back end label Aug 5, 2026
@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #13575 at eeb4f88.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit eeb4f88. Configure here.

Comment thread autogpt_platform/backend/test/test_data_updater.py

@capy-ai capy-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Added 1 comment

Comment thread .github/workflows/platform-preview-seed-fixture.yml

@coderabbitai coderabbitai 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.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/platform-preview-seed-fixture.yml:
- Line 261: Update the NOTES release text in the fixture metadata to clarify
that the bake contains no cloud database or provider credentials, rather than
claiming it holds no cloud credentials. Preserve the existing statement that it
has no read path to any real database and runs against synthetic data.
- Around line 204-220: Keep restore failures strict by removing the duplicate
platform schema creation from the restore preamble step at
.github/workflows/platform-preview-seed-fixture.yml:204-220, then retain
--exit-on-error for pg_restore. Update the restore instructions at
.github/workflows/platform-preview-seed-fixture.yml:14-22 and :243-244, and the
release-notes text at :261-261, so none advises omitting --exit-on-error as a
general rule.
- Around line 217-218: Update the fixture restore validation around the vector
and pg_trgm extension setup to verify both extensions already reside in the
platform schema and match compatible versions before invoking pg_restore. Fail
the restore before publishing when placement or versions are incompatible, and
record the baked extension versions in manifest.json.

In `@autogpt_platform/backend/test/test_data_updater.py`:
- Line 336: Issue: the lookup-key bug lacks regression coverage for the returned
graph_id shape. Add a focused pytest regression test in test_data_updater.py
that reproduces the graph_id result-shape failure, mark it with
`@pytest.mark.xfail` initially, and verify the lookup and display path involving
row['graph_id']; after the implementation fix passes, remove the xfail marker
while preserving the test.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 413ca64f-8d5e-4098-a95f-14833f31f2fa

📥 Commits

Reviewing files that changed from the base of the PR and between 0646c6c and eeb4f88.

📒 Files selected for processing (2)
  • .github/workflows/platform-preview-seed-fixture.yml
  • autogpt_platform/backend/test/test_data_updater.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
  • GitHub Check: check API types
  • GitHub Check: Seer Code Review
  • GitHub Check: type-check (3.12)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.13)
  • GitHub Check: type-check (3.13)
  • GitHub Check: type-check (3.11)
  • GitHub Check: test (3.11)
  • GitHub Check: types
  • GitHub Check: lint
  • GitHub Check: end-to-end tests
  • GitHub Check: Check PR Status
  • GitHub Check: Analyze (typescript)
  • GitHub Check: Analyze (python)
  • GitHub Check: Check PR Status
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (typescript)
🧰 Additional context used
📓 Path-based instructions (4)
autogpt_platform/backend/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development

autogpt_platform/backend/**/*.py: Use poetry run ... command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies like openpyxl
Use absolute imports with from backend.module import ... for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoid hasattr/getattr/isinstance for type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no # type: ignore, # noqa, # pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use %s for deferred interpolation in debug log statements for efficiency; use f-strings elsewhere for readability (e.g., logger.debug("Processing %s items", count) vs logger.info(f"Processing {count} items"))
Sanitize error paths by using os.path.basename() in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Use transaction=True for Redis pipelines to ensure atomicity on multi-step operations
Use max(0, value) guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...

Files:

  • autogpt_platform/backend/test/test_data_updater.py
autogpt_platform/backend/**/test/**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use snapshot testing with '--snapshot-update' flag in backend tests when output changes; always review with 'git diff'

Files:

  • autogpt_platform/backend/test/test_data_updater.py
autogpt_platform/{backend,autogpt_libs}/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Format Python code with poetry run format

Files:

  • autogpt_platform/backend/test/test_data_updater.py
autogpt_platform/backend/**/test_*.py

📄 CodeRabbit inference engine (autogpt_platform/AGENTS.md)

Create a failing test first using @pytest.mark.xfail decorator (backend) when fixing a bug or adding a feature, then implement the fix and remove the xfail marker

Files:

  • autogpt_platform/backend/test/test_data_updater.py
🧠 Learnings (11)
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-03-19T15:10:50.676Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12483
File: autogpt_platform/backend/backend/copilot/tools/test_dry_run.py:298-303
Timestamp: 2026-03-19T15:10:50.676Z
Learning: When using Python’s `unittest.mock.patch` in tests, choose the patch target based on how the imported name is resolved:
- If the code under test uses an **eager/module-level import** (e.g., `from foo.bar import baz` at module top), patch **the module where the name is looked up** (i.e., where it is used in the SUT), e.g. `patch("mymodule.baz")`.
- If the code under test uses a **lazy import** executed later (e.g., `from foo.bar import baz` inside a function/branch), patch **the source module** (e.g., `patch("foo.bar.baz")`) because the late `from ... import` will read the (potentially patched) name from the source module at call time.

For a concrete example: if `simulate_block` is imported inside an `if dry_run:` block in the SUT, then the correct test patch target is the source module path for `simulate_block` as it exists at call time (e.g., `patch("backend.executor.simulator.simulate_block")`), not the test file’s import location.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
📚 Learning: 2026-06-11T19:39:10.493Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13337
File: autogpt_platform/backend/backend/copilot/graphiti/reranker.py:0-0
Timestamp: 2026-06-11T19:39:10.493Z
Learning: In the Significant-Gravitas/AutoGPT Python backend, when calling the OpenAI Python client `chat.completions.create`, construct the `messages` payload using the concrete typed-dict variants from `openai.types.chat` (e.g., `ChatCompletionSystemMessageParam`, `ChatCompletionUserMessageParam`, etc.) rather than trying to instantiate `ChatCompletionMessageParam` directly. `ChatCompletionMessageParam` is a `Union` alias and is not constructible, so `ChatCompletionMessageParam(role=..., content=...)` should fail type checking. Build each message element with the appropriate concrete typed dict and then annotate the resulting list as `list[ChatCompletionMessageParam]` (e.g., `messages: list[ChatCompletionMessageParam] = [ChatCompletionSystemMessageParam(...), ...]`).

Applied to files:

  • autogpt_platform/backend/test/test_data_updater.py
🔇 Additional comments (4)
.github/workflows/platform-preview-seed-fixture.yml (4)

83-86: LGTM!


135-140: LGTM!


152-157: LGTM!

Also applies to: 159-162


258-260: LGTM!

Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread autogpt_platform/backend/test/test_data_updater.py
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.24%. Comparing base (c136f48) to head (eeb4f88).
⚠️ Report is 82 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #13575      +/-   ##
==========================================
+ Coverage   75.96%   76.24%   +0.27%     
==========================================
  Files        2683     2760      +77     
  Lines      203829   211757    +7928     
  Branches    19618    20311     +693     
==========================================
+ Hits       154841   161447    +6606     
- Misses      44681    45782    +1101     
- Partials     4307     4528     +221     
Flag Coverage Δ
platform-backend 83.42% <ø> (+0.63%) ⬆️
platform-frontend-e2e 30.96% <ø> (-0.46%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Platform Backend 83.42% <ø> (+0.63%) ⬆️
Platform Frontend 48.67% <ø> (-1.24%) ⬇️
AutoGPT Libs ∅ <ø> (∅)
Classic AutoGPT 28.43% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@autogpt-pr-reviewer autogpt-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📋 Automated Review — PR #13575

PR #13575 — dx(platform): bake synthetic preview seed fixture as rolling release
Author: ntindle | Files: 2

🎯 Verdict: APPROVE

PR Description Quality

⚠️ Partial — Why + What + How are all present and unusually detailed, but two spots are stale: the What section lists only fixture.dump.gz + manifest.json and omits the third published asset restore-preamble.sql (essential to the restore contract), and an inline workflow comment contradicts the description's own account of how e2e_test_data.py seeds users. Reconcile before merge so the follow-up infra-PR author isn't surprised.

What This PR Does

Adds a CI-only GitHub Actions workflow that spins up a throwaway pgvector/pg15 Postgres container, runs prisma migrate deploy + four synthetic seeders, dumps the populated platform schema, and publishes it (fixture.dump.gz + manifest.json + restore-preamble.sql) as a rolling preview-seed-fixture prerelease. This lets future preview environments restore realistic, populated data before a PR's own migrations run — surfacing migration breakage that empty DBs miss. It also fixes a one-line pre-existing KeyError in test_data_updater.py (row['agentGraphId']row['graph_id']) to match the renamed mv_agent_run_counts view column.

Specialist Findings

🛡️ Security ✅ — Core invariant holds: the job runs only against a disposable container, holds no cloud/DB credentials, consumes no untrusted input, and has no pull_request_target or github.event.* interpolation (no script-injection vector). The contents: write grant is correctly scoped to one dedicated rolling tag. Only low/informational notes.
🔵 Third-party actions pinned to mutable major tags (platform-preview-seed-fixture.yml:95,106,111) and Poetry installed via curl | python3 (:120) — consistent with existing repo convention, not a regression.

🏗️ Architecture ✅ — Clean producer/consumer split, correct docker exec for client/server major matching, sound --schema=platform + extension preamble handling, and good fail-fast release gating. Confirmed the graph_id fix retires real pre-existing debt.
🟠 Misleading comment at platform-preview-seed-fixture.yml:83 — claims e2e_test_data.py "no longer talks to Supabase at all," but the current code still calls get_supabase() / auth.admin.create_user and relies on the dead-port fallback. A maintainer trusting it could delete the dummy SUPABASE_URL, which supabase-py requires non-empty, breaking the seeder. (Flagged by: architect, quality, product — 3 specialists)

Performance ✅ — No production or hot-path code. Workflow is well-tuned (concurrency cancel-in-progress, -Z0+single gzip, 45-min cap, timeout guards). Findings are CI-efficiency only: venv not cached so poetry install rebuilds every run (:123), and best-effort seeders can burn up to ~600s of timeout-waiting per bake (:154). Non-blocking.

🧪 Testing ✅/⚠️ — The graph_id fix is verified correct against the view definition and carries zero regression risk. The gap is the publish gate: the only completeness check is platform."User" >= 1 (:190), while all three best-effort seeders can silently || exit 0, allowing a fixture with users but zero store/graph/execution rows to publish as the rolling asset.
🟠 Weak pre-publish completeness assertion (:190) — see Should Fix.

📖 Quality ✅ — Readability B. Thorough runbook-style comments and a trivial, correct field rename. Deductions for the contradictory comment (above) and 3× copy-pasted soft-failure idiom (:143), which a small shell helper would DRY up.

📦 Product ✅ — Implementation matches intent; guards fail closed. Two doc-consistency notes (stale asset list, contradictory Supabase comment) plus a low note that a rolling prerelease on the highly-watched public repo may notify release watchers / look like a product release — title already says "(rolling)", flagging as a conscious choice.

📬 Discussion ⚠️ — No merge conflicts; CI green/pending with no failures. But REVIEW_REQUIRED with only dismissed stale human reviews (Bentlybro, 3 commits since). Open unaddressed bot concerns: weekly schedule cron fires only from the default branch (master) while the workflow lives on dev, so the advertised weekly backstop is a no-op until it reaches master (:34); and no pre-publish restore validation (:184). The Cursor Bugbot graph_id objection is a verified false positive and should simply be resolved.

🔎 QA ✅ — The workflow is GHA-only and not locally executable (honestly noted). The one runtime-testable change was verified against the live DB: mv_agent_run_counts exposes columns graph_id, run_count (view def SELECT run."agentGraphId" AS graph_id), so row['graph_id'] is correct and the old key would KeyError. Auth negatives (401 no/garbage token, 200 valid) and stack smoke all passed.

🟠 Should Fix

  1. Misleading Supabase comment could break the seeder if trusted (.github/workflows/platform-preview-seed-fixture.yml:83) — The comment says e2e_test_data.py "no longer talks to Supabase," but the code still constructs a client and calls auth.admin.create_user, depending on the dead localhost:54321 port to trigger a raw-UUID fallback. Rewrite to describe the real behavior and note the dummy SUPABASE_URL must be non-empty. (Flagged by: architect, quality, product — 3 specialists)
  2. Pre-publish completeness gate is too weak (.github/workflows/platform-preview-seed-fixture.yml:190) — Only User >= 1 is asserted while seeders soft-fail via || exit 0, so a fixture missing store listings/graphs/executions can still ship as the rolling asset, defeating the workflow's purpose. Assert minimum row counts on a few representative tables (AgentGraph, StoreListing, AgentGraphExecution) and/or record them in manifest.json. (Flagged by: testing, discussion — 2 specialists)
  3. Reconcile stale PR description + confirm schedule-trigger behavior — Update the What section to list restore-preamble.sql as a third asset, and reply to the open thread confirming that the weekly schedule won't fire from dev (default branch is master) is intended or land the file on master. Also resolve the false-positive Cursor Bugbot thread on test_data_updater.py:336. (Flagged by: product, discussion — 2 specialists)

🟡 Nice to Have

  1. Cache the resolved virtualenv (platform-preview-seed-fixture.yml:123) — Set virtualenvs.in-project true + cache .venv keyed on poetry.lock so cache hits skip reinstall. (performance)
  2. Add a pre-publish restore round-trip (platform-preview-seed-fixture.yml:184) — pg_restore into a throwaway DB before publishing to catch corrupt/truncated archives; acknowledged as deferrable to the restore-side follow-up PR. (testing, discussion)
  3. Fixture-integrity hash in manifest.json — record a checksum so consumers can verify the asset before pg_restore. (security)

🔵 Nits

  1. SHA-pin third-party actions (platform-preview-seed-fixture.yml:95,106,111) — matches repo convention, informational only. (security)
  2. Hardcoded image pgvector/pgvector:pg15 in 3 places (platform-preview-seed-fixture.yml:63,175) — hoist into one env var to remove silent-drift coupling. (architect)
  3. DRY the 3× soft-failure idiom (platform-preview-seed-fixture.yml:143) — extract a shell helper. (quality)

QA Screenshots

Screenshot Description
stack smoke: marketplace renders Stack renders after checkout; marketplace loads (89KB) ✅

Human Review Needed

NO — This is a CI-only bake workflow plus a one-line test-seeder fix. It touches no system authentication, credential storage, or service trust boundary (the only credentials involved are documented non-production synthetic placeholders against a disposable container). The findings are documentation and CI-robustness improvements, not security-boundary changes.

Risk Assessment

Merge risk: LOW | Rollback: EASY (delete the workflow file and revert the one-line seeder change; the rolling release is disposable and consumed by no shipped code yet)

Local Harness

✅ All 5 checks pass — frontend lint, backend poetry run lint, frontend types, frontend test:unit, frontend build. GitHub CI: UNVERIFIED from this review (per discussion specialist, ~38/42 GitHub checks green with test (3.11/3.12/3.13) still running and no failures at time of report).


UI Testing — Variant Results

✅ local: Matview key rename verified correct against live DB (mv_agent_run_counts exposes graph_id, not agentGraphId); the workflow is CI-only and not locally executable but has no runtime regressions.

✅ hosted: The graph_id rename matches the live mv_agent_run_counts schema (old agentGraphId key provably KeyErrored, new graph_id works) and all DB objects/guards the bake workflow depends on are verified present.

Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread autogpt_platform/backend/test/test_data_updater.py

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/platform-preview-seed-fixture.yml:
- Line 63: Update the PostgreSQL service configuration in the workflow to use an
immutable image digest instead of the mutable pg15 tag, ensure the pg_dump
container-resolution logic uses that same digest-pinned reference, and record
the exact pinned image reference in manifest.json for restore consumers.
- Around line 154-162: Add a GNU timeout secondary kill duration to both seeder
commands, `e2e_test_data.py` and `test_data_updater.py`, so processes that do
not exit after the initial 300-second timeout are forcibly terminated. Preserve
the existing warning and soft-failure continuation behavior.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 470d999b-d0b2-43f5-9978-2af85e70bb2a

📥 Commits

Reviewing files that changed from the base of the PR and between d709943 and eeb4f88.

📒 Files selected for processing (2)
  • .github/workflows/platform-preview-seed-fixture.yml
  • autogpt_platform/backend/test/test_data_updater.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • autogpt_platform/backend/test/test_data_updater.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.11)
  • GitHub Check: end-to-end tests
  • GitHub Check: Check PR Status
  • GitHub Check: Check PR Status
🧰 Additional context used
🪛 Betterleaks (1.7.3)
.github/workflows/platform-preview-seed-fixture.yml

[high] 91-91: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 Checkov (3.3.9)
.github/workflows/platform-preview-seed-fixture.yml

[medium] 81-82: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (1)
.github/workflows/platform-preview-seed-fixture.yml (1)

95-111: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Other (CWE-829): Inclusion of Functionality from Untrusted Control Sphere

Reachability: External

Pin the GitHub Actions to full commit SHAs.

actions/checkout@v6, actions/setup-python@v5, and actions/cache@v5 are mutable tags. If an attacker retargets or compromises one tag, its code runs with this job's contents: write token and can replace the rolling release assets. persist-credentials: false does not prevent an action from accessing github.token. Pin each action to a verified full commit SHA and use automated dependency updates for future revisions. GitHub documents that a full SHA is the immutable action reference and that actions can access GITHUB_TOKEN through github.token. (docs.github.com)

Comment thread .github/workflows/platform-preview-seed-fixture.yml
Comment thread .github/workflows/platform-preview-seed-fixture.yml
@ntindle
ntindle added this pull request to the merge queue Aug 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 5, 2026
@ntindle
ntindle added this pull request to the merge queue Aug 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 5, 2026
@ntindle

ntindle commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

/review

@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #13575 at eeb4f88.

ntindle added a commit to djpjronline-netizen/AutoGPT that referenced this pull request Aug 6, 2026
…meout (Significant-Gravitas#13780)

### Why / What / How

**Why.** Roughly half of all `dev` merge-queue enqueues were ejecting
PRs whose own checks were fully green, and the time-to-ejection
clustered hard around 17-22 minutes. Observed live on 2026-08-04/05:

| PR | enqueued | ejected | elapsed | PR's own checks | actual cause |
|---|---|---|---|---|---|
| Significant-Gravitas#13434 | 02:55 | 03:12 | ~17 min | all green | real test failures
(credit suite) |
| Significant-Gravitas#13434 | 04:07 (2nd) | 04:29 | ~22 min | all green | **20m job
timeout** |
| Significant-Gravitas#13743 | 18:33 | 18:54 | ~21 min | all green | **20m job timeout** |
| Significant-Gravitas#13575 | 19:05 | 19:26 | ~21 min | all green | **20m job timeout** |

(A fifth ejection, Significant-Gravitas#13764 at 03:21→03:42, was also the 20m timeout.)

**What.** The `test` job in `platform-backend-ci.yml` had
`timeout-minutes: 20`, which sits *below* the job's real p95 runtime.
GitHub reports a `timeout-minutes` kill as conclusion **`cancelled`**,
not `failure` — which is why this was invisible when reading the
merge-queue runs. `.github/workflows/scripts/check_actions_status.py`
treats any conclusion outside `success`/`skipped`/`neutral` as a
failure, so a timed-out `test` leg makes **`Check PR Status`** fail, and
GitHub ejects the PR from the merge queue.

**How.** Raise the cap so it guards against a genuinely hung job instead
of acting as a performance budget, and remove the single largest source
of setup variance from the job.

### Root-cause evidence

The three ~21-22 min ejections are all the same mechanism. GitHub's own
annotation on the cancelled job (`check-runs/92414409798/annotations`):

> `failure | The job has exceeded the maximum execution time of 20m0s`

Every timed-out leg died at **1217-1222s** — exactly the 20m0s cap:

| run | merge group | leg | job total | pytest step |
|---|---|---|---|---|
| 31037832829 | pr-13575 | `test (3.12)` | 1222s | 674s (killed) |
| 31035323338 | pr-13743 | `test (3.12)` | 1219s | killed |
| 30974258262 | pr-13434 | `test (3.12)` | 1217s | 1049s (killed) |
| 30972001003 | pr-13764 | `test (3.11)` | 1218s | 1054s (killed) |

These were healthy runs killed mid-suite, not hangs — the pytest step
was still actively emitting `PASSED` lines when the runner pulled the
plug.

Sibling matrix legs in the *same* runs passed comfortably, which is what
makes this look like a "flake":

- run 31037832829: `test (3.11)` 956s ✅, `test (3.13)` 975s ✅, `test
(3.12)` **1222s ❌**
- run 30972001003: `test (3.12)` 949s ✅, `test (3.13)` 898s ✅, `test
(3.11)` **1218s ❌**

Two independent variance sources push a leg over the line:

1. **The suite's own runtime.** ~10.6k tests run **serially** —
`pytest-xdist` is not a dependency, and the pytest invocation has no
`-n`. Measured across 126 `test` legs: pytest step p50 **787s**, max
**1093s**. Two of the four kills had entirely normal setup and were
killed purely because pytest itself was still running at 1049s/1054s.
2. **Checkout.** The `test` job is the only job using `fetch-depth: 0`
(it needs base-branch refs for the poetry.lock version comparison in
"Install Poetry"). On run 31037832829 that checkout took **429s** on the
leg that died, versus **27s** and **49s** on the two legs that passed —
same commit, same run.

Measured `test`-leg duration distribution (126 legs):

| event | n | p50 | p90 | max | killed at 20m |
|---|---|---|---|---|---|
| `pull_request` | 78 | 922s | 978s | 1218s | 2 (2.6%) |
| `merge_group` | 27 | 950s | 1056s | 1222s | 2 (7.4%) |
| `push` | 21 | 928s | 954s | 976s | 0 |

`merge_group` carries the heaviest tail. It is also the most damaging
place to fail: `merge_group` has no `paths:` filter (GitHub doesn't
support one), so **every** merge group runs the full backend suite even
for PRs that cannot touch the backend — Significant-Gravitas#13434 only changed
`platform-backend-ci.yml` and `TESTING.md`.

### Before / after

The meaningful rate for a `timeout-minutes` change is the share of legs
the cap kills, not a test pass rate:

| | legs exceeding the cap | per-leg | per enqueue (3-leg matrix) |
|---|---|---|---|
| **Before** (`20m`) | 4 / 126 | 3.2% | ~9.2%; on `merge_group` legs
alone 7.4% → **~20.6%** |
| **After** (`35m`) | **0 / 126** | 0% | 0% |

No leg in the sample has ever come within 14 minutes of the new cap. The
longest *completed* leg observed is 1218s (20.3m); the killed legs were
truncated, but extrapolating from their pytest progress they would have
landed at roughly 21-25m — still comfortably inside 35m, which retains
hang detection while leaving ~40% headroom over the worst realistic run.

### Not fixed here (separate issue)

The **Significant-Gravitas#13434 02:55 ejection was a genuinely different failure mode** and
is *not* addressed by this PR. `test (3.11)` (job 92194348225) failed
with 12 failures + 7 errors, all in the credit suite:

- First failure:
`credit_concurrency_test.py::test_concurrent_spends_insufficient_balance`
— `Expected 5 failures, got 4`. One of 10 concurrent `spend_credits`
coroutines raised something that was neither a success nor
`InsufficientBalanceError`.
- Then `test_race_condition_exact_balance` — `ValueError: User not found
with ID: exact-balance-…` for a user that had just been created
successfully.
- Then everything cascaded: ~8 minutes of `25P02 current transaction is
aborted, commands ignored until end of transaction block` across
`credit_concurrency_test.py`, `credit_integration_test.py`,
`credit_metadata_test.py` and `credit_refund_test.py`.

I deliberately have **not** shipped a speculative fix for this. My
initial hypothesis (a leaked interactive transaction in the spend path)
was **disproven**: `credit.py` opens no Prisma interactive transaction
anywhere — `_add_transaction` runs a single autocommit `query_raw` CTE
with `SELECT … FOR UPDATE`, so it structurally cannot leave a connection
in an aborted state. The real poisoning vector is still open, and
reproducing it needs the full stack (Postgres + 3-shard Redis cluster +
RabbitMQ + ClamAV + FalkorDB), which I could not stand up in this
environment. Fixing it on a guess risks introducing a *new* merge-queue
failure mode, which is exactly the problem this PR exists to remove.

### Changes 🏗️

- `.github/workflows/platform-backend-ci.yml`, `test` job:
- `timeout-minutes: 20` → **`35`**, with a comment recording the
measured runtime distribution so it doesn't get tightened back into the
failure zone.
- Added **`filter: blob:none`** to the `fetch-depth: 0` checkout. This
is a blobless partial clone: every ref stays reachable (so the
base-branch `poetry.lock` lookup in "Install Poetry" is unchanged) while
the blobs for all other branches are never downloaded. If the lazy fetch
ever fails, the existing `; true` fallback already degrades to the HEAD
poetry version, so the worst case is benign.

No configuration, service, port, secret or env changes. Behaviour of the
tests themselves is unchanged.

### Checklist 📋

#### For code changes:
- [x] I have clearly listed my changes in the PR description
- [x] I have made a test plan
- [ ] I have tested my changes according to the test plan:
- [x] `python3 -c "yaml.safe_load(...)"` parses the workflow;
`jobs.test.timeout-minutes == 35` and the checkout `with:` block
resolves to `{fetch-depth: 0, filter: blob:none, submodules: true}`
- [x] `actionlint` on the changed workflow reports **5** shellcheck
findings — byte-identical to the count on `dev`, so no new lint issues
are introduced (all 5 are pre-existing, on lines this PR does not touch)
- [x] Confirmed `test` is the only job referencing `BASE_REF`, so
`fetch-depth: 0` is load-bearing there and nowhere else — it is
preserved, only made blobless
  - [x] All `pre-commit` hooks pass on the commit
- [ ] End-to-end confirmation that a `merge_group` run completes inside
35m and that the blobless checkout still resolves `git show
"origin/$BASE_BRANCH":./poetry.lock` — this can only be observed on CI,
and this PR's own `merge_group` run is the test

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Workflow-only timing and checkout tuning; no application code,
secrets, or test behavior changes.
> 
> **Overview**
> Raises the backend CI **`test`** job cap from **20m to 35m** and
documents why: serial ~10.6k-test runs often exceed 20m, GitHub marks
timeouts as **`cancelled`**, and merge-queue **`Check PR Status`**
treats that as failure—ejecting otherwise green PRs.
> 
> Adds **`filter: blob:none`** on the existing **`fetch-depth: 0`**
checkout so base-branch **`poetry.lock`** resolution for Install Poetry
stays the same while avoiding full blob downloads that sometimes
stretched checkout to hundreds of seconds on one matrix leg.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
2814a3a. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---

### CI verification (this PR's own run 31045429279)

All three legs green, and both changes behave as intended:

| leg | result | job total | headroom to 35m | checkout | pytest |
|---|---|---|---|---|---|
| `test (3.11)` | ✅ success | 935s (15.6m) | 19.4m | **9s** | 795s |
| `test (3.12)` | ✅ success | 751s (12.5m) | 22.5m | **9s** | 624s |
| `test (3.13)` | ✅ success | 925s (15.4m) | 19.6m | **14s** | 773s |

**Checkout: 9s / 9s / 14s**, against **27s / 49s / 429s** on the
pre-change baseline (run 31037832829) — the 429s outlier that blew the
budget is gone.

The one real risk in the checkout change was whether a blobless clone
could still resolve the base branch's `poetry.lock`. Confirmed from the
`Install Poetry` step log:

```
Found Poetry version 2.2.1 in backend/poetry.lock
Found Poetry version 2.2.1 in backend/poetry.lock on dev
Using Poetry version 2.2.1
```

The lazy blob fetch resolves correctly and the base-branch comparison is
unchanged.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@ntindle
ntindle added this pull request to the merge queue Aug 6, 2026

@autogpt-pr-reviewer autogpt-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📋 Automated Review — PR #13575

PR #13575 — dx(platform): bake synthetic preview seed fixture as rolling release
Author: ntindle | Files: 2

🎯 Verdict: APPROVE

PR Description Quality

✅ Has Why + What + How — the description explains the bake/restore split, the security invariant, and the seeder rationale. Two stale references remain (see Discussion): the corrected description still carries a "GoTrue-less raw-UUID fallback" test-plan checkbox that the author has since confirmed no longer applies. Docs-only, not blocking.

What This PR Does

Adds a CI-only GitHub Actions workflow (platform-preview-seed-fixture.yml) that spins up a throwaway pgvector/pg15 container, applies migrations, runs four synthetic seeders, pg_dumps the platform schema (including _prisma_migrations), and publishes it as a rolling preview-seed-fixture release. The goal is to let preview environments restore realistic, populated data before a PR's migrations run, so schema changes are exercised against real rows. It also fixes a one-line seeder bug: test_data_updater.py:336 now reads graph_id instead of agentGraphId, matching the current mv_agent_run_counts view definition (the old key would KeyError on a populated bake).

Specialist Findings

🛡️ Security ✅ — Confirmed the trigger surface is not fork-reachable (push/schedule/workflow_dispatch, all write-access), no github.event.* interpolation into run: blocks (no script injection), persist-credentials: false is set, and the fixture is genuinely synthetic with no read path to real data. Overall risk LOW.
🟠 Unpinned third-party actions (checkout@v6, setup-python@v5, cache@v5) pinned to mutable tags in a contents: write workflow that publishes downstream-consumed assets (.github/workflows/platform-preview-seed-fixture.yml:93).

🏗️ Architecture ✅ — Sound, pragmatic seed-baker; reuses established CI idioms (docker exec for pg_dump version alignment, ?schema=platform, ref: dev pinning). Coupling to the four seeder scripts is inherent and acceptable.
🟠 Best-effort load-store-agents can soft-fail and still publish a fixture with only user rows (:144); cancel-in-progress can tear the non-atomic publish sequence (:40).

Performance ✅ — CI-only, ~a few runs/day coalesced by the concurrency group; no hot path. Optional CI-cost notes only: gzip -9 on a ~5 MB disposable artifact (:212), broad backend/test/** trigger (:34), venv not cached (:129). Zero runtime impact from the test_data_updater.py string rename.

🧪 Testing ⚠️ — Verified the graph_id fix is correct against the current view definition and confirmed the sibling mv_review_stats reference is not stale. Flags that the bake pipeline's own verification is weak: the blanket || { warn; exit 0; } swallows real seeder regressions (the exact class of bug this PR fixes), and completeness is gated only by User >= 1.
🟠 Distinguish exit-code 124 (timeout) from genuine failures (:154, :161); add per-table row-count gates before publish (:190).

📖 Quality ✅ — Readability grade A; nearly every non-obvious decision is documented inline. Only minor DRY (soft-failure wrapper duplicated 3×, :159) and magic-value (timeout 300 twice, :169) nits.

📦 Product ✅ — Faithfully implements the described "bake" half with strong consumer docs. One gap: manifest.json records no per-seeder success or format_version, so a degraded fixture is invisible to the restore side (:244).

📬 Discussion ✅ — CI 20/20 green; autogpt-pr-reviewer bot APPROVED at head. Author replied individually to ~35 bot threads. The graph_id line was a standing bot↔author disagreement — now independently resolved in this review (QA + testing both verified the author is correct). Caveats: branch is 82 commits behind dev (mergeable: UNKNOWN) — refresh before merge; no standing human approval (Bentlybro's reviews were dismissed).

🔎 QA ✅ — Validated the data-layer claim end-to-end against live Postgres: mv_agent_run_counts exposes graph_id (old agentGraphId errors), the pg_dump produces a valid 69-table custom archive with _prisma_migrations included, and the documented preamble→pg_restore procedure restores cleanly with only the expected benign schema "platform" already exists collision. actionlint/YAML linters were unavailable in the sandbox, so the workflow's syntax was not independently linted (repo CI's actionlint job covers this).

🟠 Should Fix

  1. Silent-failure masking swallows real seeder regressions (.github/workflows/platform-preview-seed-fixture.yml:154, :161) — the timeout 300 ... || { warn; exit 0; } guard catches every nonzero exit, not just timeout (124). This is precisely how the graph_id KeyError this PR fixes could ship unnoticed. Capture rc=$? and only tolerate rc -eq 124; surface other failures honestly. (Flagged by: testing, architect — 2 specialists)
  2. Fixture completeness gated only by User >= 1 (:190) — if the three best-effort seeders all soft-fail, a fixture with users but no store/agent/execution rows still publishes, undermining the "exercise migrations against realistic rows" goal. Add row-count sanity checks for tables the best-effort seeders populate (AgentGraph, AgentGraphExecution, store listings). (Flagged by: testing, architect, product, discussion — 4 specialists)
  3. Unpinned third-party actions in a contents: write workflow (:93) — SHA-pin checkout@v6, setup-python@v5, cache@v5 per GitHub hardening guidance, since a moved/hijacked tag would run with a write token and poison a downstream-consumed release asset. (Flagged by: security — 1 specialist)
  4. Change-relative comment already misled reviewers (:82) — "e2e_test_data.py no longer talks to Supabase" documents a past migration, not a standing fact. Rewrite as the invariant ("does not talk to Supabase; settings read these values at import time, nothing connects"). Carried from the previous review as an open Should-Fix. (Flagged by: architect — 1 specialist)

🟡 Nice to Have

  1. Non-atomic publish + cancel-in-progress (:40) — a mid-sequence cancel can leave a new manifest with a stale fixture.dump.gz. Consider cancel-in-progress: false or excluding the publish step. Low likelihood given the 45-min window. (architect, discussion)
  2. Manifest carries no completeness/version signal (:244) — add per-seeder success flags and a format_version so the (follow-up) restore side can detect degraded/incompatible fixtures. Best agreed now while defining the contract. (product)
  3. Round-trip restore validation before publish (:184) — deferred by the author to the restore-side PR; track it explicitly so a corrupt dump can't pass every check and fail only in previews. (discussion)

🔵 Nits

  1. gzip -9 on a disposable ~5 MB artifact (:212) — default level 6 saves CPU per bake for negligible size change. (performance)
  2. Broad backend/test/** trigger (:34) — re-bakes byte-identical fixtures on unrelated test edits; narrow to the seeder files. (performance)
  3. Venv not cached (:129), soft-failure wrapper duplicated 3× (:159), timeout 300 magic value (:169) — minor. (performance, quality)

Human Review Needed

NO — This is a CI/infra workflow operating exclusively on synthetic data with a disposable Postgres container and a dummy unreachable Supabase config. It touches no system authentication/authorization, no real-credential storage, and no service trust boundary; the committed ENCRYPTION_KEY is the allowlisted non-production test key. The one application-code line is a verified string rename. The supply-chain hardening note is a Should-Fix, not a security-boundary change.

Risk Assessment

Merge risk: LOW | Rollback: EASY (delete the workflow file + revert one line; nothing consumes the fixture yet).

CI Status

Local harness: ✅ All 5 checks pass (frontend lint/types/test:unit/build, backend poetry run lint). GitHub CI reported by the discussion specialist: ✅ 20/20 checks green at head eeb4f88 (autogpt-pr-reviewer bot APPROVED). Note: branch is 82 commits behind dev with mergeable: UNKNOWN — refresh before merge.


UI Testing — Variant Results

✅ local: The test_data_updater graph_id rename is correct against the live materialized view and the workflow's bake→restore procedure reproduces end-to-end with only the documented benign schema collision.

✅ hosted: CI-only workflow plus a verified test-seeder column-key fix; the mv_agent_run_counts.graph_id rename is provably correct against the live schema and all workflow SQL operations succeed.

# backend module imports that read settings.secrets don't error.
ENCRYPTION_KEY: "dvziYgz0KSK8FENhju0ZYi8-fRTfAdlz6YLhdB_jhNw=" # pragma: allowlist secret # DO NOT USE IN PRODUCTION

steps:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟡 medium (security/supply-chain / unpinned actions)

Third-party actions (actions/checkout@v6, actions/setup-python@v5, actions/cache@v5) are pinned to mutable major-version tags rather than full commit SHAs, in a workflow that holds contents:write and publishes release assets restored into downstream preview databases. A moved/hijacked tag would execute attacker code with a write token.

Suggestion: Pin each action to a full commit SHA (e.g. actions/checkout@ # v6) per GitHub's hardening guidance for privileged workflows.

run: |
HEAD_POETRY_VERSION=$(python ../../.github/workflows/scripts/get_package_version_from_lockfile.py poetry)
echo "Using Poetry version ${HEAD_POETRY_VERSION}"
curl -sSL https://install.python-poetry.org | POETRY_VERSION=$HEAD_POETRY_VERSION python3 -

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (security/supply-chain / remote code execution)

Poetry is installed via curl -sSL https://install.python-poetry.org | python3 -, piping an unpinned remote script into the interpreter inside a job with a write-scoped token.

Suggestion: Install Poetry from a pinned installer/pipx or verify a checksum before executing; avoid piping unverified network content to python3.

concurrency:
group: preview-seed-fixture-bake
cancel-in-progress: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (security/least-privilege)

contents: write is granted at the workflow level, so all steps (including repo-defined seeders and the piped Poetry installer) run with the write token available, even though only the final publish step needs it.

Suggestion: Move the permissions block to the bake job or split publishing into a separate minimally-permissioned job to reduce blast radius.


- name: "Seed: test_data_creator (REQUIRED)"
run: poetry run python test/test_data_creator.py

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟡 medium (architect/robustness / silent degradation)

load-store-agents is best-effort (::warning:: + exit 0) even though, unlike the two Python seeders, it has no queue/cache dependency. If it fails, the release still publishes with only user rows, passing the USER_COUNT>=1 gate — producing a fixture missing store/agent data, which defeats the PR's goal of exercising migrations against populated tables.

Suggestion: Make load-store-agents required, or gate publish on row-count sanity checks for the tables each best-effort seeder populates (not just platform.User).

# Coalesce overlapping bakes: a newer commit cancels an in-flight bake since
# they would publish the same rolling asset anyway.
concurrency:
group: preview-seed-fixture-bake

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (architect/concurrency / atomicity)

cancel-in-progress can interrupt the non-atomic publish sequence (release edit -> asset upload --clobber -> tag retarget), leaving the rolling release with a new manifest but stale fixture.dump.gz, or updated assets but un-retargeted tag. Consumers restore the latest asset, so a torn update is a correctness hazard.

Suggestion: Set cancel-in-progress: false, or scope cancellation so an in-flight publish step always completes.

# --- Dump + manifest ------------------------------------------------
# pg_dump / psql run INSIDE the Postgres service container via docker exec
# so the client version always matches the server exactly (the runner's
# bundled client can lag the server major and refuse the dump). This is

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (quality/magic-value)

timeout 300 is duplicated at two seeder steps with no shared constant, so the two timeouts must be kept in sync manually.

Suggestion: Optionally hoist into a job-level env var (e.g. SEED_TIMEOUT: 300) and reference it in both steps.

--arg dev_sha "${DEV_SHA}" \
--arg migration_head "${MIGRATION_HEAD}" \
--arg restore_preamble "Apply restore-preamble.sql to the target database BEFORE pg_restore: it creates the platform schema and installs the vector + pg_trgm extensions into it (pg_dump --schema=platform omits CREATE EXTENSION for extensions inside the dumped schema). Do NOT pass --exit-on-error to pg_restore — the dump's own CREATE SCHEMA collides benignly with the preamble's." \
'{baked_at: $baked_at, dev_sha: $dev_sha, migration_head: $migration_head, restore_preamble: $restore_preamble}' \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (product/consumer-experience)

manifest.json records only baked_at/dev_sha/migration_head. Three of four seeders are best-effort (lines 155, 162): on a soft-failure the workflow still overwrites the rolling release asset, so a preview can restore a fixture missing e2e/updater data with no signal that it is degraded — partially undermining the feature's goal of exercising migrations against realistic rows. There is also no fixture format_version for the restore side to detect breaking format changes.

Suggestion: Add per-seeder success flags (e.g. seeders: {e2e_test_data: true, test_data_updater: false}) and a format_version field to manifest.json so consumers can detect degraded or incompatible fixtures before restoring.

print("\nTop 5 agents by run count:")
for row in sample_runs:
print(f" - Agent {row['agentGraphId'][:8]}...: {row['run_count']} runs")
print(f" - Agent {row['graph_id'][:8]}...: {row['run_count']} runs")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (discussion/unresolved-disagreement)

Cursor Bugbot flagged the change to row['graph_id'] as a wrong column key; the author disputes it as a false positive, citing migration 20260304123456_update_store_views that mv_agent_run_counts exposes graph_id (not agentGraphId). This is the only application-code line in the PR and the bot/author disagreement is still standing.

Suggestion: Confirm mv_agent_run_counts's column list against the referenced migration to close the disagreement before merge.

echo "PG_CID=${PG_CID}" >> "${GITHUB_ENV}"
docker exec "${PG_CID}" pg_dump --version

- name: Dump platform schema fixture (custom format, gzipped)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (discussion/deferred-concern)

Reviewer (High) noted the workflow publishes fixture.dump.gz without ever restoring it, so a corrupt/truncated archive passes all checks and only fails in every preview. Author acknowledged and deferred restore validation to the follow-up restore-side PR.

Suggestion: Ensure the follow-up infra PR adds a round-trip restore validation, or track it as an explicit follow-up issue so the deferral is not lost.

# Loud sanity check: the required seeder must have populated users.
USER_COUNT=$(docker exec "${PG_CID}" psql -U postgres -d postgres -tAc "SELECT count(*) FROM platform.\"User\";" | tr -d '[:space:]')
echo "Seeded platform.\"User\" rows: ${USER_COUNT}"
if [ "${USER_COUNT:-0}" -lt 1 ]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 🟢 low (discussion/declined-advisory)

Reviewer noted the only completeness guard (platform.User >= 1) is tautological and best-effort seeders (load-store-agents, e2e_test_data) can soft-fail, publishing a fixture missing store/agent rows while still reporting success. Author declined as advisory.

Suggestion: Consider asserting minimum row counts for other critical tables or recording per-seeder success in manifest.json so a degraded fixture is detectable by the restore side.

Merged via the queue into dev with commit 01b630f Aug 6, 2026
47 of 64 checks passed
@ntindle
ntindle deleted the dx/preview-seed-fixture-bake branch August 6, 2026 04:43
@github-project-automation github-project-automation Bot moved this from 👍🏼 Mergeable to ✅ Done in AutoGPT development kanban Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform/backend AutoGPT Platform - Back end size/l

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants