Repository navigation
perf(prd-262): worker cuts — 1,074 → 910 MB, toolchain kept - #1220
Merged
Merged
Conversation
…ries Breakdown rows 16 and 19 (19 Oct size breakdown), about 52.6 MB installed on arm64. - Row 16 (29.4 MB): /usr/bin/*lto-dump* goes in the apt layer that installs build-essential. gcc -flto runs lto1, lto-wrapper and the linker plugin, never lto-dump, which only prints LTO objects. - Row 19 (23.2 MB): the API's git trim (PRD-262 US-019) applied to the worker: git-shell, scalar, git-daemon, git-http-backend, git-http-push and git-imap-send, both copies, and git-core's second copy of git becomes a link to /usr/bin/git. git-remote-http(s) and ssh stay, so clone, pull and push over HTTPS and SSH work. scripts/ci/image_worker_toolchain.py runs in the worker image images.yml built, on both arches: no lto-dump, two units compiled and linked with -flto run; the trimmed binaries are gone, git-core's git links to /usr/bin/git, and git init, commit, branch, checkout, clone, pull, an HTTPS clone and git credential fill all work. orchestrator/tests/test_prd262_worker_diet.py pins the Dockerfile lines against what the in-image check looks for, and the images.yml step. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit a94b76d65941486bd20648b0a3cd5a940d193824)
…, not gosu Breakdown row 47 (worker smalls): gosu, 2.9 MB installed, leaves the apt list. entrypoint.sh now ends exec setpriv --reuid=worker --regid=worker --init-groups env HOME=/home/worker python -m main "$@" setpriv comes with util-linux in the python:3.12-slim base. It sets the real, effective and saved uid and gid and the worker's supplementary groups, as gosu did, but leaves HOME as root's, so the exec sets HOME=/home/worker: the canvas's Claude CLI and ~/.cache read it. - test_prd262_worker_boot.py runs the entrypoint with a stand-in setpriv and pins the new exec line. - test.yml's parity job (Railway build) now also checks the worker starts with HOME=/home/worker, besides uid 1000. - scripts/ci/image_worker_toolchain.py checks in the GHCR image that gosu is gone and that the same setpriv drop runs a command as 1000:1000 with HOME=/home/worker. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit 97a496b95dfeb8a76b8cf5ef9698f6de13249b0d)
…shared with media-render Breakdown row 21: 20.0 MB installed (123.3 MB node binary, of which .symtab, .strtab and debug sections are 20 MB). A node-stripped stage follows the node stage, byte for byte the text the media-render lane adds to services/media-render/Dockerfile: it installs binutils in the build stage only and runs strip --strip-unneeded --preserve-dates on /usr/local/bin/node. strip keeps .dynsym, so native addons still resolve Node's N-API symbols. The production stage copies the binary with COPY --link --from=node-stripped /usr/local/bin/node /usr/local/bin/node as its first instruction, so on the same python:3.12-slim digest the worker's and media-render's node layers are one blob; npm, corepack and pnpm are copied and installed as before. - scripts/ci/image_worker_toolchain.py (images.yml, both arches): node has no .symtab/.strtab/.debug_*, keeps .dynsym, prints v24 and a correct sha256; an N-API addon compiled in the image (ABI only, loaded by napi_register_module_v1) loads and answers; npm, npx, corepack and pnpm --version run on it. - test_prd262_worker_diet.py pins the stage text, the COPY line and its position, and that any Dockerfile with a node-stripped stage carries the same text. - test_prd262_node24_images.py now accepts Node's files from any stage built FROM node (and only Node's /usr/local paths), so the media lane's same change passes; test_prd262_worker_browser.py reads the new COPY line. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit 1242473d5a625db02381f1c51eda20062209c1ed)
…stall's caches Breakdown row 47 (worker smalls), about 4.1 MB installed: - npm docs and man pages (2.67 MB): a new `npm` stage, FROM node, deletes node_modules/npm/docs and /man, and the production stage copies npm from it. The image has no man, so `npm help` never showed them; `npm <cmd> --help` prints from npm's own code. - The compile-cache leftover (1.4 MB): Dockerfile:53-54 ran npm cache clean but left Node 24's /tmp/node-compile-cache, which npm writes, and npm's logs in /root/.npm. The same RUN now deletes both, last. scripts/ci/image_worker_toolchain.py checks first, before any of its checks runs npm, that none of those paths is in the image; npm, npx, corepack and pnpm still run (row 21's check). test_prd262_worker_diet.py pins the stage, the copy and the cleanup's place in the RUN. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit d485934d701bdc5fdb81ed58ef80e7a9e3bc895d)
…info and drops test suites
Breakdown rows 40 (8.1 MB) and 47's tests directories (0.62 MB).
The stable lock's RUN now ends, in the same layer, with the API's US-020 rule:
- find site-packages -type f -name '*.so*' -not -path '*.libs/*'
-exec strip --strip-debug {} +: the .debug_* sections go (31.7 MB of .so become
23.7, mostly cryptography's), .dynsym stays, and the auditwheel *.libs libraries,
rewritten by patchelf, are left alone. binutils is already there with
build-essential. The SDK's bundled CLI is installed later and matches no *.so*.
- find site-packages -type d -name tests -prune -exec rm -rf {} +: jsonschema's,
referencing's and certifi's test suites.
scripts/ci/image_worker_site_packages.py, run by a new images.yml step in the worker
image on both arches as uid 1000: every *.so* outside *.libs has no .debug_* section
and loads in a fresh interpreter (an extension exporting PyInit_<name> is imported by
its dotted name, anything else opened with ctypes), cffi, cryptography, pydantic-core
and rpds among them; no tests directory is left. test_prd262_worker_diet.py pins the
two find commands and the step.
Signed-off-by: Gerard Kavanagh <gerard161@gmail.com>
(cherry picked from commit 602bcb5ac781428c4b6609bba11ec2c30eccc708)
Breakdown row 47 (worker smalls): hiredis 1.1 MB, python-dotenv 0.12 MB. - requirements.in asks for redis instead of redis[hiredis]. redis-py falls back to its pure-Python reply parser, which the worker's queue rate (a BRPOP poll, task status writes and canvas publishes) does not notice. - python-dotenv goes: no worker module imports dotenv, and no locked package depends on it. Relocked with the lock's own command (uv pip compile ... --universal --python-version 3.12 --generate-hashes): the only change is the two entries gone; every other pin and hash is as it was. requirements-browser.lock, compiled against it, is unchanged. scripts/ci/image_worker_site_packages.py checks in the image that neither distribution is installed and that redis.asyncio imports with HIREDIS_AVAILABLE false; test_prd262_worker_diet.py pins requirements.in, both locks and that no worker module imports either. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit 6bef38a97ec9931601886bde2a0dca768c157097)
…ead of psycopg2-binary Breakdown row 27: 15.7 MB installed (psycopg2_binary.libs, 16.13 MB, out; libpq5, 0.41 MB, in). Dependency change, justified: psycopg2-binary and psycopg2 are the same project, release (2.9.13, the lock's psycopg2-binary pin and PyPI's latest) and API; only the packaging differs. The binary wheel bundles its own libpq, OpenSSL 1.1 and 3, krb5, LDAP and SASL, libraries Debian already installs in this image; built from source, psycopg2 links Debian's libpq5, which the weekly OS rebuild keeps patched. No code change: task_executions_db.py's one UPDATE imports psycopg2 as before. - requirements.in: psycopg2>=2.9.0,<3.0.0 replaces psycopg2-binary. Relocked with the lock's own command; the only change is that entry (its hashes are the sdist's and the Windows wheels'). The browser lock is unchanged. - Dockerfile: the locks stage cuts psycopg2's entry out as psycopg2.lock (the build fails unless it holds psycopg2 alone). A psycopg2 stage, on the production stage's python:3.12-slim digest, installs gcc, libc6-dev, libpq-dev and binutils there only, builds the hash-checked sdist with uv (--require-hashes --no-binary psycopg2, isolated build), installs it into /install with bytecode, strips the extension's debug info and drops uv's lock file. The production stage installs libpq5 with the toolchain and copies /install into /usr/local as its own layer. Proof: scripts/ci/image_worker_site_packages.py (images.yml, both arches, as uid 1000): no psycopg2-binary and no psycopg2_binary.libs; import psycopg2 reports libpq 17 or later and _psycopg links Debian's libpq.so.5; and psycopg2 connects to the compose stack's Postgres image, which the step starts on a throwaway network with trust auth, and runs a parameterised query. test.yml's parity job runs pip check on the Railway build. test_prd262_worker_locks.py now expects the three-way split and the build stage's --prefix install; test_prd262_worker_diet.py pins the swap, the stage, libpq5 and the step. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit dffab7a6105b1502f0a23dc11de683445c43cc84)
…ptools, without build isolation Security review of dffab7a61 (MEDIUM, supply chain), breakdown row 27. uv pip install --require-hashes checked psycopg2's sdist, but its build ran in uv's isolated build environment, which fetches the build backend (setuptools) unpinned and unhashed, past --require-hashes. - services/workspace-worker/requirements-build.in asks for setuptools>=70.1, all psycopg2 2.9.13's pyproject.toml requires (that release carries its own bdist_wheel, so no wheel package). requirements-build.lock is compiled the way the other locks are (uv pip compile --universal --python-version 3.12 --generate-hashes): setuptools==84.0.0 with its wheel's and sdist's sha256. - The psycopg2 stage copies that lock, installs it first with --system --no-cache --compile-bytecode --require-hashes --no-deps --only-binary :all: (python:3.12-slim carries no setuptools), then builds psycopg2 with --no-build-isolation on it. The build now runs only on hash-checked files; nothing else is installed in the stage. Checked locally in the scratchpad with uv 0.11.26 and Homebrew's pg_config: without the build lock, the --no-build-isolation install fails (uv asks for setuptools; it fetches nothing); with it, psycopg2 2.9.13 builds from the hashed sdist and imports. setuptools 84.0.0's wheel still ships the distutils shim that psycopg2's setup.py imports (build_ext, ccompiler.get_default_compiler, errors.CompileError) and bdist_wheel. The Docker build itself is the build stage's to prove. Guards: - test_prd262_worker_diet.py: the stage copies requirements-build.lock, installs it wheels-only before the build, builds psycopg2 with --no-build-isolation, makes no other install, and every install in it is uv with --require-hashes --no-deps from a .lock; the build lock pins setuptools alone, with hashes. - test_prd262_worker_locks.py: the build lock's compile command and flags, every line an exact pin or its hash, it satisfies requirements-build.in, and the install order now starts with it. - test_prd262_dependabot.py: requirements-build.lock is listed with the same reason as the worker's other locks (no Dependabot updater regenerates a .lock). Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit f054ad1df2e490f648692646d9883efb2f1c2dd8)
…d of aiohttp Breakdown row 36: 12.2 MB installed. aiohttp, aiohappyeyeballs, aiosignal, frozenlist, multidict, propcache and yarl leave the worker's lock. Starlette 1.9.0 and uvicorn 0.54.0 were already in it as mcp's dependencies; requirements.in now lists them (starlette>=1.0.1 for GHSA-86qp-5c8j-p5mr, uvicorn>=0.50.1, the release the platform's tests run) because the worker imports them. Relocked with the lock's own command: the seven entries go, starlette and uvicorn keep their pins, click and h11 lose an emscripten marker. aiohttp was the server only; no worker module used it as a client, so nothing moves to httpx. Every route keeps its path, its methods (GET answers HEAD), the X-Internal-Token check (constant time) and its answers. What aiohttp did implicitly is explicit: - read_json reads a body up to aiohttp's 1 MiB client_max_size; one byte more is the handler's 400 "Invalid JSON body", as before; - json_response is aiohttp's json_response byte for byte (json.dumps, application/json; charset=utf-8); - no trailing-slash redirect; a download is an attachment named after the file (RFC 5987 filename* for a name a header cannot carry, where aiohttp sent raw UTF-8 or broke on a quote); - uvicorn holds idle connections 75 s (aiohttp's keep-alive, longer than httpx's 5 s pool expiry, so the API's pooled connections never race a close), waits up to 60 s on shutdown (aiohttp's runner), logs through the worker's handlers, and never installs signal handlers: main.py's SIGTERM/SIGINT stop the worker, which stops the server, as before. Tracing (Dave's #1044, PRD-256 O2b) is an ASGI middleware, outermost as before: it matches the route the router will (route template or "unmatched" for 404 and 405), continues the caller's traceparent, records the status, marks a raised error with its type only, never traces /health or /metrics, and flushes on lifespan end. services/shared/automatos_metrics.py swaps add_aiohttp_metrics for add_starlette_metrics (the worker was its only user; the vendored copies stay byte-identical); /metrics keeps prometheus_client's exact content type and the token check, and a 404 is now labelled 404, not 500. Tests: test_prd256_o2b_worker_routes_moved.py, test_prd256_o2b_worker_span.py and test_prd262_vendored_metrics.py are ported from aiohttp's TestServer to Starlette's TestClient with the same assertions (the metrics one now also checks the status label and a raised request's 500). test_prd262_worker_starlette.py pins the body limit, the JSON bytes, HEAD, no redirect, the download header, metrics outside the token check, keep-alive, the signals and serve() stopping with the worker. scripts/ci/image_worker_site_packages.py checks in the image that the seven distributions are gone and that the worker's modules import and build the app with its 15 routes and /metrics. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit fedb6cc4ab54d650eb3f1f96202e9197f35145ab)
… limit and keep-alive, and survives a taken port Breakdown row 36, corrections to fedb6cc4a, checked against aiohttp 3.13.5's source: - Body limit: aiohttp's read() refused a body at len(body) >= client_max_size (web_request.py:675), so a body of exactly 1 MiB was already a 400 "Invalid JSON body". read_json now compares with >= too; it compared with >. - Keep-alive: aiohttp 3.13's RequestHandler default keepalive_timeout is 3630 s (web_protocol.py:186), not 75 s, and the worker's AppRunner set none. uvicorn now holds idle connections 3630 s, well past the API's httpx pool expiry (5 s). - Bind failure: uvicorn calls sys.exit when it cannot bind its port, and asyncio re-raises a SystemExit out of the event loop, which would have killed the whole worker. aiohttp's failed site.start() only ended the health task. serve() now runs uvicorn through _serve_or_log, which logs the failure and returns, so the worker keeps consuming tasks as before (Docker's healthcheck still reports it unhealthy). test_prd262_worker_starlette.py: a body of 1 MiB - 1 is parsed and one of 1 MiB is the 400; keep-alive is 3630; a new test binds the port first and checks serve() returns and logs (it fails with the guard removed: checked by mutation). The helper's docstring is rewrapped. Ran in a throwaway uv env (pytest-asyncio, no orchestrator app): test_prd262_worker_starlette, test_prd256_o2b_worker_routes_moved, test_prd256_o2b_worker_span (minus the API one-trace test, which needs core), test_prd262_vendored_metrics, F173 and F178: 43 passed on starlette 1.8.0 / uvicorn 0.50.1 (CI's orchestrator pins) and 42 passed on 1.9.0 / 0.54.0 (the worker lock's, FastAPI test excluded). Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit 0074914692b5c24aabfb7952d42b0f666e2adfdd)
… of the Starlette move Breakdown row 36. fedb6cc4a's line said the move changed no answer; the JSON answers and statuses are the same, but a download's Content-Disposition uses RFC 5987's filename*= for a name a header cannot carry as it is, and Starlette's FileResponse adds ETag, Last-Modified and Accept-Ranges. The line also names the kept 1 MiB body limit and 3630 s keep-alive (007491469). Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit fe4aa04f0b015f0d5690f67b1b16f5f0aeb70cdd)
…the Starlette move Breakdown row 36. fe4aa04f0 said downloads newly carry ETag, Last-Modified and Accept-Ranges; aiohttp 3.13.5's FileResponse sent all three as well (web_fileresponse.py). Only the RFC 5987 filename*= for a name a header cannot carry as it is differs. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit bffc96a39c49a1d623ffef4ab2fca196b6c04206)
…every build Two local builds of the shared node-stripped stage gave media-render and the worker different node layers (diff_ids 78528fcd… and 0e337ed7…) although the binary was the same file (sha256 8f01756d…, mtime 1791963666): docker save showed only usr/local/bin differing, its mtime 1792448532 against 1792442969, each build's own time. strip renames its output into /usr/local/bin, and COPY --link keeps the parent directories' times in the layer. The stage now sets the directory's time back to the binary's upstream one, so the layer depends only on the node digest. Breakdown row #22 (shared stripped node). Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit 85951e703e532657848eae5096093572f7d1fefd)
…red node layer is the same on every build The touch fix (previous commit) did not hold: rebuilt, media-render's node layer still carried usr/local/bin at the build's time (mtime 1792449603, 22:40:03Z) although the stage's own /usr/local/bin was back at the binary's 1791963666. A COPY of one file into /usr/local/bin stamps that directory when the layer is made, whatever the source directory says. Copying a tree instead keeps the tree's directory times: the stage writes the stripped binary to /node-layer/usr/local/bin/node (strip -o, dates preserved) and sets every directory of the tree to the binary's upstream time, and the images copy /node-layer/ to /. Checked with a two-stage probe built cached and with --no-cache: the file COPY gave two diff_ids (44df0b6a…, b854ab01…), the tree COPY the same one twice (63b8f90e…). Breakdown row #22 (shared stripped node). Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit d848627cf7d095bdb7eb7d57578af24c27ce2ed1)
… and libstdc++ static archives Rows #12 and #31 of the 19 Oct size breakdown. PRD-262 §3-D5 was decided on 19 Oct 2026 by Gerard: trims only, toolchain kept. gcc, g++, make, node, npm, pnpm, uv, git and Perl stay in the worker. Row #12: the sanitizer runtimes (asan, hwasan, lsan, tsan, ubsan) go, with gcc's archives, preinit objects and links for them, so -fsanitize no longer links. The list comes from dpkg -L of the installed packages that match libgcc-[0-9]*-dev and lib<name>[0-9]*, filtered to /usr/lib/.../lib<name>.* and lib<name>_*. A package one arch lacks is skipped. On Debian trixie (gcc-14 14.2.0-19) both arches have all five, and the RUN deletes 23 paths on each: the ten runtime .so files and links in /usr/lib/<triplet>, and thirteen .a, _preinit.o and .so links in /usr/lib/gcc/<triplet>/14. That is 42.2 MB on arm64 and 41.7 MB on amd64 by package file sizes. Row #31: libc.a (libc6-dev) and libstdc++.a (libstdc++-[0-9]*-dev) go, found the same way, 12.2 MB on each arch, so gcc -static and -static-libstdc++ no longer link. libstdc++exp.a stays, although the breakdown's 'how' listed it (0.9 MB): GCC 14 ships it only as a static archive, so an ordinary dynamic link of std::stacktrace (-lstdc++exp) needs it, which is more than -static. libgcc*.a, libc_nonshared.a, libssp_nonshared.a, crt*.o and the libc.so, libgcc_s.so and libstdc++.so links are never touched. Each list is a name=$(... | grep ...) assignment in the && chain, the pattern the API lane uses for ICU, so a list that comes back empty fails the RUN. ldconfig runs after both. Where: in the RUN that installs build-essential, so no layer keeps the files. The files are deleted and the packages stay registered, instead of the breakdown's dpkg --purge --force-depends: the browser step runs apt-get install later, and a purge would leave libgcc-14-dev's dependencies unmet, which would make that install fail. Tests: orchestrator/tests/test_prd262_worker_d5_trims.py pins the RUN (order, no purge, dpkg-derived lists, nothing a dynamic link needs named) and runs its trim commands under sh with stand-in dpkg-query, dpkg, rm and ldconfig, fed the trixie packages' real file lists for amd64 and arm64: exactly the 23 sanitizer paths and the two archives go on each arch, a missing libhwasan0 still builds, and no sanitizer file or a missing archive fails the RUN. scripts/ci/image_worker_toolchain.py gains three in-image checks: no sanitizer file under /usr/lib or in ld.so's cache; gcc finds neither archive but finds every file a dynamic link needs; make builds a C and a C++ hello world with gcc and g++ that run and name libc.so.6 and libstdc++.so.6 as NEEDED. test_prd262_worker_diet.py's git-trim guard and row list follow the new RUN and checks. docs/workspace-execution/_index.md says what no longer links. Not built here: no docker was run. The image build has to prove it. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit 83f574e7088da7db0ab52366cb15d891868bd9c7)
…t.o, which the compiler package ships The first local build of 83f574e70 (arm64) kept /usr/lib/gcc/aarch64-linux-gnu/14/libhwasan_preinit.o, and image_worker_toolchain.py's row 12 check failed on it (11 of 12). dpkg -S names gcc-14-aarch64-linux-gnu as its owner, not libgcc-14-dev, and Debian trixie's file lists show the same on amd64 (gcc-14-x86-64-linux-gnu). The RUN's dpkg-query now also matches 'gcc-[0-9]*-*-linux-gnu'; its dpkg -L passes the same grep, which picks only libhwasan_preinit.o from that package (libsanitizer.spec, cc1's plugins and the LTO files do not match). test_prd262_worker_d5_trims.py's fixtures gain the compiler package's real file list for both arches, and the removed set gains the preinit object, 24 paths per arch. Four of its tests fail without the Dockerfile change. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com> (cherry picked from commit dba924c950d66307877d3dcc6458fa37ead53d25)
…ed gosu The worker entrypoint drops from root to the worker user with setpriv since row 47 of the 19 Oct size breakdown took gosu out of the image. The Helm chart's comment on why the pod has no runAsNonRoot still named gosu. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 47.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 218 functions across 25 files. (8 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the main change: reducing the worker image size while retaining the required toolchain. Description check The description follows the required template and provides detailed scope, rationale, verification results, AI assistance disclosure, checklist status, and follow-up items. It also explains unchecked … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description follows the required template and provides detailed scope, rationale, verification results, AI assistance disclosure, checklist status, and follow-up items. It also explains unchecked items such as pending CI at the time of writing and the absence of UI or migrations.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 47.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 218 functions across 25 files. (8 skipped: 8 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
AutomatosAI
pushed a commit
that referenced
this pull request
Oct 20, 2026
…while the step runs test_a_playbook_step_keeps_the_loop_running_while_it_looks_up_composio failed twice on this PR (the loop stood still 0.58 s, then 0.52 s) and passes on main, #1219 and #1220. The same 44 test files ran before it in each shard. What differed is the set of test files the shard collected: three of this PR's frontend guards instead of one. With a few thousand tests collected, a full garbage collection takes about half a second, and where it lands depends on what the shard collected, not on the code under test. _watched now runs gc.collect() before it starts and pauses the collector until the step returns. The 0.3 s bar and every assertion are unchanged. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com>
Brings in #1219 media-render cuts, #1168 W7 frontend, #1222, #1226 (PRD-262 API W2-W5 and cuts), the PRD-261 theme waves and #1232. Three conflicts: - .github/workflows/images.yml: main's in-image steps after "Scan policy" (the API's stdlib bytecode layer, site-packages, botocore, scipy, system packages, left-out packages and pip layers, then the frontend's "serves with no network") and this branch's two worker steps (agent toolchain, site-packages and psycopg2) all kept, main's first, each under its own `if: matrix.name == ...`. The worker's site-packages step still follows its toolchain step, the one its "Same conditions as the step above" names. - orchestrator/tests/test_prd262_dependabot.py: both sides' lock entries kept, this branch's services/workspace-worker/requirements-build.lock and main's orchestrator/requirements*.lock and scripts/ci/reference/*.lock. - orchestrator/tests/test_prd262_node24_images.py: the code lines were the same on both sides (NODE_COPIES, NODE_LAYER, _node_stages). The module docstring and the NODE_LAYER comment take this branch's wording, which names both lanes' tests (test_prd262_worker_diet.py for the worker's #21, test_prd262_media_render_node_strip.py for media-render's #22); main's frontend #24 assertions and its NODE_COPIES comment are kept. test.yml, charts/automatos/templates/worker.yaml and orchestrator/core/workspace_client.py merged without conflict. Signed-off-by: Gerard Kavanagh <gerard161@gmail.com>
Owner
Author
|
Updated against main (ebedc7e, which includes #1219, #1168, #1222 and #1226): ba242b0.
CI on ba242b0. Green: test, images (workspace-worker amd64 and arm64 with the toolchain 12-check and site-packages steps), smoke-fresh-clone, chart, CodeQL and the rest. One non-required check is red: smoke-images' core-stack step. It's the same failure #1226 merged with, not this PR:
@daarthur: the two dependency changes for your review are under For Dave in the description. psycopg2 is built from source against Debian's libpq, with hash-checked setuptools and |
2 of 5 tasks
1 of 2 tasks
AutomatosAI
pushed a commit
that referenced
this pull request
Oct 20, 2026
…) into the nights 16–17 + socials fix wave — no conflicts; no new undefined names Signed-off-by: Gerard Kavanagh <gerard161@gmail.com>
This was referenced Oct 24, 2026
Collaborator
|
Post-merge review, as agreed in the PRD-262 close-out, against Verdict: good change, nothing to revert. No blocker or major defect. The one real item, F452, isn't a regression from this PR; it's tracked in #1309 with the measured details. What I checkedA local build of
CI on Tests on The two "For Dave" changes1. psycopg2 from source. Happy with it.
2. aiohttp → Starlette/uvicorn, and the tracing middleware (O2b). It holds up:
Smaller notes
Follow-ups
|
Merged
14 tasks done
10 of 14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
PRD-262 (Container diet), the workspace worker's code cuts from the 19 Oct size breakdown. W6 (#1166, squash f230dad) is already on main; this PR is the 16 commits the worker lane made after it (lane
feat/prd-262-w6-worker,4bc2514b0..dba924c95, local only; commit SHAs below are this branch's), replayed onto main withcherry-pick -x, plus one comment fix in the Helm chart.The worker keeps its agent toolchain: gcc, g++, make, node, npm, corepack, pnpm, uv, git and Perl all stay (§3-D5, decided by Gerard on 19 Oct as trims only). What goes is what nothing in the image runs, two dependency swaps, and the aiohttp server.
Measured, local arm64 image, installed size: 1,340 MB before PRD-262 → 1,074 MB on main today (W0–W8) → 910.1 MB with this PR. gzip of the image: 374.7 MB.
The cuts
Row numbers are the 19 Oct ranked size breakdown. MB are installed bytes on arm64, as the breakdown measured them.
lto-dumpdeleted in the build-essential RUN.gcc -fltoruns lto1, lto-wrapper and the linker plugin, never lto-dumpgitbecomes a link to/usr/bin/git. HTTPS and SSH clone, pull and push stayentrypoint.shdrops to the worker withsetpriv(util-linux, already in python:3.12-slim) and setsHOME=/home/worker, which setpriv leaves as root'sstrip --strip-unneeded;.dynsymstays, so N-API addons resolve) in anode-strippedstage that media-render carries byte for byte. Copied first withCOPY --linkas a/node-layer/tree, so both images get the same layer digest on every buildnpmstage deletes them), and the/tmp/node-compile-cacheand/root/.npmlogs the pnpm RUN left behindstrip --strip-debugon site-packages*.so*(not*.libs/, which patchelf rewrote), in the stable lock's RUNtests/directories (jsonschema, referencing, certifi)dpkg -L. 24 paths per arch.-fsanitizeno longer linkslibc.aandlibstdc++.a.-staticand-static-libstdc++no longer link.libstdc++exp.astays: GCC 14 ships it only as an archive and-lstdc++expneeds it for a dynamic linkThe breakdown's estimates add up to about 172 MB. Measured: 1,074 → 964.5 MB before the §3-D5 trims, then 910.1 MB with them (54.4 MB, the trims' estimate exactly).
Other changes that come with the cuts:
services/shared/automatos_metrics.pyand its two vendored copies (byte-identical, astest_prd262_vendored_metrics.pyrequires) swapadd_aiohttp_metricsforadd_starlette_metrics. The worker was its only user. A 404 is now labelled 404, not 500.docs/workspace-execution/file-operations.mddescribes the Starlette server and the one header that differs;_index.mdsays which builds no longer link.orchestrator/core/workspace_client.py: a comment names the worker's body limit constant (no code change).charts/automatos/templates/worker.yaml: a comment said the entrypoint drops privileges with gosu; it now says setpriv.For Dave: the two dependency changes
1. psycopg2 from source instead of psycopg2-binary (row 27).
task_executions_db.pyis unchanged.libpq5, which the weekly OS rebuild keeps patched.psycopg2build stage (same python:3.12-slim digest as production) installs gcc, libc6-dev, libpq-dev and binutils there only, builds the hash-checked sdist and strips the extension. Production installslibpq5and copies the result in as its own layer.--require-hasheschecked the sdist, but uv's isolated build fetched setuptools unpinned. Nowrequirements-build.inasks forsetuptools>=70.1(all psycopg2 2.9.13 needs),requirements-build.lockpinssetuptools==84.0.0with its sha256 hashes, the stage installs that lock first (--require-hashes --no-deps --only-binary :all:), and psycopg2 builds with--no-build-isolation. Nothing unhashed is installed.requirements-build.lock(its pip and uv updaters read only*.txtand*.in).test_prd262_dependabot.pylists it with that reason. How these locks are kept current is still open for you and Gerard.2. Starlette and uvicorn listed directly; aiohttp removed (row 36).
requirements.innow listsstarlette>=1.0.1,<2.0.0(GHSA-86qp-5c8j-p5mr, as the orchestrator) anduvicorn>=0.50.1,<1.0.0, because the worker imports them. The lock already had them through mcp (starlette 1.9.0, uvicorn 0.54.0) and keeps those pins. aiohttp, aiohappyeyeballs, aiosignal, frozenlist, multidict, propcache and yarl leave the lock. aiohttp was the server only; no worker module used it as a client.X-Internal-Tokencheck (now constant time); the 1 MiB body limit (a body of exactly 1 MiB is the 400, as aiohttp's>=); the 3630 s keep-alive (aiohttp's RequestHandler default), well past the API's 5 s httpx pool expiry; a 60 s graceful shutdown; no trailing-slash redirect. A port that can't be bound logs and ends the health server only, as aiohttp did; the worker keeps consuming tasks.filename*=, where aiohttp sent raw UTF-8 or broke on a quote.traceparent, labels by route template (or "unmatched"), never traces/healthor/metrics, and flushes on shutdown.§3-D5 and what waits until after WebSummit
How it was proven
Local image build and run (arm64, 20 Oct, from the local integration branch before the replay onto main).
services/workspace-worker/,services/shared/automatos_metrics.pyand the two in-image scripts on this branch are byte-identical to what was built (integration commit b8b7e004a).scripts/ci/image_worker_toolchain.py: 12 of 12 checks passed: no npm docs, compile cache or logs; no lto-dump andgcc -fltolinks; no sanitizer runtime, archive or link left; no libc.a or libstdc++.a and everything a dynamic link needs kept; make, gcc and g++ build hello world linked dynamically; git's server-side binaries gone; git init, commit, clone, branch, checkout and pull; an HTTPS clone and the credential protocol; node stripped, keeps.dynsymand runs; a native N-API addon loads; npm, npx, corepack and pnpm run; no gosu and setpriv drops to the worker.scripts/ci/image_worker_site_packages.py: 5 of 5: 8 stripped libraries load with no debug sections; no tests directories; psycopg2 links Debian's libpq; psycopg2 connects to Postgres and runs a query; the removed distributions are gone and their replacements import.traceparentcontinues the caller's trace in the worker's server span.INSTALL_BROWSER=truebuild (Railway's default) passes the same 12 toolchain checks and renders a PNG through the worker'sworkspace_html_to_png.Static checks on this branch (no Docker, no database), all on HEAD against
origin/main:ruff 0.16.9throughdiff-quality(CI's code-standards lane, changed lines only): 100%, 0 violations. Plainruff --select F821,F401,F811,F841on the changed files reports 6 F401s, all in the threeautomatos_metrics.pycopies (typing.Optional,CollectorRegistry), on lines this PR doesn't touch; main has the same 6.py_compileon all 24 changed.pyfiles (3.11 and 3.12),bash -n entrypoint.sh, YAML load ofimages.ymlandtest.yml: all pass.scripts/ci/check_changed_code_shape.py --base origin/main: 0 errors.test_prd262_*.pyfile except the futureagi worker client one, plustest_prd256_o2b_worker_routes_moved.py,test_prd256_o2b_worker_span.py(including the API-to-worker one-trace test) andtest_socials_worker_binary_pieces.py: 717 passed, 1 skipped (Linux/proc). Three need what isn't on this machine: the twotest_prd262_web_stack.pyroute tests (the full API app) and the real-Postgres lifecycle test intest_prd262_worker_task_executions.py.test_prd256_o5_logs_metrics.py, 9 passed and 3 need a database.test_f333_a_second_workspaces_session_files_open.pyneeds the full API app and did not run.Replay onto main
git cherry-pick -x 4bc2514b0..dba924c95(the lane's head, local only), all 16 commits, each keeping its sign-off. Two picks conflicted, both inorchestrator/tests/test_prd262_node24_images.py, which W8 (#1182) had changed on main: main'sNODE_COPIESmap andassert copied == NODE_COPIES[path]are kept, and the lane's counting of copies from every stage built FROMnode(with the/node-layer/tree read as the binary) goes under it. Its docstring and theNODE_LAYERcomment name both lanes' tests.images.yml,test.ymlandtest_prd262_dependabot.pymerged on their own; every line the lane added is present, the only lines that differ are the docstring and comment just described.Merging with the other PRD-262 PRs
Planned order: frontend (#1168), media (#1219), API, then this PR (after Dave's review). Trial merges (
git merge-tree) of this branch:feat/prd-262-media-cuts): one file,test_prd262_node24_images.py, two hunks, comments only (the docstring line and theNODE_LAYERcomment). Take this PR's side in both: it names both lanes' tests. Every code line is identical on both sides, and media's comment on its ownNODE_COPIESentry merges cleanly. After that resolution the node24 suite, media'stest_prd262_media_render_node_strip.pyand this PR'stest_prd262_worker_diet.pypass together (760 passed with the rest oftest_prd262_*). Thenode-strippedstage text, from theFROM node:24-trixie-slim@sha256:7f486cab… AS nodeline to its last RUN, is byte-identical in both Dockerfiles, on the same python:3.12-slim digest.images.yml. The frontend's "The frontend image serves with no network…" step and this PR's two worker steps (toolchain, site-packages) land at the same spot after the scan policy step. Keep all three; each has its ownif: matrix.name == ….feat/prd-262-api):images.yml, where the API's in-image steps (stdlib bytecode layer, site-packages, botocore, scipy, system tools, left-out packages, pip layers) land at the same spot: keep all of them and this PR's two. Andtest_prd262_dependabot.py, where both add entries to the same map: keep this PR'srequirements-build.lockentry and the API's lock entries.Test plan
images.ymlforworkspace-workeron amd64 and arm64, including the two new in-image steps (toolchain 12 checks; site-packages 5 checks, with psycopg2 against the compose Postgres image).test.ymlprod-image-parity: the worker's entrypoint check (uid 1000 andHOME=/home/worker), the locked-set step (pip check, SDK and uv versions) on the Railway build.workspace-workeragainstedge.psycopg2stage compiles from the hashed sdist and the build passes.workspace_html_to_pngcall in production;/metricsanswers 200; a worker trace joins the API's.How it was verified
test_prd262_worker_d5_trims.pyfail without its Dockerfile change)AUTH_EDITION=local) and hosted. Railway buildsINSTALL_BROWSER=true, proven locally above; compose and GHCR buildfalse.AI assistance
Checklist (AGENTS.md)
git commit -s)orchestrator/config.py; no hardcoded values. The worker is a separate service with its ownworker_config.py.V2copies). aiohttp,add_aiohttp_metrics, gosu and psycopg2-binary are gone, not kept alongside.orchestrator/reports/route-manifest.json: no new routes; the worker's routes are unchanged.docs/workspace-execution/file-operations.md,_index.md, the chart comment)Summary by CodeRabbit
New Features
workeridentity and sets the correct home directory.Improvements
Documentation