Skip to content

ci: per-arch image build + boot smoke on every PR - #385

Merged
aliasunder merged 7 commits into
mainfrom
worktree-arch-smoke-ci
Jul 31, 2026
Merged

ci: per-arch image build + boot smoke on every PR#385
aliasunder merged 7 commits into
mainfrom
worktree-arch-smoke-ci

Conversation

@aliasunder

@aliasunder aliasunder commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Motivation

Every arm64 image from v0.33.0 through v0.33.4 crash-looped at startup (better-sqlite3 v13's linux-arm64 prebuild requires glibc >= 2.38; the bookworm base had 2.36) and stayed invisible for five days across four releases: prod is amd64, and trivy-pr — the only PR-time image build — builds amd64 only, so nothing ever executed an arm64 artifact before release. PR #384 fixed the image and added the deps-stage assertion that executes both native bindings per-arch at build time. This PR makes executed-per-arch a PR gate.

What the job does

New Arch Smoke workflow, matrix over native runners (ubuntu-latest amd64, ubuntu-24.04-arm arm64 — no QEMU). Each leg:

  1. docker build --target local — natively executes the Dockerfile deps-stage native-binding assertion for its arch (new Database(':memory:') + require('onnxruntime-node')).
  2. Boot smoke — runs the image against a small inline fixture vault (frontmatter, tags, wikilink pair, task line — real FTS inserts) with EMBEDDING_ENABLED=false / MEMORY_ENABLED=false and a per-run generated token. /healthz 200 is a true proof of the incident class: server.ts opens the search DB, awaits the blocking FTS rebuild, and opens the OAuth DB before app.listen, exiting 1 on any failure. Then an authenticated MCP initialize round-trip is asserted (HTTP 2xx + mcp-session-id header + serverInfo in the body — a JSON-RPC error still returns 200, hence the body assertion).
  3. arm64 only: docker build --target remote as a compile check — the first-ever arm64 build of the remote target at PR time. Build-only: booting remote needs Obsidian Sync credentials, and amd64 remote coverage is already trivy-pr's job.

Design notes

  • Check contexts are arch-smoke (amd64) / arch-smoke (arm64) — explicit job name with a friendly arch key, so the ruleset contexts stay stable if runner labels ever change. Adding them to the branch ruleset is a follow-up settings change after this merges.
  • Cache: explicit per-arch scope=arch-smoke-<arch>, a deliberate divergence from the repo's bare type=gha: trivy-pr (two targets) plus both smoke legs writing mode=max into the one default scope would evict each other — worst for arm64, which shares no blobs with the amd64 writers. cache-from also lists the bare default scope so the amd64 leg piggybacks trivy-pr's layers (no-op on arm64). The APT_UPGRADE_DATE cache-bust step is reused verbatim from trivy.yml — a differing build-arg would change the apt-upgrade layer hash and break that sharing.
  • No path filters — a required check gated by paths: leaves docs-only PRs stuck on "Expected" forever; trivy-pr also runs unconditionally.
  • push: main trigger seeds the per-arch cache into the default-branch scope (readable by all PRs; PR-branch caches are not shared across PRs) and acts as a post-merge canary.
  • Crash-loop fast-fail: the healthz poll checks docker inspect '{{.State.Running}}' each iteration, so the incident's exact failure mode (startup process.exit(1)) fails in ~2s instead of burning the 60s poll window. Diagnostics step (if: failure()) dumps docker ps -a, container state JSON, and logs.
  • timeout-minutes: 20 is a kill-switch cap (default is 360), not the expected runtime — expected ~3-5 min warm-cache, ~8-12 min cold, legs parallel.
  • fail-fast: false — arch failures are independent signals; one leg must not cancel the other.

Verification

  • actionlint: zero findings on the new workflow; npm run prettier:check green.
  • Full smoke executed locally on native arm64 (Apple Silicon, the incident-class arch): the exact script block passes under bash -e in 2.2s — healthz ready on attempt 2, MCP initialize round-trip clean, and the initialize curl exits naturally when the SSE response stream closes (no --max-time timeout).
  • Failure path mutation-tested locally: booting the container without MCP_AUTH_TOKEN (startup exit 1) trips the crash-detection branch on poll attempt 1 with the startup error visible in docker logs.
  • On this PR: both legs run for the first time in CI (new pull_request workflows run from the PR branch). A deliberate red push (probe pointed at a wrong port) will verify the gate bites and the diagnostics render, then be reverted.

Also updates CONTRIBUTING.md's required-checks item to name the new checks.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests

    • Added automated smoke testing for native amd64 and arm64 Docker builds.
    • Tests now boot images, verify health and authenticated initialization, and collect diagnostics on failure.
  • Documentation

    • Updated pull request guidance to note that architecture smoke test failures block merging.

Builds the local target and boots it on native amd64 and arm64 runners
on every PR, with an authenticated MCP initialize probe; the arm64 leg
also compile-checks the remote target. Prevents the v0.33.0-v0.33.4
class of arch-specific startup breakage from reaching a release.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@umm-actually

umm-actually Bot commented Jul 30, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add --pull=never to smoke test

The docker run command should include a --pull=never flag to ensure the smoke test
runs the exact image that was just built in the prior step. Without --pull=never, if
an image tagged vault-cortex:smoke somehow already exists in the GitHub Actions
runner's local Docker daemon from cache or a previous run, the runner would silently
use the stale image instead of the one produced by the docker/build-push-action
step, making the smoke test pass for the wrong binary.

.github/workflows/arch_smoke.yml [134-144]

 - name: Boot smoke (healthz + MCP initialize)
   run: |
     MCP_AUTH_TOKEN="$(openssl rand -hex 24)"
 
-    docker run -d --name smoke -p 8000:8000 \
+    docker run -d --name smoke --pull=never -p 8000:8000 \
       -v "$RUNNER_TEMP/smoke-vault:/vault:ro" \
       -e MCP_AUTH_TOKEN="$MCP_AUTH_TOKEN" \
       -e PUBLIC_URL=http://localhost:8000 \
       -e EMBEDDING_ENABLED=false \
       -e MEMORY_ENABLED=false \
       vault-cortex:smoke
Suggestion importance[1-10]: 7

__

Why: Adding --pull=never ensures the smoke test uses the locally built image, preventing silent use of a stale cached image. The suggestion is correct and targets a valid concern about test reliability. However, docker/build-push-action with load: true typically overwrites the local tag, making the scenario unlikely, so the impact is moderate.

Medium

Comment thread .github/workflows/arch_smoke.yml
@aliasunder

Copy link
Copy Markdown
Owner Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a native amd64/arm64 GitHub Actions workflow that builds Docker targets, boots images with fixture vaults, validates health and authenticated MCP initialization, collects failure diagnostics, and documents the checks as merge-blocking.

Changes

Architecture smoke validation

Layer / File(s) Summary
Native architecture builds
.github/workflows/arch_smoke.yml
Runs the workflow on native amd64 and arm64 runners, configures caching, builds the local target, and compiles the remote target on arm64.
Fixture-backed boot validation
.github/workflows/arch_smoke.yml
Creates fixture vault content, starts the image with authentication, polls health endpoints, and validates MCP initialization and serverInfo.
Failure handling and required checks
.github/workflows/arch_smoke.yml, CONTRIBUTING.md
Logs container diagnostics after failures, always cleans up the container, and documents both architecture checks as merge-blocking.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the new per-architecture image build and boot smoke workflow running on every pull request.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-arch-smoke-ci

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.

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

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/arch_smoke.yml:
- Around line 82-85: Update the cache-from and cache-to scope expressions in the
workflow’s architecture smoke build to include the build target, using distinct
local and remote scopes derived from matrix.arch so each target reads from and
writes to its own GHA cache.

In `@CONTRIBUTING.md`:
- Around line 122-125: Update the Arch Smoke required-check description in
CONTRIBUTING.md to explicitly state that the arm64 workflow compile-checks the
Dockerfile’s remote target and that failures can block merges, covering this
path alongside the existing arm64 boot smoke 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a538342-43c4-414e-b9b9-e84e33fc4fbd

📥 Commits

Reviewing files that changed from the base of the PR and between dbe2a08 and b69dca2.

📒 Files selected for processing (2)
  • .github/workflows/arch_smoke.yml
  • CONTRIBUTING.md

Comment thread .github/workflows/arch_smoke.yml Outdated
Comment thread CONTRIBUTING.md Outdated
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@aliasunder
aliasunder merged commit 9ac0074 into main Jul 31, 2026
16 checks passed
@aliasunder
aliasunder deleted the worktree-arch-smoke-ci branch July 31, 2026 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant