Skip to content

feat(sdk): implement management sdk parity between node-sdk and go-sdk#760

Merged
dorsha merged 2 commits into
mainfrom
shuni/task-20260703183233
Jul 6, 2026
Merged

feat(sdk): implement management sdk parity between node-sdk and go-sdk#760
dorsha merged 2 commits into
mainfrom
shuni/task-20260703183233

Conversation

@dorsha

@dorsha dorsha commented Jul 3, 2026

Copy link
Copy Markdown
Member

descope/etc#16608

View workflow run

API Error: The operation timed out.


Created by Shuni 🐕

@shuni-bot-dev

shuni-bot-dev Bot commented Jul 3, 2026

Copy link
Copy Markdown

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot-dev

shuni-bot-dev Bot commented Jul 3, 2026

Copy link
Copy Markdown

🐕 Suggested Reviewers

The assignment strategy prioritizes recent contributors who have worked on the specific modules impacted by this PR, ensuring reviewers have relevant and up-to-date knowledge across different areas of the SDK.

Reviewer Reason
aviadl Recently modified multiple files related to management, including core management files and tests, indicating deep familiarity with the management SDK codebase.
yosiharan Contributed to authz and fga modules, which are key areas affected by this parity implementation and thus essential for review.
orius123 Has recent activity on authz and fga files, covering domain-specific parts of the SDK relevant to the changes.
guyp-descope Active in access key files, which are part of the management SDK components affected by this update.
slavikm Worked on audit and authz files, giving valuable insight into security and audit-related aspects of the management SDK.

Suggested by Shuni based on git history and PR context. Names are not @-mentioned to avoid notifying anyone — request a review from whoever fits best.

@shuni-bot-dev shuni-bot-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐕 Shuni's Review

This PR adds ~40 management SDK methods for node/go parity across access keys, audit, authz, FGA, JWT templates, lists, scope-claim mappings, users, and more.

Sniffed out 1 issue:

  • 1 🔴 CRITICAL: paths.ts was never updated — ~61 referenced endpoint keys don't exist, breaking compilation and every new method.

See the inline comment. This one won't hunt until it's fed — needs a bath! 🛁

Comment thread lib/management/analytics.ts
@dorsha

dorsha commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

@shuni see review comments and fix if needed, and check ci failures

@shuni-bot

shuni-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🐕 Done! Updated PR: #760

View workflow run

📋 Actions Taken

View AI response

I've fixed the PR. Here's what was wrong and what I did.

Root cause

The parity PR added ~40 new management methods but never updated the supporting layers, so the package didn't compile:

  1. paths.ts was never touched — ~61 apiPaths.* keys the new methods referenced didn't exist (whole namespaces analytics, list, jwtTemplate, scopeClaimMapping, plus dozens of keys on existing namespaces).
  2. types.ts was missing 7 exported types referenced by the new code (FGASchemaDryRunResponse, FGAMappable*, InboundApplicationTenantConsentDeleteOptions, SSOApplicationSecretResponse).
  3. list.ts used SdkResponse<boolean>, which core-js-sdk's type constraint (T extends Record<string,any>) forbids — a hard compile error.
  4. Coverage regression — the new list.ts (54% functions) and jwttemplate.ts (65%) dragged global coverage below the enforced 97% threshold.

Changes (5 files, 1 commit on shuni/task-20260703183233)

  • paths.ts — added every missing endpoint, cross-checked against the go-sdk (descope/go-sdk) so URLs and version prefixes match exactly (e.g. audit webhook is /v2/, tenant.revokeSSOConfigurationLink is /v1/ while generate stays /v2/).
  • types.ts — added the 7 missing types with shapes mirroring the go-sdk structs.
  • list.ts — dropped the illegal SdkResponse<boolean> annotations on checkIP/checkText (runtime boolean preserved; existing assertions still pass).
  • list.test.ts / jwttemplate.test.ts — added tests for the previously untested methods to restore coverage.

Verification

  • tsc --noEmit → clean (exit 0)
  • eslint + prettier --check → clean
  • jest --coverage449/449 pass, thresholds met
  • Pre-commit hooks (lint-staged) ran and passed — no bypass.

Note: I couldn't query CI logs or human review threads directly — the GH_TOKEN/GITHUB_TOKEN in this environment were empty strings, so gh API calls failed. I derived the CI failures by reproducing the build/test locally, which surfaced the compile + coverage failures described above. The one remaining local test failure (commonjs.test.ts needing dist/) is environmental and resolves once the build runs, which it now does.


View session on Shuni Portal 🐾

The parity methods referenced ~60 apiPaths keys and several types that
were never added, so the package failed to compile.

- paths.ts: add endpoints for the new access key/permission/role batch ops,
  audit webhook, FGA dry-run & mappable resources, JWT templates, lists,
  scope-claim mappings, analytics, project delete, SSO app secret/rotate,
  SSO redirect/recalculate, tenant SSO link revoke, impersonate step-up,
  and the new user recovery/custom-attribute/passkey/trusted-device ops.
- types.ts: add FGASchemaDryRunResponse, FGAMappable* types,
  InboundApplicationTenantConsentDeleteOptions, SSOApplicationSecretResponse.
- list.ts: drop the invalid SdkResponse<boolean> annotation on checkIP/checkText
  (core-js SdkResponse requires an object type).
- add tests covering the previously untested list and jwtTemplate methods
  to restore coverage thresholds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dorsha dorsha changed the title fix(sdk): implement management sdk parity between node-sdk and go-sdk feat(sdk): implement management sdk parity between node-sdk and go-sdk Jul 3, 2026
@dorsha dorsha requested a review from orius123 July 3, 2026 20:55
@dorsha dorsha enabled auto-merge (squash) July 3, 2026 21:00
@dorsha dorsha requested review from asafshen and slavikm July 4, 2026 06:37

@asafshen asafshen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment about the changed code

Comment thread lib/management/audit.ts
@dorsha dorsha merged commit 83f08bb into main Jul 6, 2026
28 checks passed
@dorsha dorsha deleted the shuni/task-20260703183233 branch July 6, 2026 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants