Skip to content

Fix processing status proxy route#489

Open
atul-upadhyay-7 wants to merge 1 commit into
FireFistisDead:masterfrom
atul-upadhyay-7:fix-processing-status-450
Open

Fix processing status proxy route#489
atul-upadhyay-7 wants to merge 1 commit into
FireFistisDead:masterfrom
atul-upadhyay-7:fix-processing-status-450

Conversation

@atul-upadhyay-7

@atul-upadhyay-7 atul-upadhyay-7 commented Jun 7, 2026

Copy link
Copy Markdown

Fixes #450. Added Express proxy route for /processing-status/ and updated server tests to verify internal token logic.

Summary by CodeRabbit

  • New Features
    • Added a processing status endpoint to check the current processing progress of a session. Includes error handling for missing or unavailable status information.

Copilot AI review requested due to automatic review settings June 7, 2026 12:28
@vercel

vercel Bot commented Jun 7, 2026

Copy link
Copy Markdown

@atul-upadhyay-7 is attempting to deploy a commit to the firefistisdead's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new Express gateway route GET /processing-status/:sessionId that proxies session status queries from the frontend to the RAG service with internal auth headers. Includes tests validating successful responses, 404 error handling, and header forwarding. Updates existing test authorization headers to use dynamically generated Supabase JWTs.

Changes

Processing status proxy endpoint

Layer / File(s) Summary
Processing status proxy endpoint
server.js
GET /processing-status/:sessionId proxies to the RAG service using ragAuthHeaders() and a 5s timeout; returns upstream JSON on success, maps upstream 404 to a fixed error response, and delegates other failures to propagateRagError.
Endpoint tests and test auth fixtures
server.test.js
Adds two route tests validating successful 200 response with payload mapping and header forwarding, and 404 error response handling. Updates authorization headers in two existing POST /process-from-url tests to use jwt.sign({ role: "authenticated" }, process.env.SUPABASE_JWT_SECRET) instead of static tokens.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • FireFistisDead/pdf-qa-bot#193: Implements corresponding RAG service /processing-status/{session_id} endpoint and auth header enforcement that this PR's proxy route depends on.
  • FireFistisDead/pdf-qa-bot#327: Frontend implementation of getProcessingStatus(sessionId) and polling logic in DocumentsView that calls the new gateway endpoint added in this PR.

Suggested labels

fix, backend, rag-service, type:testing, type:security, bug

Poem

A rabbit hops through the gateway so bright,
With /processing-status/ now working just right!
Auth headers forwarded, errors mapped true,
The frontend now sees what the RAG service does too! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding a proxy route for /processing-status/ endpoint, which directly addresses the primary objective from issue #450.
Description check ✅ Passed The description is minimal but captures the key change (fixes #450, added proxy route, updated tests). However, it lacks most required template sections like Testing, Checklist items, and Notes about implementation details.
Linked Issues check ✅ Passed The PR successfully implements requirement (1) from issue #450 by adding a GET /processing-status/:sessionId proxy route with internal auth headers and 404 handling. Tests verify proxy behavior and internal token logic.
Out of Scope Changes check ✅ Passed JWT generation updates in server tests align with internal token verification requirements. All changes focus on the proxy route and auth logic without unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@github-actions github-actions Bot added backend Express or API gateway work feature A new feature or improvement fix A targeted fix or cleanup type:testing bug Something isn't working labels Jun 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new API endpoint to fetch processing progress from the RAG service and expands/adjusts tests to cover the new behavior and updated auth requirements.

Changes:

  • Added GET /processing-status/:sessionId route that proxies to the RAG service with internal auth headers and handles upstream 404s.
  • Added tests for the new route’s success and 404 flows.
  • Updated existing tests to use a signed Supabase JWT instead of a placeholder token.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
server.js Introduces the /processing-status/:sessionId proxy route and error handling.
server.test.js Adds coverage for the new route and updates auth headers to use a signed JWT.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread server.js
Comment thread server.js
Comment thread server.test.js
Comment thread server.test.js
Comment thread server.test.js

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
server.js (1)

1106-1106: ⚡ Quick win

Validate sessionId format to fail fast on malformed input.

The sessionId URL parameter is forwarded without validation. Malformed input (non-UUID) will propagate to the RAG service. While the RAG service should handle this, gateway-level validation provides faster feedback and reduces unnecessary upstream calls.

✅ Proposed validation
 app.get("/processing-status/:sessionId", async (req, res) => {
+  const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+  if (!uuidPattern.test(req.params.sessionId)) {
+    return res.status(400).json({ error: "Invalid session ID format." });
+  }
+
   try {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server.js` at line 1106, The GET route handler registered at
app.get("/processing-status/:sessionId") forwards the sessionId without
validating its format; add an early validation step in that handler to check the
sessionId param is a well-formed UUID (e.g., UUID v4) using a regex or a UUID
validation helper (validateUuid/uuid.validate) and if invalid immediately return
a 400 response with a clear JSON error body instead of calling the downstream
RAG service; ensure the validation runs before any RAG/service calls or DB
lookups in the route handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server.js`:
- Around line 1106-1119: The GET /processing-status/:sessionId route lacks rate
limiting and must use the existing inferenceLimiter middleware to prevent
unrestricted polling; update the route definition for
app.get("/processing-status/:sessionId", ...) to apply the inferenceLimiter (the
same limiter used elsewhere like for inference endpoints) so requests pass
through inferenceLimiter before the async handler, ensuring the existing 60
req/min policy is enforced and preventing abuse/session enumeration.
- Around line 1106-1119: The GET handler for /processing-status/:sessionId
currently allows unauthenticated polling; require a session_secret query param
and validate ownership before forwarding to the RAG service: in the
app.get("/processing-status/:sessionId") handler, reject requests missing
req.query.session_secret with a 400, then call the RAG service with both
sessionId and session_secret (or perform a local ownership check) instead of
blindly proxying; keep using ragAuthHeaders() for service auth and still use
propagateRagError for downstream errors, and update the RAG service (or local
validation) so it only returns status when the session_secret matches the
sessionId.

---

Nitpick comments:
In `@server.js`:
- Line 1106: The GET route handler registered at
app.get("/processing-status/:sessionId") forwards the sessionId without
validating its format; add an early validation step in that handler to check the
sessionId param is a well-formed UUID (e.g., UUID v4) using a regex or a UUID
validation helper (validateUuid/uuid.validate) and if invalid immediately return
a 400 response with a clear JSON error body instead of calling the downstream
RAG service; ensure the validation runs before any RAG/service calls or DB
lookups in the route handler.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 89a91e71-96d1-416b-8ee8-d51ec9fea3b8

📥 Commits

Reviewing files that changed from the base of the PR and between 42ae762 and 0eafac5.

📒 Files selected for processing (2)
  • server.js
  • server.test.js

Comment thread server.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Express or API gateway work bug Something isn't working feature A new feature or improvement fix A targeted fix or cleanup type:testing

Projects

None yet

2 participants