fix: launcher restart policy + complete nginx lazy-resolution migration - #77
Merged
man4ish merged 1 commit intoSep 2, 2026
Merged
Conversation
Root cause of tonight's docs.omnibioai.org outage (502, ~4h): launcher
exited with no logged reason and no restart policy, sitting dead. nginx
resolves upstream {} blocks eagerly at startup -- one unresolvable
hostname (launcher) prevented nginx from starting at all, taking down
every route behind nginx-router, not just launcher's own.
Fixes:
1. Added restart: unless-stopped to launcher (docker-compose.yml),
matching the jupyter/rstudio/vscode convention for persistent,
dependency-critical services whose absence has outsized blast radius.
2. Completed the lazy-resolution migration nginx-router.conf was
already partway through (auth/workbench/lims/policy-engine/
hpc-policy-engine/billing-service had already been converted,
per that file's own in-progress comment, after a prior real
incident on auth-service). Converted all 24 remaining static
upstream {} blocks (31 call-sites total) to the same
set $<name>_upstream host:port; proxy_pass http://$<name>_upstream;
pattern, using Docker's embedded DNS resolver (127.0.0.11,
already globally declared) for per-request lazy resolution instead
of eager startup-time resolution. No upstream in this file
load-balances across multiple replicas, so there was no tradeoff
in converting all of them uniformly.
Verified: scratch-copy nginx -t comparison confirmed the original
config fails with the exact tonight's-incident error
(host not found in upstream "rag:8096") while the migrated version
passes cleanly. Smoke-tested all 31 converted call-sites plus 3
already-migrated ones for comparison post-reload: zero 502s, all
responses application-level (200s, and expected redirects/401s/404s
reaching real backends) rather than proxy failures. Graceful reload
only, zero container restart/downtime.
One unresolved, explicitly flagged item: control (public)'s
/_svc/control/summary route returns 401 post-migration; this location
is marked no-JWT-required in-file and the diff confirms this pass
didn't touch its selector or add any auth_request, so the 401 most
likely originates from control-center's own app logic, not from this
nginx change -- but this was not verified against the pre-migration
baseline and should be checked independently.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VSmsTvqeqcp9kg9uEVLCFx
man4ish
added a commit
that referenced
this pull request
Sep 4, 2026
* test: raise UI test coverage to 95%+ across the board packages/omnibioai-ui (Table/Card/Tabs component library): - Add branch-coverage tests for Card's padding/actions-only header, Tabs' empty/no-match tab states, and Table's null-comparator and equal-value sort branches. - Raise the branches threshold in vitest.config.ts from 90% to 95%, matching statements/lines/functions. Result: 98.59% stmts / 98.19% branches / 100% funcs / 100% lines. Root src/ui app (Electron + web UI): - Add 23 new test files under tests/ui/ covering every page and component that was previously thin or untested: Jobs, Settings, Launch, Services, RoleManagement, Cloud/LLM/HPC, the App shell, GrafanaViewer, LicenseGate, OAuthLinkConfirm, Videos, Logs, Workbench, IdeServices, Wizard, ServiceViewer, BugReport, ErrorBoundary, Mode, Sidebar/MobileNav/ UpdateBanner, and the web-build session/API/roles modules. - Extend session.test.js and store.test.js with the branches their existing tests missed (password login, OAuth link/redirect edge cases, refresh failure paths, launchSystem failure, setConfig/setSystemStatus). - Enforce a 95% threshold (statements/lines/functions/branches) in vitest.app.config.js, already wired to `npm run coverage:ui`. - Remove the leftover scratch vitest.coverage.config.js — its include list duplicated vitest.app.config.js's existing test glob — and gitignore the generated coverage/ report directory. Result: went from 47% to 99.01% stmts / 95.02% branches / 99.47% funcs / 99.47% lines. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012pAAjARYEX2VDvfGPJcbj2 * fix(nginx): forward Authorization to control-center on /_svc/control The gated /_svc/control location correctly validates the request via auth_request /internal/auth/verify -- which resolves $control_authorization from either the real Authorization header or, when absent (an iframe navigation can't set custom headers), the omnibioai_access_token cookie (map added in 1329fed, Jul 29, specifically for this iframe case). That covers the *gate*, but the actual proxied request to control-center:7070 was never told to carry that resolved header -- it just forwarded whatever the original request had, which for a cookie-only iframe navigation is nothing. control-center's own require_permission() dependency then independently 401s with "Missing or malformed Authorization header", since it re-validates the JWT itself rather than trusting nginx's gate alone. /_svc/toolserver (a few lines below) already gets this right -- proxy_set_header Authorization $control_authorization; on its proxy_pass, not just on the auth_request subrequest. /_svc/control was missing the equivalent line. This is not related to tonight's PR #77/#78 (the /report/status, /llms, /report/data, /report/public-stats gating changes in control-center's main.py) -- neither touched GET / (what the iframe actually loads), which has stayed platform.manage_infra-gated throughout both. The real chain: 1329fed (Jul 29) added the cookie fallback but only wired it into the auth_request subrequest, not the main proxy_pass -- harmless at the time, since GET / had no backend-side auth yet. 8705cbf (Sept 1, an unrelated control-center route audit) gated GET / for the first time, which is what first exposed this dormant gap as a visible failure. Verified against the live nginx-router container with a real access token from a real /auth/login call (not a synthetic JWT): - /_svc/control/ (the iframe's actual URL) with the cookie: 401 -> 200, real HTML instead of {"detail":"Missing or malformed Authorization header"}. - /_svc/control/docker/containers, /knowledge-base, /storage, /cron/jobs, /coverage/status (all gated, non-allowlist): 200 with the cookie. - No cookie at all: still 401 via nginx's own @cc_unauthorized -- the gate itself is unaffected. - The public allowlist (health/summary/report/report/data) is byte-for-byte untouched and behaves identically to before (health: 200, summary/report: 401 from control-center's own independent gating, report/data: 200 per tonight's PR #78) -- they never needed forwarding since they were never nginx-gated to begin with. Applied via: dated backup (docker/nginx-router.conf.bak-2026-09-03- pre-control-auth-forward, gitignored per existing convention, not committed), syntax-tested in a throwaway container before touching the real file, then against the live container's own nginx -t. A plain `nginx -s reload` did NOT pick up the change -- discovered live that this docker-compose service bind-mounts the single config *file* by inode, and an editor's atomic rename-on-save orphans that bind mount from any subsequent host-side edit regardless of how many times nginx reloads. Required `docker compose up -d --force-recreate nginx-router` (this one container only, ~2s of its own downtime, nothing else in the stack touched) to re-resolve the mount against the current file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VevfQdYwX27LkcGqfRAxQL * fix(web-ui): wire webApi.loadConfig() in, stop false setup banner App.jsx's header shows "Setup required — configure data directory" whenever config.settings.data_dir is unset. data_dir is an Electron-only concept (the local Docker stack's data directory, read via window.api.loadConfig() -- electron/preload.js). On the web/cloud deployment (webstudio.omnibioai.org), window.api is never injected into a plain browser tab, so that whole branch was skipped and `config` stayed stuck at its hardcoded initial default forever -- data_dir structurally can never be set there, so the banner showed unconditionally for every web visitor, regardless of anything actually being misconfigured. src/ui/lib/web/webApi.js already had a purpose-built loadConfig() for exactly this case -- returns { mode: "beta", ..., settings: {} } with a comment explaining there's nothing to load, since there's no local Docker stack to configure when connecting to an already-running backend -- but its own top-of-file comment says it was never wired into any existing shared page, "out of scope for this isolation-only pass". Two changes, both required -- wiring in the call alone would NOT have fixed the visible banner, since webApi.loadConfig()'s return value (settings: {}) is identical in shape to the hardcoded default already in useState, so it changes nothing observable by itself: 1. When window.api is unavailable, detect web mode via the existing isElectron() helper (lib/session.js -- same helper ServiceViewer.jsx already uses for its own Electron/web branching) and call webApi.loadConfig(), setting config from its result. No first-run setStep(8) redirect for this path, unlike the Electron branch -- data_dir isn't a first-run condition to redirect out of when it structurally doesn't apply. 2. Gate the banner itself on config.mode !== "beta", not just !data_dir -- "beta" is this codebase's own existing signal for "no local Docker stack" (already used identically by Launch.jsx's and Services.jsx's isBeta checks), and correctly applies whether beta mode is reached via the web deployment or via choosing "Beta" inside the Electron app itself -- both cases have no data_dir to configure, for the same reason. Electron's own branch (window.api.loadConfig() present) is completely untouched -- confirmed by diff. Existing local/hpc/cloud/hybrid-mode Electron behavior, including tests/ui/app-shell.test.jsx's own first-run-banner test (mode: "local", no data_dir -> banner still expected), is unaffected: that test and the full suite (28 files, 198 tests) pass unchanged. Grepped the whole src/ui/ tree for other config.settings readers -- only these two call sites exist anywhere. Verified: rebuilt and redeployed the web-ui container (`docker compose up -d --build web-ui`), confirmed in the actual served production bundle (not just source) that the compiled condition is `o?.mode!=="beta"&&!o?.settings?.data_dir&&(...Setup required...)` -- config.mode defaults to "beta" on web both before and after this change and nothing there ever moves it off "beta", so the banner no longer renders. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VevfQdYwX27LkcGqfRAxQL --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Root cause of tonight's
docs.omnibioai.orgoutage (502, ~4h):launcherexited with no logged reason and no restart policy, sitting dead. nginx resolvesupstream {}blocks eagerly at startup — one unresolvable hostname (launcher) prevented nginx from starting at all, taking down every route behindnginx-router, not just launcher's own.Fixes
Added
restart: unless-stoppedtolauncher(docker-compose.yml), matching thejupyter/rstudio/vscodeconvention for persistent, dependency-critical services whose absence has outsized blast radius.Completed the lazy-resolution migration
nginx-router.confwas already partway through (auth/workbench/lims/policy-engine/hpc-policy-engine/billing-servicehad already been converted, per that file's own in-progress comment, after a prior real incident onauth-service). Converted all 24 remaining staticupstream {}blocks (31 call-sites total) to the sameset $<name>_upstream host:port; proxy_pass http://$<name>_upstream;pattern, using Docker's embedded DNS resolver (127.0.0.11, already globally declared) for per-request lazy resolution instead of eager startup-time resolution. No upstream in this file load-balances across multiple replicas, so there was no tradeoff in converting all of them uniformly.Verification
nginx -tcomparison confirmed the original config fails with the exact tonight's-incident error (host not found in upstream "rag:8096") while the migrated version passes cleanly.nginx -s reload) — zero container restart, zero downtime for this change itself.launcher's restart policy applied viadocker compose up -d launcher(confirmed stateless — no data volumes, safe to recreate) and verified viadocker inspect→RestartPolicy=unless-stopped.control(public)'s/_svc/control/summaryroute returns401post-migration. This location is marked no-JWT-required in-file (# Control center — public read-only endpoints (no JWT required)), and the diff confirms this pass didn't touch its selector or add anyauth_request— only inserted aset $control_upstream ...;line. So the401most likely originates from control-center's own application logic, independent of this nginx change, but this was not verified against the pre-migration baseline (would have meant an extra live reload back to the backup for one status code). Your call whether to check this before or after merging — flagging explicitly rather than silently passing over it.🤖 Generated with Claude Code