ci(platform): validate and publish single-container images - #13759
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
…-Gravitas#13755) ### Why / What / How **Why** The single-container appliance supervises backend roles in one network namespace. Internal metrics listeners should not be exposed beyond container loopback, and notification workers must stop cleanly when Supervisor terminates the process. **What** - Binds executor and Copilot executor metrics endpoints to loopback. - Gives notification queue polling an explicit task lifecycle. - Adds regression coverage for clean cancellation and shutdown. **How** The metrics servers use an explicit loopback host. The notification runner tracks its polling task, cancels and awaits it during shutdown, and avoids leaving an event-loop-bound task behind. ### Stack Part **1 of 6** in the single-container stack. This is the bottom PR and targets `dev`. Review and merge the stack bottom-up. It supersedes the corresponding backend portion of draft Significant-Gravitas#13754. ### Changes 🏗️ - 4 focused backend source/test files. - No Docker image or configuration changes in this layer. ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] Signed commit passed the repository pre-commit suite under Node 24, including backend lint, formatting, type checking, API generation, and secret checks - [x] Notification regression coverage is included - [ ] Let PR CI validate this isolated layer #### For configuration changes: - [x] Not applicable; this layer does not change configuration ### Stack navigation Bottom → top: 1. [Significant-Gravitas#13755 — backend supervision](Significant-Gravitas#13755) 2. [Significant-Gravitas#13757 — frontend routing](Significant-Gravitas#13757) 3. [Significant-Gravitas#13756 — runtime options](Significant-Gravitas#13756) 4. [Significant-Gravitas#13758 — single-container image](Significant-Gravitas#13758) 5. [Significant-Gravitas#13759 — validation and publication CI](Significant-Gravitas#13759) 6. [Significant-Gravitas#13760 — operations documentation](Significant-Gravitas#13760) The base refs already form GitHub's required linear chain. The native Stack association is pending GitHub's repository-by-repository public-preview rollout; the Stack API currently returns its documented not-enabled response for this repository. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes shutdown ordering and timeouts for the notification RabbitMQ path and alters metrics bind behavior; mistakes could hang supervisor restarts or affect scrape access, though behavior is heavily tested and bounded. > > **Overview** > Hardens backend processes for **single-container supervision**: internal Prometheus listeners follow the same bind address as other service RPC (`pyro_host`), and the notification service shuts down RabbitMQ consumers with **bounded, non-blocking** teardown. > > **Graph and Copilot executors** now pass `addr=settings.config.pyro_host` into `start_http_server`, so metrics stay on loopback in the appliance runtime while docker-compose can still scrape via `0.0.0.0`. > > **NotificationManager** keeps strong references to the background `_run_service` future/task, routes shutdown through a new `_shutdown_service` barrier (cancel consumers, then disconnect with per-stage timeouts), and replaces the old synchronous disconnect in `cleanup()` with `run_and_wait(..., timeout=CLEANUP_TIMEOUT_SECONDS)` plus handling for already-closed or racing event loops. > > **AppService** gains an optional timeout on `run_and_wait` and skips scheduling `loop.stop()` when the shared loop is already closed. > > Regression tests cover future retention, ordered shutdown, cancellation timeouts, and loop-edge cases. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c38bd9e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5956588. Configure here.
| if [[ "$actual_platforms" != "linux/${expected_arch}" ]]; then | ||
| echo "${image_ref} does not contain exactly linux/${expected_arch}" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Wrong digest platform JSON path
High Severity
When validating each pushed platform digest, the workflow parses imagetools inspect --raw with a jq filter on .manifests[]?.platform. Those refs are single image manifests from push-by-digest, not manifest lists, so the filter yields nothing and the job rejects valid linux/amd64 and linux/arm64 digests.
Reviewed by Cursor Bugbot for commit 5956588. Configure here.
| actual_platforms="$( | ||
| docker buildx imagetools inspect --raw "$image_ref" | | ||
| jq -r '.manifests[]?.platform | select(.os == "linux" and (.architecture == "amd64" or .architecture == "arm64")) | "\(.os)/\(.architecture)"' | | ||
| sort -u | ||
| )" | ||
| if [[ "$actual_platforms" != "linux/${expected_arch}" ]]; then | ||
| echo "${image_ref} does not contain exactly linux/${expected_arch}" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Bug: The jq query in the manifest validation step incorrectly assumes a multi-platform manifest, causing it to fail when inspecting single-platform image digests and blocking image publication.
Severity: HIGH
Suggested Fix
Modify the jq query to correctly parse a single-platform image manifest. Instead of querying .manifests[]?.platform, the script should inspect the top-level .platform attribute of the manifest object itself to get the architecture information.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: .github/workflows/platform-single-container-docker.yml#L480-L488
Potential issue: In the `publish-experimental-manifest` job, a script attempts to
validate the platform of a pushed image digest. It uses the `jq` query
`.manifests[]?.platform` to extract the platform architecture. This query is designed
for multi-platform manifest lists. However, the digest being inspected points to a
single-platform image manifest, which does not have a `.manifests` array. Consequently,
the `jq` query returns an empty string. This causes the subsequent shell comparison `[[
"$actual_platforms" != "linux/${expected_arch}" ]]` to fail, incorrectly reporting a
platform mismatch and causing the workflow to exit with an error. This bug will prevent
the publication of experimental multi-platform images.
Did we get this right? 👍 / 👎 to inform future reviews.
|
/review |
There was a problem hiding this comment.
📋 Automated Review — PR #13759
PR #13759 — ci(platform): validate and publish single-container images
Author: ntindle | Files: 2
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why + What + How — the PR explains the multi-arch build/smoke/scan pipeline, the gated manual publication flow, and its place in a larger stacked series (parts 4/#13758, 6/#13760). Well-scoped.
What This PR Does
Adds CI-only surface: one GitHub Actions workflow (platform-single-container-docker.yml) and one Bash smoke harness (platform-single-container-smoke.sh). Together they build the all-in-one single-container platform image for amd64 + arm64, boot it and run a phased smoke suite (health, persistence, hostile-config rejection, redirect/host-header checks, restart recovery, Compose), run Trivy vuln/secret scans, and — only via a gated, manually-dispatched flow behind a protected environment and typed confirmations — publish immutable experimental-* digests to Docker Hub. No runtime application code changes; never creates latest.
Specialist Findings
🛡️ Security ✅ — Strong discipline: untrusted workflow_dispatch inputs passed via env: (no script injection), pull_request not pull_request_target (no secrets to forks), top-level contents: read, and Docker Hub creds gated to the protected dockerhub-experimental environment behind typed confirmations. Only low/defense-in-depth notes (shared GHA cache scope :316, tag re-validation :275). No blockers.
🏗️ Architecture ✅ — Fail-closed gating and validate→gate→re-validate-at-push→re-validate-at-manifest layering are sound. Notes: overly broad build triggers (:19) and an intentional-but-undocumented double build for provenance/SBOM (:355).
🟠 Broad paths: triggers heavy dual-arch build on unrelated changes.
⚡ Performance :74), 5–20× slower than native, which is why timeout-minutes: 240 exists; broad triggers and duplicated smoke/scan on publish compound it.
🧪 Testing ✅ — The smoke harness is the test artifact and is unusually thorough with specific assertions (exact status codes, Location headers, checksum stability, file mode 600). Gaps: async sleep 2 before the token-leak grep (:241), leak check only inspects docker logs stdout/stderr not on-disk logs (:244), no positive content assertion that a real page returns 200 (:163).
📖 Quality ✅ — Excellent comments explaining why, strict bash mode throughout. Deductions for copy-paste validation logic (three near-identical review-reference blocks :180, duplicated tag-absence check :278/:436, misnamed legal_reference_pattern :189) that could drift.
📦 Product ✅ — Requirements fully met; safety posture (fail-closed gates, no latest, immutable tags, re-smoke on pushed digest) is strong. DX concern: six exact-string approval inputs are easy to fat-finger; publish docs land in part 6.
📬 Discussion :488) with no author response. The discussion analyst notes this is likely a non-issue because sbom: true + provenance: mode=max makes each pushed digest an OCI image index (so .manifests[].platform resolves) — but it's unverified because the matrix never runs on this PR (base is feat/single-container-04-image, not dev).
🟠 Confirm or fix the manifest jq filter before it reaches a dev-targeted PR.
🔎 QA ✅ — Built the target image (BUILD EXIT: 0, 4.99GB) and ran the harness end-to-end. A clean bare docker run booted healthy in ~90s with nginx RUNNING (the headline claim). Publication-gate negative tests all rejected invalid inputs; publish jobs correctly skip on PR/merge. One transient nginx-stop-at-boot did not reproduce on a clean rerun (sandbox contention; root cause lives in base image #13758). Phase-1 bare run omits the shm_size/ulimit the compose phase declares.
🟠 Should Fix
- Confirm or fix the manifest platform jq filter (
.github/workflows/platform-single-container-docker.yml:488) —.manifests[]?.platformassumes a manifest list; two bots warn single-image push-by-digest refs could be rejected. Likely fine becausesbom: true+provenance: mode=maxyields an OCI index, but reply on the threads confirming the attestation-index behavior (or handle a top-level.platform) before this reachesdev, where the code path first executes. (Flagged by: discussion, cursor-bugbot, sentry — 3 sources) - Narrow the workflow
paths:triggers (.github/workflows/platform-single-container-docker.yml:19) —docs/**and blanketautogpt_platform/**launch a 240-min-capped dual-arch QEMU build+smoke+scan on docs-only or unrelated changes. Scope to the Dockerfile,single-container/**, bundled backend/frontend inputs, the smoke script, and the workflow. (Flagged by: architect, performance, product, ui-reviewer — 4 specialists) - Harden the token-leak assertion (
.github/scripts/platform-single-container-smoke.sh:241,244) — the fixedsleep 2before a singledocker logsgrep can pass while a token flushed later leaks, and it never inspects on-disk logs under/data. Replace the sleep with a deterministic sync point and widen the grep scope. (Flagged by: testing)
🟡 Nice to Have
- Use a native arm64 runner (
.github/workflows/platform-single-container-docker.yml:62) — replace QEMU emulation of the arm64 leg withubuntu-24.04-armto cut wall-clock from hours to minutes and reduce timing flakiness. (performance, architect, ui-reviewer) - De-duplicate validation logic (
.github/workflows/platform-single-container-docker.yml:180,278) — extract the three review-reference checks into a bash function and the tag-absence check into a shared.github/scripts/helper so fail-closed guards can't drift. (quality) - Document the deliberate second build (
.github/workflows/platform-single-container-docker.yml:355) — one comment noting the rebuild is intentional (SBOM/provenance + verify-what-you-push) so it isn't "optimized" away. (architect) - Positive content assertion (
.github/scripts/platform-single-container-smoke.sh:163) — fetch a real route (e.g./login) and assert 200/non-empty so a broken frontend can't pass on redirects alone. (testing)
🔵 Nits
- Rename
legal_reference_pattern(.github/workflows/platform-single-container-docker.yml:189) — reused for security and whole-image references; call itreview_reference_pattern. (quality) - Add diagnostic to pid assertion (
.github/scripts/platform-single-container-smoke.sh:194) — wrap the bare regex check with anecho … >&2like sibling assertions. (testing)
Human Review Needed
YES — This workflow governs how Docker Hub publish credentials are used and what gets released as a public image (a credential-handling / trust-boundary surface). The security specialist found it well-built, but the gated publication path and the unresolved manifest-filter question warrant a maintainer's eyes before it merges up to dev.
Risk Assessment
Merge risk: LOW | Rollback: EASY — CI-only, two new files, no runtime code; publish jobs cannot fire on PR/merge, and reverting is a clean file removal.
CI Status
Local harness: ✅ 5/5 checks pass (frontend lint/types/test/build, backend lint) — all 0s and effectively no-ops for this CI-only diff; not meaningful coverage of the changed files.
GitHub CI: UNVERIFIED for the new build/smoke/scan matrix — the workflow triggers only on PRs targeting dev, and this PR's base is feat/single-container-04-image, so its own architecture matrix does not execute here. QA independently built the image and booted it healthy locally as a substitute signal.
| org.opencontainers.image.licenses=LicenseRef-PolyForm-Shield-1.0.0 AND SSPL-1.0 | ||
| sbom: true | ||
| provenance: mode=max | ||
| cache-from: type=gha,scope=platform-single-container-${{ matrix.suffix }} |
There was a problem hiding this comment.
🤖 🟢 low (security/supply-chain / cache integrity)
The publish job reuses the same GHA cache scope (platform-single-container-) as the untrusted PR build-and-scan job (lines 107-108), so layers of the published public image can come from a cache other jobs write to. Mitigated by re-pull/re-smoke/re-scan of the exact pushed digest, but image integrity should not depend implicitly on GitHub cache branch-scoping.
Suggestion: Use a dedicated isolated cache scope for the publish build (e.g. append '-publish') or drop cache-from for the canonical publish build so published layers are always rebuilt clean.
| shell: bash | ||
| env: | ||
| IMAGE: ${{ env.DEPLOY_IMAGE }} | ||
| TAG: ${{ inputs.tag }} |
There was a problem hiding this comment.
🤖 🟢 low (security/input validation / defense-in-depth)
In publish-platform-digests, the 'Validate image coordinates' step validates IMAGE and tag absence but does not re-validate the TAG format before it is used in build-args (line 305) and imagetools inspect (line 283); it relies solely on the upstream publication-gate job for tag format validation.
Suggestion: Re-apply the experimental tag regex (^experimental-a-z0-9?$) and length check within this job, matching the per-job validation the manifest job already performs.
| - ".github/workflows/platform-single-container-docker.yml" | ||
| - "autogpt_platform/**" | ||
| - "docs/**" | ||
| pull_request: |
There was a problem hiding this comment.
🤖 🟢 low (security/untrusted-code execution)
The pull_request trigger builds the image and runs the full smoke harness from PR branch code on the runner. Safe because it uses pull_request (no secrets, contents: read), but relies on the repo's first-time-contributor approval setting to gate fork execution.
Suggestion: Confirm the repository requires approval to run workflows for fork/first-time contributors; no code change strictly required.
| - ".github/scripts/platform-single-container-smoke.sh" | ||
| - ".github/workflows/platform-single-container-docker.yml" | ||
| - "autogpt_platform/**" | ||
| - "docs/**" |
There was a problem hiding this comment.
🤖 🟡 medium (architect/ci-cost/trigger-scope)
pull_request/push path filters (autogpt_platform/** and docs/**) trigger a heavy dual-arch build with arm64 under QEMU emulation and a full multi-phase smoke suite (timeout 240m) on nearly every platform or docs PR, even changes that cannot affect the image.
Suggestion: Narrow paths to the Dockerfile, single-container/, and directly-bundled backend/frontend inputs; drop docs/ from the heavy build trigger or gate it behind a lighter job/label.
|
|
||
| permissions: | ||
| contents: read | ||
|
|
There was a problem hiding this comment.
🤖 🟢 low (architect/emulation-risk)
linux/arm64 is built and smoke-tested only under QEMU emulation, making timing-sensitive smoke phases (supervisor/nginx restart recovery, health waits) slower and more flake-prone against the 240-minute ceiling.
Suggestion: Use a native arm64 runner for the arm64 leg if available; otherwise document the emulation tradeoff in the operations docs (part 6).
| - ".github/scripts/platform-single-container-smoke.sh" | ||
| - ".github/workflows/platform-single-container-docker.yml" | ||
| - "autogpt_platform/**" | ||
| - "docs/**" |
There was a problem hiding this comment.
🤖 🟡 medium (product/scope / developer-experience)
The push/pull_request path filters include docs/** and a blanket autogpt_platform/**, so documentation-only or frontend-only PRs trigger a full multi-arch (amd64 + emulated arm64) build, smoke, and scan with a 240-minute timeout — work that those changes do not affect.
Suggestion: Narrow the pull_request paths to the container/backend surface (Dockerfile, single-container/**, the smoke script, and the workflow itself), or gate the arm64 leg behind a label, so routine PRs aren't blocked by an hours-long build.
| set -Eeuo pipefail | ||
|
|
||
| : "${SMOKE_IMAGE:?SMOKE_IMAGE is required}" | ||
| : "${SMOKE_PLATFORM:?SMOKE_PLATFORM is required}" |
There was a problem hiding this comment.
🤖 🟢 low (product/developer-experience / reliability)
The arm64 smoke test runs the full stack (supervisor, nginx, backend, FalkorDB) under QEMU emulation on ubuntu-latest with a 2700s (45 min) default health timeout; emulated boot is slow and flake-prone, and health failures surface opaquely to contributors.
Suggestion: Consider a native arm64 runner for the smoke leg, or document the expected runtime and raise/parameterize the timeout margin for the emulated path.
| if [[ "$actual_platforms" != "linux/${expected_arch}" ]]; then | ||
| echo "${image_ref} does not contain exactly linux/${expected_arch}" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🤖 🟠 high (discussion/unaddressed-review-concern)
Cursor Bugbot and Sentry both flag that the jq filter .manifests[]?.platform assumes a manifest list, but push-by-digest refs may be single-image manifests, potentially rejecting valid amd64/arm64 digests and blocking publication. Neither bot comment has an author response, and this code path is never executed on this PR because the workflow only triggers on PRs targeting dev while this PR's base is feat/single-container-04-image.
Suggestion: Reply to the bot threads confirming that sbom: true + provenance: mode=max makes each pushed digest an OCI image index (so the select(os=="linux"...) filter is correct), or adjust the filter to also handle a top-level .platform on single image manifests. Resolve before this reaches a dev-targeted PR where the matrix actually runs.
|
|
||
| permissions: | ||
| contents: read | ||
|
|
There was a problem hiding this comment.
🤖 🟡 medium (ui-reviewer/ci-cost-reliability)
The build-and-scan matrix builds and smoke-boots the full ~5GB, 23-program single-container image for linux/arm64 under QEMU emulation (setup-qemu-action), plus a up-to-45-min smoke test and three 30-min Trivy scans, and it is triggered on every push/PR touching the very broad paths 'autogpt_platform/' and 'docs/'. Emulated boot of an entire supervisor stack (postgres, rabbitmq, falkordb, valkey, clamav, all Python services, next, nginx) is slow and timing-fragile; I observed a transient boot flake (nginx stopped, container unhealthy) once even natively. Under QEMU this risks frequent timeouts (240-min job) and high CI cost on unrelated frontend/backend/docs PRs.
Suggestion: Narrow the trigger paths to the single-container surface (e.g. the Dockerfile single-container stages, autogpt_platform/single-container/, the compose file, the smoke script and this workflow) instead of all of autogpt_platform/ and docs/**, and/or gate the arm64 leg to run only on push to dev or on demand rather than on every PR.
| } | ||
| } | ||
|
|
||
| assert_prefixed_backend_redirect() { |
There was a problem hiding this comment.
🤖 🟢 low (ui-reviewer/test-robustness)
Phase-1's bare 'docker run IMAGE' (and the phase-2 run) do not set shm_size or ulimit nofile, but the documented compose distribution used in phase 3 explicitly sets shm_size: 2gb and nofile soft/hard 65536 with a comment that these are needed so the bundled roles stay below PostgreSQL's connection ceiling. Default Docker /dev/shm is 64MB, so phase 1 boots the bundled Postgres/FalkorDB with a materially different (smaller) resource profile than phase 3, which can make the 'literal docker run' phase flakier than the Compose phase.
Suggestion: Either document/justify that the image is expected to boot healthy with default shm/ulimits (proving the bare-run claim), or align the bare-run resource flags with the compose defaults (--shm-size=2g --ulimit nofile=65536:65536) so all three phases exercise the same resource envelope.


Why / What / How
Why
A public all-in-one image needs architecture-specific boot testing and guarded publication controls; a successful Docker build alone is not enough.
What
linux/amd64andlinux/arm64images.How
Architecture jobs validate exact local image digests before a gated publication job assembles the manifest. The workflow intentionally does not create
latest.Stack
Part 5 of 6. Base:
feat/single-container-04-image. It depends on the appliance layer and is documented by part 6. It supersedes the CI/publication portion of draft #13754.Changes 🏗️
.github/scripts/platform-single-container-smoke.sh..github/workflows/platform-single-container-docker.yml.Checklist 📋
For code changes:
For configuration changes:
dockerhub-experimentalenvironment and credentialsStack navigation
Bottom → top:
The base refs already form GitHub's required linear chain. The native Stack association is pending GitHub's repository-by-repository public-preview rollout; the Stack API currently returns its documented not-enabled response for this repository.
Note
Medium Risk
Changes affect release plumbing and public image distribution with secret scanning and strict publication gates; runtime app code is untouched but mistaken publish configuration could expose credentials or ship a bad image.
Overview
Adds CI for the all-in-one platform image: a new workflow builds
single-containeron linux/amd64 and linux/arm64, runs an end-to-end smoke harness, and fails on fixable critical Trivy findings and high/critical embedded secrets (with informational high/critical vuln reporting).The smoke script exercises bare
docker run, then a configured run with persistent/data, hostilebackend.json(listener pinning), auth redirects, hidden docs/metrics, token non-leakage in logs, supervisor/nginx restart recovery, and the documented single-container Compose file—all while asserting stableruntime.envchecksum and mode600.Manual publication is opt-in via
workflow_dispatch: typed confirmations, legal/security/SBOM references,experimental-*tags only fromdev, protecteddockerhub-experimentalenvironment, push-by-digest per arch with SBOM/provenance, re-smoke/re-scan on the exact digest, then a multi-arch manifest (nolatest).Reviewed by Cursor Bugbot for commit 5956588. Bugbot is set up for automated code reviews on this repo. Configure here.