Skip to content

feat(insurance): add the Archera comparison route and error mapping - #797

Merged
cristim merged 14 commits into
mainfrom
feat/785-archera-route
Oct 10, 2026
Merged

cristim merged 14 commits into
mainfrom
feat/785-archera-route

Conversation

@cristim

@cristim cristim commented Oct 9, 2026

Copy link
Copy Markdown
Member

PR2b-2 of #785, stacked on #796 (base is that branch; retarget to main after #796 merges). Security-sensitive: handles an API key path, authz and vendor error text.

  • GET /api/insurance/comparison behind the status route's gate: view:recommendations plus an unrestricted account scope, else 404 (scoped sessions and scoped user-API-key principals tested). Unconfigured or incomplete settings return 503 naming the missing settings (names only); an unresolvable key returns a fixed 503 naming ARCHERA_API_KEY_SECRET. No outbound request in either case.
  • Calls Client.Comparison with only the configured PlanID (no filters). The raw *insurance.Client exists only as a local in the handler.
  • Errors are mapped to fixed text: 401/403 and 404 to 502, 5xx to 502, any non-HTTPError (decode, transport, oversized body) to one fixed 502. 429 surfaces Retry-After as a header and retry_after_seconds in JSON only when the vendor gave a positive value. HTTPError.Message and the key are never returned or logged; only the status code and Retry-After are logged. clientError gains optional response headers (NewClientErrorWithHeaders).
  • The marshaled DTO is capped at 5 MiB; larger fails with 502 'comparison too large to return' instead of dropping rows.
  • Tests: production composition (env -> secret resolver -> lazy client -> router -> handler) with a RoundTripper asserting https, api.archera.ai, the documented path, x-api-key and an empty query; a vendor 401 whose body echoes the key and a fragment is absent from the response and captured logs; an AST test that both api.HandlerConfig literals in app.go pass Insurance; app.go passes hc=nil to NewProvider. Mutations that each fail a test: AllowsAll removed, HTTPError.Message passthrough, Retry-After surfaced at 0, size cap removed.

Verification (local macOS, synthetic data, no live Archera calls): go build, vet, golangci-lint 0 issues, go test for every internal package, and go test -tags integration for internal/api, internal/server and internal/purchase, all exit 0.

Refs #785 #786

🤖 Generated with Claude Code

cristim and others added 7 commits October 9, 2026 19:10
…atus endpoint

Adds internal/archera (ARCHERA_ORG_ID, ARCHERA_PLAN_ID, ARCHERA_API_KEY_SECRET;
key resolved lazily through the secret resolver, mutex-guarded so a failed
resolve is retried) and GET /api/insurance/status, gated on view:recommendations
plus an unrestricted account scope (scoped sessions get 404). Status reports
setting presence by name only and makes no outbound call. The comparison
endpoint follows in a separate PR.

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Maps pkg/insurance.Comparison to an explicit snake_case DTO: exact decimal
strings from big.Rat (null for unknown, error for non-terminating values, no
float64), vendor strings stripped of control/format characters and capped at
256 bytes, per-offer AssessProductSupport (source and evidence only when
supported), lease_attached, no org ID, currency null with a note, and the
required Archera disclosures. The route follows in a separate PR.

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
GET /api/insurance/comparison, behind the status route's gate
(view:recommendations plus an unrestricted account scope, else 404). Vendor
errors map to fixed client text, 429 surfaces Retry-After (header and JSON)
only when the vendor gave a positive value, the vendor message and key are
never returned or logged, and a response over 5 MiB fails instead of dropping
rows. clientError gains optional response headers.

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint effort/l Weeks triaged Item has been triaged impact/few Limited audience type/feat New capability labels Oct 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 8 billable files and costs up to $2.00.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 18 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 88 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 4b7d7f56-db33-4d28-b78a-cbf4ee3ff0d8

📥 Commits

Reviewing files that changed from the base of the PR and between 5bdb189 and a2736b5.


📒 Files selected for processing (8)
  • internal/api/handler.go
  • internal/api/handler_insurance.go
  • internal/api/handler_insurance_comparison.go
  • internal/api/handler_insurance_comparison_test.go
  • internal/api/handler_insurance_test.go
  • internal/api/handler_router.go
  • internal/api/router.go
  • internal/server/archera_wiring_test.go

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

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

cristim and others added 5 commits October 10, 2026 04:27
…lden fixture

One fixture gives every numeric field a distinct value (hundreds digit names
the block, units digit the field) and compares the full JSON to a golden file;
25 field-mapping mutations (swaps, nils, blanked blocks, dropped term/payment/
is_current/provider) each fail. Also tests PlanID cleaning and passes the real
testing.T to the number helpers.

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ison route

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The basis note said upfront figures are one-time dollars, but Archera provides
no currency and the DTO reports currency null. Say one-time amounts, update the
golden file, and test that no platform-authored note names a currency.

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim
cristim changed the base branch from feat/785-archera-dto to main October 10, 2026 02:49
@cristim

cristim commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

Gate review at 9f422e6 (base retargeted to main after #796 merged; the tree diff against main is only this PR's 8 files): CHANGES REQUESTED (not merging).

What holds (mutations run from git archive; each fails a test): the access gate removed; AllowsAll removed; HTTPError.Message passed through; err text logged; Retry-After sent when 0; Retry-After header renamed; 5xx mapped to non-502; the 5 MiB cap removed; hc non-nil in app.go; a filter added to the request; Insurance: removed from reinitializeAfterConnect. The distinct-value composition test pins every money field. Logs and responses never contain the key or a fragment of it (401 echo case). The scoped user API-key principal gets 404 with no outbound call.
Local (go1.26.9): go build 0, go vet 0, go test ./internal/... 0, golangci-lint api/server 0 issues, go test -count=1 -tags integration ./internal/api/ ./internal/server/ 0 under the test-suite lock (api 178.1s, server 30.2s).

Findings:

  1. (medium) internal/api/handler.go:807: deleting corsHeaders = errorResponseHeaders(err, corsHeaders) passes every test, so the Retry-After header never reaching the real response would go unnoticed. TestInsuranceComparison_RetryAfterHeaderReachesResponse calls the helper directly instead of the request path. Fix: drive the real path (HandleRequest/executeRequest, or the Router through to the LambdaFunctionURLResponse) with a vendor 429 Retry-After: 120 and assert resp.Headers["Retry-After"] == "120"; assert the header is absent for the no/zero Retry-After cases.
  2. (low) internal/server/archera_wiring_test.go only checks that the Insurance key exists, so Insurance: nil in reinitializeAfterConnect passes. Assert the value is the app.archera selector (or archeraProvider in the first literal).
  3. (nit) Swapping the 404 message ("organization or plan was not found") with the 401/403 one passes. Assert the exact message per status case in TestInsuranceComparison_VendorErrorsAreSanitized.
    CI: the full matrix has not run since the retarget; the fix push will trigger it.

cristim and others added 2 commits October 10, 2026 05:06
…pin wiring

Adds a HandleRequest-level test for the Retry-After header and
retry_after_seconds (present when positive, absent at 0 or missing), pins the
message for every mapped vendor status, and makes the app.go wiring test
require the same provider instance in reinitializeAfterConnect.

Refs #785 #786

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim

cristim commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

Gate re-run at a2736b5: CLEAN, merging.

  • Since 9f422e6: a normal merge of main (5bdb189, feat(insurance): add the Archera comparison DTO with exact decimals #796) plus one test-only commit (handler_insurance_comparison_test.go +61, archera_wiring_test.go +6). The tree diff against main is only this PR's 8 files.
  • All three findings are closed, each proven by mutation:
    • Deleting corsHeaders = errorResponseHeaders(err, corsHeaders) (handler.go:807) now fails the HandleRequest-path test.
    • Insurance: nil in reinitializeAfterConnect now fails the wiring test.
    • Swapping the 404 and 401 messages now fails the per-status message test.
  • The earlier mutations still fail: access gate removed, AllowsAll removed, HTTPError.Message passthrough, err text logged, Retry-After when 0, header renamed, 5xx code, 5 MiB cap, hc non-nil, filter added, Insurance dropped. One survivor, passing ErrKeyUnavailable's text through, is harmless because that text is fixed.
  • Local from git archive (go1.26.9, GOWORK=off, -mod=readonly): go build 0, go vet 0, go test ./internal/... 0, golangci-lint api/server/archera 0 issues. go test -count=1 -tags integration ./internal/api/ ./internal/server/ ./internal/purchase/ 0 under the test-suite lock (api 118.7s, server 27.0s, purchase 17.5s).
  • CI: the full matrix ran, 26/26 pass, mergeStateStatus CLEAN at this SHA, base main.

@cristim
cristim merged commit f7ffabc into main Oct 10, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/l Weeks impact/few Limited audience priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/feat New capability urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant