fix: let the backend detect the axiom region - #60
Conversation
Axiom has no regional console or API hosts (app.axiom.co / api.axiom.co everywhere); the region only surfaces as the org's edge deployment under Settings > General, which the old prompt pointed at vaguely. Collect the token first and detect the region from /v2/orgs (defaultEdgeDeployment) with /v2/datasets (edgeDeployment) as fallback; only ask — with exact directions — when detection is ambiguous. A 401 from either probe surfaces as a bad-token error instead of a wrong region. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MefBGfjDGUX8nRkF1YrE9w
|
@claude Please review this PR at head 1b61660. CI is green on Node 20.x, 22.x, and 24.x. The review procedure is in REVIEW.md. Generated by Claude Code |
The API now serves region as optional on integrations.connect and detects it from the token server-side, so the client-side probing of Axiom's /v2/orgs and /v2/datasets is gone. The axiom flow sends the connect request without region (--region stays as an explicit override) and only asks for the region when the backend answers 422 — interactively via a picker pointing at Axiom Settings > General > Edge deployment, non-interactively as a usage error hinting at --region. mapApiError now returns ApiError (a CLIError carrying the HTTP status), with 422 mapped to a usage error, so commands can react to specific statuses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MefBGfjDGUX8nRkF1YrE9w
|
@claude please review the new head (51c029f). The branch was reworked: the client-side probing of Axiom's Generated by Claude Code |
There was a problem hiding this comment.
LGTM: clean rework that matches the described flow and the live API contract, with solid test coverage.
Verified against head 51c029f:
- Confirmed via a fresh
npm run codegenagainst the live spec thatregionis genuinely optional server-side now ({ type: "axiom"; workspaceId: string; region?: "us-east-1" | "eu-central-1"; apiToken: string }ingenerated/client.ts), so this isn't racing ahead of the backend deploy. - The earlier client-side probing of Axiom's
/v2/orgsand/v2/datasetsis fully gone — grepped the repo (src and test) fordefaultEdgeDeployment/v2/orgs/v2/datasets, no hits. No orphaned code. connectAxiom(src/commands/integration/connect.ts:151-176) only intercepts a 422 when noregionwas already sent, so a bad explicit--regionor a rejected picked region can't loop — it just propagates. Good, deliberate choice, not a bug.--regionis validated up front for axiom (connect.ts:597-604) before the wizard runs, so a typo errors immediately in both interactive and non-interactive modes.- Non-interactive/agent path throws a
CLIError(ExitCode.USAGE) with a--regionhint instead of hanging on a prompt; interactive path shows the backend's message plus the exact Axiom UI path vianote(), then re-picks withpromptSelectOrBack, matching the rest of the wizard's Back/Cancel conventions. mapApiErrornow returnsApiError extends CLIError, so existing call sites typed againstCLIErroraren't broken (checkedclient/http.tsand other catch sites).- Ran the repo's own procedure end to end:
npm ci,npm run codegen(fetched the live spec fine),npm run typecheck,npm run lint,npm run test— all green, 311/311 tests passing, matching the PR body's claim. - New tests (
test/integration-connect-axiom.test.ts) cover: success without region, 422→usage-error in non-interactive mode, rethrow when region was already explicit, and rethrow of non-422 errors untouched.test/errors.test.tscovers the new 422 mapping and that.statusis preserved.
One thing worth a human's eye rather than a blocker: mapApiError's 422 case moved from the default branch (ExitCode.GENERAL) to an explicit ExitCode.USAGE. That mapping is used globally in client/http.ts for every API call, not just Axiom connect, so it changes the exit code for any other 422 the backend might return elsewhere in the CLI. Semantically USAGE seems like the right call for "unprocessable" and nothing else in the suite relies on 422→GENERAL, but flagging it since it's a small scope creep beyond the Axiom flow — worth a mental note in case another 422 producer surfaces later expecting the old code.
Nit: no test exercises the interactive retry branch of connectAxiom (prompt → re-picked region → success) — understandable given promptSelectOrBack isn't easily mocked here, and the non-interactive + rethrow paths are the ones with real branching logic, so not blocking.
CI is green on Node 20.x/22.x/24.x (checked via the API, not just the badge). Merge is otherwise a human call per this org's policy.
Generated by Claude Code
There was a problem hiding this comment.
Auto-approved: Claude reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.
Generated by Claude Code
Requested by boris · Slack thread
Before:
polylane integration connect --type axiomopens by asking for the Axiom edge deployment region (US East 1 / EU Central 1) and points at "your organization settings" — a place where the region is genuinely hard to find, since Axiom's console and API URLs are the same for every region.After: the flow asks only for the API token and connects — the backend detects the region from the token. The region question appears only when the backend answers that it couldn't (422), and then it says exactly where to look: Settings > General > Edge deployment in Axiom. Non-interactive/agent runs surface the backend's message as a usage error with a
--regionhint;--regionstill wins as an explicit override.This removes the one question in the Axiom connect flow that users couldn't reliably answer, by letting the backend answer it.
How: the API now serves
regionas optional onintegrations.connectand detects it from the token server-side, answering 422 with an explanatoryerror.detailwhen it can't. The client is regenerated from the live spec; the earlier client-side probing of Axiom's/v2/orgsand/v2/datasetsis gone.mapApiErrornow returnsApiError(aCLIErrorcarrying the HTTP status, 422 → usage error) so the connect flow can react to the 422 and retry with the picked region.Checks:
npm run typecheck,npm run lint,npm run test(311 pass),npm run buildall green.🤖 Generated with Claude Code
https://claude.ai/code/session_01MefBGfjDGUX8nRkF1YrE9w
Generated by Claude Code