Skip to content

ci(docker): run the image before publishing it - #2189

Merged
igorls merged 3 commits into
developfrom
ci/docker-smoke-test
Aug 8, 2026
Merged

ci(docker): run the image before publishing it#2189
igorls merged 3 commits into
developfrom
ci/docker-smoke-test

Conversation

@igorls

@igorls igorls commented Aug 8, 2026

Copy link
Copy Markdown
Member

Follow-up to #2187 and #2188, as offered there.

Why

The Docker workflow built both images and never started a container, and never parsed a Compose file. A green run only ever meant "the Dockerfile compiles". Both bugs fixed this week break the first command the README tells a user to run, and both sailed through:

What this adds

scripts/docker-smoke.sh, covering the documented paths:

  1. compose config on docker-compose.yml and deploy/docker-compose.server.yml
  2. entrypoint dispatch — both cli … and bare passthrough
  3. mine a mounted directory → asserts a drawer is filed
  4. search from a separate container → asserts the text comes back verbatim
  5. a real MCP stdio JSON-RPC handshake → initialize, tools/list, and a mempalace_search call whose result must contain the drawer

It asserts on returned content, not just exit codes — the mine crash did exit non-zero, but the verbatim read-back is what actually proves the palace holds the text. Keeping it in a script rather than inline YAML means it runs identically on a laptop: scripts/docker-smoke.sh <image>.

Wiring

A new smoke job builds amd64 natively with load: true (buildx can't load a multi-arch manifest), then runs the script. build gains needs: smoke, so a failing smoke test blocks publication instead of being noticed after the fact.

Cache: smoke reads the publish job's gha cache (identical amd64 layers) but writes to scope=smoke, so an amd64-only export never lands on top of the multi-arch one — same split the build-gpu job already uses.

Verification

Not just "it passes" — I checked it fails for the right reasons, against real builds:

scenario result
current develop all 5 pass, exit 0
#2188 defect reintroduced fails at step 1: docker-compose.yml is not a valid Compose file, exit 1
#2187 defect reintroduced fails at step 3: 'mine /work' exited non-zero, exit 1

shellcheck and actionlint both clean.

Two notes

  • Step 3 reaches Chroma's S3 once for the ~80 MB embedding model, so the job is network-dependent. That download is part of a user's first run, so I'd rather cover it than stub it — but it's the most likely source of future flakes, and worth remembering before blaming a PR.
  • Failure output is clipped to 500 columns; a rejected embedding batch otherwise prints a full 384-dim vector on one line and buries the message.

The Docker workflow built both images and never started a container,
and never parsed a Compose file. A green run therefore only meant the
Dockerfile compiled. Two defects that break the very first documented
command shipped past it: `docker-compose.yml` carried a bare
`environment:` key that made Compose reject the file outright (#2188),
and `_embed_texts` handed chromadb `np.float32` scalars so `mine`
aborted on the first drawer (#2187).

Add `scripts/docker-smoke.sh`, which exercises what the README tells
users to run:

  1. `compose config` on docker-compose.yml and the server compose file
  2. entrypoint dispatch for both `cli ...` and bare passthrough
  3. `mine` a mounted directory, asserting a drawer is filed
  4. `search` from a *separate* container, asserting the stored text
     comes back verbatim — this is the assertion that matters, since
     storing user words exactly is the promise the palace makes
  5. a real MCP stdio JSON-RPC handshake: initialize, tools/list, and a
     mempalace_search call whose result must contain the drawer

It asserts on returned content, not just exit codes, and lives in a
script rather than inline YAML so it runs identically on a laptop:
`scripts/docker-smoke.sh <image>`.

The new `smoke` job builds amd64 natively with `load: true` (buildx
cannot load a multi-arch manifest) and reads the publish job's cache
while writing its own scope, so an amd64-only export never lands on top
of the multi-arch one. `build` now needs it, so a failing smoke test
blocks publication rather than being noticed afterwards.

Verified by reintroducing each defect against a real build: the compose
regression fails at step 1, the embedding regression at step 3, and the
current tree passes all five. Failure output is clipped to 500 columns
because a rejected embedding batch otherwise prints a whole 384-dim
vector on one line and buries the message.
@igorls
igorls requested a review from milla-jovovich as a code owner August 8, 2026 13:27
Copilot AI lite review requested due to automatic review settings August 8, 2026 13:27

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01a4a26414

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/docker-smoke.sh

# First run also downloads the ~80 MB embedding model into the volume, so this
# doubles as a check that a cold container can reach and cache it.
mine_out="$(docker run --rm -v "$VOLUME:/data" -v "$WORKDIR:/work" "$IMAGE" mine /work 2>&1)" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make the bind-mounted fixture traversable by the container

On ubuntu-latest, and on any developer host whose UID is not 1000, mktemp -d creates WORKDIR with mode 0700 owned by the host user, while this image runs as the fixed UID 1000. The process receiving this bind mount therefore cannot traverse /work to read notes.md, so the mine step either errors or reports zero drawers and the new required smoke job blocks every build and publication. Grant the directory suitable read/execute permissions or create the fixture with ownership accessible to the container before mounting it.

Useful? React with 👍 / 👎.

The first CI run failed at step 3 with

    PermissionError: [Errno 13] Permission denied: '/work/mempalace.yaml'

`mktemp -d` creates the fixture 0700 owned by the runner user. Bind
mounts carry host ownership through unchanged, and the image runs as
uid 1000, so the container could not stat inside /work. Docker
Desktop's uid mapping hides this on macOS, which is why it passed
locally and only failed on Linux.

Model an ordinary project checkout instead — 0755 dir, 0644 file — which
is the shape that makes the README's `-v /path/to/project:/work` work
against a normal repo.
@igorls
igorls merged commit c7a026b into develop Aug 8, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants