Improve ingestion & query service - #75
Open
tekrajchhetri wants to merge 80 commits into
Open
Conversation
Track all mutating actions (ingestion, named-graph registration, crash recovery) as W3C PROV-O triples in a dedicated provenance named graph (https://brainkb.org/provenance/), queryable via SPARQL. Postgres keeps job execution state; Oxigraph is the provenance source of truth. - core/provenance.py: PROV-O builders (IngestionActivity/RegistrationActivity/ RecoveryActivity) with typed user/system agents, plus Graph Store HTTP write and CONSTRUCT->JSON-LD retrieval helpers. Writes are best-effort. - insert.py: emit provenance from run_ingest_job (all terminal states), create_named_graph, and recover_stuck_jobs; stop embedding PROV into domain data (files upload unmodified); add GET /provenance/job and /provenance/named-graph (application/ld+json). - PROVENANCE_MODEL.md: design and reference.
- query_provenance_jsonld: Oxigraph returns HTTP 406 for Accept application/ld+json (it serializes Turtle/N-Triples/N-Quads/RDF-XML only), so request Turtle and convert to JSON-LD locally with rdflib. Keeps the JSON-LD API contract independent of the triplestore's output formats. - construct_for_job: also traverse inbound prov:used so a job's provenance bundle includes the recovery activity (and its system agent) that acted on the job, not just forward links.
Track which triples each ingestion job adds, not just activity-level metadata. Validated end-to-end against a live Oxigraph. - Each job stages triples in a per-job delta graph (https://brainkb.org/provenance/delta/{job_id}), then merges into the target via SPARQL ADD (idempotent set union). The delta graph is preserved as the exact change record; a brainkb:IngestionDelta PROV-O entity records the derivation, target, delta graph, and added triple count. - Gated by TRACK_TRIPLE_DELTAS (default on); disable to upload directly to the target (delta graphs persist, roughly doubling stored triples). - New endpoints: GET /provenance/delta (added triples as JSON-LD), /provenance/delta/history (change history for a graph), /provenance/delta/compare (diff two jobs' deltas: A-only/B-only/shared). - provenance.py: delta_graph_for, merge_delta_into_target, count_graph_triples, construct_delta_content, delta_history_for_graph, compare_deltas; job CONSTRUCT now includes the delta entity. - PROVENANCE_MODEL.md: document the delta model and endpoints.
Registration was recorded twice: once in the named-graph registry graph (metadata/named-graph, via named_graph_metadata) and again as a separate RegistrationActivity in the provenance graph. Both asserted the registration timestamp and made the graph IRI a subject. Keep the registry graph as the single home for registration facts and attribute it to the registering user there (prov:wasAttributedTo); drop the duplicate RegistrationActivity from the provenance graph. - shared.py: named_graph_metadata takes an optional agent_uri and adds prov:wasAttributedTo on the registry entry. - insert.py: create_named_graph passes the agent URI; remove the duplicate build_registration_provenance write (and its import). - provenance.py: remove build_registration_provenance (now unused). - query.py: /query/registered-named-graphs returns registered_by. - PROVENANCE_MODEL.md: document registration living in the registry graph.
Scopes (require_scopes) — GET reads require 'read', mutations require 'write':
- read added: GET /insert/jobs, /insert/user/jobs/detail,
/insert/jobs/check-recoverable, /provenance/job, /provenance/named-graph,
/provenance/delta, /provenance/delta/history, /provenance/delta/compare,
/query/registered-named-graphs
- write added: POST /insert/jobs/recover, POST /register-named-graph
- unchanged: /insert/{raw,files}/knowledge-graph-triples (write),
/query/taxonomy (read), /query/sparql/ (write+admin, arbitrary query),
/register + /token (public)
Docstrings: document the difference between /query/registered-named-graphs
(the registry/catalog of graphs) and /provenance/named-graph (the PROV-O
ingestion/activity history of a graph); expand descriptions on all provenance
and delta endpoints.
Arbitrary SPARQL is a powerful, unrestricted capability; require the 'admin' scope (dropping the redundant 'write'), and keep it off the default 'read' scope used by fixed-shape read endpoints.
Users/teams create owner-controlled spaces, keep them private, or publish them publicly for anyone (incl. unauthenticated clients) to read. Supports a decentralized, IRI-addressable model (https://brainkb.org/space/{slug}). Storage split (confirmed): Postgres holds identity/teams/enforcement (spaces, space_members, space_graphs); Oxigraph holds all KG data + provenance plus a best-effort RDF mirror of each space manifest (metadata/spaces graph). - core/spaces.py: CRUD, per-request authorization (public=anonymous read; private=members; write=owner/editor), and SPARQL-Update RDF mirror. Legacy unmapped graphs fall through to existing scope checks (backward compatible). - core/routers/spaces.py: POST/GET /spaces, GET /spaces/{slug}, PATCH visibility, POST/DELETE members, POST graphs, GET /spaces/{slug}/data (public = anonymous). - security.py: get_current_user_optional (token used if present, never 401) for anonymous public reads. - insert.py: ingestion now enforces space write-authorization on the target graph, in addition to the write scope and user-identity check. - main.py: create spaces tables on startup; mount spaces router. - SPACES_MODEL.md: design + endpoints. Validated end-to-end against the live stack (15/15): private owner ingest/read, outsider ingest/read denied (403), anonymous read denied while private, public flip enables anonymous read + listing, public grants read-not-write, RDF mirror.
Stop leaking private graph existence via the registry listing: graphs in a private space the caller is not a member of are omitted. Public-space and legacy (unmapped) graphs remain listed. The endpoint now loads the caller identity (get_current_user) and excludes hidden graphs via spaces.hidden_graphs_for(). Also clarify in SPACES_MODEL.md that job-scoped provenance is intentionally owner-only and ingestion is always restricted to activated JWT users with valid credentials + space owner/editor membership (never anonymous); only reads of public spaces are anonymous. Validated live: owner sees private+public graphs; non-member sees only public.
- query_service/README.md: document auth/scopes, ingestion + jobs, PROV-O provenance, triple-level delta endpoints, and private/public spaces; add architecture note (Postgres = identity/enforcement, Oxigraph = graph data + provenance). - readme.md: expand the Query Service bullet to mention provenance, deltas, and spaces, with pointers to the model docs.
Search over the knowledge graphs that respects space visibility. Hybrid design: Postgres holds a full-text locator index (graph_search_index: subject, text, named graph, owning space) populated at ingest; a query runs in Postgres (fast, filtered by space visibility/membership) to locate subjects, then the matched triples are fetched from Oxigraph (the source of truth for KG data). - core/search.py: index_graph_subjects (indexes a graph's/-delta's subject literals), reindex_graph_space, and search() with the access filter (anonymous -> public spaces only; authenticated -> public + member spaces + legacy). Data for the located subjects is fetched from Oxigraph as JSON-LD. - main.py: create graph_search_index (GIN full-text) on startup; mount router. - run_ingest_job: index the target graph's subjects after merge (best-effort). - spaces.attach_graph: point existing index rows at the space so search picks up the workspace immediately (inline SQL to avoid an import cycle). - routers/search.py: GET /search (optional auth; space-scoped or full). - READMEs updated. Validated live (9/9): anon finds public term + data from Oxigraph; anon/outsider cannot see private term; owner finds private; scoped search respects membership; anon scoped to a private space returns nothing.
Indexing a large graph inline was slow and delayed ingest jobs. Move it off the ingest path into an in-process async task queue with durable status. - core/indexing.py: asyncio queue + single background consumer, durable index_tasks table, atomic queued->running claim (safe across gunicorn workers), and startup recovery (re-queue tasks left over from a crash). Supports 'ingest' (one graph) and 'backfill' (reindex every user graph, with progress). - main.py: create index_tasks table; start the consumer on startup. - insert.py: ingest now ENQUEUES indexing (non-blocking) instead of awaiting it, so jobs finish without waiting on indexing. - routers/search.py: POST /search/reindex (admin, background backfill) and GET /search/index-tasks (status). - README updated. Validated live: ingest job completes in ~1s while indexing runs in background; background task indexes the subject and it becomes searchable; backfill reindexed 3/3 graphs in the background.
Ingestion stays submit-and-forget/background, but a burst of concurrent submissions could previously run unbounded and exhaust memory / the DB pool / Oxigraph and crash the worker. Add a per-worker asyncio.Semaphore limiter (MAX_CONCURRENT_INGEST_JOBS, default 3): run_ingest_job now acquires a slot before processing; excess jobs return immediately and wait as 'pending' until a slot frees (backpressure without a queue). Effective global cap ~= cap x workers. Validated live: 6 concurrent submissions all accepted in ~0.07s (submit-and- forget intact) and all completed 'done', throttled, no crash.
SPARQL (Oxigraph) + SQL (Postgres) queries to verify ingested graphs, provenance, per-job deltas, spaces manifest/membership, and the search index, with instructions to run via the API or directly against Oxigraph/Postgres.
…I access only Authorization now comes from the user's roles (joined by email to Web_user_profile -> Web_user_role), mapped to capabilities, layered with space membership. JWT scopes remain only an API-access gate. - core/rbac.py: roles->capabilities policy; delegated grants (user_capability_grants); SuperAdmin>=Admin>write>read>none hierarchy; admin-intrinsic caps (grant, sparql_admin) are NOT delegatable (no escalation); query_service never assigns roles (role assignment stays Django-owned). - Space types: 'individual' (any write-capable user) vs 'team' (Admin/SuperAdmin or granted create_team_space). - Enforcement: create space (by type), ingest, recover, arbitrary SPARQL, space management, and reads (no-role -> public content only). - Admin endpoints: GET/POST /admin/capabilities[/grant|/revoke]. - main.py: space_type column + user_capability_grants table. - RBAC_MODEL.md: full model. Validated live (14/14): no-role denied (public read only); Lab Member creates private + ingests but not team; Admin creates team + SPARQL; delegated grant upgrades Lab Member to create team spaces; non-admins can't grant; admin-intrinsic caps rejected for delegation.
Within a space, restrict an action to a global role, a space role, or specific
members — e.g. 'only Admins may write here', 'only these Lab Members may read',
'let this member manage'. Layers on top of capabilities + owner/editor/viewer
membership; owner and global Admin/SuperAdmin always bypass (no lockout).
- space_access_rules table (action, subject_type[global_role|member|space_role],
subject_value).
- spaces.py: rule CRUD, matches_access_rule (pure match) and
space_action_permitted (owner/admin bypass; no-rules -> allow for read/write).
- Enforced on: reads (get space / space data), ingest (insert raw+files), and
manage (members/visibility/graphs, via _can_manage grant).
- Endpoints: GET/POST/DELETE /spaces/{slug}/access-rules (manager only; GET
member/manager).
- RBAC_MODEL.md updated.
Validated live (9/9): write Admin-only rule blocks a Lab Member editor but owner
bypasses; member rule then allows the Lab Member; read Admin-only rule blocks a
member read (owner bypass); manage rule grants a member management.
POST /api/admin/users/activate and /deactivate (by email), Admin-gated, using the existing jwt_user_repo.activate_user/deactivate_user. Enables admins to activate accounts via API (e.g. after password self-registration) instead of only the UI/DB.
…ntial Web_user_profile becomes the single user of record; Web_jwtuser is demoted to a 1:1 credential linked via a new profile_id FK (email backfill for existing rows). Per-service token isolation is preserved (each service keeps its own secret) — this unifies identity, not tokens. - schema: add Web_jwtuser.profile_id (FK -> Web_user_profile, SET NULL, indexed) in the ORM + an idempotent inline migration with case-insensitive email backfill (usermanagement bootstrap). - usermanagement: new provision_identity() as the single path that ensures profile + linked credential + default role (Curator) + bootstrap-superadmin; OAuth callback refactored onto it (drops _ensure_jwt_user_shell + duplicated default-role/bootstrap blocks) and now sets profile_id. - query_service: /api/register now provisions a canonical profile, assigns the default role, and links the credential (best-effort so it never blocks signup) — fixes password users having no roles. - query_service tokens now carry sub/scopes/user_id/profile_id/roles/auth_source to match usermanagement's v2 token shape, still signed with query_service's own secret. Roles stay informational; rbac re-reads them from the DB. - docs: AUTH_UNIFICATION.md design + Phase 1 implementation status. Verified live: migration + backfill, fresh register -> profile+link+Curator, both services' /api/token return the same claim shape, protected endpoints OK.
usermanagement becomes the sole token issuer; a single login mints a short-lived refresh token, exchanged for narrow per-service access tokens (aud=<service>). Services verify via the published JWKS and require their own audience, so a token minted for one service can't be replayed against another (containment enforced by aud, not shared secrets). Additive: legacy HS256 tokens still validate, so this is a safe migration rather than a cutover. usermanagement: - tokens_rs256.py: RS256 key load from env PEM/FILE, else a process-shared ephemeral key persisted to a file (all uvicorn workers agree — per-worker ephemeral keys break cross-worker verification). JWKS builder, refresh/access minting, refresh verification. - routers/sso.py: GET /.well-known/jwks.json, POST /api/auth/login (refresh), POST /api/auth/exchange (per-audience access; roles/scopes re-read fresh from the DB, active + ban checks enforced here). - configuration.py: issuer, private key, TTLs, allowed audiences. query_service: - jwks.py: sync JWKS fetch/cache + RS256 verification requiring iss + aud. - security.py: decode_token_any() tries RS256 (SSO, aud-checked) then legacy HS256; wired into get_current_user(_optional), verify_scopes/require_scopes, and websocket auth. - configuration.py: SSO JWKS URL, issuer, audience. docs: AUTH_UNIFICATION.md Phase 2 status + deployment env + remaining rollout (ml_service/chat_service/MCP, then retire legacy HS256).
Make the RS256 SSO key zero-touch for deployment: the unified container's start.sh generates a persistent key at /app/secrets/um_jwt_private.pem on first boot (only if no key is configured), so the JWKS kid stays stable across the 4 usermanagement gunicorn workers and across redeploys. An explicit USERMANAGEMENT_JWT_PRIVATE_KEY_PEM/_FILE still takes precedence. - Dockerfile.unified: openssl key-gen block in start.sh; exports USERMANAGEMENT_JWT_PRIVATE_KEY_FILE (inherited by supervised processes). - docker-compose.unified.yml: mount ./secrets:/app/secrets so the key persists. - .gitignore: ignore secrets/. - env.template: document that the key is auto-provisioned; override is optional. - AUTH_UNIFICATION.md: update deploy notes.
genpkey already emits PKCS#8; the -pkcs8 flag is invalid and made key generation fail, so the container silently fell back to the /tmp ephemeral key (still shared across workers, but not on the persistent ./secrets volume). Drop -pkcs8 so the key lands at /app/secrets/um_jwt_private.pem and survives redeploys with a stable JWKS kid. Same fix in the env.template hint.
…oped) Extend single-issuer SSO verification beyond query_service so per-audience tokens work across services. Additive — legacy HS256 tokens still validate. usermanagement (now accepts its own SSO tokens): - verify_token() tries an RS256 access token minted for aud=usermanagement (verified with our OWN public key — we are the issuer, no network) before the legacy HS256 v2 token. Flows through get_current_user / require_admin / scopes / ban-check unchanged. - tokens_rs256: add verify_access_token(token, audience) + user_id claim in access tokens; exchange now stamps jwt_user_id. - add "usermanagement" to the exchangeable audiences (config + env.template). ml_service: - new core/jwks.py (httpx, sync) verifies RS256 via the issuer's JWKS and requires aud=ml_service. - decode_token_any() tries RS256 then legacy HS256; wired into get_current_user, verify_scopes/require_scopes, decode_jwt (covers SSE), and the websocket path. - SSO config (JWKS URL, issuer, audience) in configuration.py. Verified live (hot-swap): usermanagement-aud and ml-aud tokens accepted (200); a query_service-aud token is rejected at each (401, containment holds); legacy HS256 tokens still work. chat_service deferred (not in use).
- query_service/README.md: Auth section now documents dual verification (RS256/JWKS SSO with aud=query_service + legacy HS256), and that /register provisions a canonical profile + default role. - usermanagement_service/README.md: document the SSO endpoints (/.well-known/jwks.json, /api/auth/login, /api/auth/exchange), auto-provisioned signing key, and that it accepts aud=usermanagement SSO tokens on its routes. - top-level readme.md / README.md: describe usermanagement as the identity + SSO issuer and add an Authentication section pointing to AUTH_UNIFICATION.md.
… a group)
Previously per-space access rules could only *restrict*, and ingest required
owner/editor membership — so there was no way to let a whole group ingest into a
team space without adding each user individually. Now a write access rule GRANTS
write:
- spaces.can_write_space(space, email): write allowed if global Admin, owner/
editor membership, OR a matching write access rule (global_role / member /
space_role). Returns a reason for clear 403s.
- insert.py: both ingest endpoints use can_write_space instead of the old
membership-only authorize() + restrict-only space_action_permitted() combo
(drops the now-unused authorize import). The INGEST capability (write-capable
role) is still required separately, so a read-only group can't ingest.
So an admin/space-manager can add {action=write, subject_type=global_role,
subject_value="Lab Member"} and every Lab Member can ingest into that space; remove
the rule to revoke.
Docs: query_service/README.md gains a "Capabilities & roles (RBAC)" section
(capability meanings, role→capability mapping, delegation, SuperAdmin vs Admin,
and giving a group ingest access to a team space).
Verified live: rule present → Lab Member ingest 200, non-group 403; rule removed
→ 403.
Adds role/group-level capability grants so an admin can give a custom group (e.g. "uk_collaborator") a global KG capability without per-user grants — the missing piece next to per-user grants and per-space access rules. - new role_capability_grants table (role, capability), created at startup. - rbac: role_granted_capabilities(roles); capabilities(email) now = role-derived caps ∪ role/group grants ∪ per-user grants. grant/revoke/list_role_capability. - spaces admin router: GET /admin/capabilities/available (catalog + which are delegatable), GET /admin/capabilities/role, POST grant-role / revoke-role. Admin+SuperAdmin only; only GRANTABLE_CAPS delegatable (grant/sparql_admin stay admin-intrinsic — no escalation). Verified live: uk_collaborator [read_private] -> grant ingest -> [ingest, read_private]; sparql_admin refused (400).
…ban only)
Enforce SuperAdmin > Admin and make ban (not delete) the removal mechanism.
- Only a SuperAdmin may assign/remove the Admin (or SuperAdmin) role and ban an
Admin account; regular Admins manage non-admin users only. SuperAdmin role
stays fully protected (no strip/ban). Added _is_superadmin/_require_superadmin
(honors the bootstrap-superadmin allowlist).
- User deletion is DISABLED (DELETE /users/{id} -> 405): we don't delete
accounts — ban instead (reversible, preserves provenance/audit history).
- Fix a latent MissingGreenlet in ban_user: build the response from locals
captured before commit instead of touching expired ORM attributes.
Verified live: Admin assign/remove Admin + ban Admin -> 403; SuperAdmin -> 200;
delete -> 405.
…, no-delete (ban) policy
Lets the MCP/skill complete an OAuth login without the web UI. The browser
sign-in (user consent) is unavoidable, but the result is picked up out-of-band
via a short paste-code instead of a frontend redirect.
- Web_oauth_state gains a `mode` ('web'|'cli'); new Web_oauth_cli_result table
(code -> SSO refresh token, single-use, short-lived) + repo.
- POST /api/auth/cli/start {provider} -> authorize URL (state marked cli).
- OAuth callback branches on mode: for cli it provisions as usual, mints an SSO
refresh token, stores it behind a short code, and renders a minimal
"copy this code" page (no SPA).
- POST /api/auth/cli/exchange {code} -> refresh token (reads it inside the
session to avoid MissingGreenlet), single-use.
Verified: cli/start (globus) 200 with authorize URL; exchange of a seeded code
returns the token and reuse is refused (400); success page renders the code.
The real Globus click-through is verified on deploy.
The one-time login paste-code was 8 chars (~39 bits). Raise it to 20 chars over the 30-symbol unambiguous alphabet (~98 bits), grouped in 4s, clamped to fit the String(32) code column, env-configurable via USERMANAGEMENT_CLI_CODE_LEN. It stays short-lived (~10 min) + single-use; the extra entropy is defense-in-depth against brute force in the window.
…d dummy example) Show that USERMANAGEMENT_BOOTSTRAP_SUPERADMIN_EMAILS supports multiple emails and note the runtime path (an existing SuperAdmin can grant the role). Use dummy placeholder emails in the template.
…session The OAuth callback handed the UI a 30-min HS256 token that NextAuth stores and never refreshes, so after 30 min /api/users/me (and the profile/activity routes that reuse the session token) returned 401/404. Make create_access_token_v2 take an expires_minutes override and have the OAuth callback mint the web-session token with USERMANAGEMENT_WEB_SESSION_TTL_MIN (default 720 = 12h). Password-login and per-service tokens keep their short defaults.
…enew The OAuth callback now also mints a longer-lived SSO refresh token (aud=brainkb-auth, USERMANAGEMENT_WEB_REFRESH_TTL_MIN, default 7d) and returns it to the UI as ?refresh=. The UI exchanges it at /api/auth/exchange (audience=usermanagement) to renew its short access token without re-login. create_refresh_token gains an expires_minutes override. Verified: refresh -> exchange -> /api/users/me 200.
Auth unification
Add longer web-session and issue a web refresh token
/api/auth/exchange looked the credential row up with the active-only
jwt_user_repo.get_by_email, so it 401'd "Account inactive" for every
Globus/ORCID/GitHub user. OAuth onboarding creates that row as a SHELL with
is_active=False on purpose — an OAuth user has no usable password, and the shell
exists only to supply a stable user_id claim (see the get_by_email_any_status
docstring, which already says OAuth flows must use it). The result: an OAuth user
could log in and mint a refresh token that nothing would ever accept.
That broke two flows on the same line. The MCP/skill paste-code login
(cli/start -> cli/exchange -> exchange) dead-ended at the last hop, so
brainkb_whoami read authenticated:false right after a successful login and a PAT
could never be minted — minting needs a session token, and the only way to one
from a refresh token is this endpoint. The UI's silent renew goes through the
same call (oauth.py mints web_refresh precisely "exchanged by the UI at
/api/auth/exchange"), so web sessions died at TTL instead of renewing.
Not a bare swap to get_by_email_any_status, because is_active is overloaded:
POST /api/admin/users/deactivate flips the same column, so dropping the check
would make deactivation a no-op here. The refresh token records how it was
issued — auth_source="password" from /auth/login, the provider name from OAuth —
so the check now applies only to password credentials, where is_active really is
the deactivation switch. OAuth accounts are removed by banning, which the
is_banned -> 403 check below already enforces.
Also distinguishes a missing row ("Unknown account") from a switched-off one
("Account inactive"), which were previously the same message.
usermanagement: let OAuth accounts exchange a refresh token
get_current_user verified the token, then looked the caller up with
get_user(email), whose SQL filters `AND is_active = True`. An OAuth caller's
credential row is the SHELL usermanagement provisions with is_active=False — they
have no usable password, the row exists only to carry a stable user_id — so the
lookup found nothing and raised 401 "Could not validate credentials" for every
Globus/ORCID/GitHub user, with a valid, correctly-audienced, correctly-signed
token. Same root cause as the /api/auth/exchange fix in the previous commit, one
service further down.
Worse than a plain error on the optional path: get_current_user_optional swallows
the failure and returns None, so a signed-in OAuth user read those endpoints as
ANONYMOUS. list_spaces answered {"spaces": []} — indistinguishable from "you own
no spaces", and it was reported to a user as exactly that, while authenticated
endpoints 401'd alongside. That combination reads like a token/issuer mismatch and
sent debugging after the wrong thing entirely.
get_user takes include_inactive (default False, so nothing else changes) and the
three token-verification call sites pass it based on the token's own auth_source
claim: relaxed for OAuth, strict for password credentials where is_active is the
switch POST /api/admin/users/deactivate flips. authenticate_user, the actual
password path, is untouched and still refuses inactive rows. Banned accounts are
unaffected — that is enforced separately on the profile.
Readme update
…cope table
A Globus SuperAdmin got 403 "Insufficient scopes" from every query_service admin
route, including /api/admin/capabilities, while usermanagement's own admin routes
worked. Setting USERMANAGEMENT_BOOTSTRAP_SUPERADMIN_EMAILS did not help, and it was
never going to: promote_bootstrap_superadmins assigns the Admin and SuperAdmin ROLES
to a UserProfile and writes nothing else.
Four paths mint tokens, and until now they disagreed about where scopes come from:
/api/auth/session-exchange scopes from roles ("RBAC is authoritative")
/api/pat/exchange scopes from roles
/api/auth/login scopes from Web_jwtuser_scopes
/api/auth/exchange scopes from Web_jwtuser_scopes <- the MCP path
Web_jwtuser_scopes is the legacy Django table, populated only for accounts created
through the old admin. An OAuth account has no rows in it: its Web_jwtuser row is
the shell created to supply a stable user_id claim. So the refresh-token exchange
minted `roles: ["Admin", "SuperAdmin"]` alongside `scopes: ["read"]`.
query_service gates its admin routes on the scope claim — require_scopes(["admin"])
runs as a dependency, before the rbac.is_admin() check inside the handler that would
have passed — and it has no bootstrap-email allowlist of its own
(config.bootstrap_superadmin_emails appears nowhere in query_service). That
combination is why the symptom looked like a missing audience: the same identity
could list users through usermanagement, which honours the allowlist, and could not
read capabilities through query_service, which trusts the token.
Both refresh-token paths now union the stored scopes with the ones the user's roles
imply. Union rather than replacement, because a legacy account may hold an
explicitly granted scope that no role implies and dropping it would be a silent
downgrade. This also removes the need for a PAT as a workaround — the PAT exchange
only worked because it already derived scopes from roles.
Verified on the pure functions: a Globus SuperAdmin with no scope rows now yields
["admin", "read", "write"], a Curator ["read", "write"], a user with no roles
["read"], and a legacy account keeps a scope no role implies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
usermanagement: derive token scopes from roles, not just the legacy scope table
Nothing could read a review its author marked Public. Every synth-scholar route
depends on get_current_user, GET /reviews is owner-scoped, and _session_or_404
404s for non-owners — so the public web pages at /knowledge-base/synth-scholar
failed for anonymous visitors and showed signed-in ones their own reviews. The
is_public flag was write-only.
Add /api/synth-scholar/public/reviews{,/{id},/{id}/log,/{id}/export}. Three rules
hold across all four:
* no get_current_user, so an anonymous browser can read them;
* every lookup goes through store.list_public / get_public, which require
is_public AND completed — the filter is in SQL, not the caller, so other
people's drafts are never shipped to a browser to be filtered there;
* an unpublished id answers 404, not 403, so the status code does not confirm
it exists.
list_public skips the runtime-state merge: a completed review has none, and
consulting it would leak progress for a row being re-run.
Export shares one _export_session helper with the authenticated route (the two
were identical past the access check) and one _EXPORT_FORMAT_PATTERN constant, so
a format added for signed-in users cannot silently 400 on a public review.
No behaviour change to the authenticated routes. Nothing new is exposed: the
detail payload already excludes openrouter_api_key from run_request, and these
routes serve only what an author explicitly published.
add an unauthenticated read surface for published reviews
ml_service dies at boot on EC2 — gunicorn exit 3 (WORKER_BOOT_ERROR) after 2.7s,
four times, then supervisor gives up and 8007 stays dead while the container
reports healthy on 8000/8004/8010.
Exit 3 with no bind means the workers raised before the app object existed, and
`from structsense import kickoff` is the only unguarded heavy import on that path
(core/main.py already guards the SynthScholar one for exactly this reason). It was
imported in two places:
* core/routers/structsense.py — never used. This module reaches structsense only
through core.shared.run_kickoff_with_config. Removed.
* core/shared.py — the real one. Now guarded, with kickoff = None on failure and
a 503 raised per-request from run_kickoff_with_config.
So a broken structsense install now costs the extraction endpoints only. Everything
that never touches kickoff keeps serving: GET /api/ner and the saved-annotation
surface behind /knowledge-base/ner, and the whole /api/synth-scholar tree.
Why the install can be broken while the image builds green (Dockerfile.unified):
the structsense step falls back to `--no-deps` when the legacy resolver fails, so
crewai/litellm never arrive — and ml_service's requirements.txt lists neither, so
nothing fills the gap. The RUN still exits 0. Added an import check to that step so
this fails the build. (Not a precedence bug: `A || B && C` already groups as
`(A || B) && C`; the parentheses added are only for the reader.)
Diagnosis note for next time: gunicorn's traceback goes to
/var/log/supervisor/ml_service.err.log, not the container's stdout, so
`docker logs brainkb-unified` shows only supervisord's exit codes.
…tion The traceback names the cause exactly: structsense -> crewai -> litellm -> openai openai/_vendor/httpx_aiohttp/transport.py:17 aiohttp.SocketTimeoutError AttributeError: module aiohttp has no attribute SocketTimeoutError requirements.txt pinned aiohttp==3.8.6, and Dockerfile.unified installs that file AFTER structsense — so the pin downgraded the aiohttp that crewai/litellm/openai had just pulled in. Recent openai vendors httpx_aiohttp, which needs aiohttp.SocketTimeoutError (added in 3.10). Import raised, all 6 workers died before the app object existed, supervisor gave up, 8007 stayed dead while the container looked healthy on 8000/8004/8010. Changed to `aiohttp>=3.10,<4` — a floor, not a pin, so pip can reconcile with whatever openai/litellm require. chat_service and usermanagement_service keep 3.9.1; neither imports openai. Also: both import guards caught the wrong exception. This failure is an AttributeError, so `except ImportError` does not catch it — the guard added in c970966 would not have prevented this outage, and main.py's SynthScholar guard had the same hole since it was written. Both now catch Exception: a four-package import chain can fail in any number of ways, and each must cost one feature rather than the process. Verified the widening matters rather than assuming it: `except ImportError` lets an AttributeError through, `except Exception` does not.
…override Each service kept its own hardcoded list and they had drifted. chat_service was actually broken: it was missing https://brainkb.org and https://www.brainkb.org entirely, so the production UI was blocked from it on the main domain while beta and sandbox worked. It also carried "http://127.0.0.1:300" — a typo for :3000. Dropped the schemeless "localhost:3000" entries from usermanagement and chat: the browser's Origin header always carries a scheme, so they could never match. Dropped ml_service's "http://localhost" and its two :3001 entries — nothing in the repo serves the UI on 3001, and CORS_ALLOWED_ORIGINS covers anyone who does. All four now share one 6-entry default, plus CORS_ALLOWED_ORIGINS (comma-separated) so a new domain does not mean editing four files. Documented in env.template. Not a fix for the reported failure. The synth-scholar public route reporting "No Access-Control-Allow-Origin" from brainkb.org is ml_service being down: the ALB answers 502 (server: awselb/2.0) and its error page has no CORS headers, so an outage is indistinguishable from a CORS misconfiguration in the browser console. https://brainkb.org was already in ml_service's list. Noted in both the code and env.template, since this will mislead again.
Feat/synth scholar public reads
…ards, CORS Brings the three commits added after PR #81 was merged. The aiohttp one is the reason ml_service could not boot (gunicorn exit 3), so deploying this branch without it would reintroduce that crash on the next --no-cache rebuild: c970966 survive an unimportable structsense; fail the build instead be4eac8 unpin aiohttp 3.8.6, widen the import guards to Exception 29c1fb0 make the four CORS origin lists identical, add CORS_ALLOWED_ORIGINS
…BrainKB into improve-ingestion-query-service
This is why ml_service worked in local Docker and died on the unified/EC2 build. The two files installed the same requirements with different resolvers: ml_service/Dockerfile : pip install -r requirements.txt (works) Dockerfile.unified : pip install --use-deprecated=legacy-resolver -r (broken) The legacy resolver does not backtrack and does not check consistency, so a later requirement silently downgrades an earlier one. `aiohttp>=3.10` in requirements.txt was therefore not enough on the unified build: structsense==0.0.4 drags an old crewai/litellm that tolerates aiohttp 3.8.x, that won, and openai's vendored httpx_aiohttp failed at import with AttributeError: module aiohttp has no attribute SocketTimeoutError That is why the deployed build had the new code (confirmed: the ml_service CORS list no longer allows http://localhost:3001) yet still hit the original error. The legacy resolver is kept for the structsense install alone, which needs it. requirements.txt now resolves the same way it does locally. The explicit aiohttp upgrade after it is a safety net for this one regression, not the fix — nothing in the tree wants aiohttp <3.10 (litellm requires >=3.14.2). Also adds ml_service/scripts/verify_imports.py, run at the end of the build. It checks aiohttp.SocketTimeoutError and imports both structsense and synthscholar, failing the build with the real traceback if either is broken. Both are guarded at runtime so one bad dependency cannot kill the process — correct, but it means a broken install is invisible: the container starts, /api/health returns 200, and the routers are simply never mounted. That is how the synthscholar breakage stayed hidden until /api/synth-scholar/health started 404-ing.
The build failed at the ml_service pip step: with the modern resolver, this tree
cannot be satisfied. structsense==0.0.4 drags an old crewai/litellm that holds
aiohttp below 3.10, and openai's vendored httpx_aiohttp references
aiohttp.SocketTimeoutError at import time. The legacy resolver hid that by
installing an inconsistent set; the modern one refuses.
That conflict was never confined to structsense. synthscholar imports openai too, so
the same old aiohttp broke it, which is why every /api/synth-scholar route 404'd
including the pre-existing ones — the router was never mounted. One dead pin cost
two features.
So structsense is no longer installed, and its three extraction WebSocket endpoints
are no longer registered:
/ws/ner/{client_id}
/ws/extract-resources/{client_id}
/ws/pdf2reproschema/{client_id}
Registration is conditional on STRUCTSENSE_AVAILABLE (new, exported from
core/shared.py) via a small _extraction_ws decorator, so the paths simply do not
exist rather than accepting a WebSocket upgrade and failing after the client has
uploaded a PDF. GET /api/ws-info now reports extraction_available: false and names
what is disabled, instead of advertising paths that 404.
Everything that only touches stored data keeps working, which is the point:
GET /ner, GET /structured-resource, both save endpoints, GET /job/{task_id}.
/knowledge-base/ner depends on GET /ner.
Nothing is deleted. The install is commented in place with what re-enabling needs:
a structsense release whose crewai/litellm accept aiohttp>=3.10, or the two stacks
will keep fighting over it. verify_imports.py checks synthscholar only for now, with
a note not to re-add structsense without restoring the install.
/knowledge-base/ner and /knowledge-base/resources are public pages, but they read
GET /api/ner and GET /api/structured-resource, which require a credential. An
anonymous visitor got 403 {"detail":"Not authenticated"} — get_current_user rejects
for having no credential at all, before require_scopes(["read"]) is consulted. So
those pages showed an error to everyone not signed in.
Adds GET /api/public/ner and GET /api/public/structured-resource. They serve the
same documents as the authenticated routes, which is safe here rather than by
accident: saved annotations carry documentName, processedAt, sourceType,
sourceContent and the extracted entities, and the write path
(upsert_ner_annotations / upsert_structured_resources) records no submitter
identity — so there is nothing per-user to leak and no visibility flag to honour.
A comment marks these as the place to filter if per-record visibility is ever added.
`limit` is capped at 200 (the authenticated routes document 1000 and enforce
nothing). These are open to the internet and the payload includes sourceContent,
which can be a whole paper.
Same shape as the synth-scholar public routes added in 8fc6b5c: no
get_current_user, and read-only.
Build failed with: error: resolution-too-deep × Dependency resolution exceeded maximum depth Not a structsense problem — structsense was already removed in 21d543d, and the failing step does not mention it. synthscholar drags pydantic-ai -> openai and sentence-transformers -> torch, and solved together with the exact `==` pins in requirements.txt on an empty base image the graph exceeds what pip will search. The hundreds of "psycopg 3.1.18 does not provide the extra 'async'" lines are noise, not the cause: no psycopg release has ever had an `async` extra (checked 3.1.18, 3.2.x, 3.3.4), pip ignores unknown extras, and the repetition just shows how much backtracking was happening. So synthscholar moves out of requirements.txt into its own pip step. Two smaller solves succeed where one large one does not, and requirements.txt goes first so its pins are established before synthscholar resolves against them. This also explains local-vs-EC2, which I had wrong earlier: the standalone ml_service/Dockerfile does it in one solve because tiangolo/uvicorn-gunicorn-fastapi preinstalls fastapi/uvicorn/pydantic, constraining the search. python:3.11-slim starts empty, so pip explores everything. The resolver change in b622f5f surfaced this rather than causing it — the legacy resolver was silently accepting an inconsistent set. Separately, fixes the layer ordering that made every deploy slow. Each service did `COPY <service>/` BEFORE `pip install`, so editing any .py file invalidated that service's pip layer and reinstalled everything — including torch, ~8 minutes on ml_service alone. Now each copies only requirements.txt, installs, then copies source, so pip layers are keyed on requirements.txt alone. verify_imports.py moves to its own RUN after the source copy, since it needs the code. `--no-cache` is no longer needed to pick up a dependency change; a requirements.txt edit invalidates the layer on its own.
ml_service died on boot with
File "/app/ml_service/core/shared.py", line 31
from bs4 import BeautifulSoup
ModuleNotFoundError: No module named 'bs4'
bs4 was never declared — it arrived as a transitive dependency of structsense.
Dropping structsense in 21d543d took it away, and core/shared.py imports it directly.
Rather than fix that one and wait for the next crash, parsed every import in core/ and
diffed against requirements.txt. Four were undeclared and genuinely needed:
beautifulsoup4 core/shared.py
python-dotenv core/configuration.py
rdflib core/shared.py (currently survives via synthscholar)
PyYAML core/shared.py
Deliberately not added: pydantic and starlette (guaranteed by the fastapi pin, and
unbounded entries risk resolution-too-deep again), pytest (test-only), structsense
(guarded, intentionally absent), synthscholar (its own pip step).
The deeper problem was the build gate. verify_imports.py checked the two optional AI
stacks but never the app itself, so a missing dependency in ml_service's OWN code
sailed through — bs4 broke via core/shared.py, a module the script never touched. It
now imports core.main, the same module gunicorn loads, and treats only
ModuleNotFoundError as fatal: missing env or database at build time is expected and
says nothing about the image, since importing core.main mounts routers but opens no
connections.
Needed sys.path fixing too — `python scripts/verify_imports.py` puts scripts/ on
sys.path, not the service root, so `import core.main` would have failed misleadingly
whatever was installed.
…ceback
ml_service is up and NER/structured-resource work, but /api/synth-scholar/* is still
absent from the OpenAPI spec. So `import synthscholar` succeeds (the build's check
passed) while `core.synth_scholar.routes` does not — and core/main.py mounts that
router inside a try/except, so the failure is swallowed, the service starts, and
/api/health returns 200 with an entire feature missing.
Two gaps, both closed here:
* The gate checked the PACKAGE, not the router module. Importing synthscholar tells
you nothing about whether the router that uses it can import. Added
core.synth_scholar.routes as a fatal check — synthscholar is deliberately
installed, so if it is present and the router still cannot import, that is a
defect rather than a configuration choice.
* Failures printed only str(exc). Diagnosing one meant a docker exec into a running
container to reproduce the import by hand, because the frame that fails is the
only thing identifying the bad dependency. Now prints the full traceback.
Note this makes the next build FAIL until the router imports. That is intended: a
green build shipping a silently disabled feature is what produced the last several
hours of 404s.
Also a correction to my own analysis, recorded so nobody repeats it: I reported
ROB_DOMAINS as missing from synthscholar 0.0.11's agents.py. It is present, at line
327, as an annotated assignment (`ROB_DOMAINS: dict[str, list[str]] = {...}`). The
checker only walked ast.Assign, not ast.AnnAssign. Every name core/synth_scholar/
imports does exist in 0.0.11 — verified against the wheel.
Root cause of /api/synth-scholar/* being absent, found at last. The four services share ONE Python environment: docker-compose.unified.yml defines a single brainkb-unified container and supervisor runs api_tokenmanager, query_service, ml_service, usermanagement_service and oxigraph as programs inside it. So the last pip install wins, and the install order was: line 129 ml_service aiohttp>=3.10 installed line 137 verify_imports.py PASSES (aiohttp is 3.10+ at this moment) line 142 usermanagement aiohttp==3.9.1 DOWNGRADES it At runtime ml_service then sees 3.9.1, and openai's vendored httpx_aiohttp fails on aiohttp.SocketTimeoutError (added in 3.10 — verified against the 3.9.5/3.10.11/ 3.11.18/3.12.15/3.14.3 wheels). core/main.py catches it, the router never mounts, and the service reports healthy with a whole feature missing. That also explains why the build kept passing: verification ran inside the ml_service block, i.e. before the downgrade. It now runs LAST, after every pip step, so it checks the environment the container actually runs. The aiohttp upgrade moved there too, as a backstop rather than the fix. Audited every requirements.txt for the same class of problem — 12 packages are required by more than one service with differing specs. Most are last-wins on patch versions and harmless; two were real violations of another service's constraint: aiohttp usermanagement/chat ==3.9.1 vs ml_service >=3.10 (this outage) sqlalchemy usermanagement ==2.0.23 vs ml_service >=2.0.30 (latent) Both relaxed, with a note in each file explaining that an exact pin here overrides what another service needs. Not touched: fastapi, gunicorn, rich, pydantic-settings and the rest, where the differing specs are all mutually satisfiable.
…_service instead The push in oxigraph_push.py writes review RDF straight into Oxigraph over GSP. It is the fastest way to get triples into the store and the worst way to get them into BrainKB: no ingest job, no PROV-O provenance, no search-index row, and the named graph belongs to no space, so nothing but an Admin SPARQL query can see it. Reviews now reach BrainKB the way all other RDF does — the TTL export goes through query_service, which records a job, a delta and a real user as the agent. Leaving both paths on gives every review two graphs rather than one. Review ids are unique, so reviews never collide with each other; the problem is that the two writers disagree about the IRI by a single character. _named_graph_for returns prefix + review_id with no trailing slash, while the ingest path registers and writes .../<review_id>/ — query_service normalises the registry lookup (check_named_graph_exists appends a slash) but writes the IRI verbatim (create_job(graph=named_graph_iri)). So the store ends up holding a governed graph and an unregistered shadow copy, with search and provenance describing only one of them. That is worse than either outcome alone. backfill_oxigraph_push.py now refuses to run rather than reporting success against a no-op: with the flag off, _make_config returns None for every review and the script would have logged a clean backfill while nothing left the process. Worst possible outcome for a one-shot repair tool. The env-var table and module docstrings say which path is current, so the next person reading either file finds the ingest route instead of re-enabling this one.
Route Review RDF Through BrainKB Ingest and Disable Direct Oxigraph Push
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.
This PR makes several improvements to existing implementations, such as introducing private, public graph/space. Some major changes are: