Skip to content

fix(scripts): preserve uv extras across make dev restarts (#2754)#2767

Open
fancyboi999 wants to merge 1 commit intobytedance:mainfrom
fancyboi999:fix/2754-uv-sync-wipes-postgres-extras
Open

fix(scripts): preserve uv extras across make dev restarts (#2754)#2767
fancyboi999 wants to merge 1 commit intobytedance:mainfrom
fancyboi999:fix/2754-uv-sync-wipes-postgres-extras

Conversation

@fancyboi999
Copy link
Copy Markdown
Contributor

Summary

Fixes #2754make dev ran uv sync unconditionally on every restart, wiping any optional extras (asyncpg / psycopg / langgraph-checkpoint-postgres / psycopg-pool) the user had installed manually with uv sync --extra postgres.

The Docker image-build path already solved this via the UV_EXTRAS build-arg in backend/Dockerfile; the local serve.sh path and docker-compose-dev.yaml's startup command were the remaining outliers.

What changed

  • scripts/serve.sh now resolves extras before uv sync:
    1. honors UV_EXTRAS env var (parity with backend/Dockerfile and docker/docker-compose.yaml — no new convention introduced).
    2. falls back to parsing config.yamldatabase.backend: postgres or legacy checkpointer.type: postgres auto-pins --extra postgres, so the common case needs zero extra config.
  • scripts/detect_uv_extras.py (new) — stdlib-only resolver. Has to run before the venv exists, so it cannot depend on PyYAML.
  • docker/docker-compose-dev.yaml — startup uv sync (and its self-heal fallback) now honors UV_EXTRAS too via $${UV_EXTRAS:+--extra $$UV_EXTRAS} (matches backend/Dockerfile:51). Users set UV_EXTRAS=postgres in project-root .env (already loaded via env_file: ../.env).
  • backend/.../persistence/engine.py — asyncpg-missing error message now points at the persistent flow.
  • config.example.yaml — install instructions cover all three paths (local / docker dev / docker image build).

Why not --no-sync?

A separate suggestion in the issue thread was to switch to uv run --no-sync for the dev path. That would solve the symptom but in dev paths it would also drop:

  • the auto-apply of new pyproject.toml deps after git pull / branch switch,
  • the self-heal (uv sync || (recreate venv && retry)) branch in docker-compose-dev.yaml:128.

--no-sync is already in use for the production CMD (backend/Dockerfile:101) where it's appropriate. For dev, keeping uv sync and letting it know which extras the user actually wants is the more robust direction.

Test plan

  • Unit tests: tests/test_detect_uv_extras.py — 19 tests covering env parsing, YAML parsing edge cases (comments, quotes, nested layers), env-vs-config precedence, explicit DEER_FLOW_CONFIG_PATH, root-vs-backend lookup precedence, missing files.
  • Regression: tests/test_persistence_scaffold.py — 22/22 still passing (the postgres error-message assertion still matches; the new wording keeps the uv sync --extra postgres substring).
  • Boundary: tests/test_harness_boundary.py — still passing.
  • Static: ruff check + ruff format clean. bash -n scripts/serve.sh clean. yaml.safe_load parses docker-compose-dev.yaml cleanly.
  • End-to-end loop (local path) — really executed:
    • Reproduced the bug: uv sync --quiet (no flags) wiped asyncpg from venv ✅ (matches issue report)
    • Fix path A (UV_EXTRAS=postgres): detect_uv_extras.py--extra postgresbash word-splits → uv sync --quiet --extra postgresasyncpg, psycopg, psycopg-binary, psycopg-pool all preserved ✅
    • Fix path B (auto-detect via DEER_FLOW_CONFIG_PATH to a temporary database.backend: postgres config): same result, all preserved ✅
  • Docker dev path — not run end-to-end. Docker daemon was not available in my test environment. The change in docker-compose-dev.yaml mirrors backend/Dockerfile:51 exactly (same ${UV_EXTRAS:+--extra $UV_EXTRAS} pattern, escaped as $$ per docker-compose convention) and yaml parses cleanly, but I could not actually start the container and inspect the venv. Reviewer please verify, or I can iterate if a reviewer reports issues.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes recurring loss of optional uv extras (notably postgres) across make dev / dev-container restarts by ensuring uv sync is invoked with the correct extras inferred from environment/config.

Changes:

  • Update local dev launcher to detect and pass uv extras into uv sync during startup.
  • Add a stdlib-only extras detector script plus unit tests to validate env/config precedence and YAML edge cases.
  • Propagate UV_EXTRAS into Docker dev startup uv sync, and update Postgres-related guidance/error messaging.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/serve.sh Runs extras detection before uv sync and splats resolved flags into the backend dependency sync.
scripts/detect_uv_extras.py New stdlib-only resolver for UV_EXTRAS / config.yaml → --extra ... flags.
docker/docker-compose-dev.yaml Ensures container startup uv sync honors UV_EXTRAS as well.
config.example.yaml Updates Postgres install guidance to reflect the new extras-preservation behavior across paths.
backend/tests/test_detect_uv_extras.py Adds unit tests for the detector’s env parsing and minimal YAML parsing behavior.
backend/packages/harness/deerflow/persistence/engine.py Improves missing-asyncpg error message with the persistent install flow.

Comment thread scripts/serve.sh Outdated
Comment thread docker/docker-compose-dev.yaml Outdated
Comment thread scripts/detect_uv_extras.py Outdated
@fancyboi999 fancyboi999 force-pushed the fix/2754-uv-sync-wipes-postgres-extras branch from 3035c30 to 1c8cf8c Compare May 9, 2026 02:28
@WillemJiang
Copy link
Copy Markdown
Collaborator

@fancyboi999 thanks for the contribution. Here are some additional comment

  1. Docker-compose command is a single very long line

docker/docker-compose-dev.yaml:132 is now a ~350-character one-liner. This is hard to read, review, and maintain. Consider extracting to a shell script (e.g., scripts/docker-entrypoint-dev.sh) that can be properly formatted and linted, or at minimum use YAML multiline string syntax:

command: >
  sh -c '
    cd backend &&
    if [ -n "$$UV_EXTRAS" ] && ! printf ...
    ...
  '
  1. Multi-extra inconsistency between local and Docker dev paths

The local path (detect_uv_extras.py) supports comma/whitespace-separated extras (UV_EXTRAS=postgres,ollama), but the Docker dev path only supports a single extra — the grep validation '^[A-Za-z][A-Za-z0-9_-]*$' rejects commas. If someone sets UV_EXTRAS=postgres,ollama in their .env, local make dev works but make docker-start aborts with an error. This should either be documented as intentional or harmonized.

  1. Silent detector failures in serve.sh

scripts/serve.sh:177:
UV_EXTRAS_FLAGS=$("$DETECT_PYTHON" "$REPO_ROOT/scripts/detect_uv_extras.py" 2>/dev/null || true)
Stderr is suppressed and failures are silently swallowed. If the detector crashes (e.g., Python version incompatibility, file permission issue), the user gets no indication that extras detection was skipped.
Consider logging to stderr on failure:

UV_EXTRAS_FLAGS=$("$DETECT_PYTHON" "$REPO_ROOT/scripts/detect_uv_extras.py" 2>/dev/null || echo "[serve.sh] Warning: extras detector failed, proceeding without extras" >&2)

Or at least don't redirect stderr during development.

…e#2754)

`make dev` ran `uv sync` unconditionally on every restart, wiping any
optional extras the user had installed manually with
`uv sync --all-packages --extra postgres`. The Docker image-build path
already solved this via the `UV_EXTRAS` build-arg in backend/Dockerfile;
the local serve.sh path and the docker-compose-dev startup command
were the remaining outliers.

`scripts/serve.sh` now resolves extras before `uv sync`:
  1. honors `UV_EXTRAS` (parity with backend/Dockerfile and
     docker/docker-compose.yaml — no new convention introduced);
  2. falls back to parsing config.yaml — `database.backend: postgres`
     or legacy `checkpointer.type: postgres` auto-pins
     `--extra postgres`, so the common case needs zero extra config.
  3. detector stderr is no longer suppressed, so whitelist warnings or
     crashes surface to the dev terminal (review feedback).

Detection lives in `scripts/detect_uv_extras.py` (stdlib-only — has to
run before the venv exists). Extra names are validated against
`^[A-Za-z][A-Za-z0-9_-]*$` so a stray shell metacharacter in `.env`
cannot reach `uv sync` downstream (defense in depth).

`docker/docker-compose-dev.yaml`'s startup command is now extracted to
`docker/dev-entrypoint.sh` (review feedback — the inline command had
grown to a ~350-char one-liner). The script:
  - parses comma/whitespace-separated UV_EXTRAS, applying the same
    `^[A-Za-z][A-Za-z0-9_-]*$` whitelist as the local detector;
  - emits one `--extra X` flag per token, so `UV_EXTRAS=postgres,ollama`
    works in Docker dev too (harmonized with local — review feedback);
  - calls `uv sync --all-packages` (PR bytedance#2584) so workspace member
    extras (deerflow-harness's postgres extra) are installed;
  - keeps the existing self-heal `(uv sync || (recreate venv && retry))`
    branch;
  - exposes `--print-extras` for dry-run testing.

The compose file mounts the script read-only at runtime, so script
edits take effect on `make docker-restart` without an image rebuild.

The `--no-sync` alternative (a separate suggestion in the issue thread)
was considered but rejected for dev paths because it would drop the
self-heal branch and the auto-pickup of new pyproject deps. `--no-sync`
is already in use for the production CMD (`backend/Dockerfile:101`)
where it's appropriate.

Updates the asyncpg-missing error message to include the
`--all-packages` flag (matching bytedance#2584) plus the persistent install flow,
and expands `config.example.yaml` so all three install paths
(local / docker dev / docker image build) are documented with their
multi-extra capabilities.

Tests:
  - `tests/test_detect_uv_extras.py` (21 tests) — local-path env parsing,
    YAML edge cases, env-vs-config precedence, whitelist rejection of
    shell metacharacters.
  - `tests/test_dev_entrypoint.py` (15 tests) — docker-path validation
    via `--print-extras`, multi-extra parsing, metacharacter abort.
  - `tests/test_persistence_scaffold.py` (22 tests, unchanged) — passes
    with the merged `--all-packages --extra postgres` error message.
@fancyboi999 fancyboi999 force-pushed the fix/2754-uv-sync-wipes-postgres-extras branch from 1c8cf8c to d09d885 Compare May 9, 2026 13:19
@fancyboi999
Copy link
Copy Markdown
Contributor Author

@WillemJiang thanks for the detailed feedback. All three addressed in d09d885f:

1. Long one-liner extracted to docker/dev-entrypoint.sh

The compose command: is now a two-element list pointing at the script:

command: ["sh", "/usr/local/bin/dev-entrypoint.sh"]
volumes:
  - ./dev-entrypoint.sh:/usr/local/bin/dev-entrypoint.sh:ro
  ...

Mounted read-only so script edits take effect on make docker-restart without an image rebuild. The script is POSIX-sh (alpine-safe), shellcheck-clean, and self-documents the responsibilities (UV_EXTRAS validation → uv sync --all-packages → self-heal → uvicorn handoff).

2. Multi-extra harmonized between local and Docker dev

docker/dev-entrypoint.sh now parses UV_EXTRAS the same way as scripts/detect_uv_extras.py: split on comma/whitespace, validate each token against ^[A-Za-z][A-Za-z0-9_-]*$, emit one --extra X per token. So UV_EXTRAS=postgres,ollama works in both make dev and make docker-start. The Docker image-build path (backend/Dockerfile:51) still treats it as a single token since build args don't go through that script — documented in config.example.yaml and the detector docstring.

The script gained a --print-extras dry-run hook used by the new tests/test_dev_entrypoint.py (15 tests) so the validation logic has real coverage, not just shell sanity.

3. Detector stderr no longer suppressed in serve.sh

UV_EXTRAS_FLAGS=$("$DETECT_PYTHON" "$REPO_ROOT/scripts/detect_uv_extras.py" \
    || { echo "[serve.sh] detect_uv_extras.py failed (exit $?) — proceeding without extras" >&2; echo ""; })
  • whitelist warnings (e.g. ignoring invalid UV_EXTRAS entry ';') now surface to the dev terminal,
  • detector crashes print an explicit [serve.sh] detect_uv_extras.py failed line on stderr,
  • || { ...; echo ""; } keeps set -e from killing dev startup when the detector fails (graceful degrade — proceed without extras).

Total verification (all green on the rebased branch):

  • 59 unit tests (test_detect_uv_extras 21, test_dev_entrypoint 15, test_persistence_scaffold 22, test_harness_boundary 1)
  • ruff check + ruff format clean
  • sh -n dev-entrypoint.sh, bash -n serve.sh clean
  • end-to-end loop: bug reproduced under plain uv sync, both fix paths (UV_EXTRAS env + config.yaml auto-detect) preserve asyncpg/psycopg/psycopg-binary/psycopg-pool
  • shell injection bait (UV_EXTRAS="; rm -rf /") is rejected by both whitelists with stderr abort

Docker dev path itself was not started end-to-end (no docker daemon in my test env) — Reviewer please confirm if a real run uncovers anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewing The PR is in reviewing status

Projects

None yet

Development

Successfully merging this pull request may close these issues.

database选择postgres,本地执行uv sync --extra postgres,每次执行make dev都需要重新安装postgres依赖

3 participants