Skip to content

fix(docker): lock the e2e runner's cargo test dependency graph - #1596

Merged
OlympusLedgerOrg merged 1 commit into
mainfrom
claude/pr1567-followup-locked-test
Aug 9, 2026
Merged

fix(docker): lock the e2e runner's cargo test dependency graph#1596
OlympusLedgerOrg merged 1 commit into
mainfrom
claude/pr1567-followup-locked-test

Conversation

@OlympusLedgerOrg

@OlympusLedgerOrg OlympusLedgerOrg commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to the CodeRabbit review round on #1567. Re-checked every item from that review against current main before touching anything:

  • Admin key interpolation (docker/compose.audit.yml, both services) — already on main: OLYMPUS_ADMIN_KEY: ${OLYMPUS_ADMIN_KEY:?set OLYMPUS_ADMIN_KEY before running the audit harness}.
  • Headless production refusal gate (src-tauri/src/bin/olympus-server.rs) — already on main: OLYMPUS_ENV=production/prod exits 2 before any DB work.
  • DATABASE_URL-free error message — already on main: generic "Could not connect to external database..." message, no interpolated URL.
  • Embedded-Postgres cleanup on both shutdown paths — already on main: bind-failure path calls stop_db() before exit(2); normal shutdown reports stop_db() errors instead of unconditionally claiming success.
  • docker/Dockerfile.audit --locked — already on main.
  • DOCKER-APP-TESTING-2026-07-28.md — already on main: output fences tagged text, run instructions require OLYMPUS_ADMIN_KEY.

One gap remained: the e2e service's cargo test invocation in docker/compose.audit.yml was still missing --locked, so the audit-image builder stage could silently resolve a different dependency graph for the test binary than Cargo.lock pins (the Dockerfile.audit build step already had this flag, but the compose-level test command didn't). This PR adds it.

Test plan

  • cat docker/compose.audit.yml | pnpm exec prettier --check --stdin-filepath docker/compose.audit.yml — passes clean
  • Local pre-push gate (cargo clippy --workspace -D warnings, full nextest suite incl. embedded-Postgres binaries, doctests) — all green
  • Verified by grep that items 1–4 and 6 from the review are present on main prior to this change

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved end-to-end test reliability by ensuring dependency versions match the project lockfile during test execution.

Follow-up to PR #1567's CodeRabbit review: the `Dockerfile.audit` build
already uses --locked, but the e2e service's `cargo test` invocation in
compose.audit.yml did not, so the audit-image builder stage could
silently resolve a different dependency graph for the test binary than
Cargo.lock pins. All other items from that review round (admin-key
interpolation, the headless binary's production refusal gate, the
DATABASE_URL-free error message, embedded-Postgres cleanup on both
shutdown paths, and the DOCKER-APP-TESTING doc corrections) already
landed on main via the earlier follow-up commits on that PR branch —
verified present here before making this change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 64a6083d-2882-475d-a9fa-1fee0ad39e71

📥 Commits

Reviewing files that changed from the base of the PR and between be49393 and 1a6ec81.

📒 Files selected for processing (1)
  • docker/compose.audit.yml

📝 Walkthrough

Walkthrough

The audit E2E runner now passes Cargo’s --locked flag. Cargo must use the existing lockfile during test execution.

Changes

Audit E2E test execution

Layer / File(s) Summary
Lockfile-enforced test command
docker/compose.audit.yml
The E2E Cargo test command now includes --locked.

Estimated code review effort: 1 (Trivial) | ~2 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 describes the main change: adding locked Cargo dependency resolution to the E2E runner.
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.

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.

@OlympusLedgerOrg
OlympusLedgerOrg merged commit c062bbe into main Aug 9, 2026
38 checks passed
@OlympusLedgerOrg
OlympusLedgerOrg deleted the claude/pr1567-followup-locked-test branch August 9, 2026 15:39
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