Skip to content

fix(docker): drop the null environment: block that invalidated compose - #2188

Merged
igorls merged 2 commits into
developfrom
fix/compose-null-environment-block
Aug 8, 2026
Merged

fix(docker): drop the null environment: block that invalidated compose#2188
igorls merged 2 commits into
developfrom
fix/compose-null-environment-block

Conversation

@igorls

@igorls igorls commented Aug 8, 2026

Copy link
Copy Markdown
Member

The bug

Every documented Compose command fails, on any machine, before anything starts:

$ docker compose run --rm mcp cli search "GraphQL"
validating /…/docker-compose.yml: services.mcp.environment must be a mapping

environment: was declared with nothing but comments under it. YAML parses that as null, and Compose rejects the file — so docker compose build, docker compose run, and docker compose down all refuse to do anything. The README points users at this file, and it has never worked.

Reported symptom in #2177 ("no real working isolated docker setup").

The fix

Comment the key out together with its example entries, and record in the file why it cannot be left bare, so the next person adding an override does not reintroduce it. The examples move to mapping syntax, so uncommenting them produces a valid block rather than a list.

Verification

  • docker compose config on the shipped file: rejected before, valid after.
  • deploy/docker-compose.server.yml checked for the same defect — unaffected, validates clean.
  • The documented flow, run against an image built from this tree: docker compose run --rm mcp cli mine … files a drawer, and a second docker compose run --rm mcp cli search … returns it verbatim from the persisted named volume.

Note

CI builds the Docker image but never runs it, and never validates the Compose files — which is why a file that fails on the first command shipped. A smoke step in docker-publish.yml (compose config on both files, plus one docker run of the built image) would have caught this; happy to send it separately if you want it in 3.7.0 too.

`environment:` was present with nothing but comments beneath it, so YAML
parsed it as null and Compose refused the whole file:

    services.mcp.environment must be a mapping

That is every documented Compose command — `docker compose build`,
`docker compose run --rm mcp`, `docker compose down` — failing before
anything starts, on any machine. The README points at this file.

Comment the key out along with its example entries, and say in the file
why it cannot be left bare. The examples switch to mapping syntax so
uncommenting them yields a valid block.

Verified with `docker compose config` on the shipped file (now valid)
and by running the documented flow against a build of this tree: mine a
mounted directory through `docker compose run --rm mcp cli mine`, then
read it back with `... cli search`, which returns the drawer verbatim.
`deploy/docker-compose.server.yml` was checked for the same defect and
is unaffected.
Copilot AI lite review requested due to automatic review settings August 8, 2026 12:23
@igorls
igorls requested a review from milla-jovovich as a code owner August 8, 2026 12:23

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.

Pull request overview

Fixes a Docker Compose YAML validation failure that prevented any documented docker compose workflow from running by removing a null-valued environment: block and preserving the intended override examples in a safe, commented form.

Changes:

  • Commented out the environment: key (and rewrote the example overrides as mapping syntax) to prevent YAML from parsing the block as null.
  • Added an in-file explanation of the Compose validation failure mode to reduce the chance of regression.
  • Documented the fix in the changelog.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
docker-compose.yml Removes the invalid null environment: block while keeping valid, commented override examples and an explanation of the Compose/YAML pitfall.
CHANGELOG.md Records the Compose validation fix and its impact on documented Docker flows.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@igorls
igorls merged commit 0ff93ca into develop Aug 8, 2026
8 checks passed
@igorls
igorls deleted the fix/compose-null-environment-block branch August 8, 2026 13:20
pull Bot pushed a commit to FaZios/mempalace that referenced this pull request Aug 8, 2026
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 (MemPalace#2188),
and `_embed_texts` handed chromadb `np.float32` scalars so `mine`
aborted on the first drawer (MemPalace#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.
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