feat(sdk): CommsAPI.listTenants — E9.4 registry read - #95
Conversation
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_137d032e-376f-4990-b73d-bd6ac328523f) |
|
Warning Review limit reachedNext included review available in 43 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 91 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
Reviewer's GuideAdds Sequence diagram for the CommsAPI tenant registry readsequenceDiagram
participant Caller
participant CommsAPI
participant WaveClient
participant Gateway
Caller->>CommsAPI: listTenants()
CommsAPI->>WaveClient: get(/v1/comms/tenants)
WaveClient->>Gateway: GET /v1/comms/tenants
Gateway-->>WaveClient: CommsTenantListResult
WaveClient-->>CommsAPI: org and tenant metadata
CommsAPI-->>Caller: tenants[] without api_key
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Summary by QodoAdd CommsAPI.listTenants to read comms tenant registry (E9.4)
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| async listTenants(): Promise<CommsTenantListResult> { | ||
| return this.client.get<CommsTenantListResult>(`${this.basePath}/tenants`); | ||
| } |
There was a problem hiding this comment.
💡 Edge Case: listTenants has no pagination while other list* SDK methods do
Every other list method in this SDK (listSubscriptions, listTips, listPayouts, listAssistants, listSuggestions in creator.ts/studio-ai.ts) accepts PaginationParams and returns PaginatedResponse<T>, but listTenants() takes no params and returns a bare { org, tenants[] } shape. If an org accumulates many tenants, this call has no way to page through results and risks large unbounded responses. Confirm with the gateway whether /v1/comms/tenants supports limit/cursor query params; if so, thread PaginationParams through this call for consistency with the rest of the SDK.
Was this helpful? React with 👍 / 👎
| describe("CommsAPI.listTenants (E9.4)", () => { | ||
| it("GETs /v1/comms/tenants and returns the org's rows", async () => { | ||
| const { client, post } = mockClient(); | ||
| const get = vi.fn(async () => ({ org: "acme", tenants: [{ client_id: "acme", pod_id: "pod-1", key_id: "k1", created_at: "2026-08-24T00:00:00Z" }] })); | ||
| Object.assign(client, { get }); | ||
| const api = new CommsAPI(client); | ||
| const r = await api.listTenants(); | ||
| expect(get).toHaveBeenCalledWith("/v1/comms/tenants"); | ||
| expect(r.tenants[0].pod_id).toBe("pod-1"); | ||
| expect(JSON.stringify(r)).not.toContain("api_key"); | ||
| expect(post).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
💡 Quality: PR description claims an untested 'empty-row safety' case
The PR description states the test suite covers 'empty-row safety' (an org with no tenants returning an empty list), but the only new test in comms.test.ts (lines 43-55) mocks a response with one tenant row — there is no assertion against tenants: []. Add a test case asserting listTenants() returns { org, tenants: [] } cleanly when the org has no tenants, to actually verify the claimed behavior.
Was this helpful? React with 👍 / 👎
|
Note Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by September 1. Add seats for more headroom. Code Review 👍 Approved with suggestions 0 resolved / 2 findingsAdds CommsAPI.listTenants() to retrieve comms tenant records without exposing API keys. Consider adding pagination to match other list methods and verifying the empty-row test case mentioned in the description. 💡 Edge Case: listTenants has no pagination while other list* SDK methods doEvery other list method in this SDK (listSubscriptions, listTips, listPayouts, listAssistants, listSuggestions in creator.ts/studio-ai.ts) accepts PaginationParams and returns PaginatedResponse<T>, but listTenants() takes no params and returns a bare { org, tenants[] } shape. If an org accumulates many tenants, this call has no way to page through results and risks large unbounded responses. Confirm with the gateway whether /v1/comms/tenants supports limit/cursor query params; if so, thread PaginationParams through this call for consistency with the rest of the SDK. 💡 Quality: PR description claims an untested 'empty-row safety' case📄 src/tests/comms.test.ts:43-55 The PR description states the test suite covers 'empty-row safety' (an org with no tenants returning an empty list), but the only new test in comms.test.ts (lines 43-55) mocks a response with one tenant row — there is no assertion against 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Code Review by Qodo
1. Missing listTenants changelog entry
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| async listTenants(): Promise<CommsTenantListResult> { | ||
| return this.client.get<CommsTenantListResult>(`${this.basePath}/tenants`); |
There was a problem hiding this comment.
1. Missing listtenants changelog entry 📘 Rule violation § Compliance
This PR adds the user-facing CommsAPI.listTenants() method and its public result types, but the diff does not add an entry under CHANGELOG.md's Unreleased section. Consumers therefore receive an undocumented API surface change.
Agent Prompt
## Issue description
The new public `CommsAPI.listTenants()` API is user-facing but is not documented in the changelog's Unreleased section.
## Issue Context
Add an entry describing the new comms tenant registry read endpoint, returned fields, required `comms:read` scope, and that API-key values are never returned.
## Fix Focus Areas
- CHANGELOG.md[7-7]
- src/comms.ts[43-44]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| async listTenants(): Promise<CommsTenantListResult> { | ||
| return this.client.get<CommsTenantListResult>(`${this.basePath}/tenants`); |
There was a problem hiding this comment.
2. Export list result types 🐞 Bug ⚙ Maintainability
CommsAPI.listTenants() publicly returns CommsTenantListResult, but the package entry point exports only the pre-existing comms request and tenant types. Consumers importing from the SDK root cannot name or import the new method's result types, so the advertised public TypeScript API is incomplete.
Agent Prompt
## Issue description
The newly added public `CommsAPI.listTenants()` method returns `CommsTenantListResult`, but `src/index.ts` does not re-export `CommsTenantListResult` or `CommsTenantListRow` from the package entry point.
## Issue Context
The method is exposed through the SDK root, while consumers commonly import public types from that same root. Direct module tests do not catch the missing package-root exports.
## Fix Focus Areas
- src/index.ts[345-348]
- src/comms.ts[52-62]
Update the package-root exports to include both new public result interfaces, and add a type-level/export test if this repository has coverage for package entry-point exports.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Qodo Fixer🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (2) 🔗 Fix PR: #96 This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR. Prompt for coding agent Process — 2 fixed
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: low. Left a non-blocking comment and did not approve because Cursor Bugbot skipped with incomplete results (usage limit), even though Cursor Security Agent passed with no findings. Human review is needed; no reviewers were assigned because no eligible reviewers were available beyond the author.
Sent by Cursor Approval Agent: Pull Request Router and Approver
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR adds a new public SDK capability for reading the comms tenant registry, including a new endpoint contract and exported response types. Pagination behavior and incomplete package-root type exports remain to be validated, so the public API change warrants human review. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
1 participant
feat(sdk): CommsAPI.listTenants — E9.4 registry read
WHAT
wave.comms.listTenants()GETs /v1/comms/tenants and returns the caller org's tenant rows — client_id, pod_id, key_id, created_at — with the API-key VALUE structurally absent (the mint-once contract: the value is never stored, so the read surface cannot contain it). Requires the comms:read scope (the gateway PR #1337 added it alongside comms:write). An org with no tenants returns an empty list.WHY
The four-renderings law: E9.4 shipped its API cell (gateway #1337, merged). The SDK cell closes the read loop — the write (createTenant) mints a NEW key per call, so listing is the only way to answer "what do I already have" without minting another credential. The console cell follows.
VERIFICATION
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Low Risk
Read-only SDK wrapper around an existing gateway endpoint; no auth or credential storage changes in this diff.
Overview
Adds
CommsAPI.listTenants()so callers can read their org’s comms tenant registry via GET/v1/comms/tenants, complementingcreateTenant(which mints a new key each time).The method is typed with new
CommsTenantListResult/CommsTenantListRowshapes (client_id,pod_id,key_id,created_at— no secret material). Docs call outcomms:readand the mint-once rule thatapi_keymust not appear on the list response.Tests assert the GET path, row fields, that serialized JSON never contains
api_key, and thatpostis not used for this read.Reviewed by Cursor Bugbot for commit dd65412. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by Sourcery
Add the SDK read path for retrieving an organization’s comms tenants.
New Features:
CommsAPI.listTenants()to retrieve the caller organization’s comms tenant registry.Tests: