Add request and error context controls#111
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAUTH-04A introduces request/correlation ID middleware, structured API errors, OpenAPI documentation, and compatibility-preserving auth/task response handling. It also updates workstream planning, review evidence, tests, contract checks, and operational documentation while leaving PostgreSQL rate controls inactive. ChangesAUTH-04A planning and contracts
Runtime API behavior
Validation and operational evidence
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RequestContextMiddleware
participant FastAPI
participant AuthDependencies
participant TaskRouter
Client->>RequestContextMiddleware: Send request with optional ID headers
RequestContextMiddleware->>RequestContextMiddleware: Validate or generate request and correlation IDs
RequestContextMiddleware->>FastAPI: Forward request with IDs in scope state
FastAPI->>AuthDependencies: Resolve authentication when required
FastAPI->>TaskRouter: Execute task route
TaskRouter-->>FastAPI: Return success or structured domain error
FastAPI-->>RequestContextMiddleware: Produce response
RequestContextMiddleware-->>Client: Return canonical ID headers and error payload when applicable
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md:
- Around line 218-220: The AUTH-04A lifecycle notes are stale because
implementation and internal review have passed. In
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md
lines 218-220, remove the preimplementation re-review gate or clearly mark it
historical; in .agent-loop/WORK_QUEUE.md lines 59-63, update the “Proposed Next”
text so contract repair/re-review is no longer presented as active and reflects
PR publication being active.
In
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md:
- Around line 15-16: Update the paragraph beginning “The user explicitly started
AUTH-04” so the reference to “PR `#110`” remains on the same Markdown line as the
surrounding sentence; do not alter the wording or other review content.
In `@backend/tests/test_api_controls.py`:
- Around line 469-476: Update the request and response header schemas in
api_controls to include the canonical lowercase RFC 4122 UUID pattern alongside
the existing uuid format and request length constraints. Extend the test
covering request and response headers to assert the expected pattern for both
X-Request-ID and X-Correlation-ID, not just their presence and required flags.
In `@docs/operations_authorization_service.md`:
- Around line 118-131: Update the canonical error object example in the
documentation to include the existing top-level detail compatibility field, or
explicitly label the snippet as only the nested error portion. Keep the
documented nested error fields unchanged and ensure the example matches the
stated compatibility contract.
🪄 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: 118e6023-b7f9-4e92-a641-92ba24cf8a58
📒 Files selected for processing (21)
.agent-loop/LOOP_STATE.md.agent-loop/REVIEW_LOG.md.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DISCOVERY.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04-request-api-controls.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04B-postgres-rate-controls.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.mdbackend/app/api/deps/auth.pybackend/app/core/api_controls.pybackend/app/main.pybackend/app/modules/tasks/router.pybackend/scripts/api_contract_e2e.pybackend/tests/test_api_controls.pybackend/tests/test_auth.pybackend/tests/test_tasks.pydocs/operations_authorization_service.md
Comment on lines
+218
to
+220
| The user explicitly started parent AUTH-04 on 2026-07-13. Required plan review | ||
| split it before implementation. Only this repaired 04A contract is active; | ||
| runtime edits remain gated on its required preimplementation re-review. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== FILES ==\n'
git ls-files '.agent-loop/LOOP_STATE.md' '.agent-loop/WORK_QUEUE.md' '.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md'
printf '\n== LOOP_STATE excerpt ==\n'
sed -n '1,260p' .agent-loop/LOOP_STATE.md | nl -ba | sed -n '1,260p'
printf '\n== WORK_QUEUE excerpt ==\n'
sed -n '1,140p' .agent-loop/WORK_QUEUE.md | nl -ba | sed -n '1,140p'
printf '\n== CHUNK excerpt ==\n'
sed -n '180,250p' .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md | nl -ba | sed -n '180,250p'Repository: Flow-Research/workstream
Length of output: 410
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== LOOP_STATE ==\n'
cat -n .agent-loop/LOOP_STATE.md | sed -n '1,260p'
printf '\n== WORK_QUEUE ==\n'
cat -n .agent-loop/WORK_QUEUE.md | sed -n '1,140p'
printf '\n== CHUNK ==\n'
cat -n .agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md | sed -n '180,250p'Repository: Flow-Research/workstream
Length of output: 23436
Update AUTH-04A lifecycle wording
These entries still read as preimplementation-gated, while .agent-loop/LOOP_STATE.md now says implementation and internal review passed and PR publication is active.
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md#L218-L220: remove the “preimplementation re-review” gate or mark this note historical..agent-loop/WORK_QUEUE.md#L59-L63: update the “Proposed Next” text so it no longer says contract repair/re-review is active.
📍 Affects 2 files
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md#L218-L220(this comment).agent-loop/WORK_QUEUE.md#L59-L63
🤖 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
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md
around lines 218 - 220, The AUTH-04A lifecycle notes are stale because
implementation and internal review have passed. In
.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04A-request-error-context.md
lines 218-220, remove the preimplementation re-review gate or clearly mark it
historical; in .agent-loop/WORK_QUEUE.md lines 59-63, update the “Proposed Next”
text so contract repair/re-review is no longer presented as active and reflects
PR publication being active.
Source: Coding guidelines
Comment on lines
+15
to
+16
| The user explicitly started AUTH-04 after AUTH-03 memory merged through PR | ||
| #110. Required L1 review split request/error context from PostgreSQL rate |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep PR #110`` on one Markdown line.
Line 16 is parsed as a malformed heading rather than the continuation of the sentence.
Proposed fix
-The user explicitly started AUTH-04 after AUTH-03 memory merged through PR
-#110. Required L1 review split request/error context from PostgreSQL rate
+The user explicitly started AUTH-04 after AUTH-03 memory merged through
+PR `#110`. Required L1 review split request/error context from PostgreSQL rate📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| The user explicitly started AUTH-04 after AUTH-03 memory merged through PR | |
| #110. Required L1 review split request/error context from PostgreSQL rate | |
| The user explicitly started AUTH-04 after AUTH-03 memory merged through | |
| PR `#110`. Required L1 review split request/error context from PostgreSQL rate |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 16-16: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🤖 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
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04A-pr-trust-bundle.md
around lines 15 - 16, Update the paragraph beginning “The user explicitly
started AUTH-04” so the reference to “PR `#110`” remains on the same Markdown line
as the surrounding sentence; do not alter the wording or other review content.
Source: Linters/SAST tools
Comment on lines
+469
to
+476
| request_headers = { | ||
| parameter["name"] for parameter in operation["parameters"] | ||
| } | ||
| assert {"X-Request-ID", "X-Correlation-ID"} <= request_headers | ||
| for response in operation["responses"].values(): | ||
| assert {"X-Request-ID", "X-Correlation-ID"} <= set(response["headers"]) | ||
| assert response["headers"]["X-Request-ID"]["required"] is True | ||
| assert response["headers"]["X-Correlation-ID"]["required"] is True |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline backend/app/core/api_controls.py --items all
rg -n -C4 'minLength|X-Request-ID|invalid_request' \
backend/app/core/api_controls.py backend/tests/test_api_controls.pyRepository: Flow-Research/workstream
Length of output: 12067
🏁 Script executed:
#!/bin/bash
sed -n '130,320p' backend/app/core/api_controls.py
printf '\n---\n'
sed -n '1,220p' backend/tests/test_api_controls.pyRepository: Flow-Research/workstream
Length of output: 14935
🏁 Script executed:
#!/bin/bash
rg -n -C3 '"pattern"|format": "uuid"|minLength|maxLength|X-Request-ID|X-Correlation-ID' backend/app/core/api_controls.py backend/tests/test_api_controls.py
python3 - <<'PY'
from uuid import UUID, uuid4, RFC_4122
def ok(v):
try:
p = UUID(v)
return len(v) == 36 and p.int != 0 and p.variant == RFC_4122 and p.version in range(1, 9) and str(p) == v
except Exception:
return False
cases = {
"lower_v4": str(uuid4()),
"upper_v4": str(uuid4()).upper(),
"compact": str(uuid4()).replace("-", ""),
"nil": "00000000-0000-0000-0000-000000000000",
"v9_like": "00000000-0000-9000-8000-000000000001",
}
for name, v in cases.items():
print(name, ok(v))
PYRepository: Flow-Research/workstream
Length of output: 6677
Add the canonical UUID pattern to these OpenAPI header schemas. backend/app/core/api_controls.py still documents the request context headers with only format: uuid (plus length on request params), but the middleware accepts only lowercase RFC 4122 UUID strings and rejects uppercase, nil, compact, and unsupported-version values. Add the matching pattern for both request and response headers, and have backend/tests/test_api_controls.py:469-476 assert that schema instead of only header presence.
🤖 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 `@backend/tests/test_api_controls.py` around lines 469 - 476, Update the
request and response header schemas in api_controls to include the canonical
lowercase RFC 4122 UUID pattern alongside the existing uuid format and request
length constraints. Extend the test covering request and response headers to
assert the expected pattern for both X-Request-ID and X-Correlation-ID, not just
their presence and required flags.
Comment on lines
+118
to
+131
| Errors add this canonical object while retaining the existing top-level | ||
| `detail` or domain `code/details` compatibility fields: | ||
|
|
||
| ```json | ||
| { | ||
| "error": { | ||
| "code": "invalid_request", | ||
| "message": "Request validation failed", | ||
| "details": {}, | ||
| "correlation_id": "00000000-0000-4000-8000-000000000001", | ||
| "retryable": false | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Make the example match the compatibility contract.
The prose says existing top-level fields remain, but the example omits detail. Include it or label this explicitly as only the nested error portion.
🤖 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 `@docs/operations_authorization_service.md` around lines 118 - 131, Update the
canonical error object example in the documentation to include the existing
top-level detail compatibility field, or explicitly label the snippet as only
the nested error portion. Keep the documented nested error fields unchanged and
ensure the example matches the stated compatibility contract.
abiorh-claw
approved these changes
Jul 13, 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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
PR Trust Bundle: WS-AUTH-001-04A
Chunk
WS-AUTH-001-04A- Request And Error ContextGoal
Give every API response canonical request and correlation identifiers and a
stable privacy-bounded error object without changing product authority or
breaking existing clients.
Human-Approved Intent
The user explicitly started AUTH-04 after AUTH-03 memory merged through PR
#110. Required L1 review split request/error context from PostgreSQL rate
controls before implementation. This PR contains only the reviewed 04A child;
04B and later authorization chunks remain inactive.
What Changed
middleware.
status codes, and headers.
Why It Changed
Workstream needs traceable API failures and a dependable error contract before
authority enforcement and rate controls expand, but existing consumers cannot
be forced through a breaking response migration.
Design Chosen
Pure ASGI middleware reads raw header pairs, stores effective IDs on request
scope state, and edits only
http.response.startheaders. Shared responsebuilders and exception handlers own JSON shapes. Existing compatibility fields
remain additive beside the canonical nested object.
Alternatives Rejected
BaseHTTPMiddleware, response buffering, or response-body rewriting.Scope Control
The production change remains below the contract's 500 non-comment line hard
stop. No route, dependency consumer, database model, migration, setting,
permission, role, grant, actor state, rate counter, or product lifecycle changed.
Product Behavior
Clients receive canonical
X-Request-IDandX-Correlation-IDheaders onsuccess and failure. Errors gain a required nested object with stable code,
message, bounded details, correlation ID, and retryability while retaining
existing compatible fields and headers. Product authorization is unchanged.
Acceptance Criteria Proof
Real-ASGI tests cover missing, valid, malformed, duplicate, non-ASCII, nil,
uppercase, non-hyphenated, unsupported-version, and comma-joined identifiers;
concurrency; success and every required error class; validation privacy;
streaming; background tasks; duplicate unrelated headers; task compatibility;
OpenAPI schemas; and exact route/dependency inventories.
Tests And Checks
90.82, 90.70, and 92.36 percent respectively.
drill passed.
Gates, additive test delta, and diff hygiene passed.
114 passes; its exact order passes after repair. GitHub Backend must provide
the complete-suite rerun before merge.
Test Delta
No assertion,
pytest.raises, skip/xfail marker, orskipTestline was removedor weakened. New proof exercises actual FastAPI routes through ASGI transport.
CI Integrity
No workflow, package, dependency, threshold, exclusion, or bypass changed.
GitHub Backend and Agent Gates remain mandatory before merge. The separate
whole-app coverage initiative remains paused at its official 79.249908 percent
baseline; this chunk's changed-file threshold is at least 90 percent.
Reviewer Results
Senior engineering, QA/test, security/auth, product/ops, architecture, CI
integrity, docs, reuse/dedup, and test-delta review pass. Production was reviewed
at
cdcaf77; final reviewed candidate4fd6db9received exact-head confirmationfor both additive test repairs with identical production blobs and no weakened
tests.
External Review
GitHub Backend, Agent Gates, CodeRabbit, and explicit human review begin after
the ready PR is published. None substitutes for the completed internal review.
Remaining Risks
IdentityIssuerVerifieradoption depends on the sharedexternal-service adapter foundation and a separate reviewed AUTH chunk.
separately reviewed removal is safe.
Follow-Up Work
After this PR merges and memory is updated, AUTH-04B still requires a separate
explicit user start. AUTH-05 and later cutover chunks remain proposed.
Human Review Focus
Inspect duplicate-ID rejection, response-start-only header mutation, error and
log privacy, exact legacy compatibility, per-route OpenAPI truth, and unchanged
route/dependency inventories.
Human Merge Ownership
Only the user may approve and merge this PR. Publication is not merge approval,
and no later AUTH chunk starts automatically.
Summary by CodeRabbit
New Features
Bug Fixes
Tests