Repository navigation
fix(docker): bundle Lark broker runtime helpers - #7542
Conversation
|
@zHaooJ thanks for your contribution, please click the CLA button to sign the CLA, otherwise we cannot merge your code. |
willem-bd
left a comment
There was a problem hiding this comment.
Reviewed at 6502d7f. Verified the root cause and the fix end-to-end, including a real container run:
- Root cause confirmed: #7200 (582b1f5) added
from deerflow.utils.atomic_write import cleanup_failed_atomic_writetolark_broker.py, but the image copied only the single module, so both entrypoint modes died at import. The bundled closure is exactly right:atomic_write.py→file_revision_metadata.py, both stdlib-only; a full import sweep oflark_broker.py(including the embedded shim script) finds no other non-stdlib imports. The/opt/broker/deerflownamespace-package layout works because the harness's owndeerflow/utils/has no__init__.pyeither, and/opt/brokerissys.path[0]under the entrypoint'spython3 /opt/broker/lark_broker.py. - Red/green at container level: built the final stage with and without the new COPY against this head. Without it, both
serveandinstall-shimfail withModuleNotFoundError: No module named 'deerflow'; with it,install-shimwrites the shim andserveanswers{"ok": true}on 127.0.0.1:8788. Added image content is two files, ~7 KB — minimal; base images andLARK_CLI_VERSIONpinning are unchanged. - Tests:
test_lark_broker_image.py2 passed (PYTHONPATH pinned to the PR tree, import path asserted into it); compose explicit-bind suitetest_compose_default_bind_host.py15 passed (compose files untouched); ruff check + format clean on the new test. Guidance budgets:backend/tests/AGENTS.mdat 28,203/49,152 — the only AG002 error in the tree is the pre-existingruntime/AGENTS.mdchain (99,004 > 98,304), which involves no file this PR touches. - Mounting rules: compose/Helm untouched; the
entrypoint.shchanges are comment-only, so credential-mount behavior is unchanged.
One non-blocking nit inline about keeping the hand-curated closure discoverable at the import site.
|
|
||
| COPY --from=builder /opt/lark-cli /opt/lark-cli | ||
| # Single stdlib-only module drives both install-shim and serve. | ||
| # Bundle the broker's stdlib-only dependency closure without installing the harness. |
There was a problem hiding this comment.
This bundle is a hand-curated import closure — correct today (lark_broker.py imports only deerflow.utils.atomic_write, which imports deerflow.utils.file_revision_metadata; no other non-stdlib imports exist), but nothing at the import site flags the contract: if a future change adds another deerflow.* import to lark_broker.py or to either helper, this COPY list must grow or both broker modes break at runtime again (this exact failure class already shipped once via #7200). test_lark_broker_image.py does catch the drift in CI, but only after the fact. Consider a one-line pointer near lark_broker.py's from deerflow.utils.atomic_write import ... — its module docstring still promises plain stdlib-only self-containment — naming this COPY list as the thing to keep in sync.
There was a problem hiding this comment.
Thanks for the suggestion. Addressed in 9daf54f: the module docstring now clarifies that the broker and its bundled DeerFlow utilities use only the standard library. A comment beside the atomic_write import points to docker/lark-cli-broker/Dockerfile and documents the need to keep its Python COPY list in sync with transitive dependencies. The existing isolated image regression tests remain in place, and all CI checks are passing.
Why
The optional Lark CLI broker image copies
lark_broker.pybut omits its shared atomic-write helpers. The broker now importsdeerflow.utils.atomic_write, which also importsdeerflow.utils.file_revision_metadata. In the standalone image, bothinstall-shimandservefail during import withModuleNotFoundError: No module named 'deerflow'.What changed
/opt/broker/deerflow/utils/, so both broker modes can start without installing the harness.python -I -S.Surface area
frontend/backend/applanggraph.json, or prompt changedocker/or sandboxed executionskills/backend/pyproject.tomlorfrontend/package.jsonBug fix verification
backend/tests/test_lark_broker_image.py.ce0fa4c1, green with this fix: yes, both tests reproduced the missingdeerflowimport before the COPY change.Validation
Run with the existing Python virtual environment and repository packages on
PYTHONPATH:python scripts/check_agent_guidance.py --base-ref 3d5107dc0dd13a1f83b696b6682483aea64da896 --head-ref HEAD --github-annotations(repository root) — 56 AGENTS.md, 0 errors, 0 warnings. The branch includes upstream fix(ci): keep runtime guidance within the chain budget #7522, which fixes the previous guidance-chain budget failure.python -m pytest tests/test_lark_broker_image.py tests/test_lark_broker.py tests/test_atomic_cleanup_preserves_primary_error.py tests/test_agent_guidance_check.py -q(backend) — 46 passed, 27 skipped; POSIX-only tests skipped on Windows.python -m pytest tests/blocking_io -q --tb=short --maxfail=1(backend) — 285 passed, 8 skipped, before and after this follow-up.ruff check . --fixandruff format .— passed; 2,363 files already formatted.python -m pytest -m 'not live' --ignore=tests/blocking_io tests/ -q --maxfail=1 --tb=short(backend) — 1 failed, 4,004 passed, 157 skipped, 14 deselected. The remaining failure istest_cached_history_saver.py::test_fanout_compose_matches_inner_canonical_order[asyncio]: expected write task IDs["task-a", "task-b"], actual[]. This same failure was reproduced in earlier local baseline runs; the test and its production modules are unchanged from the original PR commit.pnpm checkcould not complete because the existing dependency tree is incomplete (eslint-plugin-react-hooksis missing). This PR changes no frontend files.git diff --check— passed.A full Docker image build was not run because the local Docker engine is unavailable. The isolated tests exercise the Python runtime file layout and startup behavior, but do not validate the image build or Lark CLI binary download.
AI assistance
Tool(s) used: OpenAI Codex.
How you used it: Codex investigated the missing image dependency, wrote and verified the regression tests, made the packaging and documentation changes, and reviewed the final diff. The human acknowledgment below is pending and must be completed by the submitter after reviewing the change.