feat(integration): post-OAuth accessible-resources picker + set-metadata CLI/SDK#142
Merged
Merged
Conversation
…ata CLI/SDK
Cross-repo consumer of cloud's new GET .../accessible-resources and
PUT .../metadata routes. Together with the cloud-side change this
closes the loop on Jira/Confluence multi-tenant setup (cloud#535):
operators no longer need to dive into the Nango dashboard to pick a
cloudId after OAuth completes.
TS SDK (packages/sdk/typescript):
- WorkspaceHandle.listAccessibleResources(provider) returns
[{ id, url, name?, scopes?, avatarUrl? }] for Atlassian-family
providers; Cloud's typed 400 for non-Atlassian providers surfaces
as CloudApiError so callers can branch on the code.
- WorkspaceHandle.setIntegrationMetadata(provider, metadata) PUTs the
metadata namespace (full replacement); rejects non-object metadata
locally before hitting Cloud so the failure mode is immediate.
- 6 new vitest cases covering happy path, junk-entry filtering,
cloud error pass-through, and runtime guards.
Python SDK (packages/sdk/python):
- RelayFileClient and AsyncRelayFileClient gain list_accessible_resources
and set_integration_metadata. Both clients also accept a
cloud_base_url constructor arg because cloud's control plane is a
different host from the relayfile data plane.
- 6 new respx-backed tests across sync + async paths.
Go CLI (cmd/relayfile-cli/main.go):
- `relayfile integration connect jira` (and `... confluence`) now
calls accessible-resources after OAuth completes:
* 0 sites: warn and fail (continuing would queue a sync that
cannot succeed).
* 1 site: auto-set { cloudId, baseUrl } and log "auto-selected".
* >1 sites: numbered picker with default-to-1 on blank input,
validates range, retries up to 3 times then errors out.
Non-Atlassian providers skip the picker entirely so the existing
connect flow is unchanged.
- New `relayfile integration set-metadata PROVIDER KEY=VALUE [...]`
verb for general-purpose metadata writes (any provider, not just
Jira). v1 accepts flat KEY=VALUE pairs only; nested keys are
documented as a future enhancement.
- The picker uses a single bufio.Reader for the retry loop rather
than promptLine, because promptLine creates a fresh bufio reader
per call and drops bytes buffered past the first newline — that
would break the retry path when stdin is a pre-loaded reader.
- Usage banner updated; 7 new Go tests covering set-metadata happy
path, KEY=VALUE validation, single-site auto-select, multi-site
interactive pick, blank-input default, retry-then-error, and the
non-Atlassian no-op path.
Validation:
- npm test --workspace packages/sdk/typescript: 144 passing
- pytest packages/sdk/python: 52 passing
- go test ./cmd/relayfile-cli/: passing
- go build ./...: clean
Depends on cloud#540; this PR is draft until that lands and deploys.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces cloud-control-plane integration setup capabilities across the SDK and CLI. TypeScript and Python SDKs gain public methods to list accessible OAuth resources and update integration metadata. The CLI adds a ChangesIntegration Metadata and Accessible Resources
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔴 Atlassian site picker missing from runSetup flow, causing Jira/Confluence setup to fail
The Atlassian site picker (runAtlassianSitePicker) is only wired into the runIntegrationConnect path (cmd/relayfile-cli/main.go:1464-1468) but is completely absent from the runSetup path. When an operator runs relayfile setup --provider jira, the code calls connectCloudIntegration (line 562) or ensureCloudIntegration (line 566) and then proceeds directly to waitForInitialSync (line 571) without ever invoking the site picker. The PR's own comments at cmd/relayfile-cli/main.go:1457-1462 state that "Cloud's Jira/Confluence sync bails with a clear error if metadata.cloudId is unset and the grant has >1 site." This means the guided relayfile setup --provider jira flow will complete OAuth but then hang or fail on the initial sync wait because no cloudId metadata was set for multi-site grants.
(Refers to lines 560-574)
Prompt for agents
The runSetup function in cmd/relayfile-cli/main.go (around lines 560-574) calls connectCloudIntegration or ensureCloudIntegration and then waitForInitialSync, but never invokes the Atlassian site picker for jira/confluence providers. The fix should mirror what runIntegrationConnect does at lines 1464-1468: after the connectCloudIntegration/ensureCloudIntegration call succeeds but before waitForInitialSync, check isAtlassianProvider(selectedProvider) and if true call runAtlassianSitePicker(tokenSet.APIURL, record.ID, joined.Token, selectedProvider, stdin, stdout). The stdin parameter is already available in runSetup's signature. This should be added in both the createdWorkspace and the else branch, or after the if/else block but before the waitForInitialSync call.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/sdk/typescript/src/setup.test.ts (2)
568-587: ⚡ Quick winAssert the “no cloud call” guarantee explicitly in the local-validation test.
The test title says validation happens before Cloud, but it currently doesn’t verify fetch call count after invalid inputs.
Proposed patch
- queueFetch(makeJoinResponse()) + const fetchMock = queueFetch(makeJoinResponse()) @@ await expect( // `@ts-expect-error` - exercising runtime guard handle.setIntegrationMetadata("jira", ["array"]) ).rejects.toThrow(/metadata must be a plain object/) + expect(fetchMock).toHaveBeenCalledTimes(1)🤖 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 `@packages/sdk/typescript/src/setup.test.ts` around lines 568 - 587, The test currently checks runtime validation errors from handle.setIntegrationMetadata but doesn't assert that no network call was made; after each invalid-call expect(...).rejects, also assert the fetch/mock queue hasn't been consumed (i.e., verify no Cloud fetch was attempted). Update the test around RelayfileSetup/handle.setIntegrationMetadata and queueFetch(makeJoinResponse()) to check the fetch mock call count or remaining queue length after each rejection (use the existing fetch-mocking utilities in the test suite to assert zero additional fetches occurred).
424-525: ⚡ Quick winAdd one normalization-focused test for provider input casing/whitespace.
Given the new
WorkspaceIntegrationProvider | stringsupport, a dedicated test for" JiRa "(success) and""(throws) would lock in the intended contract.🤖 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 `@packages/sdk/typescript/src/setup.test.ts` around lines 424 - 525, Add a normalization-focused unit test in the existing setup.test.ts suite that covers the new WorkspaceIntegrationProvider | string support: create a test that uses RelayfileSetup and joinWorkspace to call handle.listAccessibleResources with a provider string containing mixed case and surrounding whitespace (e.g., " JiRa ") and assert it behaves like "jira" (returns the expected resources and issues the same request URL/Authorization), and add another assertion or separate test that calling listAccessibleResources with an empty string ("") rejects/throws (matching the SDK’s error behavior). Reference the RelayfileSetup class and the listAccessibleResources method when adding the test so it validates input trimming/lowercasing and empty-string rejection.packages/sdk/python/tests/test_client.py (1)
661-707: ⚡ Quick winAdd async negative-path parity tests for the new APIs.
Please mirror the sync negative cases in async (
non-dict metadataand cloud400 invalid_metadata/provider_has_no_accessible_resources) to lock in behavior parity.As per coding guidelines,
packages/sdk/python/**/*client*.py: “Maintain both sync and async client behavior when adding endpoints or retry/error handling.”🤖 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 `@packages/sdk/python/tests/test_client.py` around lines 661 - 707, Add async negative-path tests mirroring the sync suite to ensure parity: in the TestIntegrationSetupAsync class add a test that calls AsyncRelayFileClient.set_integration_metadata with a non-dict metadata argument and asserts the client raises the same exception as the sync version; also add tests that mock the cloud endpoints (accessible-resources and integrations/jira/metadata) to return 400 JSON errors like {"ok": False, "error": {"code": "provider_has_no_accessible_resources"}} and {"ok": False, "error": {"code": "invalid_metadata"}} and assert the async client (AsyncRelayFileClient.list_accessible_resources and set_integration_metadata) surface those error conditions exactly as the sync tests expect.
🤖 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 `@cmd/relayfile-cli/main_test.go`:
- Around line 2603-2615: The test HTTP handler currently silently returns 200 {}
for unexpected paths, letting waitForInitialSync and the connect flow miss or
call the wrong routes without failing; update the default branch of the switch
(the handler that matches strings.HasPrefix for
"/v1/workspaces/ws_123/syncs/jira" and "/v1/workspaces/ws_123/fs/tree") to fail
the test on any unexpected URL: return a non-2xx status (e.g.
http.StatusNotFound or http.StatusInternalServerError) and include the request
path in the response body (or panic) so the test fails loudly when
waitForInitialSync (the function under test) does not hit the real
"/v1/workspaces/<id>/sync/status" endpoint.
In `@cmd/relayfile-cli/main.go`:
- Around line 1454-1468: The Atlassian site picker is being invoked
unconditionally even when ensureCloudIntegration(...) did not perform a new
OAuth connect, which can overwrite existing metadata; change
ensureCloudIntegration to surface whether a new connection was created (e.g.,
return (bool, error) or set a returned/side-channel flag) and update the caller
in main.go to capture that "created" result from
ensureCloudIntegration(cloudCreds.APIURL, record.ID, joined.Token, provider,
requestedBackend, record.LocalDir, *timeout, !*noOpen, stdout) and only call
runAtlassianSitePicker(cloudCreds.APIURL, record.ID, joined.Token, provider,
stdin, stdout) when the created flag is true and isAtlassianProvider(provider)
is true; keep error handling identical for the updated ensureCloudIntegration
signature.
- Around line 2044-2055: The metadata parsing loop currently accepts keys like
"foo.bar" or "a[b]" but the CLI contract requires flat keys only; update the
loop that builds the metadata map (the for _, raw := range rest[1:] block that
sets key, value and metadata[key]=value) to validate the trimmed key and reject
any nested-looking keys (e.g., containing '.', '[' or ']') by returning an error
(use the existing fmt.Errorf pattern and include raw and key for context) so
only plain flat KEY names are accepted before adding to metadata.
In `@packages/sdk/python/src/relayfile/client.py`:
- Around line 880-885: The code silently returns the original input variable
metadata when the cloud response is malformed (payload missing or has non-dict
"metadata"); instead, change the behavior to fail fast by raising a clear
exception (e.g., ValueError or a module-specific InvalidResponseError) that
includes the offending payload for debugging. Replace the current pattern (if
isinstance(payload, dict): updated = payload.get("metadata"); if
isinstance(updated, dict): return updated; return metadata) with a strict
validation that raises when payload is not a dict or payload.get("metadata") is
missing/not a dict, and apply the same change to the other matching block in
client.py that uses the same payload/metadata pattern.
In `@packages/sdk/typescript/src/setup.ts`:
- Around line 838-850: The current validation accepts non-plain objects
(Date/Map) and the response check allows arrays; update the input and output
guards in the setIntegrationMetadata flow: for the input `metadata` (before
calling this._setup.requestJson) require typeof metadata === "object" &&
metadata !== null && !Array.isArray(metadata) && Object.getPrototypeOf(metadata)
=== Object.prototype (or equivalent "plain object" check) and reject otherwise;
after the request, ensure `response.metadata` is also a plain object (same
check: object, non-null, not Array, prototype === Object.prototype) before
returning it as Record<string, unknown>, and throw or surface an error if the
response shape is invalid. Use the same strict checks around the response from
this._setup.requestJson with operation "setIntegrationMetadata".
---
Nitpick comments:
In `@packages/sdk/python/tests/test_client.py`:
- Around line 661-707: Add async negative-path tests mirroring the sync suite to
ensure parity: in the TestIntegrationSetupAsync class add a test that calls
AsyncRelayFileClient.set_integration_metadata with a non-dict metadata argument
and asserts the client raises the same exception as the sync version; also add
tests that mock the cloud endpoints (accessible-resources and
integrations/jira/metadata) to return 400 JSON errors like {"ok": False,
"error": {"code": "provider_has_no_accessible_resources"}} and {"ok": False,
"error": {"code": "invalid_metadata"}} and assert the async client
(AsyncRelayFileClient.list_accessible_resources and set_integration_metadata)
surface those error conditions exactly as the sync tests expect.
In `@packages/sdk/typescript/src/setup.test.ts`:
- Around line 568-587: The test currently checks runtime validation errors from
handle.setIntegrationMetadata but doesn't assert that no network call was made;
after each invalid-call expect(...).rejects, also assert the fetch/mock queue
hasn't been consumed (i.e., verify no Cloud fetch was attempted). Update the
test around RelayfileSetup/handle.setIntegrationMetadata and
queueFetch(makeJoinResponse()) to check the fetch mock call count or remaining
queue length after each rejection (use the existing fetch-mocking utilities in
the test suite to assert zero additional fetches occurred).
- Around line 424-525: Add a normalization-focused unit test in the existing
setup.test.ts suite that covers the new WorkspaceIntegrationProvider | string
support: create a test that uses RelayfileSetup and joinWorkspace to call
handle.listAccessibleResources with a provider string containing mixed case and
surrounding whitespace (e.g., " JiRa ") and assert it behaves like "jira"
(returns the expected resources and issues the same request URL/Authorization),
and add another assertion or separate test that calling listAccessibleResources
with an empty string ("") rejects/throws (matching the SDK’s error behavior).
Reference the RelayfileSetup class and the listAccessibleResources method when
adding the test so it validates input trimming/lowercasing and empty-string
rejection.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c9260420-139e-408f-8052-95013ebfc451
📒 Files selected for processing (6)
cmd/relayfile-cli/main.gocmd/relayfile-cli/main_test.gopackages/sdk/python/src/relayfile/client.pypackages/sdk/python/tests/test_client.pypackages/sdk/typescript/src/setup.test.tspackages/sdk/typescript/src/setup.ts
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
1 participant
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.
Summary
Cross-repo consumer of cloud's new
GET .../accessible-resourcesandPUT .../metadataroutes (see https://github.com/AgentWorkforce/cloud/pull/540). Together with that change this closes the loop on Jira/Confluence multi-tenant setup (cloud#535): operators no longer need to dive into the Nango dashboard to pick acloudIdafter OAuth completes.TS SDK (
packages/sdk/typescript)WorkspaceHandle.listAccessibleResources(provider)returns[{ id, url, name?, scopes?, avatarUrl? }]for Atlassian-family providers. Cloud's typed 400 for non-Atlassian providers surfaces asCloudApiErrorso callers can branch on the code.WorkspaceHandle.setIntegrationMetadata(provider, metadata)PUTs the metadata namespace (full replacement, not merge); rejects non-object metadata locally before hitting Cloud.WorkspaceIntegrationProvider | stringso callers can target dynamically-discovered providers without an SDK release.Python SDK (
packages/sdk/python)RelayFileClientandAsyncRelayFileClientgainlist_accessible_resources(workspace_id, provider)andset_integration_metadata(workspace_id, provider, metadata).cloud_base_urlconstructor arg because cloud's control plane is a different host from the relayfile data plane (https://agentrelay.com/cloudvshttps://api.relayfile.dev).Go CLI (
cmd/relayfile-cli/main.go)relayfile integration connect jira(and... confluence) now calls accessible-resources after OAuth completes:{ cloudId, baseUrl }and log "auto-selected"relayfile integration set-metadata PROVIDER KEY=VALUE [KEY=VALUE...] [--workspace NAME] [--yes]— general-purpose, usable for any provider (JiracloudId/baseUrl, linearteam_id, custom API hosts, etc.). v1 is flat-keys-only; nested keys deferred.integration connect jira/confluencedoes post-OAuth site selection.Test plan
npm test --workspace packages/sdk/typescript— 144/144 passing (6 new)pytest packages/sdk/python— 52/52 passing (6 new)go test ./cmd/relayfile-cli/— passing (7 new tests: set-metadata happy path, KEY=VALUE validation, single-site auto-select, multi-site interactive pick, blank-input default, retry-then-error, non-Atlassian no-op)go build ./...— cleanrelayfile integration connect jiraagainst a workspace with 2+ Atlassian sites, confirm prompt + selection + sync startJudgment calls
setMetadata(full replacement) rather thanupdateMetadata(merge) because the CLI confirmation prompt becomes truthful only with full replacement; callers wanting merge can read existing metadata first.bufio.Readerrather than going throughpromptLinebecausepromptLinecreates a fresh bufio reader per call and drops bytes buffered past the first newline — that breaks the retry loop when stdin is a pre-loaded reader (tests / piped scripts)._*,connection_*,auth_*,provider_config_key,connection_idkeys to keep Nango plumbing safe, but allows these as nested keys under operator-supplied parents so legitimate domain metadata like{ custom: { auth_url: "..." } }still works.🤖 Generated with Claude Code