Migrate GraphQL layer from graphene to strawberry (query shapes pinned by golden SDL)

[JSv4]

Graphene → Strawberry migration

Status: migration in progress — resolver porting fan-out underway. Will be updated on this PR as it completes; do not merge yet.

What this does

Replaces the graphene / graphene-django GraphQL layer (schema types, resolvers, mutations) and the graphene-level auth middlewares with strawberry-graphql, with a machine-verified guarantee of zero query-shape changes:

• Golden SDL contract: the full graphene SDL was captured at migration time as config/graphql/schema.graphql (10.6k lines). opencontractserver/tests/test_schema_parity.py structurally compares the served strawberry schema against it — every type, field, argument name/type, nullability wrapper, interface, enum member, and printed default must match exactly. The parity test is green.
• Shared runtime (config/graphql/core/): relay global IDs + Node interface in graphene wire format (base64("TypeName:pk")), countable/PDF-page-aware connection factories, a faithful port of graphene-django's connection resolution (arrayconnection cursors, RELAY_CONNECTION_MAX_LIMIT=100, 1-based offset→after conversion), django-filter FilterSet argument mapping incl. GlobalIDFilter decoding, GenericScalar/JSONString/BigInt scalars, permission-annotation resolvers, DRF-serializer mutation bases, auth decorators with graphql_jwt-compatible error messages.
• Auth middlewares replaced: graphql_jwt.middleware.JSONWebTokenMiddleware + the API-key graphene middleware are gone. Per-request authentication happens once in config/graphql/views.py::GraphQLView.get_context via the standard AUTHENTICATION_BACKENDS chain (JWT / Auth0 / API-key / session). tokenAuth / verifyToken / refreshToken are strawberry-native ports preserving long-running refresh-token laziness.
• Security hardening preserved: DepthLimitValidationRule + DisableIntrospection attach via strawberry's AddValidationRules (which appends to the full spec rule set — the graphene replace-the-rules trap is structurally impossible). The GCS file-URL pre-warm middleware became a strawberry SchemaExtension.
• Tests keep their substantive cases: graphene.test.Client → drop-in config/graphql/testing.py::Client (same result dict shape); GraphQLTestCase ported for endpoint-level tests; schema.execute(...) → schema.execute_sync(...).
• graphene-django removed from requirements/INSTALLED_APPS; strawberry-graphql==0.320.3 added; django-graphql-jwt retained only as a JWT signing/backend utility (its graphene middleware/mutations are no longer used).

How the port was done

The strawberry schema skeleton (394 types, all field/argument shapes) was generated by introspecting the live graphene schema, guaranteeing shape parity by construction. Custom resolver bodies (465 stubs) are being ported verbatim module-by-module — roots are Django model instances in both frameworks, so bodies transfer nearly unchanged, keeping the service-layer permission patterns intact (the opencontracts.E001 architecture check stays green).

Verification

• test_schema_parity — green (schema shape vs golden SDL)
• test_og_metadata_queries — 18/18 green (first fully ported module)
• test_token_expiration — 10/10 green (JWT auth path)
• Full backend suite — pending completion of the porting fan-out

🤖 Generated with Claude Code

https://claude.ai/code/session_01SZxDJP7pKPSryHv7d5tiWm

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[claude]

Review: Graphene → Strawberry GraphQL migration

Reviewed via the local merge commit (dc214be...aec1b89), which let me diff the full 194-file / +35548/-25213 changeset directly rather than through gh pr diff (which 406s on this PR — it exceeds GitHub's 20k-line diff cap).

Overview
This replaces the graphene/graphene-django GraphQL layer with strawberry-graphql end-to-end: schema types, resolvers, mutations, auth middleware, and the permission-annotation mixin, backed by a golden-SDL parity test (config/graphql/schema.graphql + test_schema_parity.py) intended to guarantee zero query-shape drift. The PR is explicitly marked WIP ("do not merge yet"), and the review below is consistent with that — it surfaces exactly the kind of gaps you'd expect from an in-flight porting fan-out.

Strengths

• Golden SDL contract is a genuinely good approach for a mechanical framework swap — pinning the schema shape structurally (types/fields/args/nullability/enums/defaults) is a much stronger guarantee than eyeballing a diff of this size.
• config/graphql/views.py::authenticate_request is a clean, well-reasoned replacement for the old JWT + API-key graphene middlewares — single auth pass via AUTHENTICATION_BACKENDS, session precedence preserved, JWT errors re-surfaced as GraphQL-shaped errors in dispatch().
• config/graphql/core/permissions.py is a careful, faithful port of AnnotatePermissionsForReadMixin. Worth calling out: resolve_object_shared_with explicitly documents and preserves a pre-existing graphene-era bug (the this_model_permission_id_map lookup in that path was always empty, so shared-with entries silently raised KeyError and got swallowed) rather than silently fixing it — the right call for a parity-focused migration, but it should become a tracked follow-up bug fix once this lands (the one test exercising this path is currently broken — see below).
• CLAUDE.md and requirements/base.txt comments were updated in the same PR to reflect the new architecture, per the project's own docs convention.
• changelog.d/graphene-strawberry-migration.changed.md fragment added per the changelog convention.

Confirmed test breakage (should block merge as-is)
Grepping the merged tree for imports of modules this PR deletes turned up real, currently-broken tests:

• opencontractserver/tests/test_mention_permissions.py — ~35 local imports of from config.graphql.queries import Query across CorpusMentionPermissionTestCase, DocumentMentionPermissionTestCase, MentionIDORProtectionTestCase, CorpusScopedMentionSearchTestCase, AgentMentionCorpusScopingTestCase. config/graphql/queries.py is fully deleted by this PR. This file is untouched by the diff (0 changes) and is IDOR/permission-relevant.
• opencontractserver/tests/test_document_path_migration.py (lines 479/491/508/526) — TestDocumentPathTypeCaching's 4 tests import from config.graphql.graphene_types import DocumentPathType; graphene_types.py is fully deleted. File is untouched by the diff.
• opencontractserver/tests/test_user_privacy.py:483 — _resolve_shared_with() (used by test_shared_with_returns_slug_only and siblings) does from config.graphql.graphene_types import CorpusType and calls CorpusType.resolve_object_shared_with(...) as a classmethod. That symbol is gone; the equivalent logic moved to the free function config.graphql.core.permissions.resolve_object_shared_with(instance, info). Notably, this is the only regression test for the deliberately-preserved KeyError-swallowing quirk mentioned above, so that behavior is currently untested. (The rest of this file's graphene.test.Client imports were correctly updated to config.graphql.testing.Client; this one call site was missed.)
• opencontractserver/tests/test_security_hardening.py:1902-1986 — TestDRFMutationValidationError and TestIOSettingsRequiredFieldsGuard import DRFMutation/DRFDeletion/_require_io_setting from config.graphql.base, which is fully deleted (346 lines removed, no visible core/ equivalent test). This is a security-hardening test file by name and purpose — worth prioritizing.
• scripts/test-django-ratelimit.py:99-100 — manual script, lower priority, but from config.graphql.mutations import CreateLabelset / from config.graphql.queries import Query will also break.

All five are local (function-scope) imports, so pytest collects the files fine and only fails at the specific test methods — consistent with the PR's own "Full backend suite — pending completion of the porting fan-out" caveat. Recommend a final repo-wide sweep for references to config.graphql.base, graphene_types, queries, mutations, custom_connections, jwt_overrides, and permissioning.permission_annotator.mixins before dropping WIP status — a plain import grep won't catch call-site signature changes like the CorpusType classmethod → free-function move above.

Other observations

• config/graphql/_port_manifest.json (2327 lines, all 465 stubs) hardcodes absolute paths from the migration tool's sandbox, e.g. "ref": "/home/user/oc-graphene-ref/config/graphql/research_types.py:52". That path won't resolve for anyone else and reads as leftover scaffolding from the automated porting run rather than durable project documentation. Consider dropping it before merge, or repointing ref at repo-relative paths if it's meant to stay as an audit trail.
• The schema-parity test guarantees shape but says nothing about resolver behavior (pagination edge cases, permission-annotation caching, GenericScalar/JSONString/BigInt serialization could all differ subtly while still matching the SDL). Worth confirming the full backend suite passes, not just the three modules named green in the PR description, given ~465 hand/tool-ported resolver bodies across ~90 files.
• Given the WIP status and that this is fundamentally an atomic framework swap (hard to land incrementally since the golden-SDL test needs the whole schema to match at once), the overall size is understandable — flagging only that a true line-by-line review of every resolver body wasn't practical here and should lean on the full test suite once green.

Suggested next steps before removing WIP status

1. Fix the broken test files above (plus the manual script).
2. Run the full backend suite (pytest -n auto --dist loadscope) and confirm green, not just the 3 modules named in the PR description.
3. Decide on _port_manifest.json's fate (keep with cleaned-up refs, or drop).

[claude]

Review

This is a huge, carefully-engineered migration (60k+ lines) with a genuinely strong verification story — the golden-SDL parity test, the deliberate preservation of graphene-django's get_node/get_queryset asymmetry, and the honest documentation of preserved quirks (e.g. the resolve_object_shared_with KeyError-swallowing behavior) all show real care. I reviewed it with several parallel deep-dives into the core runtime (config/graphql/core/), auth/JWT/schema wiring, and a sample of the largest ported resolver files, cross-checking findings against git diff dc214be..HEAD myself. Per the PR description this is explicitly WIP/"do not merge yet," so treat the following as the checklist for that point, not a blocker on today's state.

🔴 Critical — IDOR regression: 7 singular "by ID" query fields lost permission filtering

get_node_from_global_id (config/graphql/core/relay.py:239-269) resolves a type via its registered get_node hook, or — if none is registered — falls back to entry.model._default_manager.get_queryset().get(pk=_pk) (after applying get_queryset if one exists). That fallback is fully unfiltered when a type has neither hook.

Seven model-backed types are registered with no get_queryset/get_node at all, and each has a live top-level singular query field that forwards straight to this unfiltered path:

• relationship → config/graphql/annotation_queries.py:1374 (RelationshipType, registered annotation_types.py:2014)
• annotationLabel → annotation_queries.py:~1469 (AnnotationLabelType, annotation_types.py:1445)
• labelset → annotation_queries.py:~1556 (LabelSetType, annotation_types.py:1683)
• chatMessage → config/graphql/conversation_queries.py:432 (MessageType, conversation_types.py:1929)
• fieldset → config/graphql/extract_queries.py:174 (FieldsetType, extract_types.py:1495)
• column → extract_queries.py:241 (ColumnType, extract_types.py:1639)
• datacell → extract_queries.py:492 (DatacellType, extract_types.py:1889)

I verified three of these directly against the pre-migration graphene resolvers (git show dc214be:...):

• old resolve_relationship called BaseService.filter_visible(Relationship, info.context.user, request=info.context).get(id=django_pk) — the port drops this entirely.
• old resolve_chat_message was @login_required and called BaseService.get_or_none(ChatMessage, django_pk, info.context.user, request=info.context) — the port has neither the auth gate nor the permission check, so any unauthenticated caller can fetch arbitrary private conversation messages by forging base64("MessageType:<id>") (trivial, not a secret).
• old resolve_datacell used BaseService.get_or_none(Datacell, django_pk, info.context.user, request=info.context) — same drop, leaking extraction results across corpora/documents the caller has no access to.

The corresponding plural resolvers a few lines below each (relationships, annotationLabels, labelsets, fieldsets, columns, datacells) still correctly call BaseService.filter_visible(...) — only the singular-by-ID path was missed in the port. This is the same bug class the PR already fixed for Corpus (commit 2a1bbf1, giving CorpusType a custom permission-aware get_node) — that audit just wasn't extended to these 7 types.

Suggested fix: give each of these types a get_node/get_queryset hook mirroring the old resolver (BaseService.get_or_none/filter_visible), the same pattern already used for DocumentType/AnnotationType (get_queryset) and ExtractType/AnalysisType/ConversationType (get_node). Worth a final grep for other model-backed register_type(...) calls with no hook + a live singular field before considering this done — I didn't exhaustively check every type (e.g. CorpusReferenceType, NoteRevisionType, ModerationActionType, AssignmentType, CorpusCategoryType, CorpusActionType are also registered without hooks; I didn't verify whether each has an exposed singular resolver or whether the underlying data is sensitive enough to matter).

🟠 Medium — unhandled AuthenticationFailed crashes the request on a bad API key

config/graphql/views.py:46 (authenticate_request) calls django.contrib.auth.authenticate(request=request) for any request carrying an Authorization header, which walks into ApiKeyBackend.authenticate() (config/graphql_api_token_auth/backends.py:66-105). That backend raises rest_framework.exceptions.AuthenticationFailed for a malformed/unknown/inactive-user API key — not a graphql_jwt exception. GraphQLView.dispatch (views.py:60-73) only catches graphql_jwt.exceptions.JSONWebTokenError, so AuthenticationFailed propagates unhandled out of the view.

Under graphene this same exception was raised inside ApiKeyTokenMiddleware.resolve, i.e. inside graphql-core's per-field resolution, so the execution engine caught it and returned a normal {"errors": [...]} 200 response. Now get_context() runs before query execution begins, outside that try/except, so the same input produces an unhandled 500 (or a DEBUG traceback) instead. Failure scenario: any GraphQL request sent with Authorization: Token <bad-key> when USE_API_KEY_AUTH=True. Not an auth bypass, but a real robustness regression, and no existing test (test_security_hardening.py, test_jwt_utils.py) exercises an invalid API key through the view. Suggest catching AuthenticationFailed alongside JSONWebTokenError in dispatch().

🟡 Worth a second look (pre-existing, not introduced by this PR)

• GlobalIDFilter type confusion (config/graphql/core/filtering.py:90-102): decodes a relay global ID but discards the type-name half, so a global ID encoded for the wrong type (base64("AnnotationLabel:5")) filters by pk 5 under any filter regardless of the label. Confirmed this is a line-for-line port of graphene_django.filter.filters.GlobalIDFilter and predates this PR — not a regression, and the decoded value only ever reaches the ORM as a parameterized filter kwarg (no injection), but it's an inherited footgun on top of whatever visibility filtering wraps the queryset. Worth a follow-up issue.
• resolve_object_shared_with (config/graphql/core/permissions.py:213-276): the docstring itself documents that this_model_permission_id_map is looked up empty, so any object with an actual per-user share raises KeyError inside the loop — caught only by except AttributeError. This is preserved faithfully from the graphene original (verified byte-for-byte against dc214be), so it's not a regression, but since it's now explicitly documented rather than silently inherited, it'd be a good time to file a follow-up to actually fix it (or confirm it's genuinely unreachable, e.g. if the map is always populated by a wrapping call before this runs).

Everything else checked out clean

• corpus_types.py's custom get_node (the earlier IDOR fix) is correct and intentionally not wired to the cached BaseService path for the top-level corpus(id:) query, per a documented incident with stale cached objects across permission mutations mid-request.
• CorpusType.documents resolver correctly uses CorpusDocumentService.get_corpus_documents_visible_to_user (the MIN-permission variant) per the CLAUDE.md Reconcile CorpusObjsService corpus-as-gate vs GraphQL MIN-permission semantic #1682 invariant — no regression there.
• annotation_mutations.py / extract_mutations.py: ownership/hybrid permission checks, require_permission's inverted-truthiness convention, and structural-annotation read-only enforcement are all intact.
• opencontractserver/tests/architecture/test_graphql_service_layer.py and the opencontracts.E001 check (opencontractserver/shared/checks.py) have zero diff — the inline-Tier-0 enforcement mechanism wasn't weakened, and no new inline visible_to_user/user_can composition was found in config/graphql/'s added lines.
• Depth-limit + introspection-disable wiring via AddValidationRules genuinely appends to (rather than replaces) the spec rule set, avoiding the graphene "replace-the-rules" trap the migration explicitly set out to fix; config/graphql/security.py itself is byte-identical to before.
• core/scalars.py has no unsafe deserialization (no eval/pickle); core/mutations.py's DRF permission gating and generic-error-on-exception behavior matches the old DRFMutation/DRFDeletion exactly.
• schema.py's schema.graphql_schema = schema._schema reaches into a strawberry-private attribute for backwards compatibility — documented and low-risk, but worth a comment/upstream-issue if strawberry's internal layout changes across versions.

Test coverage / CI

The PR description is upfront that the full backend suite is still pending completion of the porting fan-out, and that matches what I see in gh pr checks: linter, the e2e job (Login + Navigate All Views), and redis-integration were still in progress at review time, and I don't see a Django/pytest backend-suite check in the status rollup at all yet. Given the IDOR finding above, I'd flag opencontractserver/tests/test_add_annotation_idor.py (already touched by this PR) as a template — extending IDOR-style coverage to relationship/chatMessage/datacell/fieldset/column/annotationLabel/labelset singular queries would have caught this mechanically, and would be good regression insurance before merge.

[JSv4]

Thanks — the 🔴 IDOR finding was spot-on and is now fixed in 99202ae, and I extended the audit as you suggested.

Method: I grepped every get_node_from_global_id(info, id, only_type_name="X") call site and cross-checked each target X against its register_type hooks. That surfaced the 7 you flagged plus 7 more the same bug affected:

• Your 7: relationship, annotationLabel, labelset, chatMessage, fieldset, column, datacell
• Also fixed: analyzer, gremlinEngine (its only_type_name is GremlinEngineType_READ — the underscore made it easy to miss), agent (AgentConfigurationType), badge, userexport, userimport, and assignment

Each now registers a get_node hook mirroring the graphene resolver — BaseService.get_or_none / filter_visible, AgentConfigurationService.get_agent_by_id for agent, and assignment keeps its deprecated superuser-or-participant gate. Of the types you named as unverified: moderationAction was already safe (custom gated resolver via conversation.can_moderate, not the fallback path); corpusReference/noteRevision/corpusCategory/corpusAction aren't exposed as singular by-ID queries.

🟠 medium: also fixed — GraphQLView.dispatch now catches DRF AuthenticationFailed alongside JSONWebTokenError, so a bad API key returns a 200 GraphQL error instead of an unhandled 500.

Regression insurance: added test_singular_node_idor.py — a structural guard that fails if any type resolved via get_node_from_global_id lacks a permission hook (so this class can't silently return), plus a behavioral non-owner-denied check.

The two 🟡 pre-existing items (GlobalIDFilter type-name discard, resolve_object_shared_with KeyError) are faithful ports and out of scope for this migration — leaving them as-is for a separate follow-up as you suggested.

---

Generated by Claude Code

[claude]

Review

This PR already has two thorough automated review passes on it (the IDOR finding on singular-node resolution and the AuthenticationFailed-500 finding), both of which the author fixed in 99202ae with good regression coverage (test_singular_node_idor.py, extended to 14 types beyond the original 7 flagged). I focused on (a) verifying those fixes and the previously-flagged broken test imports are actually resolved in the current tree, and (b) areas not yet covered by prior passes. Reviewed via the local merge commit checkout since gh pr diff 406s on this PR's size.

Verified fixed since last review

• The 5 broken test-file imports flagged previously (test_mention_permissions.py, test_document_path_migration.py, test_user_privacy.py, test_security_hardening.py, scripts/test-django-ratelimit.py) — grepped the tree for config.graphql.{queries,graphene_types,base,mutations,custom_connections,jwt_overrides} imports and found none. Clean.
• _port_manifest.json (the scaffolding file with hardcoded /home/user/oc-graphene-ref/... paths) is gone from the tree.
• test_singular_node_idor.py and test_fk_visibility_traversal.py are present and, per changelog.d/2139-strawberry-singular-idor.security.md / 2139-fk-traversal-idor.security.md, cover both the singular-node and FK-traversal IDOR classes structurally (fails if any registered type lacks a hook), which is good insurance against a third instance of this bug class reappearing during the remaining porting fan-out.

New observation — tokenAuth/updateMe have no rate limiting
config/graphql/user_mutations.py has zero graphql_ratelimit/graphql_ratelimit_dynamic usage — no import, no decorator — unlike every other mutations module in this migration (annotation_mutations.py, conversation_mutations.py, badge_mutations.py, etc. all decorate their mutations). Notably m_token_auth (user_mutations.py:164, the login mutation) and m_update_me are unprotected at the GraphQL layer, despite a RateLimits.AUTH_LOGIN = "5/m" tier existing in config/ratelimit/rates.py — it's just never referenced outside config/admin_auth/views.py (the separate Django-admin login view).

I couldn't diff this file against the pre-migration graphene source directly (no history in this shallow checkout), so I can't confirm whether this is a regression introduced by the port or a pre-existing gap in the graphene code that's simply being carried over faithfully (which would make it out of scope for this PR, per the same reasoning the author gave for the GlobalIDFilter/resolve_object_shared_with pre-existing quirks). Worth a quick check against the graphene baseline — if tokenAuth was already unprotected, this is a good candidate for a fast independent follow-up (GraphQL login is a natural credential-stuffing target and AUTH_LOGIN already exists for exactly this purpose); if it was protected and the port dropped it, it should block WIP removal alongside the other IDOR-class fixes.

Minor — config/graphql/schema.py extra-types wiring is repetitive
The _extra_types += [v for v in vars(_module).values() if hasattr(v, "__strawberry_definition__")] block (schema.py:138-332) is copy-pasted ~30 times, one per imported module. Per the DRY guidance in CLAUDE.md, this collapses cleanly to a loop over a list of source modules. Not a correctness issue, just ~190 lines of pure boilerplate in a file that otherwise reads well.

Note — unrelated changelog fragments/commits present
The file list includes changelog.d/customs-ruling-citation-enrichment.added.md, changelog.d/ingest-corpus-convertible-ext.fixed.md, changelog.d/relationship-embedding-batching.fixed.md, and opencontractserver/corpuses/management/commands/enrich_customs_rulings.py, none of which relate to the graphene-to-strawberry migration. Given the PR shows 34 commits, this is very likely just main catch-up merges rather than intentional scope creep, but worth double-checking the diff is clean of unintended inclusions before this lands, since a PR this large is easy to lose track of.

Everything else I spot-checked

• config/graphql/core/relay.py, permissions.py, auth.py, mutations.py, filtering.py, views.py, jwt_auth.py, schema.py — all read as careful, well-commented ports with the rationale for non-obvious decisions documented inline (e.g. the deliberate non-fix of resolve_object_shared_with's KeyError-swallowing, the is_type_of shim for strawberry's stricter interface-membership check, the (ValueError, TypeError, OverflowError) guard around malformed pks). No unsafe deserialization, no obviously-inline Tier-0 permission composition.
• test_schema_parity.py is a genuinely rigorous structural diff (types/fields/arg types/printed defaults/interfaces/enum members) rather than a shallow smoke test — good confidence anchor for a mechanical framework swap this size.
• Settings/auth-backend wiring (config/settings/base.py) looks correctly carried over; AUTHENTICATION_BACKENDS chain and INSTALLED_APPS changes match the PR description.

Recommendation
Consistent with the PR's own "do not merge yet" status: confirm the tokenAuth rate-limiting question above, run the full backend suite (not just the 3 named modules) before dropping WIP, and this looks close to ready. The verification story here (golden SDL parity + the two rounds of IDOR-hardening tests) is a strong foundation for a migration of this size.