Sync demo/ios-nsls2 with main - #115
Merged
Anubhuti Sinha (anubhutisinha04) merged 103 commits intoAug 7, 2026
Merged
Conversation
Address the highest-consequence manager-core bugs from the 2026-07-02 backend review (reports 00-summary / 07-queueserver-manager-core, plan Phase 1). C1 — the worker-state poll task (_periodic_worker_state_request) died permanently on a double set_result of _fut_manager_task_completed: env-open holds MState.CREATING_ENVIRONMENT while it runs the plans/devices download + config-service sync, so the next poll iteration re-entered the same branch and called set_result on the already-resolved future -> InvalidStateError killed the task. Heartbeats run in a separate task, so the watchdog stayed satisfied while the manager silently stopped processing plan reports, task results and shutdown notifications. Fix: guard resolution behind _complete_manager_task(); split the loop body into _periodic_worker_state_request_once() and wrap the loop in try/except mirroring _heartbeat_generator. C2 — in unified (co-hosted HTTP) mode, HTTP handlers call _dispatch_command concurrently with the sequential 0MQ loop and the autostart _start_plan, so the IDLE check-then-act in _start_plan races (two queue_start both pass IDLE). Fix: serialize all command-handler execution with a single asyncio.Lock (_get_dispatch_lock) covering _dispatch_command (0MQ + HTTP loopback) and the autostart _start_plan call. The 0MQ REP loop is already sequential, so the lock is uncontended there — no 0MQ behavior change. H1 — the plan report was fetched with the default 0.5 s pipe timeout and cleared on read; one lost response marked a *completed* plan failed and re-queued it (duplicate execution). Fix: fetch with the long timeout + one retry; the worker now retains the report until the next command_reset_worker (delivered flag gates re_report_available to avoid re-processing), so a retry re-fetches it. H4 — the multiprocessing-pipe receive threads swallowed EOFError with `pass`, so after the peer closed the pipe poll() returned ready immediately forever (100% CPU busy-spin). Fix: log once, mark the thread stopped and break. Deferred to follow-ups (noted in review plan): H2/1.5 worker-death detection + env open/close timeouts; M1 task-result ack; 1.7 overlay timeout/atomicity; 1.8 sync-task exception surfacing; H3 ZMQCommSendThreads lock leak (0MQ code slated for removal in Phase 6). Tests: tests/manager/test_manager_core_criticals.py (14 isolated unit tests, no shared redis/ports) covering the set_result guard, poll-loop resilience, the real poll-iteration re-entry scenario, dispatch-lock serialization, plan-report timeout+retry, the worker delivered-flag semantics, and pipe-EOF thread exit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tart Addresses Copilot review on NSLS2#83. The worker's '_request_state_handler' (now) and the pre-existing '_request_plan_report_handler' acquire '_re_report_lock', but it was created (worker.py) AFTER 'self._comm_to_manager.start()' launched the pipe receive thread that dispatches those handlers — a 'request_state' / 'request_plan_report' arriving in that startup window would hit 'with None:' -> TypeError and could break worker<->manager comms. Move the 'threading.Lock()' creation to just before '_comm_to_manager.start()'. It can't move to __init__ (a threading.Lock isn't picklable for the 'spawn' start method, so it must be created in the child process inside run()). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix(queueserver): manager-core criticals (C1/C2/H1/H4)
…H2/M7) Phase 1 follow-up to NSLS2#83 (2026-07-02 backend review, report 07). H2 — a SIGKILLed/OOM-killed RE Worker left the manager stuck in EXECUTING_QUEUE (or CREATING_ENVIRONMENT) forever: '_worker_request_state' just timed out each poll and nothing detected the dead process; recovery required an operator 'environment_destroy'. Add worker-death detection to the poll loop: after '_worker_state_timeout_limit' (5) consecutive 'request_state' pipe timeouts while an environment exists, confirm with the watchdog (the authority on process liveness) and, only if it reports the process dead, run recovery — '_kill_re_worker_task' for a live environment (pushes any running plan back to the queue, releases locks, returns to IDLE), or resolve '_fut_manager_task_completed' to unblock a hung env open/close so its existing failure path cleans up. We never act on timeouts alone, so a legitimately slow env-open (large profile collection, hardware connect) is never aborted — the watchdog keeps reporting the process alive and we keep waiting. The counter resets on any good state response and at env open; '_worker_death_handled' prevents re-triggering. Chose this watchdog-confirmed approach over a blind 'asyncio.wait_for' on the env open/close future awaits (the review's alternative suggestion) precisely because env-open can legitimately take a long time; a fixed timeout would abort healthy slow opens, whereas the watchdog check cannot false-positive. M7 — in PipeJsonRpcSendAsync, a late/duplicate pipe response arriving after 'wait_for' cancelled '_fut_recv' could call 'set_result' on a done/cancelled future -> InvalidStateError, swallowed inside the detached '_response_received' task. Guard with 'if _fut_recv is not None and not _fut_recv.done()'. Also keep strong references to the detached callback tasks created by '_conn_received' / '_conn_sent' (asyncio only holds weak references, so they could be GC'd mid-execution). Deferred: M1 task-results ack (already uses the long timeout; a correct list-ack needs a real ack protocol — own follow-up PR). Tests: tests/manager/test_worker_liveness.py (10 isolated unit tests) covering below-threshold no-op, alive-worker-not-killed, executing->kill, handled-once, creating->future(False), closing->future(True), no-environment no-op, good-response counter reset, and the two M7 late-response guards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The queueserver suite is wall-clock-bound (many tests boot real RE Manager processes) and split via pytest-split. Two problems, both measured 2026-07-02: - Stale/unbalanced durations: the committed .test_durations had 942 stale entries (renamed/removed tests, e.g. the old test_zmq_api.py ids) and ~684 currently-collected tests with no recorded duration, so pytest-split estimated ~19% of the suite and balanced the 3 groups poorly. - 3 groups is too few: real serial wall-clock is ~118 min, so even a perfect 3-way split is ~40+ min/group. Changes (zero-risk — no tests added, removed, or modified): - .github/workflows/queueserver-tests.yml: split widened 3 -> 6 groups (--splits 6, matrix group [1..6], job names, header comment). - .test_durations: regenerated from a full serial run (USE_IPYKERNEL=true pytest --store-durations), pruned of stale entries and including the 14 new tests from test_manager_core_criticals.py (NSLS2#83). Result: stale=0, missing=0 over 3311 collected tests; pytest-split balances the 6 groups to ~19-20 min each. Regen env: Python 3.12, redis:7, bitnamilegacy/openldap. The recording run was 3225 passed / 64 xpassed / 1 failed / 7 errors in 1:57:40; all 8 failures/errors were the known-flaky 'RE Manager failed to start' timeouts (tests/http/test_access_control + one CLI test) and do not affect recorded durations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-in parity)
Top parity gap flagged in the 2026-07-02 backend review (reports 05/06/07,
plan item 2.1). Real beamline startup scripts and upstream's own sample
profiles import from manager subpaths, e.g.:
from bluesky_queueserver.manager.profile_tools import set_user_ns, load_devices_from_happi
from bluesky_queueserver.manager.annotation_decorator import parameter_annotation_decorator
The shim was top-level only, so those imports raised ModuleNotFoundError under
this fork — and because the distribution is named 'bluesky-queueserver' (it
shadows upstream so the api client's Requires-Dist resolves in-tree), users
couldn't install upstream alongside to get them either.
Alias the manager subpaths to the in-tree implementation via sys.modules:
'bluesky_queueserver.manager' -> queueserver_service.manager, plus
'.profile_tools', '.annotation_decorator', '.profile_ops'. These modules are
already imported by queueserver_service/__init__.py, so this adds no import
cost. The 0MQ-era '.comms' / '.json_rpc' / '.logging_setup' subpaths are
intentionally NOT aliased (that surface is being retired; HTTP-only direction).
Tests: tests/manager/test_bluesky_queueserver_shim.py — full top-level export
list present, manager subpaths importable, alias identity == the in-tree
modules, the documented beamline-profile import idioms work, and the 0MQ-era
subpaths remain absent.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…m + task exc) - Worker-death detection now uses a new '_watchdog_confirm_worker_dead' helper that recovers ONLY on a positive 'worker_alive: False'. Any inability to reach the watchdog (CommTimeoutError / any exception) is treated as "cannot confirm" and returns False so the poll loop keeps waiting instead of killing a possibly-alive worker. This also fixes the 3.9/3.10 case the reviewer flagged, where 'asyncio.TimeoutError' != 'TimeoutError' so a 'CommTimeoutError' from '_watchdog_is_worker_alive' would escape its 'except', bubble into the poll loop's blanket 'except', and defeat recovery. - PipeJsonRpcSendAsync: the detached callback tasks' done-callback now retrieves the task exception (and discards from '_background_tasks') via a shared '_on_background_task_done', so a raising '_response_received' / '_response_sent' is logged instead of surfacing as an "unretrieved task exception" warning. - Tests: '_watchdog_confirm_worker_dead' semantics (dead / alive / comm-timeout / unexpected-shape), no-recovery-when-watchdog-unreachable, and done-callback exception/cancellation handling for the pipe callback tasks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ci: rebalance queueserver test split (regenerate durations, 3→6 groups)
Remove references to internal planning documents from the shim and its test (the code should be understandable without them), and reword the ZeroMQ-layer note in plain terms.
Reword the worker-liveness test docstring and a section header to describe intent in plain terms, without referencing internal planning documents.
Reword the test docstring and section headers to describe what each test covers in plain terms, without references to internal planning documents.
docs: make manager-core test comments self-explanatory
feat(queueserver): alias bluesky_queueserver.manager.* subpaths (drop-in parity)
…namespace The HTTP/WebSocket API server is implemented in-tree as queueserver_service.http, but nothing provided the upstream-compatible bluesky_httpserver import namespace or a distribution named bluesky-httpserver. As a result `import bluesky_httpserver` (and the submodule imports that beamline startup scripts, uvicorn factories, and YAML configs rely on) broke, and a third-party `Requires-Dist: bluesky-httpserver` would pull the upstream distribution in alongside this fork -- colliding on the import package and clobbering the `start-bluesky-httpserver` console script. This mirrors the existing bluesky_queueserver shim: a second thin distribution that re-exports the in-tree implementation. - New standalone distribution under backend/queueserver_service/bluesky-httpserver/ claims the bluesky-httpserver distribution name and ships the bluesky_httpserver import package. It pins bluesky-queueserver==1.0.0 in lockstep (the shim maps the implementation's internal module layout, so a version-skewed pair must be uninstallable). - A meta-path finder lazily ALIASES bluesky_httpserver.<sub> onto the already-imported queueserver_service.http.<sub> -- the same module object, never re-executed, so settings singletons, DB engines and isinstance checks stay consistent across both import paths. It handles literal `import x.y`, `from x.y import z`, and nested subpackages (bluesky_httpserver.authorization.*). A bare `import bluesky_httpserver` stays lightweight (no FastAPI stack), matching upstream's __version__-only __init__. - The start-bluesky-httpserver console script is intentionally NOT re-declared here; the bluesky-queueserver distribution already owns it (re-declaring would recreate the clobber this shim exists to prevent). - Installed alongside the main package in the Dockerfile and the queueserver-tests CI job (`-e . -e ./bluesky-httpserver`). - Tests (tests/http/test_bluesky_httpserver_shim.py): lightweight bare import, submodule import + identity aliasing, nested subpackage, from-imports + server entrypoints, config-driven module:object dotted-path load, __version__ from dist metadata, lockstep pin, and console-script sole ownership. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix(queueserver): worker-death detection + pipe late-response guard (H2/M7)
… response The worker previously cleared its completed-task results as soon as the manager read them. If that read's response was lost (pipe timeout), the results were gone — the affected tasks stayed "running" in the manager forever. The worker now keeps results until the manager acknowledges them: the manager sends the task UIDs it received on the previous successful download, and the worker drops those before returning the rest. Results stay available until acknowledged, so a lost response is simply re-fetched on the next poll. Storage is keyed by task UID, so a re-delivered result is acknowledged again but not stored twice. Tests: worker retains results until acknowledged and drops acknowledged UIDs; the manager tracks and sends acknowledgements, keeps results after a dropped response and re-fetches them, and does not re-store an already-received result.
Because task results now stay available until acknowledged, the periodic worker-state poll would schedule a new download every cycle while a batch was pending, running several concurrently and flooding the shared worker pipe — which could starve other worker communication (e.g. plan-report delivery) and leave the manager stuck in 'executing_queue'. Guard the download so only one runs at a time; a concurrent trigger is a no-op and the next poll re-runs it once the current one finishes. This keeps the worker-pipe usage to a single in-flight task-results request, as before.
…shim feat(queueserver): bluesky-httpserver shim distribution + import namespace (2.2)
…rce-unlock, and device update Three related correctness fixes in the device registry: - Enable/disable now persist to the database before mutating the in-memory registry (matching create/update/delete). Previously the in-memory flag was flipped first, so a failed write left memory and the database disagreeing until restart; it also mutated the live spec object in place. - Force-unlock is now all-or-nothing: if any named device is unknown, nothing is unlocked and no audit entry is written (it returns 404). Previously it cleared locks for the valid devices, then returned 404 and skipped the audit log, leaving state changed with no record. - Updating a device now re-homes its shared/standalone PVs instead of deleting the index entries. Dropping a PV in an update that another device also lists (or that is registered standalone) previously destroyed the surviving entry, making that PV 404 until restart. The re-homing logic is now shared with device removal. Tests: enable/disable leave memory unchanged when the write fails; force-unlock with a mix of known and unknown devices changes nothing and writes no audit; updating a device to drop a shared PV re-homes it to the other owner and a standalone PV reverts to standalone.
Initialize '_loading_task_results' in the manager constructor so the single-flight guard has a defined value even if a download is attempted before an environment is opened, and drop the duplicate reset in the env-open path.
Add an in-process compatibility suite that exercises the real PyPI `bluesky-queueserver-api` client (`REManagerAPI` from `bluesky_queueserver_api.http`) against a live RE Manager + HTTP server. The REST + WebSocket surface this client speaks is a stable public contract, but until now it was only exercised against a running container by the integration exerciser; there was no fast, in-process test that fails when a request or response shape drifts. - `test_side_c_api_client_compat.py` (single-user API-key mode): status/ping, config, allowed/existing plans & devices, permissions get/set/reload, the full queue-editing set (add, add_batch, get, update, move, remove, clear), history, environment open/close, plan execution + history result, script_upload and function_execute with task_status/task_result, the HTTP console monitor, Run Engine pause/resume/stop, locks, and error semantics (a rejected request raises, an admin-only endpoint and a wrong API key return HTTP errors). The manager is started with console publishing enabled so the console monitor has output to stream. `whoami()` is marked xfail: `/api/auth/whoami` currently returns HTTP 500 (tracked separately); the test flips to xpass when fixed. - `test_side_c_auth.py`: token/session auth through the client against a password-authenticator-backed server -- login, token-authenticated requests, session refresh, minting an API key, logout, and rejection of bad credentials. - A dedicated `queueserver-side-c` CI job runs both files for fast per-PR signal (they also run as part of the full suite). This is the acceptance gate for the HTTP contract as the 0MQ transport is retired. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix(queueserver): don't lose completed-task results on a dropped pipe response
test(queueserver): drive the bluesky-queueserver-api client over HTTP
The RE Manager command endpoints declare Pydantic response models so the generated SDKs and static openapi.json carry real type information. Those models used extra="forbid", which makes FastAPI raise a ResponseValidationError (HTTP 500) whenever the manager returns a key the model doesn't declare. The manager dicts can grow new keys over time (for example, when tracking upstream bluesky-queueserver), so a forbid policy turns an otherwise-valid response into a 500. Upstream does not validate or strip response fields. Switch the shared RMResponse base to extra="allow" so unknown keys pass straight through to the client while the declared fields keep their types. The regenerated shared-schema/queueserver_service.openapi.json reflects this (additionalProperties: false -> true on the affected models); the drift test still pins it. The response-model round-trip and drift-sentinel tests keep catching shape changes at test time rather than in production. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix(configuration-service): registry integrity for enable/disable, force-unlock, device update
The docstrings implied new-key drift is generally caught by the drift-sentinel tests, but only a few routes (status, history, lock_info) assert exact key sets; the rest are only round-tripped and, under extra="allow", would not fail on extra keys. Reword to state that limitation explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allow unknown keys in RE Manager HTTP responses
Two monitoring-layer fixes in direct_control_service. Blocking CA connect no longer holds the global monitor lock. PVMonitorManager.subscribe built the EpicsSignal, waited up to 5 s for the connection, and took the initial read while holding self._lock. The value/meta update handlers reacquire that same lock on the CA dispatch thread, so a single dead PV's connection timeout stalled fan-out for every monitored PV, and N unreachable PVs froze monitoring for ~N x 5 s. The connect and initial read now run outside the lock; the lock is taken only to commit (or roll back) the caches, with a per-PV connect lock serializing concurrent first-touches so no PV opens two connections. The pv-socket also subscribes new PVs concurrently instead of serially. Client disconnect during an in-flight first subscribe no longer leaks a CA monitor. Both the pv-socket and device-socket committed their bookkeeping under the lock and then awaited the blocking EPICS subscribe. If the client disconnected during that await, its teardown ran while the signal wasn't registered yet (a no-op), then the subscribe completed and registered a live CA monitor plus callback that nothing referenced -- broadcasting to an empty set and, because teardown matches callbacks by identity, keeping the signal alive forever. After the subscribe returns, both sockets now re-check under the lock that the PV/device still has clients (and, on the pv-socket, still owns its callback slot) and unsubscribe immediately if not. Adds regression tests: connect/read runs outside the lock, and disconnect racing the EPICS subscribe leaks no monitor on either socket (including the all-failed device path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A PV that fails to connect (or fails while registering its CA monitors) is never reached by any unsubscribe path, so its per-PV connect-lock entry persisted forever — a slow leak for repeatedly-failing PV names. Both subscribe() failure paths now drop the connect lock (guarded on _signals so a live subscription committed by a racing subscriber is never disturbed), and a regression test covers both the connect-failure and subscribe-registration- failure cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…be-leak Fix PV monitor lock stalls and disconnect-during-subscribe monitor leaks
Add a contract guard that pins the queueserver endpoints the finch frontend's qServer client depends on: each must remain present (path + method) in the committed OpenAPI contract. A backend change that drops or renames one now fails CI here instead of silently breaking the frontend. The endpoint list mirrors finch src/api/qServer/requests.ts and is the source of truth to update when the frontend's calls change. The test reads only the committed schema, so it is fast and needs no running server. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Parse the committed OpenAPI schema once via lru_cache instead of per parametrized case, and assert the top-level 'paths' key is present so a malformed schema fails with a clear message rather than a bare KeyError. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
configuration_service calls direct_control's POST /api/v1/devices/enrich via DirectControlClient, whose request/response models (EnrichmentSpec / EnrichmentResult) are hand-mirrored from direct_control's published contract. Add a guard that pins those client models to the committed shared-schema/direct_control.openapi.json: the request spec fields must equal the contract's EnrichmentSpec (all required), and the result fields must equal the contract's EnrichmentResultItem with `ok` required. A field rename or removal on either side now fails CI here instead of silently degrading every enrichment to ENRICHMENT_UNAVAILABLE at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t-guard Guard the finch frontend's required queueserver endpoints
…st helper
RegistryClient and CoordinationClient issued HTTP calls through a per-call-site
local handle (`client = await self._get_client(); await client.get(path)`),
unlike configuration_service's and queueserver_service's clients. Introduce a
`_request(method, path, **kwargs)` helper on each (mirroring
queueserver_service's ConfigServiceClient._request) and route every call through
it.
Behavior is unchanged — the helper uses the same lazily-created shared client and
`client.request("GET", path)` is equivalent to `client.get(path)`. The benefit is
a single, consistent, statically-analyzable HTTP call surface per client.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The shared _request helper forwarded arbitrary **kwargs to httpx, which contradicted its docstring's claim of a consistent, analyzable call surface. Replace **kwargs with an explicit keyword-only `timeout` parameter — the only per-call override any call site uses (the health probe) — so the helper's surface is exactly what the docstring promises. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The config client reads body["results"] unconditionally, so the contract must keep 'results' required, not merely present. The response test only checked 'results' in properties; add a check that it is in the schema's required list, mirroring the existing request-side 'items' and result 'ok' guards, so a drift to optional is caught here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
evict_idle_monitors() pops the PV from _signals under its lock but calls signal.destroy() after releasing the lock, so destroy is strictly later than the removal. The test broke its wait loop on _signals removal and then asserted signal.destroyed, so under CI load it could observe the removal before destroy ran and fail intermittently. Wait on signal.destroyed (the strictly-later observable) instead, then assert the _signals removal that necessarily preceded it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
evict_idle_monitors() pops the PV from _signals under its lock but calls signal.destroy() after releasing the lock, so destroy is strictly later than the removal. The test broke its wait loop on _signals removal and then asserted signal.destroyed, so under CI load it could observe the removal before destroy ran and fail intermittently. Wait on signal.destroyed (the strictly-later observable) instead, then assert the _signals removal that necessarily preceded it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-helper Route direct_control's config-service calls through a _request helper
…ract Guard the configuration_service ↔ direct_control enrich contract
process_exception is called from the except block of ~70 handlers across every router and maps the in-flight exception to an HTTPException: RE Manager request timeouts to 408, everything else to 400. That mapping was only exercised indirectly (a custom-router fixture and flaky ZMQ-timeout integration tests). Add a focused unit test pinning both branches directly, so a regression in this shared error mapper fails fast and deterministically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pin process_exception's HTTP error-status mapping
A foreign IOC bound to 5064 (another project's simulator) was silently reused as the test IOC, hanging every EPICS test on connect timeouts. DIRECT_CONTROL_TEST_IOC_PORT now picks a free port for both the caproto server and the CA clients.
…tbound HTTP Each backend service must run standalone as well as composed, and the resolver enrichment callback was the one place a dependency arrow reversed: configuration_service called direct_control's /devices/enrich, while direct_control (and everything else) calls configuration_service - the only service-to-service cycle in the system. - direct_control: new POST /api/v1/devices/resolve resolves dotted device addresses to PV names by live introspection, using its registry provider (config-service in http mode, local file in standalone mode) and the existing ophyd-cache walk. Handles the ophyd FormattedComponent runtime placeholders static resolution cannot. Replaces the config-facing /devices/enrich endpoint (its only consumer). - configuration_service: /devices/resolve is now static-only; needs_enrichment is a terminal outcome pointing at direct_control's resolve. The direct-control client, CONFIG_DIRECT_CONTROL_URL, the enrichment cache, and the enrichment_unavailable outcome are removed. The service makes no outbound HTTP calls at all. - OpenAPI schemas regenerated; docs and orientation pages updated.
Both suites pass against 0.21.0 (config 438, direct-control 343); pyproject constraints were already open so CI resolves latest.
The guard pinned config-service's enrich client models against direct_control's published schema. With the enrich client removed and live resolve moved to direct_control, there is no cross-service enrich wire left to pin; the resolve endpoint's request/response shapes are covered by direct_control's own tests and the OpenAPI drift gate.
Upstream merged 'Updating authenticators from latest in Tiled' (PR NSLS2#81, 2026-08-03), re-aligning bluesky-httpserver's auth with tiled v0.2.12 after ~3 years of divergence. This port keeps the fork wire-compatible with the OIDC login workflows the bluesky-queueserver-api client is adding (its PR NSLS2#62). Adopted wholesale (imports rewritten to queueserver_service.http): authenticators.py (OIDC incl. Entra + proxied + device-code flow, mode flag removed), protocols.py (new; class-type route wiring), database core/orm + pending-sessions migration, schemas additions. app.py wiring, DB auto-upgrade, robust shutdown, and the WebSocket first-message auth handshake are hand-ported into the fork's build_app and split routers. Fork-local behavior re-applied on top: case-insensitive WS auth schemes, WS query-param fallback, async IdP token exchange (upstream's new exchange_code still blocks the event loop), OpenAPI docs on auth routes. Bearer tokens are now accepted on WebSockets (new upstream contract); the WS test asserting they were rejected is updated accordingly. Tests: upstream's new authenticator/OIDC/database test modules ported; OIDC fixtures added to conftest; test server HTTP port overridable via QSERVER_TEST_HTTP_PORT (a foreign container on 60610 otherwise absorbs the whole suite). OpenAPI schema regenerated (additive only). Auth suites green: 28 authenticator + 14 database/OIDC + 12 WS auth + Side-C auth; response-model/shim/drift sentinels 64 passed.
- plan_queue_ops: remove the completed item's UID before registering the re-queued copy in loop mode; the UID dict otherwise grows by one entry per cycle (upstream memory-leak fix). - profile_ops: only evict script-local modules from sys.modules after a startup-script load; unconditional eviction breaks common library modules on Python >= 3.13. Also drops a stray debug print. - profile_ops: fix a shadowed loop variable in annotation processing (type_patterns reused as the loop variable).
The Side-B CI job collects tests/http for the OpenAPI drift test with only the base install present; module-level cryptography/jose/respx imports in conftest broke that collection. The auth/OIDC fixtures now import their deps inside the fixture bodies.
- Decode Bearer tokens explicitly on both direct WebSocket auth paths: calling get_current_principal() outside FastAPI left decoded_access_token at its Depends(...) default (a truthy sentinel), which broke WS Bearer auth. Invalid/expired tokens now fail closed; regression asserts added to the WS precedence test. - Fix decoded_access_token annotation (Optional[dict]) and the invalid 'Principal or None' return annotations (Optional[...], 3 sites). - Replace mutable defaults: UserSessionState.state and Session.state use default_factory; schema regenerated (default no longer emitted). - Docs: use the canonical queueserver_service.http.authenticators paths in configuration examples (3 sites); fix 'acheived' typo.
…ups per device A batch addressing the same device repeatedly now hits the registry once per lookup kind instead of once per address (get_device_pvs has no client-side cache, so a 200-address single-device batch could cost up to 400 sequential HTTP calls). Failed lookups are memoized too. Regression test asserts the call counts.
queueserver: port the tiled-v0.2.12 auth stack (upstream PR NSLS2#81) + three upstream fixes
…-control Move live device resolve to direct_control; drop config-service's outbound call
Anubhuti Sinha (anubhutisinha04)
approved these changes
Aug 7, 2026
Anubhuti Sinha (anubhutisinha04)
merged commit Aug 7, 2026
4681737
into
NSLS2:demo/ios-nsls2
46 of 59 checks passed
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
3 participants
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.
Merges current
main(745f619) into the demo branch — first sync since early July. Conflict-free merge.Notable backend changes the demo branch picks up:
GET /api/queue/item/{item_uid}, registry disable-state handling on bootstrap + diff, tightened per-plan lockingThe demo's
queueserver-reproflow runs the real upstream queueserver via pixi, so it is unaffected;pods/iosruns the in-tree backends and picks these up directly.