feat(ri-exchange): symmetric session-auth approve path (closes #300)

[cristim]

Mirrors the in-dashboard approve pattern from #286 onto the RI Exchange flow.

Closes #300.

Changes

• Three-mode dispatch in approveRIExchange handler: session with approve-any/approve-own → RBAC approve; legacy email token → unchanged path; neither → 401.
• Migration 000053: nullable approved_by + created_by_user_id columns on ri_exchange_history. approved_by stamped best-effort after every session-authed approval.
• Frontend Approve button in RI Exchange history panel, visible only when canApproveRIExchangeRow returns true.
• 8 new backend unit tests + frontend coverage; tsc clean, 61 jest suites pass.

Summary by CodeRabbit

New Features

• Users can now approve pending RI exchanges directly through authenticated sessions with role-based access controls (admins approve any; other users approve only their own submissions)
• Approval tracking now records submitter and approver information for audit purposes

Database

• Schema updated to persist creation and approval metadata for RI exchanges

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4c0f97e7-5a41-410e-9deb-1e35a2911964

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR extends RI exchange approval to support session-based RBAC authorization alongside the existing email-token legacy path. It adds a dashboard Approve button, refactors the approval dispatcher with three-mode routing, implements approve-any/approve-own authorization rules, persists creator and approver attribution, and provides comprehensive test coverage for both auth flows.

Changes

RI Exchange Dual-Auth Approval Feature

Layer / File(s)	Summary
Database schema and entity types internal/database/postgres/migrations/000053_*, internal/config/types.go	Migration adds nullable created_by_user_id and approved_by TEXT columns to ri_exchange_history. RIExchangeRecord Go struct gains corresponding optional pointer fields with JSON tags for audit attribution.
Store interface and Postgres persistence internal/config/interfaces.go, internal/config/store_postgres.go	StoreInterface adds StampRIExchangeApprovedBy(ctx, id, approverEmail) method. Postgres store updates all RI exchange queries (GetRIExchangeRecord, GetRIExchangeRecordByToken, GetRIExchangeHistory, TransitionRIExchangeStatus, GetStaleProcessingExchanges) to select/scan/return new audit columns. Implements approval stamping with error handling and queryRIExchangeRecords helper updates for nullable field scanning.
Frontend API client and type contracts frontend/src/api/index.ts, frontend/src/api/riexchange.ts, frontend/src/api/types.ts	Exports new approveRIExchange(id: string): Promise<void> function that POSTs to /ri-exchange/approve/{id}. Extends RIExchangeHistoryRecord interface with created_by_user_id and approved_by optional fields for history rendering.
Frontend UI approval button and click handler frontend/src/riexchange.ts	Adds imports for showToast and getCurrentUser. Introduces canApproveRIExchangeRow authorization predicate (pending status only, admin always allowed, non-admin checks creator-match). Conditionally renders Approve button in history table Actions column. Wires click handler: confirmation dialog → disable button → call API → show success/error toast → reload history on success.
Backend session authorization helpers internal/api/handler_ri_exchange.go (lines 697–761)	Three new methods: sessionHasApproveRight (lightweight check for any approve permission via admin fast-path or resource permission check); authorizeSessionApproveRIExchange (enforce approve-any/approve-own rules, reject 403 if creator-id mismatch on approve-own); fetchAndAuthorizeRIExchange (validate UUID, load record, enforce pending status, authorize session before returning).
Backend approval dispatcher and session flow internal/api/handler_ri_exchange.go (lines 579–667)	approveRIExchange refactored into three-mode dispatcher: routes session-present with approve rights to approveRIExchangeViaSession; token-present to legacy path; otherwise falls through for session-gated rejection. approveRIExchangeViaSession acquires session, fetches/authorizes, transitions pending→processing atomically, executes approved exchange, stamps approved_by email (best-effort; failures logged, not returned).
Router wiring and test mock infrastructure internal/api/router.go, internal/api/mocks_test.go, internal/analytics/collector_test.go, internal/purchase/mocks_test.go, internal/scheduler/scheduler_test.go, internal/server/test_helpers_test.go	Router handler passes full req object to approveRIExchange. Test mock stores across all suites extended with StampRIExchangeApprovedBy stub method for compile/call coverage.
Backend test coverage for dual-auth paths internal/api/handler_ri_exchange_test.go, internal/config/store_postgres_nil_db_test.go, internal/config/store_postgres_pgxmock_test.go, internal/api/coverage_extras_test.go	Existing token-only tests updated to match new signature. New tests: TestApproveRIExchange_SessionAdmin (admin approval without token), TestApproveRIExchange_SessionApproveOwn (owner can approve, non-owner rejected 403), TestApproveRIExchange_LegacyTokenStillWorks (backwards-compat). Nil-DB panic test. Pgxmock column/row schema updates for audit fields.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• LeanerCloud/CUDly#505: Adds rate-limiting guards to the same RI exchange approve/reject HTTP route handlers, complementing this PR's approval authorization logic at the endpoint wrapper layer.

Poem

A rabbit with approvals to give, 🐰
Now slides through dashboards, no email link to sieve,
Dual-paths converge where tokens once reigned—
Session and stamp, the creator's domain sustained,
Pending to processing, one click remains! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%.	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 PR title clearly describes the main change: implementing a symmetric session-auth approve path for RI exchanges, mirroring purchase approval work and closing issue #300.
Linked Issues check	✅ Passed	The implementation successfully addresses all coding requirements from issue #300: three-mode dispatch (session RBAC + legacy token + 401), approveRIExchangeViaSession with transitions and attribution stamping, schema migration, frontend Approve button with proper predicates, and comprehensive backend/frontend test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with issue #300 objectives: handler refactoring, session authorization, schema migration, frontend UI, and test coverage. No extraneous modifications detected beyond the symmetric approval implementation scope.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/wave6-purchase-features

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/api/router.go (1)

704-709: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use resolveApprovalToken(req) for RI exchange approval token extraction.

Reading the token only from req.QueryStringParameters["token"] keeps sensitive approval tokens in URL/query logs and diverges from the hardened purchase flow.

💡 Proposed fix

 func (r *Router) approveRIExchangeHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) {
 	if err := r.h.checkRateLimit(ctx, req, "approve_cancel_public"); err != nil {
 		return nil, err
 	}
-	return r.h.approveRIExchange(ctx, req, params["id"], req.QueryStringParameters["token"])
+	token := resolveApprovalToken(req)
+	return r.h.approveRIExchange(ctx, req, params["id"], token)
 }

🤖 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 `@internal/api/router.go` around lines 704 - 709, In approveRIExchangeHandler
replace direct query access of the token (req.QueryStringParameters["token"])
with the secure extractor resolveApprovalToken(req) so the handler calls
r.h.approveRIExchange(ctx, req, params["id"], resolveApprovalToken(req)); this
ensures token extraction follows the hardened purchase flow and avoids exposing
sensitive tokens in URL/query logs — update the callsite in
approveRIExchangeHandler to use resolveApprovalToken instead of reading
QueryStringParameters directly.

🤖 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 `@frontend/src/riexchange.ts`:
- Around line 1009-1016: The catch currently covers both approveRIExchange and
loadExchangeHistory so a refresh failure is reported as an approval failure;
split the flows: call await api.approveRIExchange(id) in its own try block, show
the success toast and only then call loadExchangeHistory in a separate try/catch
that handles refresh errors (e.g., show a different error toast like "Failed to
refresh exchange history" but do not claim approval failed). Ensure btn.disabled
is correctly managed (disable before approval, re-enable in both catch blocks as
needed) and reference the functions approveRIExchange, loadExchangeHistory and
the btn.disabled flag when making these changes.
- Around line 904-911: The canApproveRIExchangeRow predicate only allows admins
or the record owner to approve, missing users granted the approve-any:purchases
permission; update canApproveRIExchangeRow (and use getCurrentUser()) to also
return true when the current user has the approve-any:purchases permission
(e.g., check user.permissions.includes('approve-any:purchases') or call the
existing permission helper if present) while keeping the existing pending-status
and owner/admin checks and the created_by_user_id equality check.

In `@internal/api/handler_ri_exchange_test.go`:
- Line 353: The test currently ignores the return values from
h.approveRIExchange(ctx, req, id, ""), which can hide failures; update the three
call sites (the one shown and the other occurrences around the same test) to
capture the returned error (and result if applicable) and assert it is nil (or
assert the expected error) immediately after the call so the test fails on
regressions rather than only on side effects; reference the call to
h.approveRIExchange and the local vars ctx, req, id when making the assertion.

In `@internal/api/handler_ri_exchange.go`:
- Around line 591-625: The approveRIExchange function currently returns
immediately when sessionHasApproveRight(ctx, session) == nil by calling
approveRIExchangeViaSession, which prevents trying the token path if
approveRIExchangeViaSession returns a permission-denied error for a non-creator;
change the flow so you still call approveRIExchangeViaSession when
sessionHasApproveRight == nil but inspect its returned error: if
approveRIExchangeViaSession returns a permission-denied error (use
isPermissionDenied) and a non-empty token was provided, do NOT return and
instead continue to the token branch (calling validateExchangeApproval /
TransitionRIExchangeStatus / executeApprovedExchange); for any other error or
success, return the result as before. Use the existing symbols
approveRIExchange, sessionHasApproveRight, approveRIExchangeViaSession,
isPermissionDenied, validateExchangeApproval, TransitionRIExchangeStatus, and
executeApprovedExchange to locate and implement this change.

---

Outside diff comments:
In `@internal/api/router.go`:
- Around line 704-709: In approveRIExchangeHandler replace direct query access
of the token (req.QueryStringParameters["token"]) with the secure extractor
resolveApprovalToken(req) so the handler calls r.h.approveRIExchange(ctx, req,
params["id"], resolveApprovalToken(req)); this ensures token extraction follows
the hardened purchase flow and avoids exposing sensitive tokens in URL/query
logs — update the callsite in approveRIExchangeHandler to use
resolveApprovalToken instead of reading QueryStringParameters directly.

🪄 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

Run ID: eb2fbf94-29df-4449-b804-819cd2937ba2

📥 Commits

Reviewing files that changed from the base of the PR and between bc8831b and bd20098.

📒 Files selected for processing (20)

• frontend/src/api/index.ts
• frontend/src/api/riexchange.ts
• frontend/src/api/types.ts
• frontend/src/riexchange.ts
• internal/analytics/collector_test.go
• internal/api/coverage_extras_test.go
• internal/api/handler_ri_exchange.go
• internal/api/handler_ri_exchange_test.go
• internal/api/mocks_test.go
• internal/api/router.go
• internal/config/interfaces.go
• internal/config/store_postgres.go
• internal/config/store_postgres_nil_db_test.go
• internal/config/store_postgres_pgxmock_test.go
• internal/config/types.go
• internal/database/postgres/migrations/000053_ri_exchange_approver_creator.down.sql
• internal/database/postgres/migrations/000053_ri_exchange_approver_creator.up.sql
• internal/purchase/mocks_test.go
• internal/scheduler/scheduler_test.go
• internal/server/test_helpers_test.go

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.