Skip to content

Feat nxe phase - #65

Merged
kjdev merged 2 commits into
mainfrom
feat/nxe-phase
Sep 2, 2026
Merged

kjdev merged 2 commits into
mainfrom
feat/nxe-phase

Conversation

@kjdev

@kjdev kjdev commented Sep 2, 2026 •

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Improvements
    • JWT request processing now uses a configurable priority for PREACCESS and ACCESS checks.
    • Handler ordering is consistent and no longer depends on module build or load order.
    • Builds now include and validate the required phase-integration component.
  • Bug Fixes
    • Added error handling for cases where JWT phase-handler registration cannot be completed.
  • Documentation
    • Updated the unreleased changelog with the new handler registration and ordering behavior.

nginx's ngx_http_init_phase_handlers() walks each phase's handler
array tail-to-head, so directly pushing onto conf->phases[...].handlers
made this module's execution order relative to other same-phase
modules (e.g. an httpsig or OAuth2 bearer-token module) depend on
--add-module / load_module order rather than on nginx.conf intent.

Add the nxe-phase submodule and register the PREACCESS and ACCESS
phase handlers through nxe_phase_add_handler() with a fixed priority
(NXE_PHASE_PRIO_JWT), so the order no longer depends on build/load
order.
@coderabbitai

coderabbitai Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: f082c241-0320-47f5-a446-12298169628f

📥 Commits

Reviewing files that changed from the base of the PR and between e100d9d and d1682f5.

📒 Files selected for processing (6)
  • .gitmodules
  • CHANGELOG.md
  • Dockerfile
  • config
  • nxe-phase
  • src/ngx_http_auth_jwt_module.c

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The build now includes the pinned nxe-phase submodule. JWT PREACCESS and ACCESS handlers register through nxe_phase_add_handler() at NXE_PHASE_PRIO_JWT.

Changes

nxe-phase Integration

Layer / File(s) Summary
Submodule and build wiring
.gitmodules, nxe-phase, Dockerfile, config
The build pins, copies, validates, compiles, and links the nxe-phase submodule.
Priority-based handler registration
src/ngx_http_auth_jwt_module.c, CHANGELOG.md
JWT handlers use nxe_phase_add_handler() at NXE_PHASE_PRIO_JWT. Registration failures return NGX_ERROR.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d1682

The PR integrates nxe-phase into module registration and build configuration; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant ngx_http_auth_jwt_post_conf
  participant nxe_phase_add_handler
  participant NGINX phase handler arrays
  ngx_http_auth_jwt_post_conf->>nxe_phase_add_handler: Register PREACCESS handler at NXE_PHASE_PRIO_JWT
  nxe_phase_add_handler->>NGINX phase handler arrays: Add handler by configured priority
  ngx_http_auth_jwt_post_conf->>nxe_phase_add_handler: Register ACCESS handler at NXE_PHASE_PRIO_JWT
  nxe_phase_add_handler->>NGINX phase handler arrays: Add handler by configured priority
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (5 skipped: 5 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the main change: adding nxe-phase integration and handler registration. It is concise and related to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/nxe-phase

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kjdev
kjdev merged commit d1682f5 into main Sep 2, 2026
9 checks passed
@kjdev
kjdev deleted the feat/nxe-phase branch September 2, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant