Repository navigation
Isolate generated-code execution per chat session; ship the AD Workbench image - #14
Merged
Merged
Conversation
…entials from it The Chainlit app serves many chats from one process, but the REPL that runs LLM-generated Python kept its namespace, captured plots and stdout capture in module-level globals. Two people asking a question at the same moment therefore collided: run A received run B's printed output and B's OUTPUT_DIR, so A's generated files were written into B's run directory. Because each execution restored whatever sys.stdout it found on entry rather than the one it replaced, a single overlap also left the process writing into a discarded buffer, silently swallowing every subsequent print. Reproduced with two overlapping run_python_repl calls, then end to end with two browser sessions whose code steps overlapped for 21.8s: each now keeps its own variables and its own output directory across turns. - Execution state is keyed by observability.session_id_var, which the app binds per chat thread. Callers outside a bound session (notebooks, CLI) share one default session, preserving the "variables persist between calls" contract. The registry is LRU-capped (BIOMNI_MAX_REPL_SESSIONS) because a closed tab fires no teardown callback. - Stdout is routed through one installed dispatcher instead of swapped, so no execution can strand the stream. - utils.run_with_timeout spawns a raw thread, which does not inherit contextvars; it now replays the caller's context. Without this every concurrent run lands in the default session and the isolation does nothing. - observability.setup_logging unwraps the router before binding its handler, or records emitted during a code step would land in that snippet's user-visible output and never reach the log pipeline. New biomni/credentials.py strips credential-shaped variables from os.environ around every exec and restores them after. Stripping the real mapping rather than passing a filtered copy into the exec globals is deliberate: a bare `import os` defeats the copy, and subprocesses inherit the parent environment regardless. Windows are reference-counted so overlapping runs share one and none waits on another. In-process consumers that genuinely need a credential read through credentials.getenv, which sees past an open window, so a chat opening during another chat's code step still builds a working client and a readiness probe firing then does not report the pod unready. This is not a sandbox and does not claim to be: it closes the accidental and casual paths. Also fixes a collision found by the same end-to-end test: run directory ids were timestamp plus prompt slug, and every caller creates the directory with exist_ok=True, so two runs started in the same second from the same prompt shared one directory and overwrote each other's report and trace. The UI's starter buttons send fixed text, so this is a demo-day scenario, not a contrived one. Ids now carry a random token after the timestamp, which keeps listings sorting chronologically.
Three points from their reply need something on our side. Issuer. They confirmed `sub` is stable within a Keycloak realm and offered to send the issuer as well. Worth taking: `sub` is unique within an issuer's realm, not globally, so without `iss` an IdP change could hand one person another person's preferences and run history. `iss` is now read from the user context header (or BIOMNI_AUTH_ISSUER_HEADER) and folded into the storage key when present. It has to be sent from the first deployment or not at all - starting to send it later re-keys every existing user, who then finds an empty history. Both properties are covered by tests so the hazard cannot be reintroduced silently. CSV escaping. They confirmed the gateway will emit valid CSV. The parser already handled a quoted comma; it now also collapses a doubled quote inside a quoted value per RFC 4180, which is what "valid CSV" means for a name like `Smith ""Bud"", Jr.`. If they settle on backslash escaping instead, _split_context_pairs is the single function that changes. Pod reclamation. They confirmed `active: false` will eventually shut pods down and asked us to account for it now. Reclaiming a pod out from under a connected user destroys their in-memory REPL state - preferences, run records and threads are on the volume and survive, but loaded frames and fitted models do not. While at least one browser session is connected the idle window therefore widens to BIOMNI_STATUS_OPEN_SESSION_INACTIVITY_SECONDS (default 12h) instead of the base 4h, which covers a long reading-and-thinking gap without pretending a tab left open since Tuesday is work in progress. With nobody connected the shorter window still applies, so an abandoned pod stays reclaimable within a working day. Work in flight remains unconditionally active.
…ches The image built from environment.yml, which supplies 10 of the ~113 libraries biomni/env_desc.py advertises to the model - and the system prompt tells the model to prefer locally installed ones. The agent was therefore free to write scanpy / gseapy / biopython code that could only fail at the import. The AD Workbench environment is the one that matches the catalogue, so it becomes the default for the Dockerfile and compose build. Trimmed 41 pinned entries first - about 780 MB of the 4.19 GB site-packages, measured per distribution and verified against the advertised catalogue, which loses nothing: - transformers / tokenizers / sentencepiece. One call site (systems_biology.py) does `pipeline(model="InstaDeepAI/ChatNT", trust_remote_code=True)`, which needs a torch backend the image did not install, so it could not run; nothing else imports them. - scvi-tools, pertpy, pyro-ppl, numpyro, lightning, pytorch-lightning, torchmetrics, tensorboard, tensorstore, arviz. None appear in the catalogue, so the agent can never propose them, and numpyro was the only reason jax and jaxlib (245 MB) were pulled in at all. torch and torchvision stay - cellpose and harmony-pytorch are advertised and require them. - The Jupyter *server* stack (jupyter, jupyterlab, notebook and friends). trace.ipynb is assembled as plain JSON in a1.py; nothing imports nbformat and nothing serves notebooks. ipykernel, ipywidgets, nbclient and nbconvert stay - gget, cooler and others require them. - streamlit and its exclusive dependencies; pyomo with sparsecca (cobra uses optlang); tooluniverse, which appears in no file outside the env definitions. - opencv-python, keeping opencv-python-headless: identical `import cv2`, and headless is the right build for a server. Both were installed. - fastparquet, redundant with the pinned conda pyarrow. - dataclasses==0.6, a Python 3.6 backport of a stdlib module that has no business on 3.11. PyTDC still drags it in transitively; the stdlib wins on sys.path, so it is inert, but it should not be pinned deliberately. Kept deliberately, having checked: blinker (flask), sphinx and sphinx-rtd-theme (scholarly requires them) and rdkit-pypi (PyTDC requires it). Removing those pins saves nothing - pip reinstalls them transitively - and only makes a pinned environment less honest about what it contains. rdkit and rdkit-pypi install the same rdkit/ directory, so one silently overwrites the other; that conflict is upstream in PyTDC and wants fixing separately. Also pinned aiobotocore, boto3 and botocore, the only unpinned entries in an otherwise fully pinned file. s3fs constrains aiobotocore, which constrains botocore, so pip backtracked through hundreds of boto3 releases on every build and could settle on a different triple each time. Verified: the conda section solves for linux-64 (before and after, identically), and the pip section resolves cleanly to 554 packages, where it previously backtracked without converging.
… toolchain Three changes that follow from making the AD Workbench environment the default. **The catalogue no longer lies.** biomni/env_desc.py lists ~113 libraries and the system prompt tells the model to "prioritize locally installed tools and libraries", but no deployment has all 113 - the minimal environment has ten, and even the AD Workbench one has no R and no docking tools. The agent was free to plan around gseapy or DESeq2 and then fail at the import, several steps in, in front of the user. biomni/env_probe.py narrows the catalogue at agent construction, which is the single point both the retriever and the system prompt read from. A missing library is not like a missing data-lake file: that gets fetched on demand, so listing and tagging it is honest, whereas a library cannot be produced at run time. So entries are hidden rather than tagged. Detection is metadata-only and side-effect free - importing 113 packages to see which work would cost seconds and run arbitrary import code in the agent's process. Each name is tried as an installed distribution, then an importable top-level module, then an executable (most of the bioinformatics catalogue is binaries, not Python packages), then an R package via one cached Rscript call. The interpreter's own bin is searched as well as PATH, because run_chainlit.sh starts the app without activating the environment. Two safety valves: the filter fails open if nothing at all resolves, since that means the probe is wrong rather than the image empty, and BIOMNI_ADVERTISE_ALL_LIBRARIES=true disables it for a deployment that installs libraries after the image is built. On this machine it resolves 77 of 113. **The image stops carrying its own compiler.** adworkbench_env.yml pins conda-forge's `compilers` because several pip dependencies build from source, but the runtime stage copies the environment wholesale, so gcc/g++/gfortran shipped in the final image. docker/prune-build-tools.sh removes the toolchain as the last builder step, leaving the compiler *runtime* libraries (libgcc_s, libstdc++, libgfortran) and binutils alone - the latter because ctypes.util.find_library shells out to ld/objdump on Linux. Static archives go too, since nothing can link them once the compiler is gone. The script verifies its own work: every module still on disk must still import, so a bad prune fails the build instead of producing an image that dies on first use. It is a no-op on an environment that never had compilers, so the minimal build runs it unchanged. **CI stops building a multi-GB image for changes that cannot affect it.** A new `scope` job picks the full environment when a PR touches biomni_env/, docker/ or the Dockerfile - exactly the changes that can break it - and the minimal one otherwise, as a fast canary that still proves the Dockerfile, the package install and the entrypoint. Push and manual runs always build what actually ships. Two bugs found in the same file while there: adworkbench_env.yml was not in the PR trigger paths at all, so a change to the now-default environment triggered no build; and push events ran both `build` and `publish`, building the same image twice, where a failure in `publish` already publishes nothing. Also extends the advisory mypy scope to the two modules added this week.
Found by building it. scikit-misc, which scanpy needs for highly_variable_genes(flavor="seurat_v3"), publishes a manylinux x86_64 wheel but none for aarch64, so on arm64 pip falls back to a source build and its meson step fails during metadata generation. That is the first blocker; there may be others behind it. No effect on where this actually runs - Hetzner Cloud, Azure and the GitHub Actions ubuntu runners are all x86-64. It matters when building on an Apple Silicon laptop, where Docker defaults to the native architecture and the build fails with an error that looks like a broken environment rather than a missing wheel. Noted in the Dockerfile next to the env-file choice and in the VM deployment guide, both with the --platform flag to pass. The minimal environment.yml builds on both architectures.
It runs once, inside the image build, against an environment that only exists there - so it was the one build-critical artifact with no way to be exercised before a release, and it deletes files out of the conda prefix. If it takes one shared library too many, the result is an image whose scientific stack no longer imports. The prefix is synthetic but mirrors conda-forge's gcc layout, which is the part that is easy to get wrong: the target directory is named for the build triple, so there is a case for aarch64 as well as x86_64. The tests pin down both halves of the contract - the toolchain goes, and libgcc_s / libstdc++ / libgfortran and binutils stay - plus the no-op path the minimal environment takes, the error on a missing prefix, and that the self-check actually runs. The prefix is built with `venv` rather than by symlinking an interpreter in. A symlinked interpreter resolves its prefix to the synthetic directory, finds no environment there and falls back to the base installation, so the self-check imported the whole development stack on every run - 33s for the file. Building a real environment takes it to 7.7s, with the remaining time in the one test that deliberately uses a populated interpreter to cover the import branch.
…r use The image was pulling torch (821 MB), triton (156 MB) and twelve nvidia-cu12 wheels (2.0 GB) - cuDNN alone is 571 MB - into a deployment that has no GPU and is not going to get one. Measured from the build log on linux/amd64, where those wheels actually apply; they carry sys_platform == "linux" markers, so a resolve on macOS does not show them and understates this by an order of magnitude. Nothing needed them except two catalogue entries. torch was required only by cellpose and harmony-pytorch, torchvision only by cellpose, and opencv-python-headless only by cellpose. cellpose is "cell segmentation in microscopy images", which is not what an AD genomics deployment does and is impractical on CPU anyway. harmony-pytorch is a real capability, so it is replaced rather than dropped: harmonypy is the same Harmony algorithm with a C++ backend, depends on numpy alone, is 10.5 MB, and is what scanpy.external.pp.harmony_integrate calls - so the single-cell integration path now works from scanpy, which it did not before. Added to the library catalogue in both the academic and commercial variants. Also removes fill-voids and roifile, cellpose-only utilities left orphaned. imagecodecs is deliberately kept: it is a tifffile codec backend for spatial work, not GPU baggage. The cellpose and harmony-pytorch catalogue entries stay. The catalogue describes the reference environment and biomni/env_probe.py hides whatever this deployment lacks, so a build that does install them still gets them described - the point of the filter is that the catalogue no longer has to be edited to stay honest. Verified: the pip section still resolves cleanly (547 packages, down from 554) and scanpy, anndata, scikit-image, gget, rdkit, tiledbsoma and numba are intact.
…e script Ten findings from the review of this branch. The three that mattered most were all introduced by this work. **The scrub was hiding a secret the server needs.** CHAINLIT_AUTH_SECRET matches the SECRET marker, and Chainlit re-reads it on every JWT operation, so while one chat ran a code step every other user reconnecting or resuming a thread hit `assert secret` and got a 500. It is now never scrubbed - and hiding it bought nothing anyway, since chainlit_ui/persistence.py writes it to $BIOMNI_STATE_DIR/threads/auth_secret where generated code can read it regardless. **The scrub only covered Python.** The #!R, #!BASH and #!CLI paths in A1's execute node called run_with_timeout directly, so `#!CLI printenv ANTHROPIC_API_KEY` returned the key in the tool result. All four paths now open the window, at the call site rather than inside the executor - run_with_timeout abandons threads it cannot kill, and a window opened in one of those would never be closed. That abandonment is handled properly rather than papered over: windows are counted per thread, and run_with_timeout releases exactly the abandoned thread's share on timeout. A first attempt reset the global depth on exit instead, which looked leak-proof and broke concurrency - two interleaved windows and the first one out restored the credentials while the second chat was still executing. The tests caught it. **The allowlist was writable from inside a window.** Neither override variable is credential-shaped, so a snippet could set BIOMNI_SCRUB_ENV_ALLOW=ANTHROPIC_API_KEY in one execution and read the key in the next, process-wide. Both lists are now read once. Also from the review: - prune-build-tools.sh deleted binutils. binutils_linux-64 ships its tools *as* triple-prefixed binaries with the bare names as symlinks, so the `*-conda-linux-gnu-*` sweep removed the real ld/ar/nm/objdump and left four dangling links - present enough to pass an existence check, and broken. ctypes.util.find_library shells out to those. The test missed it by writing plain files where conda ships symlinks; it now models the real layout. The first fix used `find | grep -Ev`, which under pipefail exits 1 when find matches nothing - the no-op path - so the exclusion is done in bash instead. - a1.py copies data_lake_dict. The catalogue on the next line was already fixed; this one still aliased biomni.env_desc, so one chat's uploaded filenames entered the system prompt of every agent built after it. - BIOMNI_MAX_REPL_SESSIONS is parsed with a guard. A typo or an empty ConfigMap value raised on import, so the pod never booted. - REPL session eviction is TTL-aware. LRU order is by last *use*, so the victim was whichever chat had gone longest without executing - a researcher reading a result, who then gets NameError for frames they correctly believe they loaded. That is the same user status.py's 12-hour window protects; evicting them here undid it one layer down. Idle entries go first, and evicting a live one is now a WARNING that names the knob. - env_probe's safety valve is proportional. "Nothing detected" could never fire: numpy, pandas and pyarrow are hard dependencies and catalogue entries, so a systemic probe failure quietly hid 110 of 113 entries instead of failing open. - protocols.py resolves its token per call. The module is only ever imported by generated code, inside the window, so a module-level constant computed through credentials.getenv handed out a long-lived copy of a value the scrub hides. - opencv-python-headless is restored. Seven tool modules import cv2 and the tool registry is built from static tool_description modules that the catalogue filter never reaches, so dropping it left registered tools that ModuleNotFound mid-plan - the failure env_probe exists to prevent, one layer down. - The router install is locked and unwraps nested routers. Concurrent chats could both wrap a foreign stream, and observability unwraps only one level. - workflow_dispatch built nothing: every job was gated on pull_request or push, so "Run workflow" produced a green run that did nothing. The scope job's `git diff | grep -q` also exits 141 under pipefail on a large diff, and its regex missed pyproject.toml. Not addressed, and worth knowing: output from threads that generated code spawns (joblib, sklearn n_jobs) still bypasses the capture, which is a regression against the old process-global stdout swap; matplotlib's figure manager is still process-global; query_chatnt remains registered but cannot run without transformers; and literature.py has no Azure-Anthropic credential path now that get_llm no longer writes one into os.environ.
…gn-in **Chat history was silently off for every compose deployment.** The conversation list on the left needs a writable state directory, and docker-compose.yml never set BIOMNI_STATE_DIR or mounted anywhere for it. resolve_threads_dir() then fell back to the workspace root - which compose mounts read-only - found it unwritable and returned None, so Chainlit ran with no data layer: no thread list, no resumable conversations, and no run records surviving a restart. Nothing failed loudly; the sidebar was just always empty. Compose now sets BIOMNI_STATE_DIR=/app/state and mounts BIOMNI_STATE_HOST_PATH there, outside the read-only user-data mount. **Password sign-in, for a deployment with no gateway in front.** Set BIOMNI_DEMO_PASSWORD and Chainlit puts its own login in front of the app: one shared password, any username. The username becomes the identity (identity.password_identity, source="password"), which makes is_authenticated true - so preferences, run records and the conversation list all key off it exactly as they do for a gateway-supplied subject, and each person who picks a distinct name gets their own history without anyone administering accounts. The name is slugged before it becomes a storage key, and a name that slugs to nothing is rejected rather than shared. Two details that make it actually gate anything: - The callback is registered conditionally, not with a decorator. Chainlit renders a login form whenever a password callback exists, so an unconditional registration would put one in front of gateway deployments that already authenticate upstream. - header_auth_callback stops falling through to the shared local user when a password is configured. That fallback exists because an un-gated deployment has no front door anyway - but once there is one, returning a user without asking would make the login decorative. Verified against a running app: /auth/config reports requireLogin, a wrong password gets 401 and a correct one 200, and the state directory gains its threads database. Both paths build the Chainlit user through one helper, so the two ways in cannot drift on what identity they produce.
The run directory lives on the server, and the completion message told the user its path. That only helps somebody with a shell on the box - not a demo attendee, and not anyone on the hosted deployment. Plots already rendered inline, but report.md, trace.ipynb, trace.json and any generated CSVs had no way off the machine at all. Each finished run is now zipped and attached to the completion message, so the whole thing is one click. The archive is written to a hidden `.packages` sibling rather than inside the run directory - otherwise it lands inside the next archive of itself, and shows up in the run's own file listing. Runs over BIOMNI_MAX_PACKAGE_MB (default 200) are reported with their size instead of packaged: a single run can write a multi-GB intermediate, and zipping that would stall the app and the browser rather than help anyone. Empty runs and a missing directory say so rather than attaching nothing.
The run packaging added in the previous commit writes an archive per run and never removed one. On the shipped Kubernetes manifest BIOMNI_OUTPUT_ROOT points at a 20Gi PVC and the run directories are themselves never deleted, so the volume would have filled roughly twice as fast, forever - a regression for the gateway deployment rather than the demo box it was written for. Archives are pure derived data: the files are still on disk, so dropping an old one costs nothing but regenerating it. The newest BIOMNI_MAX_PACKAGES (default 20) are kept and the rest removed as each new one is written, and the archive just created can never be the one pruned. Also spells out in deploy/k8s/biomni-ad.yaml the settings a gateway deployment should care about, including one it must NOT set: BIOMNI_DEMO_PASSWORD puts Chainlit's own login form in front of the app, which is for deployments with no gateway. There, it would add a second and weaker door in front of a real one.
A deployment that puts a TLS proxy in front needs to stop publishing the app itself, and `ufw deny 8000` does not do it: Docker writes its own iptables rules ahead of ufw's chain, so a published port stays reachable from the internet however the firewall is configured. Binding the publish to 127.0.0.1 is the reliable way, so the address is now a variable. Default is unchanged (0.0.0.0), so nothing moves for a deployment without a proxy. The demo box sets BIOMNI_BIND_ADDRESS=127.0.0.1 and reaches the app only through Caddy, which terminates TLS.
…durable output
**Every second save was blank.** _capture_matplotlib_plots closed each figure
after capturing it, and it runs from the savefig/show monkey patches - that is,
in the middle of the user's own plotting code. So the extremely common
plt.savefig("x.png")
plt.savefig("x.pdf")
wrote a correct 13 KB PNG and then a blank 1 KB PDF, because by the second call
there was no figure left. Anything that kept working on a figure after saving it
hit the same thing. Capture must observe, not dispose: the generated code owns
the figure's lifetime. Duplicates were already prevented by the identity check,
so closing bought nothing. Now 7 KB and the figure survives.
**The "results are lost when the application restarts" warning was false under
compose.** Output resolution falls through to a "cwd-fallback" candidate that is
flagged ephemeral by construction, and compose never set BIOMNI_OUTPUT_ROOT - so
the app warned that results would not survive a restart while they were landing
on a bind mount and persisting perfectly well. The Kubernetes manifest has
always declared it; compose had not, the same omission as BIOMNI_STATE_DIR.
The warning itself is worth keeping - it is correct when there really is no
volume, which is exactly when somebody needs to be told. It just has to be told
the truth about this deployment.
Fourteen commits added settings that only appeared where they were used - some in the Kubernetes manifest, some in comments, several nowhere at all. Anyone running this had no single place to look, and BIOMNI_ADVERTISE_ALL_LIBRARIES, BIOMNI_REPL_SESSION_TTL_SECONDS and BIOMNI_BIND_ADDRESS were undiscoverable outside the source. docs/configuration.md now carries a Deployment settings section covering all fifteen, grouped by what an operator is actually deciding - serving and access, storage, run artifacts, execution and memory, monitoring - and each entry says what goes wrong if it is left alone, since that is the part the variable name never conveys. The two that quietly break a deployment are called out plainly: BIOMNI_STATE_DIR, without which there is no conversation list, and BIOMNI_OUTPUT_ROOT, without which the app warns about losing results it is not losing.
Not closing on save was the right fix for the blank PDF, but it left a leak I introduced with it: pyplot's figure manager is process-global, so every figure any chat ever drew stayed resident for the life of the pod - and one chat could see another's in plt.get_fignums(), which an integration check across the new subsystems made obvious. The oldest figures beyond BIOMNI_MAX_OPEN_FIGURES (default 50) are closed at the end of each execution rather than during it. That keeps the Jupyter-like behaviour the persistent namespace implies - a figure drawn in one step is still usable in the next - while the registry stops growing without limit. Closing the oldest rather than the newest is deliberate: the figure just drawn is the one the next step is most likely to want. Verified the blank-PDF fix still holds with the cap in place.
… import hygiene Fifteen verified findings. Three of the worst were introduced by this branch in the last few commits. **The sign-in password was itself being scrubbed.** BIOMNI_DEMO_PASSWORD matches the PASSWORD marker, so for the length of every code step - up to 600s - demo_password() read empty. header_auth_callback then stopped refusing anonymous callers and minted a session for anyone who asked, while a correct password typed in that window was rejected. Both healed when the step ended, so it presented as a flake rather than an open door. Exempted alongside CHAINLIT_AUTH_SECRET. **An abandoned thread's late finally released a window twice.** release_thread already accounted for the thread, then its own finally ran the decrement unconditionally, dropping the global depth to zero and writing every credential back into os.environ while a *different* chat's generated code was mid-exec. The decrement is now conditional on the thread still being counted. **/proc/self/environ was never covered, and the docs claimed it was.** os.environ.pop calls unsetenv, which rewrites libc's environ array but never touches the stack block the kernel exposes - so a one-liner reads every credential during a window, needing no knowledge of this codebase. That is not the "determined attacker" case the module disclaims. The claim about subprocess coverage was wrong and is corrected rather than papered over; closing it properly means never holding the values in the initial environment. **Three plot bugs, two of them mine from an hour ago.** Capture iterated every open figure, which once figures stopped being closed meant: re-encoding the whole registry on every savefig (N(N+1)/2 renders, 6.5x slower at 20 figures), sweeping other sessions' charts into this session's plot list and LLM context, and calling plt.figure(n) per figure - so a bare plt.savefig() after a capture wrote whichever figure the loop left current. It now captures exactly the figure the patched call acted on. Separately, _bound_open_figures closed the lowest-numbered figures, which are the longest-lived and therefore another chat's - reintroducing the blank-file bug across sessions. It now only closes figures the current execution created. **Module-level imports of absent packages.** Dropping torch left genetics.py importing it at module scope, so `from biomni.tool.genetics import liftover_coordinates` - nothing to do with torch - killed all nine of that module's registered tools. Fixed, and the same pre-existing breakage in genomics.py (esm, pybiomart) and bioimaging.py (nibabel, SimpleITK, nnunet) with it: all 23 tool modules now import. bioimaging uses the idiomatic deferred pattern, since two of those names appear in annotations that evaluate at class definition. genomics is ported to harmonypy's run_harmony, the API of the package that replaced harmony-pytorch. **Fatal env parsing.** _SESSION_TTL_SECONDS and the two package knobs parsed with bare float()/int() at import - the failure the guarded helper beside them exists to prevent. os.getenv returns "" for a set-but-empty ConfigMap key, so the pod would CrashLoopBackOff before uvicorn binds, naming a tuning knob. **The router now delegates unknown attributes** to the stream it wraps, so sys.stdout.buffer and .reconfigure work again - they had been broken process-wide and permanently after any chat ran code. It also reports isatty() False and refuses fileno() while capturing, so rich and tqdm stop writing ANSI into the agent's observation and subprocess stops writing past the buffer. **Test hygiene.** tests/test_run_packaging.py imported chainlit_app at module scope, whose sys.exit(1) aborts collection with INTERNALERROR and zero tests on any interpreter where that import fails - the local gate is authoritative, so a green CI matrix would not have caught it. Two credential tests parked 30s daemon threads holding scrub windows that woke under later tests; they are now released and joined. The REPL tests left a router on sys.stdout for the whole session, making one test order-dependent and failing under `pytest -s`, and cleared the root logger's handlers; both are saved and restored per test. Verified: 618 pass under plain, `-s` and randomised ordering; no threads survive the run; all 23 tool modules import.
…d state dir The remaining review findings. **AD1 wrote its report and its outputs to different directories.** AD1.go builds a run directory, then delegates to A1.go which built a second one - so OUTPUT_DIR, and therefore every plot and CSV the generated code saved, pointed at directory B while trace.json, report.md and trace.ipynb were written into directory A. The user is told about A, which has no outputs. Timestamp-and-slug ids used to collide often enough to mask it; adding a random token made it happen every time. A1.go now reuses a directory the caller already made, and skips the second _summarize_topic_with_llm call that came with it. **discard_repl_session had no caller.** It is now called from the existing @cl.on_chat_end, which is where a chat's Python state should be released - otherwise each conversation's namespace, holding whatever dataframes and models it loaded, stays resident until the LRU cap pushes it out. **Session eviction did not do what its docstring said.** Both branches popped the least-recently-used entry and the TTL only chose the log level, so a live chat could be evicted while a genuinely idle one sat untouched. Idle entries are now preferred, and taking a live one remains a warning naming the knob to raise. **state/ was untracked but not ignored.** Compose now mounts it by default, and it holds the JWT signing secret and the chat database.
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.
Nine commits, all verified locally: 603 tests, ruff check and format clean, mypy at its pre-existing baseline, and the image built and exercised end to end on the demo box.
The headline fix
The REPL that runs LLM-generated code kept its namespace, captured plots and stdout capture in module-level globals shared by the whole process, while the app serves many chats from one process. Two people asking a question at the same moment collided: run A received run B's printed output and B's
OUTPUT_DIR, so A's generated files were written into B's run directory. And because each execution restored whateversys.stdoutit found rather than the one it replaced, a single overlap left the process writing into a discarded buffer, silently swallowing every later print.Reproduced with two overlapping calls, then end to end with two browser sessions whose code steps overlapped for 21.8s. Execution state is now keyed by chat session; stdout is routed rather than swapped;
run_with_timeoutreplays the caller's context into the thread it spawns (without that, every concurrent run lands in the default session and the isolation does nothing).Credentials
biomni/credentials.pystrips credential-shaped variables fromos.environaround everyexec. Stripping the real mapping rather than passing a filtered copy is deliberate: a bareimport osdefeats the copy, and subprocesses inherit the parent environment regardless. All four language paths are covered -#!CLI printenv ANTHROPIC_API_KEYreturned the key before this.This is not a sandbox and does not claim to be. Anything the process can recover, code running in the process can recover. It closes the accidental and casual paths.
Environment and image
Ships the AD Workbench environment, because that is the one the agent is told it has -
env_desc.pyadvertises ~114 libraries and the prompt says to prefer locally installed ones, while the minimal environment supplied ten.biomni/env_probe.pynarrows the catalogue to what is actually installed, so the agent can no longer plan aroundgseapyand fail at the import.Trimmed ~780 MB of unreachable packages, then dropped the GPU stack: torch, triton and twelve nvidia-cu12 wheels, 2.98 GB measured on linux/amd64, existing solely for cellpose and harmony-pytorch.
harmonypyreplaces the latter at 10.5 MB and is whatscanpy.external.pp.harmony_integrateactually calls.docker/prune-build-tools.shdrops the compiler toolchain after the build - 933 MB on the real target - and verifies its own work.Also
issfolded into the storage key, RFC 4180 escaping, and/statusno longer reports inactive under a connected user (pods get reclaimed on that signal).BIOMNI_DEMO_PASSWORDsign-in for a deployment with no gateway.workflow_dispatchpreviously triggered no job at all.Review
A code review of this branch raised 21 findings; 10 are fixed here, including three I introduced - a scrubbed
CHAINLIT_AUTH_SECRETthat 500'd other users mid-execution, the shell paths bypassing the scrub, and an allowlist re-armable from inside a window. The commit messages record what was not fixed: output from threads generated code spawns still bypasses capture, matplotlib's figure manager is still process-global,query_chatntremains registered but cannot run, andliterature.pyhas no Azure-Anthropic path.