feat(insurance): compare one stored recommendation with its Archera line item

[cristim]

What and why

GET /api/insurance/comparison?recommendation_id=<id> compares one stored recommendation with its own Archera line item. Without the param the plan comparison is unchanged. Extracts h.insuranceClient(ctx) from the plan handler (no behaviour change).

Order: auth gate, ID parse, exactly one stored row (before any secret or vendor call), projection (422 if unsupported), then under a 20s child context (client and secret resolution, all LineItems, SelectLineItem, scoped Comparison with LineItemIDs=[selected] and all three payment options), then a reread on the parent context compared with Equal, then the DTO with the existing 5 MiB guard. No line item or an ambiguous one returns the unavailable DTO with no Comparison call. No vendor text or key reaches errors or logs.

Raw query: the hook fires when the key is in the map or in the unescaped raw keys (split on & and ;). More than one raw occurrence is 400; raw-present but map-missing yields an empty ID, 400.

Error mapping

Condition	Status
empty or over 1024 byte ID, repeated key	400
denied / scoped session	existing 403 / 404
zero rows	404
several rows, or row changed/deleted/store error on reread	409
store error or no store	503
unsupported recommendation	422
client without LineItems	503
deadline	504
vendor error	existing mapping (502, 429 with Retry-After)

Tests and mutations (synthetic fixtures, through the real Router)

All new tests are in handler_insurance_recommendation_test.go with fake store/client and decoded synthetic vendor JSON. Hand mutation probes, each reverted, each fails a named test: auth gate (DeniedOrScoped), exact-one (StoreOutcomes/duplicate), Equal (RereadChange), reread (RereadChange/deleted), deadline mapping (VendorFailures/deadline), payment options and selected ID (MatchesThroughRouter), duplicate guard (QueryRules encoded_repeated, semicolon), router hook (MatchesThroughRouter, i.e. the plan DTO is returned before the hook exists). Not covered by this PR: real HTTP client path (PR4).

Size note: about 526 added lines, over the 400 target; tests are 326 of them.

Refs #799

Stack: based on #1058 (DTO), which is on #1056 and #1054. Base is feat/799-archera-recommendation-dto.

🤖 Generated with Claude Code

[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: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: ef196041-5c39-4dc6-a8da-e143686ea7d3

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

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[cristim]

Independent review verdict: CLEAN at 5fd1350

Reviewer: a fresh Claude agent (Sonnet 5.5, high effort, security focus), not the author, with a cold brief. It reviewed this PR's own commit (df66be15..5fd1350a). The body's error table and ordering were treated as claims. Coordinator: claude-cc-reviewer.

Blocking findings: none.

Verified

• AuthZ.

  • requireInsuranceAccess (handler_insurance.go:36-49) requires view recommendations plus scope.AllowsAll(), and otherwise returns 404. This is the same gate as the plan comparison.
  • Denied, scoped and account-restricted sessions get 404 and make no store or vendor call. The test DeniedOrScopedMakesNoCalls pins this.
  • The client and secret come from the deployment-wide h.insurance, not from the row.

• Ordering. The handler runs these steps in order:

  1. the gate;
  2. ID parse;
  3. the exactly-one-row check;
  4. ProjectRecommendation (422 if it fails);
  5. a 20 s child context, inside which it gets the client and secret, then LineItems, then SelectLineItem, then the scoped Comparison (selected line item ID, all three payment options);
  6. the reread;
  7. building the DTO, with the 5 MiB guard.

  The reread cannot be skipped. The unavailable-DTO path also goes through recommendationUnchanged.

• Error table. Each row matches the code: 400, 404, 409, 503, 422 and 504, with 502 and 429 coming from mapInsuranceError.

• No leaks. Every error message is fixed text. The only vendor-derived values are the HTTP status and Retry-After. Both are logged as an integer and a duration, and Retry-After is rounded to whole seconds.

• Query parsing.

  • Repeated, encoded or ;-separated keys return 400, and so do an empty ID and one over 1024 bytes.
  • recommendation_id[] and mixed-case keys are left unmatched, so they fall through to the unchanged plan path.
  • When the param is absent, existing callers are unaffected (router.go:524; TestInsuranceRecommendationAbsentParamKeepsPlanComparison).

• Refactor. The extracted insuranceClient is a byte-for-byte lift of the old inline block.

• Mutation probes. Each one failed a named test:

  • skip the reread;
  • accept a repeated key;
  • map the deadline to 502;
  • resolve the secret before the row check;
  • remove the auth gate.

Local verification (fixture-based, GOWORK=off)

• go build ./... and go vet ./internal/api/ are clean.
• go test -race ./internal/api/... -run Insurance passes.
• Local lint was blocked by another session's lock. CI Lint Code must pass after the retarget.

Non-blocking notes

1. ListStoredRecommendations(RecommendationFilter{ID:id}) (handler_insurance_recommendation.go:120) has no tenant clause, because the recommendations table has no tenant column (store_postgres_recommendations.go:291). Isolation relies on one database per tenant plus the full-access gate. If tenants ever share a database, this lookup will need a scope clause.
2. After the gate, a 404 versus a 409 reveals only that a recommendation exists, and only to viewers who already have full access.
3. A NUL or invalid-UTF-8 ID reaches Postgres and returns 503 instead of 400. It fails closed, but a 400 would be cleaner.
4. A cancelled parent context returns 504, or 409 on the reread. Both fail closed.

Gate

• The PR is stacked on feat(archera): build the recommendation comparison response with exact decimals and null currency #1058, which is blocked on 5 lint issues. CI here is partial (frontend only) until it targets main. It will merge after feat(archera): build the recommendation comparison response with exact decimals and null currency #1058, once it has been retargeted and full CI is green.
• Labels are mirrored from feat(insurance): per-recommendation Archera quotes (blocked on an Archera API mapping) #799.
• CodeRabbit skipped the review because of the base branch.

[cristim]

Split to meet the 400-line cap, with every test kept. The vendor fetch and the insuranceClient extraction moved to #1063 (PR3a, 248 added lines), and this PR is now based on it. This PR (489ca273, 377 added lines) keeps the router hook, query rules, handler order, post-fetch reread and router-level tests. GOWORK=off go test ./internal/api/ passes at 489ca27 (3844 tests). Each guard's mutation probe was re-run on the PR that now owns it, and each one failed a named test.

[cristim]

Re-review after the restack onto #1063. CLEAN at 489ca27, reviewed by the same reviewer as the 5fd1350 verdict (Sonnet 5.5, high). The head was re-checked right before reporting.

• Fetch group moved to feat(insurance): fetch one recommendation's Archera comparison #1063 unchanged. fetchRecommendationComparison, readRecommendationVendor, lineItemClient and the 20 s timeout now live in feat(insurance): fetch one recommendation's Archera comparison #1063 as FetchRecommendationComparison. The logic is identical:

  • 504 on deadline;
  • mapInsuranceError for errors;
  • SelectLineItem, with no vendor call when nothing matches;
  • a scoped comparison with LineItemIDs=[selected] and all three payment options.

• getInsuranceRecommendation keeps the same order. It now calls h.FetchRecommendationComparison, and the steps run as before:

  1. auth gate;
  2. ID parse;
  3. exactly-one row check, which still precedes any secret or vendor call;
  4. projection (422);
  5. fetch;
  6. reread on the parent context, still unskippable;
  7. DTO and the 5 MiB guard.

  Query parsing, the error table and the router hook are unchanged.

• Probes (each reverted). Each one fails:

  • skipping the reread;
  • accepting a repeated key;
  • resolving the secret before the row check;
  • removing the auth gate.

• Checks. go build, go vet and go test -race ./internal/api/... -run Insurance pass. These are fixture-based.

• Earlier notes still stand. The store lookup has no tenant clause, so isolation rests on the full-access gate and one DB per tenant. A NUL or invalid-UTF-8 ID returns 503 instead of 400, which fails closed.

Gate: merges after #1058 and #1063, once retargeted to main with full CI green. (claude-cc-reviewer)

[cristim]

Independent delta review of 489ca27 (previous: 5fd1350): clean

Reviewer: fresh agent (non-author), dispatched by claude-cc-pr-merge-2.

• 5fd1350 is not an ancestor; the PR was split. git range-diff df66be15...5fd1350a a8d6be38...489ca273 shows one commit whose fetch and insuranceClient extraction moved out to feat(insurance): fetch one recommendation's Archera comparison #1063 (a8d6be3, CLEAN). The rest of the diff is unchanged: +377 across handler_insurance_recommendation.go, its test and router.go.
• Auth and rec provenance:
  • requireInsuranceAccess runs first (handler_insurance_recommendation.go:60-61; handler_insurance.go:36-49): view recommendations plus scope.AllowsAll(), with 404 otherwise. There is no store, secret or vendor call before it.
  • The request supplies only the id lookup key: non-empty, at most 1024 bytes, and a single occurrence, including the encoded and ; spellings (:64-67).
  • rec is archera.ProjectRecommendation of the single stored row (:68-72, :103-117): 0 rows gives 404, more than 1 gives 409, a store error gives 503. No identity field comes from the request. The plan id and the client come from config.
  • The post-fetch reread requires rec.Equal (:80), otherwise 409.
  • Probe: no-op'ing the auth gate fails TestInsuranceRecommendationDeniedOrScopedMakesNoCalls.
• GOWORK=off go vet is clean, and go test ./internal/api/ ./internal/archera/ passes 4002.
• Non-blocking, as before: the lookup has no tenant clause because the table has no tenant column (one DB per tenant plus the full-access gate), and a NUL or invalid UTF-8 id returns 503, not 400.
• An unparam lint fix from the author is incoming; that commit gets its own delta check.