You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Strawberry API still pulls in Graphene through django-graphql-jwt and uses graphql-relay helpers. Remove graphene, graphene-django, django-graphql-jwt, and graphql-relay while preserving JWT behavior, the served schema, opaque IDs, pagination, and permission checks.
The Extract pipeline also stalled when a first-message badge celebration covered the “New Extract” button. Give the celebration an accessible dialog name and have the browser test close it before continuing. Prepare the version display and collated changelog for the v3.1.0 release.
Changes
Own the shared JWT runtime in config/jwt_auth/ using Django and the existing PyJWT dependency. GraphQL, REST, Auth0, WebSockets, and MCP use the same replacement helpers.
Preserve GRAPHQL_JWT settings and legacy default handler paths, JWT error messages, refresh-token behavior, and the optional refresh app's database tables and migration history.
Replace Relay ID/cursor helpers and pagination slicing with small local implementations. Remove test-only Graphene resolver aliases and the schema compatibility accessor; existing auth assertions remain unchanged.
Update Python embedded in frontend E2E helpers to use the replacement JWT/ID utilities, and guard imports in frontend tests and CI workflows.
Handle badge celebrations during the complete PDF extraction/export E2E workflow. Run one bounded CI attempt, retain its failure trace, and upload diagnostics even when the job is cancelled.
Set the displayed version to v3.1.0 and collate the pending changelog fragments into the release entry.
Create validation extensions per request, expand SDL parity coverage, and add dependency, ID, JWT, and refresh-storage contract tests. Keep Strawberry pinned at 0.323.2; check 0.327.0 separately.
Deployment note: If INSTALLED_APPS explicitly includes graphql_jwt.refresh_token, replace it with config.jwt_auth.refresh_token. Existing refresh-token data and migration records are preserved. The default deployment does not enable this optional app. See migration details and verification notes.
Broad affected backend suite, pytest -n 4 --dist loadscope: 4,634 passed, 75 subtests passed, one optional measurement test skipped across 219 modules, with all four removed packages physically uninstalled. python -m pip check passes.
The same 181 existing auth/schema/Relay/scalar/security tests pass before and after the change; a differential check matches 32,400 pagination combinations against the original implementation.
Strawberry 0.327.0 candidate: 248 contract/auth/permission/security tests passed, including 10 subtests.
Golden schema files are unchanged. python scripts/validate_frontend_graphql.py validates all 292 frontend documents; tsc --noEmit passes.
WebSocket auth E2E CI: all 6 tests passed without retries, including URL-token rejection, private-document rejection, VCR agent chat, and in-band token refresh.
Full Extract PDF workflow, against an isolated backend with the four removed packages uninstalled: 1 passed in 6.2 minutes, including PDF ingestion, populated extraction cells, CSV export, iteration creation, and comparison. The recorded trace confirms the badge dialog was dismissed.
Dependency import/manifest guards: 3 passed, including the embedded-Python regression check.
Full mypy check: 1,584 source files pass.
pre-commit run --all-files: all hooks pass.
All 21 PR checks are green on cc547259f059cee5b579cfec65c9d8d33be423dd, including backend, frontend component/unit/build checks, all three browser integration workflows, Redis, and security scans. CI tested merge tree e182e582fb5f8d2300e0a4aa13e873fdc8bf2d07 against the current main branch.
Compatibility review
The pagination differential includes cursor offsets -4, -1, and values beyond the collection bounds. Negative QuerySet slicing already exists in the original wrapper; out-of-range before behavior and the refresh queryset’s annotate-only expired() behavior also match the removed libraries. This dependency removal preserves those existing contracts. JWT settings are read dynamically to avoid stale signing keys or handlers.
Checklist
Tests pass locally for affected and new code
pre-commit run --all-files passes
TypeScript compiles cleanly
Changelog fragments collated for v3.1.0; version display updated
No new third-party dependencies introduced
Contributor License Agreement
By submitting this pull request, you agree to license your contribution
under the project's Contributor License Agreement.
Review: Remove Graphene dependencies while preserving auth and API contracts
Great scope of testing documented in the PR description (schema-parity, differential pagination checks, contract tests across two Strawberry versions). I focused on the parts that are easiest to miss in a large mechanical port: the vendored Relay pagination math and the new JWT settings/token-issuance code. Findings below, most severe first.
1. Crafted after cursor with a negative offset crashes QuerySet-backed connections
get_offset_with_default/cursor_to_offset (opencontractserver/utils/ids.py:47-58) do not validate the decoded integer — any syntactically valid arrayconnection:<int> cursor is accepted, including negative ones. If a client sends after: base64("arrayconnection:-5"), slice_start becomes negative (e.g. -4) and is never clamped before being used at line 468:
For a Django QuerySet (the common case for every paginated connection field), negative slicing raises AssertionError: Negative indexing is not supported, an unhandled 500 reachable by any authenticated client on any connection field. Note start_offset = max(slice_start, 0) at line 447 clamps the other use of the value but not slice_start itself, so the fix is likely just slice_start = max(slice_start, 0) right after line 437.
2. Negative before cursor isn't clamped, so it stops constraining the page
When before decodes to a negative offset, the guard 0 <= before_offset is false, so end_offset silently stays at array_length instead of reflecting "nothing before this point" (which should yield an empty page). A client passing before: base64("arrayconnection:-1") gets the entire unbounded result instead of an empty one, and has_next_page (line 483, computed against before_offset) becomes similarly inconsistent. Worth clamping explicitly (e.g. end_offset = min(end_offset, max(before_offset, 0))) rather than skipping the bound entirely outside [0, array_length).
This only annotates an expired boolean and returns the full queryset — there's no trailing .filter(expired=True). It isn't called anywhere in the current tree (grepped config/ and opencontractserver/), so there's no active bug today, but the name/shape strongly invites a future RefreshToken.objects.expired().delete() cleanup job to nuke every refresh token, not just stale ones. Worth fixing now or removing the method if genuinely unused.
4. graphql_jwt. → config.jwt_auth. handler-path rewrite is a blind prefix replace
This rewrites any string starting with graphql_jwt., not just the known legacy default handler paths. A deployment with a genuinely custom handler that happens to live under a graphql_jwt.*-prefixed module (unlikely, but the whole point of this compatibility shim is unusual legacy configs) gets silently remapped to config.jwt_auth.*, which either ImportErrors far from the real cause or — worse — coincidentally resolves to an unrelated callable. Consider allowlisting the specific legacy paths you intend to remap (the ones enumerated in docs/architecture/graphql_strawberry_migration.md) instead of a bare substring rewrite.
5. JWTSettings.__getattr__ re-resolves handlers via import_string on every access
config/jwt_auth/settings.py:47-62
Every jwt_settings.JWT_*_HANDLER access (i.e. every authenticated GraphQL/REST/WebSocket request touching JWT) re-runs import_string's module split + import_module + getattr from scratch — no memoization like DRF's APISettings/upstream django-graphql-jwt (which caches resolved values via setattr and only busts the cache on setting_changed). The comment above jwt_settings = JWTSettings() (line 65-66) suggests this is a deliberate trade-off for override_settings correctness in tests, which is reasonable, but it does mean import-string resolution is now on the hot authentication path for every request in production. Worth confirming this is an acceptable cost, or adding a lightweight cache keyed on the resolved setting that's invalidated via the setting_changed signal (matching the pattern this replaces).
6. Token minting is inlined in three places instead of calling the new get_token() helper
config/graphql/jwt_auth.py:117-118, config/graphql/user_mutations.py:177-178, vs. config/jwt_auth/shortcuts.py:7-10
The login mutation and the refresh mutation both reimplement the JWT_PAYLOAD_HANDLER + JWT_ENCODE_HANDLER pair inline rather than calling this helper (this mirrors upstream graphql_jwt's own duplication between shortcuts.get_token and its decorators, so it may be intentional fidelity to the ported structure — flagging in case consolidating onto the shared helper wasn't considered). Minor, but a future change to token issuance (e.g. an added claim) applied only to get_token() won't reach these two call sites, and nothing currently pins them together.
Not flagged as a bug, but worth a sanity check
The differential pagination test mentioned in the PR description ("32,400 pagination combinations against the original implementation") — did that combination space include negative/out-of-range cursor offsets? If the comparison only exercised well-formed forward cursors (0..array_length), it wouldn't have caught #1/#2 above, since both require a cursor value outside the "normal" range that a well-behaved client would never construct but any GraphQL client legitimately can.
Overall this is a careful, well-tested port — the pagination edge cases above are the kind of thing that's easy to miss when porting arithmetic-heavy slicing logic away from a battle-tested library, and are worth a couple of added test cases (negative after/before cursors on a QuerySet-backed connection) rather than a large rework.
JSv4
changed the title
Remove Graphene dependencies while preserving auth and API contracts
Remove Graphene dependencies and prepare OpenContracts v3.1.0
Sep 8, 2026
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 freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Strawberry API still pulls in Graphene through
django-graphql-jwtand usesgraphql-relayhelpers. Removegraphene,graphene-django,django-graphql-jwt, andgraphql-relaywhile preserving JWT behavior, the served schema, opaque IDs, pagination, and permission checks.The Extract pipeline also stalled when a first-message badge celebration covered the “New Extract” button. Give the celebration an accessible dialog name and have the browser test close it before continuing. Prepare the version display and collated changelog for the v3.1.0 release.
Changes
config/jwt_auth/using Django and the existing PyJWT dependency. GraphQL, REST, Auth0, WebSockets, and MCP use the same replacement helpers.GRAPHQL_JWTsettings and legacy default handler paths, JWT error messages, refresh-token behavior, and the optional refresh app's database tables and migration history.v3.1.0and collate the pending changelog fragments into the release entry.Deployment note: If
INSTALLED_APPSexplicitly includesgraphql_jwt.refresh_token, replace it withconfig.jwt_auth.refresh_token. Existing refresh-token data and migration records are preserved. The default deployment does not enable this optional app. See migration details and verification notes.Test plan
Full backend CI: 11,329 passed, 26 skipped, 382 subtests passed; required backend gate passed.
Broad affected backend suite,
pytest -n 4 --dist loadscope: 4,634 passed, 75 subtests passed, one optional measurement test skipped across 219 modules, with all four removed packages physically uninstalled.python -m pip checkpasses.The same 181 existing auth/schema/Relay/scalar/security tests pass before and after the change; a differential check matches 32,400 pagination combinations against the original implementation.
Strawberry 0.327.0 candidate: 248 contract/auth/permission/security tests passed, including 10 subtests.
Golden schema files are unchanged.
python scripts/validate_frontend_graphql.pyvalidates all 292 frontend documents;tsc --noEmitpasses.WebSocket auth E2E CI: all 6 tests passed without retries, including URL-token rejection, private-document rejection, VCR agent chat, and in-band token refresh.
Full Extract PDF workflow, against an isolated backend with the four removed packages uninstalled: 1 passed in 6.2 minutes, including PDF ingestion, populated extraction cells, CSV export, iteration creation, and comparison. The recorded trace confirms the badge dialog was dismissed.
Extract PDF workflow CI: 1 passed in 5.6 minutes without retries, with coverage enabled.
Existing badge component suite: 10 passed.
Dependency import/manifest guards: 3 passed, including the embedded-Python regression check.
Full mypy check: 1,584 source files pass.
pre-commit run --all-files: all hooks pass.All 21 PR checks are green on
cc547259f059cee5b579cfec65c9d8d33be423dd, including backend, frontend component/unit/build checks, all three browser integration workflows, Redis, and security scans. CI tested merge treee182e582fb5f8d2300e0a4aa13e873fdc8bf2d07against the current main branch.Compatibility review
The pagination differential includes cursor offsets
-4,-1, and values beyond the collection bounds. Negative QuerySet slicing already exists in the original wrapper; out-of-rangebeforebehavior and the refresh queryset’s annotate-onlyexpired()behavior also match the removed libraries. This dependency removal preserves those existing contracts. JWT settings are read dynamically to avoid stale signing keys or handlers.Checklist
pre-commit run --all-filespassesContributor License Agreement
By submitting this pull request, you agree to license your contribution
under the project's Contributor License Agreement.