Skip to content

[fix] deploy rebuilds a project's own image - #99

Merged
malickyeu merged 1 commit into
mainfrom
fix/deploy-rebuilds-images
Jul 31, 2026
Merged

[fix] deploy rebuilds a project's own image#99
malickyeu merged 1 commit into
mainfrom
fix/deploy-rebuilds-images

Conversation

@malickyeu

Copy link
Copy Markdown
Contributor

Summary

This started as the roadmap item "upload build: contexts to a remote host". It
ends somewhere else, so the short version first:

The roadmap item was based on a wrong premise, and building it would have been
wasted work.
Docker's build API takes the build context as a tar stream from
the client
, so a remote daemon has always received your local ./app folder and
built the image there. Verified before writing any code: the image ends up only on
the remote daemon and contains a file that exists only in the local context.

That is genuinely different from a bind mount — which nothing uploads, and which is
precisely why the dcseed-* volume seeding exists. Conflating the two is what put
the item on the roadmap. It's now retired in NEXT.md with the reasoning, so it
doesn't come back.

What the investigation did find is a real bug, and it's the fix here.
docker compose up -d builds a service only when its image is missing. So a
project with a build: section was built on its first deploy and never again:
every later deploy reused the original image no matter what changed in the
Dockerfile or its context. Edit a Dockerfile in the project editor, hit Deploy, and
the CLI reports Container Running and exits 0 — a wrong answer delivered as a
success, which is the worst shape for a bug in a tool whose whole promise is "the
files in the editor are what runs."

Deploy now passes --build.

Type of change

  • Bug fix
  • New feature
  • Docs only
  • Refactor / chore

Checklist

  • go test -short ./... and go vet ./... pass
  • gofmt gate is clean (gofmt -l $(git ls-files '*.go') after staging)
  • Frontend type-checks (cd web && npx tsc --noEmit)
  • Rebuilt and committed web/dist
  • Added/updated tests for the change
  • Updated docs/ and added a CHANGELOG.md entry

Notes for reviewers

Why --build unconditionally rather than a toggle. It is a no-op for services
that only pull an image, so the cost lands exactly where the benefit is: on
projects that actually declare build:, which are precisely the ones that were
silently going stale. That avoids a UI toggle nobody would find before being bitten
once. POST with {"build": false} opts out for the rare case where re-sending a
large context over a slow link matters more than freshness — the request field is a
*bool so absent means rebuild, and false is distinguishable from unset.

MCP matches the UI (mcpDeployProject passes true). Not a widening of the
MCP surface: up already builds a missing image, so deploying a project with a
build: section could always run its Dockerfile. What it fixes is the
inconsistency where the same project deployed via MCP would keep a stale image
while the UI refreshed it.

Two tests, and both were checked against the bug rather than assumed:

  • TestIntegrationComposeUpRebuildsChangedContext pins both halves — that
    without --build a context change is ignored, and with it the change lands.
    The negative half is the point: a test asserting only "the new content appears"
    would pass for the wrong reason if compose ever rebuilt incidentally, and would
    never have caught the original bug.

  • TestRemoteBuildContextDeployEndToEnd (gated on DC_REMOTE_DOCKER) covers the
    combination nothing else reached: a project that builds its own image and
    bind-mounts a project file
    , deployed to a daemon that can't see the folder.
    The two mechanisms travel completely differently and meet in one
    -f compose.yml -f override.json invocation. It also asserts the build context
    is not misclassified as a bind needing seeding.

    Verified by mutation: flipping that deploy to build=false fails it with
    /marker.txt = "BUILT-V1" after the edit. Run against a docker:28-dind
    sidecar at tcp://127.0.0.1:12500.

Also corrected a UI string that had become false. The remote-host hint still
said bind-mounting projects were "blocked at deploy … until file sync lands"
seeding shipped a while ago. It now describes what actually happens, including that
build: contexts are uploaded with the build.

Test data: all containers/images created during verification were removed; the
dind sidecar is torn down separately.

`docker compose up -d` builds a service only when its image is MISSING, so
a project with a build: section was built once and never again. Editing a
Dockerfile and hitting Deploy silently kept the old image running, with
the CLI reporting nothing worse than "Container Running". Deploy now
passes --build; it is a no-op for image-only services. POST {"build":
false} opts out, and the MCP deploy tool matches the UI.

Retires the "upload build: contexts to a remote host" roadmap item as
based on a wrong premise: Docker's build API sends the context as a tar
from the client, so a remote daemon has always received the local folder
and built the image there. Verified end to end against a second daemon,
together with the bind seeding it is often confused with.
Copilot AI review requested due to automatic review settings July 31, 2026 08:46

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.

@malickyeu
malickyeu merged commit 78b8c90 into main Jul 31, 2026
3 checks passed
@malickyeu
malickyeu deleted the fix/deploy-rebuilds-images branch July 31, 2026 08:57
malickyeu added a commit that referenced this pull request Jul 31, 2026
…104)

README/docs/README/NEXT: stacks can now be edited & redeployed, project
deploy rebuilds, build: contexts upload with the build. docs/mcp.md: the
MCP deploy tool passes --build like the UI, and why that isn't a wider
surface. NEXT: the version matrix is CI-verified, not just local.

Records what repeated testing turned up, in the places it will be read
before the next test is written (feature-tests skill, docs/testing.md,
CONTRIBUTING): mutation-test every guard and check it fails for the right
reason — three tests here passed while guarding nothing, because compose
validation or a shared status code was doing the rejecting. Plus the
daemon fixture traps: t.Context() is cancelled before t.Cleanup runs, a
pentest whose guard fails leaves real containers behind, and a killed run
poisons the next one.

Also logs a gap found while auditing: StackRedeploy still runs a plain
up -d, so a CLI stack with build: has the same staleness bug #99 fixed
for Projects.
malickyeu added a commit that referenced this pull request Jul 31, 2026
StackRedeploy ran a plain `docker compose up -d`, so a CLI-discovered
stack declaring build: kept running the image from its first deploy no
matter what changed in its Dockerfile or context on the host — the same
staleness bug fixed for Projects in #99, on the other code path.

Found while auditing the docs against the merged code: the two deploy
paths are separate, and only one had been corrected.

The test builds the stack with the real compose CLI so the labels
StackRedeploy works from are genuine, then edits the context and
redeploys through the app. Verified by mutation: without --build it
fails with marker "v1".
malickyeu added a commit that referenced this pull request Jul 31, 2026
StackRedeploy ran a plain `docker compose up -d`, so a CLI-discovered
stack declaring build: kept running the image from its first deploy no
matter what changed in its Dockerfile or context on the host — the same
staleness bug fixed for Projects in #99, on the other code path.

Found while auditing the docs against the merged code: the two deploy
paths are separate, and only one had been corrected.

The test builds the stack with the real compose CLI so the labels
StackRedeploy works from are genuine, then edits the context and
redeploys through the app. Verified by mutation: without --build it
fails with marker "v1".
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