fix(docker): don't abort Docker startup when .env is missing - #4956
Conversation
…relative Windows Docker reports a generic file-not-found when env_file targets are missing, or when compose paths are doubled. Make docker-start copy .env examples and invoke compose with filenames relative to docker/. Co-authored-by: Cursor <cursoragent@cursor.com>
willem-bd
left a comment
There was a problem hiding this comment.
Two blocking concerns:
-
[P1] Existing backend tests fail. The new
prepare_compose_envpreflight exits before the mocked Compose command intest_compose_commands_set_deer_flow_root_before_composefor bothlogs --gatewayandrestart. I reproduced this locally as 2 failures / 6 passes, andbackend-unit-testsis now failing in CI. Please update the fixture to provide the compose/example files, or separate theDEER_FLOW_ROOTsetup from environment preparation. -
[P2] The reported reproduction is unchanged. Issue #2416 invokes
docker-compose -f docker/docker-compose-dev.yaml up --builddirectly, which bypassesscripts/docker.sh; therefore neither the new env-file creation nor the wrappercdbehavior applies. The base compose filename was already relative and normal wrapper invocations already ran aftercd "$DOCKER_DIR"; only the AIO overlay path changed. Please either address the direct invocation or explicitly document that the supported resolution ismake docker-startand adjust the issue claim accordingly.
e18261f to
516a58b
Compare
Address review feedback on bytedance#4956. [P1] prepare_compose_env aborted before the mocked COMPOSE_CMD in test_compose_commands_set_deer_flow_root_before_compose, because the temp root had no compose file or .env examples. Seed them in the fixture so the preflight reaches the mock. [P2] .env is gitignored, so a fresh clone has none and a direct `docker compose -f docker/docker-compose-dev.yaml up --build` aborts on Windows before scripts/docker.sh can help. Mark the dev env_file entries `required: false` so a missing .env is not fatal, and document that direct Compose must be run from the repository root. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the careful review — both concerns are addressed in 5b4cbc7. [P1] Backend testsReproduced on the previous head: I took the fixture route. I kept the preflight where it is rather than splitting it from the [P2] Direct invocation, and the issue claimYou're right, and it goes further than that: I can't reproduce #2416 at all. The reported is the "compose file itself is absent" error — So I've dropped the What this PR does fix is adjacent and reproducible every time.
Marking the dev Two things I'd like your call on:
If you'd rather not take the compose change at all, I'm glad to reduce this to the fixture |
willem-bd
left a comment
There was a problem hiding this comment.
The previous test failures and the missing-env direct-invocation case are improved, and the targeted suites plus current CI are green. Three compatibility issues remain: the undocumented Compose 2.24 minimum, the direct command rendering incorrect host paths without DEER_FLOW_ROOT, and non-start commands creating configuration files.
| - PROVISIONER_API_KEY=${PROVISIONER_API_KEY:-} | ||
| env_file: | ||
| - ../.env | ||
| - path: ../.env |
There was a problem hiding this comment.
[P1] Please declare or avoid the new Compose 2.24 minimum. The long-form env_file required field is not understood by older Compose v2 clients, so configurations that previously ran make docker-start now fail while parsing this file, before the wrapper-created env files can help. The prerequisites currently list Docker Desktop or Engine without a Compose version floor, and the wrapper has no version check. Either preserve the previous syntax/support or document Compose >= 2.24 and fail early with an actionable version check, including for direct callers.
| `make docker-start` reads `config.yaml` and starts `provisioner` only for provisioner/Kubernetes sandbox mode. | ||
|
|
||
| Prefer this wrapper over invoking Compose yourself. If you do run Compose | ||
| directly, do it from the repository root (`docker compose -f docker/docker-compose-dev.yaml`), |
There was a problem hiding this comment.
[P2] This direct command also needs to set DEER_FLOW_ROOT to the absolute checkout path. Running the documented command with it unset emits warnings and renders DEER_FLOW_HOST_BASE_DIR as /backend/.deer-flow and THREADS_HOST_PATH as /backend/.deer-flow/threads. Those are host-side paths in AIO/provisioner mode, so the command gets past the missing env file but remains misconfigured. Please document the platform-appropriate setting or make Compose derive it reliably.
| export DEER_FLOW_ROOT="$PROJECT_ROOT" | ||
| fi | ||
|
|
||
| prepare_compose_env |
There was a problem hiding this comment.
[P2] Please keep logs, stop, and restart read-only with respect to configuration. prepare_compose_env calls ensure_from_example for both env files, so these commands now mutate a fresh checkout and can even refuse to stop running containers if an example file is unavailable. Since the Compose entries are optional after this change, split the shared preflight into compose-file validation plus DEER_FLOW_ROOT setup, and run env-file creation only from start.
…ead-only Address the second review round on bytedance#4956. - Document Compose >= 2.24 (CONTRIBUTING, README, compose header) and fail early from make docker-start with an actionable message; probe both `docker compose` and the hyphenated `docker-compose` binary. - Document DEER_FLOW_ROOT for direct Compose callers (bash + PowerShell); leave the variable without a $PWD fallback because PowerShell/cmd do not export it. - Split prepare_compose_env: compose_preflight is shared and read-only; ensure_env_files runs only from start. - Expand tests for version boundaries, hyphenated fallback, env-file creation, and read-only stop/logs/restart behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the second review — all three compatibility points are addressed in 0bedc01 (on top of 5b4cbc7). Summary of this round
[P1] Compose >= 2.24Chose declare + fail early, not “preserve the old syntax” — keeping the short-form Where it is declared
Wrapper check (
Direct callers Compose YAML cannot run a version check. Direct [P2]
|
DEER_FLOW_ROOT warnings |
rendered DEER_FLOW_HOST_BASE_DIR |
|
|---|---|---|
| unset | 3 | /backend/.deer-flow |
| set as documented | 0 | <checkout>/backend/.deer-flow |
The “why no $PWD fallback” rationale is also in a comment above THREADS_HOST_PATH in the compose file so a later edit does not reintroduce it by accident.
[P2] Keep logs / stop / restart read-only
Split the old prepare_compose_env as requested:
compose_preflight→require_compose_file+require_compose_version+ensure_deer_flow_root(read-only; used by start/logs/stop/restart)ensure_env_files→ only called fromstart
Live check on a fresh tree (examples present, no .env yet)
| command | creates .env? |
|---|---|
logs --gateway / stop / restart |
no |
start (ensure_env_files) |
yes |
Also verified: with no .env.example at all, logs/stop/restart still complete (they no longer inherit ensure_from_example's exit 1).
Tests added / extended
- Version floor: 2.23.3 / 2.5.0 reject; 2.24.0 / desktop / 3.x pass; empty → warn+proceed
- Hyphenated fallback: plugin missing +
docker-compose2.24.0 passes; 2.23.3 rejects ensure_env_filescreates from examples; does not overwrite existing.envlogs/stop/restartsetDEER_FLOW_ROOT, create no env files, run without examples- Existing
test_dev_compose_env_files_are_optionalstill locks the long-form syntax
Local targeted run: 47 passed (plus one unrelated local AV false-positive deleting skillscan/orchestrator.py mid-import — not present in CI).
Intentionally out of scope / open calls
docker/docker-compose.yaml(prod /make up) still has short-form requiredenv_file. Same missing-.envexposure. Left out to keep this PR scoped — say the word and I will mirror the optional syntax + docs there.- Direct callers still have no programmatic version gate (inherent). Docs + header quote the old error.
- Undetectable Compose version warns rather than hard-fails. Prefer hard-fail? Easy to flip.
startalways printsUsing DEER_FLOW_ROOT=...(previously only when the script set a default). Small UX change; intended for visibility.
Happy to trim further (e.g. docs-only + fixture-only) if you would rather not take the Compose 2.24 floor at all — but that would mean dropping required: false and leaving the fresh-clone direct-invocation failure on Windows.
willem-bd
left a comment
There was a problem hiding this comment.
Thanks for addressing the previous review rounds. The earlier concerns are resolved, but one compatibility issue remains on 0bedc015:
[P2] The docker-compose fallback is only used for version detection. _compose_version_short() accepts a valid standalone docker-compose installation when the docker compose plugin is unavailable, but COMPOSE_CMD remains hardcoded to docker compose. As a result, the standalone 2.24 binary passes require_compose_version(), then start, logs, stop, and restart immediately invoke the unavailable plugin and fail.
I reproduced this by making docker compose version unavailable and returning 2.24.0 from docker-compose version: preflight succeeds, but the next $COMPOSE_CMD operation calls docker compose and exits. Please select and retain the Compose executable for both the version check and subsequent operations (preferably as an array), and extend the fallback test through an actual wrapper operation rather than stopping at require_compose_version().
Focused validation otherwise passed: 32 targeted tests, Compose config rendering with Compose 5.1.4, bash -n, and git diff --check.
The version probe could accept a standalone docker-compose install while COMPOSE_CMD stayed hardcoded to `docker compose`, so preflight passed and start/logs/stop/restart then failed. Keep the selected executable in COMPOSE_BIN (array), refresh COMPOSE_CMD from it in the current shell, and extend the fallback test through an actual stop invocation. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks — good catch. Fixed in abf0c6b. What was wrong
There was a second footgun in the first attempt at fixing this: assigning What changed
Verification
One residual note (not changed here): execution still goes through the |
Related to #2416 — but see the review discussion: the exact error in that issue is a
different failure mode and I could not reproduce it, so this PR no longer claims to close it.
Why
.envandfrontend/.envare gitignored, so a fresh clone has neither. Onmain, bothdocumented entry points then abort on Windows with a confusing "cannot find the file
specified" from Compose, before any service starts:
.env)maindocker compose -f docker/docker-compose-dev.yaml up --build(repo root)env file ... not foundmake docker-startenv file ... not foundMaking the
env_fileentries optional uses Compose's long-formpath/requiredsyntax, which needs Compose >= 2.24. This PR declares that floor, fails early from
the wrapper with an actionable message, and documents
DEER_FLOW_ROOTfor direct callers.What changed
make docker-startcopies.env/frontend/.envfrom examples when missing.env_fileentries arerequired: false(Compose >= 2.24).docker compose, thendocker-compose) andexits with an upgrade hint when too old.
logs/stop/restartonly run a read-only preflight (compose file + version +DEER_FLOW_ROOT); they never create env files.DEER_FLOW_ROOT(bash + PowerShell). No${DEER_FLOW_ROOT:-$PWD}fallback: PowerShell/cmd do not export
PWD, so a default would silently break on Windows.cd docker/with relative filenames.CONTRIBUTING.md,README.md, andREADME_zh.md.Surface area
frontend/backend/applanggraph.json, or prompt changedocker/or sandboxed executionskills/backend/pyproject.tomlorfrontend/package.json.envis tolerated by the dev compose file;Compose < 2.24 is rejected by
make docker-startwith an upgrade message;logs/stop/restartno longer create env filesBug fix verification
.envdeleted):config/up --build --dry-runsucceed here; fail onmain.services.gateway.env_file.0 must be a string(documented in the compose header).DEER_FLOW_ROOTset: 0 warnings; host paths render to the checkout.logs/stop/restarton a clean tree: no.envcreated; still succeed without.env.example.Open points for maintainers
docker/docker-compose.yaml) still uses short-formenv_file— left out of scope on purpose.AI assistance
Tool(s) used: Cursor
How you used it: AI helped reproduce the Windows Docker / Compose failures, draft the
script and compose changes, run red/green verification (including a real Compose v2.23.3
client), and write this PR text. I reviewed every line.