Repository navigation
GRIP onboarding V3: identify gateway users, route model calls through the LLM proxy, explain SMB access - #16
Merged
Conversation
The AD Workbench deployment team reported that Biomni-AD was not detecting the user and could not read the workspace share, and asked for every model call to go through the platform's LLM proxy. This covers those and the rest of the "AI Apps Onboarding V3" checklist. Identity (biomni/identity.py, chainlit_app.py) - A chat takes its identity from the Chainlit login, not from the websocket's headers. A login cookie that names a different user or workspace than the gateway's headers, or one issued by an earlier release, is refused with a 401, and the page signs in again. - Ai-App-User-Context and Ai-App-Workspace-Context are parsed as strict CSV and are authoritative: while either is present the single-value headers are ignored, and a header with an unterminated quote or a repeated sub, iss, email or uuid is rejected whole. The websocket's headers are read from the raw ASGI list, where an underscore alias cannot pass for the real header. - BIOMNI_AUTH_*_HEADER settings that point at a context header are ignored with a startup warning, and UTF-8 names that arrive as latin-1 are repaired. - A reopened conversation whose sign-in no longer matches is not replayed, and a live run can only be followed or stopped by its owner. LLM proxy (biomni/llm_proxy.py, biomni/llm.py) - BIOMNI_LLM_PROXY_URL, _API_KEY and _SCHEMA (openai or anthropic) send every model call to the proxy with the bearer token and the session's X-User-Id (sub) and X-Workspace-Id (uuid), attached to each request from the identity bound to the running session. The identity follows the code step into the concurrent.futures pools it starts, and a call with no gateway identity is refused before it is sent. - Proxy refusals (401, 403, 404, 429, 5xx) are explained in plain words and end the run, instead of it carrying on without a model. - The Claude web search, which calls Anthropic directly, is withdrawn while the proxy is configured, and the prompts name only tools the agent can call. Hardcoded model ids now follow BIOMNI_LLM. Storage (biomni/workspace_prefs.py) - One storage_check log line per location with the live mount options and, for a location the app cannot use, the reason and the fix: SMB ownership options and the remount they need, cifsacl, FUSE, read-only image paths, hostPath with fsGroup. Users see the same explanation in the chat and in Settings, and the agent is told when the workspace cannot be read instead of seeing it as empty. Health and deployment - /readyz gates on the configured workspace, and on the proxy token and BIOMNI_TRUST_AUTH_HEADERS when the proxy is configured. - docs/grip_deployment.md maps the checklist item by item, the k8s manifest uses GRIP's volume layout, and Chainlit is pinned to 2.11.1 with aiosqlite and asyncpg in place of chainlit[custom-data]. - samples/ holds a synthetic table for the guide's smoke test. Also fixed - A LangGraph streaming race that dropped a code step from the transcript and showed an observation twice (chainlit_ui/graph_stream.py). - The sign-in page showed Chainlit's logo instead of the app's. - The settings dialog lost its paragraph breaks and did not refresh after a save. - CI's mypy step covers nine more modules.
deploy/k8s/biomni-ad.yaml holds several documents separated by ---, as Kubernetes manifests do, so the hook rejected it on every commit that touched it.
A data lake file that exists counts as downloaded, and downloads were written in place. One cut short by anything but a Python exception - the process killed, the pod evicted or OOM-killed, a KeyboardInterrupt - left a truncated file that no later query fetched again, so every analysis that needed the dataset read the fragment. The local copy of BindingDB here was 1.2 MB of 6.25 GB. With the data lake on a persistent volume, as the GRIP manifest now puts it, the damage also outlives the pod. Downloads now go to a hidden partial file beside the target, renamed into place once every byte has arrived (checked against Content-Length), so the file exists complete or not at all. Partial files a killed process left are removed once untouched for 15 minutes. Requests time out instead of holding the chat turn on a stalled connection, and one thread per process downloads a given file, so two chats that need it at once no longer write into the same file - where the second read it half-written - or fetch it twice. Reproduced against the real bucket by killing a download mid-way: the next fetch returned success within a second and kept the 12 MB fragment of a 1.5 GB file. After the fix it finds nothing at the target and downloads the file again.
Measured against the bucket: the data lake is 76 files and 15.05 GB, of which BindingDB is 6.25 GB, and AD1's catalogue data adds 1.41 GB. The docs said 9 GB (measured on a local copy whose BindingDB was truncated) or the older ~11 GB. Loaded whole, BindingDB takes about 13 GiB in pandas (extrapolated from a 50 MB sample), more than the documented minimum memory, so the guide now says so. Several per-dataset sizes in ARCHITECTURE.md were off by an order of magnitude. The guide also says that a download cut short is fetched again rather than left half-written.
The guard that refuses a cookie the gateway's headers do not match was checked only by a throwaway script. This keeps that script's checks - another user's cookie, another workspace, the shared 'local' and 'anonymous' cookies the previous release issued, a released gateway cookie, an underscore copy of the context header, another user's conversation - as a test driving Chainlit's HTTP API in a fresh interpreter, since chainlit_app reads its settings when imported. Passes on Chainlit 2.11.1 (the image) and 2.9.6; with the guard disabled it fails on eight of its 18 checks. Skipped without the chainlit extra.
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.
This PR covers everything the AD Workbench (GRIP) deployment team raised:
X-User-IdandX-Workspace-Id, OpenAI and Anthropic schemas);docs/grip_deployment.mdis the operator guide, and the best place to start reviewing.Not detecting the user
Biomni-AD already parsed the
Ai-App-User-ContextandAi-App-Workspace-ContextCSV headers.Four things stopped it recognising users:
BIOMNI_TRUST_AUTH_HEADERSwas off. It stays off by default, but the app now says so at startup, when a chat opens, and in Settings.BIOMNI_AUTH_*_HEADERsettings could turn the whole CSV into the user id. Such settings are now ignored, and a startup warning says to remove them.The context headers are now strict and authoritative:
sub,iss,emailoruuidgiven twice, is rejected whole.Ai_App_User_Context) cannot pass for the real header.LLM proxy
BIOMNI_LLM_PROXY_URL,BIOMNI_LLM_PROXY_API_KEYandBIOMNI_LLM_PROXY_SCHEMA(openaioranthropic) send every model call to the proxy. That covers the agent, its plans, and the tools that call a model.Each call carries the bearer token plus the session's
X-User-Id(sub) andX-Workspace-Id(uuid).concurrent.futurespools it starts.BIOMNI_LLM.Storage
On SMB, ownership comes only from the mount options, and changed options apply only when the node mounts the share again.
That matches "still cannot read" after new options were applied.
A partly applied set (
dir_mode=0770with the share still owned by uid 0) makes the share unreadable, not just read-only.storage_checkline per location, with the live mount options. For a location it cannot use, the line gives the reason and the fix: SMB options and remount,cifsacl, FUSE, read-only image paths, hostPath andfsGroup. The storage account's name is left out.I reproduced this on a real CIFS mount in Docker and verified the fix there.
Checklist and deployment
docs/grip_deployment.mdmaps each item of the V3 checklist.configuration.md,README.md,ARCHITECTURE.mdand.env.exampleare updated to match.deploy/k8s/biomni-ad.yamlnow uses GRIP's volume layout. Its pod no longer waits forever on a data path the image does not have./readyzchecks the configured workspace. When the proxy is configured, it also checks the proxy token andBIOMNI_TRUST_AUTH_HEADERS.aiosqliteandasyncpginstead ofchainlit[custom-data], whose GCS SDK upgradedrequestspast whatarxivaccepts.samples/holds a synthetic table for the guide's smoke test.Also fixed
check-yamlhook accepts the multi-document manifest.Review
Four review passes (identity, LLM proxy, storage, docs and deployment) raised 32 findings: 2 high, 8 medium and 22 low.
All are addressed here.
The two high findings:
tests/test_login_guard.pynow runs the reviewer's attack sequences against the real app. It fails on eight of its 18 checks with the guard disabled.Some findings asked for tests of code in
chainlit_app.py. Only the login guard got a unit test. The session-start and storage code there is covered by the browser E2E below instead.Verification
pytestpasses 795 tests in the full environment. In clean venvs built the way CI builds them (pip install -e . pytest), 760 pass and 35 are skipped, on both Python 3.11 and 3.12. Every skipped test needs an optional extra: chainlit, matplotlib, anthropic or PyPDF2.ruff checkandruff format --check(0.12.7);compileall;kubernetes-validate --strictfor 1.30 on the manifest.pip checkis clean. I did not rebuild the runtime stage locally: it only copies app files, and this machine has 8 GiB of free disk. CI builds the full image.Known gaps
BindingDB_All_202409.tsvis 6.25 GB on disk and about 13 GiB once loaded whole, and nothing tells the agent to read it selectively. A question that loads all of it can exhaust a pod sized to the documented recommendation, and the guide says so. The follow-up is a size hint in its data lake description, or a cap on what the agent may load.🤖 Generated with Claude Code