fix: update create org params to match procedure #80
Conversation
🦋 Changeset detectedLatest commit: 3419471 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds exported interfaces Changes
Sequence Diagram(s)(omitted — changes are interface/param updates and validations, not a new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/sdk-core/src/repository/interfaces.ts (1)
913-913: Update method signature to use CreateOrganizationParams.The
createmethod signature still uses an inline object type with the oldhandleparameter instead of the newly definedCreateOrganizationParamsinterface. This creates a type mismatch with the implementation.🔧 Proposed fix
- create(params: { name: string; description?: string; handle?: string }): Promise<OrganizationInfo>; + create(params: CreateOrganizationParams): Promise<OrganizationInfo>;packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
77-110: Update JSDoc to reference handlePrefix instead of handle.The JSDoc comments still reference the old
handleparameter in multiple places:
- Line 83:
@param params.handle- Line 92: mentions "If you provide a custom handle"
- Line 107:
handle: "reforestation"These should be updated to
handlePrefixto match the new parameter name.📝 Proposed JSDoc updates
* @param params - Organization parameters * @param params.name - Display name for the organization * @param params.description - Optional description of the organization's purpose - * @param params.handle - Optional custom handle. If not provided, one is auto-generated. + * @param params.handlePrefix - Handle prefix for the organization (without SDS domain) * @returns Promise resolving to the created organization info - * @throws {@link NetworkError} if organization creation fails + * @throws {@link ValidationError} if required parameters are missing + * @throws {@link NetworkError} if organization creation fails * * @remarks * The creating user automatically becomes the owner with full permissions. * - * **Handle Format**: Handles are typically formatted as - * `{name}.sds.{domain}` (e.g., "my-team.sds.hypercerts.org"). - * If you provide a custom handle, it must be unique on the SDS. + * **Handle Format**: The handlePrefix is combined with the SDS domain to form + * the full handle (e.g., "gainforest" becomes "gainforest.sds.hypercerts.org"). + * The handlePrefix must be unique on the SDS. * * @example Basic organization * ```typescript * const org = await repo.organizations.create({ * name: "Climate Action Team", + * handlePrefix: "climate-action", * }); * console.log(`Created org: ${org.did}`); * ``` * - * @example With description and custom handle + * @example With description and handlePrefix * ```typescript * const org = await repo.organizations.create({ * name: "Reforestation Initiative", * description: "Coordinating tree planting projects worldwide", - * handle: "reforestation", + * handlePrefix: "reforestation", * }); * ```packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
16-74: Add test coverage for validation errors.The implementation now validates required parameters (
handlePrefixandname), but there are no test cases covering the validation error paths. Per coding guidelines, minimum test coverage is 70%.🧪 Proposed validation tests
Add these test cases in the "create" describe block:
it("should throw ValidationError when handlePrefix is missing", async () => { await expect( orgOps.create({ name: "Test Org", handlePrefix: "" }) ).rejects.toThrow(ValidationError); await expect( orgOps.create({ name: "Test Org" } as any) ).rejects.toThrow(ValidationError); }); it("should throw ValidationError when name is missing", async () => { await expect( orgOps.create({ name: "", handlePrefix: "test" }) ).rejects.toThrow(ValidationError); await expect( orgOps.create({ handlePrefix: "test" } as any) ).rejects.toThrow(ValidationError); });
🤖 Fix all issues with AI agents
In @packages/sdk-core/src/repository/interfaces.ts:
- Around line 92-107: OrganizationOperations.create currently uses an inline
param type with `handle` that conflicts with the exported
CreateOrganizationParams (which uses `handlePrefix`); update the
OrganizationOperations.create signature to accept CreateOrganizationParams
(replace the inline param type at the create method with the exported
CreateOrganizationParams type and rename any `handle` parameter to
`handlePrefix` inside that method), and then update the duplicate
CreateOrganizationParams in packages/sdk-react/src/types.ts to match the
sdk-core definition (use `handlePrefix` and the same optional/required fields)
and fix any call sites to pass handlePrefix instead of handle.
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
117-122: Consider using Zod for validation instead of manual checks.The coding guidelines specify: "Use Zod schemas for runtime validation". The current manual validation approach is not aligned with this requirement and doesn't provide type safety or detailed error messages.
As per coding guidelines, consider creating a Zod schema:
♻️ Proposed Zod-based validation
At the top of the file, after imports:
import { z } from "zod"; const CreateOrganizationParamsSchema = z.object({ name: z.string().min(1, "Organization name is required"), handlePrefix: z.string().min(1, "Handle prefix is required"), description: z.string().optional(), });Then in the create method:
async create(params: CreateOrganizationParams): Promise<OrganizationInfo> { const userDid = this.session.did || this.session.sub; if (!userDid) { throw new NetworkError("No authenticated user found"); } - if (!params.handlePrefix) { - throw new ValidationError("Missing handlePrefix"); - } - if (!params.name) { - throw new ValidationError("Missing name"); - } + try { + CreateOrganizationParamsSchema.parse(params); + } catch (error) { + throw new ValidationError("Invalid organization parameters", error); + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.ts
**/tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/tests/**/*.test.ts: Test files should mirror source structure and be placed in tests/ directory with *.test.ts extension
Minimum test coverage: 70% for lines, functions, and statements; 60% for branches
Use mocks from src/testing/ for unit tests
Use fixtures from tests/utils/fixtures.ts in unit tests
Files:
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts
🧬 Code graph analysis (3)
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
packages/sdk-core/src/index.ts (1)
NetworkError(114-114)
packages/sdk-core/src/repository/interfaces.ts (1)
packages/sdk-react/src/types.ts (1)
CreateOrganizationParams(207-211)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (3)
packages/sdk-core/src/repository/interfaces.ts (1)
CreateOrganizationParams(92-107)packages/sdk-core/src/repository/types.ts (1)
OrganizationInfo(82-97)packages/sdk-core/src/core/errors.ts (2)
NetworkError(209-220)ValidationError(170-181)
🔇 Additional comments (3)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
10-10: Good addition of ValidationError import.The import correctly follows the error hierarchy from the coding guidelines.
.changeset/fix-create-org-params.md (1)
1-7: LGTM - Clear changeset documentation.The changeset accurately describes the changes: new interface, parameter rename from
handletohandlePrefix, and validation additions.packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
29-48: Tests correctly updated to use handlePrefix.The test cases properly reflect the parameter change from
handletohandlePrefix.
Kzoeps
force-pushed
the
fix/minor-bugs
branch
from
219dc41 to
a675626
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
16-74: Add tests for validation errors.The implementation now validates required fields (
handlePrefixandname) and throwsValidationErrorwhen they're missing (lines 117-122 inOrganizationOperationsImpl.ts). However, there are no tests covering these validation error paths. As per coding guidelines, minimum test coverage is 70% for lines, functions, and statements.🧪 Suggested test cases
Add these test cases within the
describe("create", ...)block:it("should throw ValidationError when handlePrefix is missing", async () => { await expect( orgOps.create({ name: "Test Org", handlePrefix: "" }) ).rejects.toThrow(ValidationError); }); it("should throw ValidationError when name is missing", async () => { await expect( orgOps.create({ name: "", handlePrefix: "test" }) ).rejects.toThrow(ValidationError); });You'll also need to import
ValidationError:import { NetworkError, ValidationError } from "../../src/core/errors.js";packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
77-110: Update JSDoc to referencehandlePrefixinstead ofhandle.Line 83 still references the old parameter name
@param params.handle, but it should be@param params.handlePrefix. The example on line 107 also references the old parameter name.📝 Proposed documentation fix
/** * Creates a new organization. * * @param params - Organization parameters * @param params.name - Display name for the organization * @param params.description - Optional description of the organization's purpose - * @param params.handle - Optional custom handle. If not provided, one is auto-generated. + * @param params.handlePrefix - Handle prefix for the organization (required). Combined with SDS domain to form full handle. * @returns Promise resolving to the created organization info * @throws {@link NetworkError} if organization creation fails + * @throws {@link ValidationError} if required parameters are missing * * @remarks * The creating user automatically becomes the owner with full permissions. * * **Handle Format**: Handles are typically formatted as * `{name}.sds.{domain}` (e.g., "my-team.sds.hypercerts.org"). - * If you provide a custom handle, it must be unique on the SDS. + * The handlePrefix is combined with the SDS domain to form the full handle. * * @example Basic organization * ```typescript * const org = await repo.organizations.create({ * name: "Climate Action Team", + * handlePrefix: "climate-action", * }); * console.log(`Created org: ${org.did}`); * ``` * - * @example With description and custom handle + * @example With description * ```typescript * const org = await repo.organizations.create({ * name: "Reforestation Initiative", * description: "Coordinating tree planting projects worldwide", - * handle: "reforestation", + * handlePrefix: "reforestation", * }); * ``` */
🤖 Fix all issues with AI agents
In @packages/sdk-react/src/types.ts:
- Around line 207-211: The CreateOrganizationParams interface currently marks
handlePrefix as optional but must be required to match the core SDK and runtime
validation; update the CreateOrganizationParams definition so handlePrefix is a
non-optional string (remove the ? from handlePrefix) and ensure any call sites
constructing CreateOrganizationParams provide handlePrefix to avoid
ValidationError at runtime.
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
117-122: Good defensive validation, but consider using Zod schemas.The runtime validation for required fields is good defensive programming. However, as per coding guidelines for
**/*.{ts,tsx}, "Use Zod schemas for runtime validation." Consider defining a Zod schema forCreateOrganizationParamsto provide more robust validation and better error messages.♻️ Optional: Zod-based validation approach
Define a Zod schema in a separate validation file or at the top of this file:
import { z } from "zod"; const CreateOrganizationParamsSchema = z.object({ name: z.string().min(1, "Organization name is required"), handlePrefix: z.string().min(1, "Handle prefix is required"), description: z.string().optional(), });Then use it in the create method:
async create(params: CreateOrganizationParams): Promise<OrganizationInfo> { const userDid = this.session.did || this.session.sub; if (!userDid) { throw new NetworkError("No authenticated user found"); } try { CreateOrganizationParamsSchema.parse(params); } catch (error) { throw new ValidationError("Invalid organization parameters", error); } // ... rest of implementation }This provides better error messages and aligns with the coding guidelines.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-react/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk-core/src/repository/interfaces.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-react/src/types.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.ts
**/tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/tests/**/*.test.ts: Test files should mirror source structure and be placed in tests/ directory with *.test.ts extension
Minimum test coverage: 70% for lines, functions, and statements; 60% for branches
Use mocks from src/testing/ for unit tests
Use fixtures from tests/utils/fixtures.ts in unit tests
Files:
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts
🧠 Learnings (1)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Applied to files:
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts
🧬 Code graph analysis (2)
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
packages/sdk-core/src/index.ts (1)
NetworkError(114-114)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (3)
packages/sdk-core/src/repository/interfaces.ts (1)
CreateOrganizationParams(92-107)packages/sdk-core/src/repository/types.ts (1)
OrganizationInfo(82-97)packages/sdk-core/src/core/errors.ts (2)
NetworkError(209-220)ValidationError(170-181)
🔇 Additional comments (4)
.changeset/fix-create-org-params.md (1)
1-7: LGTM!The changeset accurately documents the parameter rename and validation additions.
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
29-33: LGTM!The parameter rename from
handletohandlePrefixis correctly applied in the test.packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (2)
10-10: LGTM!Correctly imports
ValidationErrorfrom the error hierarchy, as per coding guidelines.
13-13: LGTM!Correctly imports the new
CreateOrganizationParamsinterface.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (1)
160-176: Consider using nullish coalescing for consistency.The implementation correctly accepts
GrantAccessParamsand falls back tothis.repoDidwhenparams.repoDidis not provided. However, using??instead of||would be more precise, as it only falls back onundefined/nullrather than any falsy value.♻️ Suggested refinement
body: JSON.stringify({ - repo: params.repoDid || this.repoDid, + repo: params.repoDid ?? this.repoDid, userDid: params.userDid, permissions, }),packages/sdk-core/src/repository/interfaces.ts (1)
92-107: Interface placement breaks documentation flow.
CreateOrganizationParamsis inserted between the JSDoc documentation block (lines 28-91) andCreateHypercertParams(line 108). The extensive example comments above describe hypercert creation parameters, not organization parameters. Consider movingCreateOrganizationParamsto a more appropriate location, such as nearOrganizationOperations(around line 918) where it's actually used.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/fix-create-org-params.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.tspackages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/interfaces.ts
**/tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/tests/**/*.test.ts: Test files should mirror source structure and be placed in tests/ directory with *.test.ts extension
Minimum test coverage: 70% for lines, functions, and statements; 60% for branches
Use mocks from src/testing/ for unit tests
Use fixtures from tests/utils/fixtures.ts in unit tests
Files:
packages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.ts
🧠 Learnings (1)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces should use PascalCase with descriptive names
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
🧬 Code graph analysis (1)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (1)
packages/sdk-core/src/repository/interfaces.ts (1)
GrantAccessParams(818-831)
🔇 Additional comments (4)
packages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.ts (1)
79-93: LGTM!The test correctly verifies that an explicit
repoDidparameter is propagated to the request body, complementing the existing tests that use the default repository DID. The test follows the established patterns in this file.packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (1)
12-12: LGTM!Import correctly added for the new
GrantAccessParamstype.packages/sdk-core/src/repository/interfaces.ts (2)
817-831: LGTM!The
GrantAccessParamsinterface is well-documented and correctly placed near theCollaboratorOperationsinterface that uses it. The optionalrepoDidparameter enables flexibility for organization repository operations while defaulting to the session DID.
840-840: LGTM!Method signatures correctly updated to use the new typed parameter objects instead of inline object definitions.
Also applies to: 922-925
Kzoeps
force-pushed
the
fix/minor-bugs
branch
from
e255cd8 to
7827eee
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-core/src/repository/interfaces.ts (1)
833-840: JSDoc missing documentation forrepoDidparameter.The
@paramblock documentsuserDidandrolebut omits the new optionalrepoDidfield fromGrantAccessParams.📝 Suggested documentation update
/** * Grants repository access to a user. * * @param params - Grant parameters * @param params.userDid - DID of the user to grant access to * @param params.role - Role to assign + * @param params.repoDid - Optional repository DID (defaults to session.did) */ grant(params: GrantAccessParams): Promise<void>;
🤖 Fix all issues with AI agents
In @packages/sdk-core/src/repository/interfaces.ts:
- Around line 92-107: Move the CreateOrganizationParams interface so it
immediately follows the CreateHypercertParams JSDoc block to ensure the
documentation matches the intended interface; then resolve the handlePrefix type
mismatch by making handlePrefix required across the surface area (update the
corresponding type in sdk-react from handlePrefix?: string to handlePrefix:
string) so callers match the repository.organizations.create() contract, or
alternatively add explicit validation in the React hook that calls
repository.organizations.create() to throw or prompt when handlePrefix is
missing—update references to CreateOrganizationParams, CreateHypercertParams,
handlePrefix, and repository.organizations.create() accordingly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts
- packages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.ts
- .changeset/fix-create-org-params.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/src/repository/interfaces.ts
🧠 Learnings (1)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces should use PascalCase with descriptive names
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
🧬 Code graph analysis (1)
packages/sdk-core/src/repository/interfaces.ts (4)
packages/sdk-react/src/types.ts (3)
CreateOrganizationParams(207-211)RepositoryRole(428-428)OrganizationInfo(426-426)packages/sdk-core/src/repository/Repository.ts (2)
RepositoryRole(23-23)OrganizationInfo(25-25)packages/sdk-core/src/types.ts (2)
RepositoryRole(50-50)OrganizationInfo(52-52)packages/sdk-core/src/index.ts (2)
RepositoryRole(24-24)OrganizationInfo(26-26)
🔇 Additional comments (2)
packages/sdk-core/src/repository/interfaces.ts (2)
817-831: LGTM!The
GrantAccessParamsinterface is well-structured with clear documentation. The optionalrepoDidfield properly addresses the PR objective of allowing callers to specify a target repository instead of defaulting tosession.did.
918-925: LGTM!The updated signature properly uses
CreateOrganizationParams, aligning with the PR objectives. The interface-level documentation adequately describes each field.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (1)
196-209: Interface mismatch:revokeacceptsrepoDidin implementation but not in interface.The implementation accepts an optional
repoDidparameter, but theCollaboratorOperationsinterface at line 848 ininterfaces.tsonly declares{ userDid: string }. This inconsistency will cause type errors for consumers using the interface type.🔧 Suggested fix in interfaces.ts
Update the interface to include the optional
repoDid:- revoke(params: { userDid: string }): Promise<void>; + revoke(params: { userDid: string; repoDid?: string }): Promise<void>;packages/sdk-core/src/repository/interfaces.ts (1)
840-861: Interface inconsistencies with implementation (see related comments).The
grantsignature correctly usesGrantAccessParams. However:
- Line 848:
revokeis missing the optionalrepoDidparameter that the implementation accepts- Line 858:
listdeclaresrepoDid: stringas required, but the implementation treats it as optionalThese mismatches should be corrected to ensure type consistency between interface and implementation.
🔧 Proposed fix
/** * Revokes repository access from a user. * * @param params - Revoke parameters * @param params.userDid - DID of the user to revoke access from + * @param params.repoDid - Optional repository DID (defaults to current repo) */ - revoke(params: { userDid: string }): Promise<void>; + revoke(params: { userDid: string; repoDid?: string }): Promise<void>; /** * Lists all collaborators on the repository. * * @param params - Optional pagination parameters * @param params.limit - Maximum number of results (1-100, default 50) * @param params.cursor - Pagination cursor from previous response + * @param params.repoDid - Optional repository DID (defaults to current repo) * @returns Promise resolving to collaborators and optional cursor */ - list(params?: { limit?: number; cursor?: string; repoDid: string }): Promise<{ + list(params?: { limit?: number; cursor?: string; repoDid?: string }): Promise<{ collaborators: RepositoryAccessGrant[]; cursor?: string; }>;
🤖 Fix all issues with AI agents
In @packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts:
- Around line 239-244: The implementation of list in CollaboratorOperationsImpl
accepts an optional repoDid but the CollaboratorOperations interface declares
repoDid as required; update the CollaboratorOperations interface (the list
method signature) to make repoDid optional (repoDid?: string) so it matches
CollaboratorOperationsImpl, and update any related type references or overloads
that assume a required repoDid to accept the optional form to keep types
consistent.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk-core/tests/repository/CollaboratorOperationsImpl.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/interfaces.ts
🧠 Learnings (1)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces should use PascalCase with descriptive names
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
🧬 Code graph analysis (1)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (2)
packages/sdk-core/src/repository/interfaces.ts (1)
GrantAccessParams(818-831)packages/sdk-core/src/repository/types.ts (1)
RepositoryAccessGrant(66-73)
🔇 Additional comments (5)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (2)
10-13: LGTM!The imports are correctly updated to bring in
GrantAccessParamsfrom the interfaces module.
160-176: LGTM!The
grantmethod correctly implements the optionalrepoDidoverride with proper fallback tothis.repoDid.packages/sdk-core/src/repository/interfaces.ts (3)
92-107: LGTM!The
CreateOrganizationParamsinterface is well-documented with clear JSDoc comments. ThehandlePrefixfield with the example clarifies the expected format, and the interface follows the PascalCase naming convention per coding guidelines.
817-831: LGTM!The
GrantAccessParamsinterface is well-structured with clear documentation explaining the optionalrepoDidoverride behavior.
922-925: LGTM!The
createmethod signature correctly uses the newCreateOrganizationParamsinterface, aligning with the PR objective of requiringhandlePrefixinstead ofhandle.
Kzoeps
force-pushed
the
fix/minor-bugs
branch
2 times, most recently
from
f57967d to
7389e2b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/sdk-core/src/repository/interfaces.ts (2)
28-167: FixCreateHypercertParamsJSDoc examples/types mismatch (workScope, dates).
The examples useworkTimeframeFrom/workTimeframeToandworkScope: "...", but the interface definesstartDate/endDateandworkScopeas an object (withinAllOf/withinAnyOf/withinNoneOf). This will mislead users and breaks copy/paste docs.Proposed doc fix (update example fields to match the interface)
- * workScope: "Food Security", - * workTimeframeFrom: "2024-01-01", - * workTimeframeTo: "2024-06-30", + * workScope: { withinAllOf: ["Food Security"] }, + * startDate: "2024-01-01", + * endDate: "2024-06-30",
819-860:repoDidsupport is inconsistent (docs mention it; types/signatures don’t).
GrantAccessParamsdoesn’t includerepoDid, but downstream docs/PR objective indicate it should.CollaboratorOperations.listJSDoc mentionsparams.repoDid, but the signature doesn’t accept it, and the doc line has a typo (params.repoDid-).
This is a contract/documentation bug and blocks the “target repo/org override” feature.Proposed fix (types + signatures + doc typo)
export interface GrantAccessParams { /** * DID of the user to grant access to */ userDid: string; /** * Role to assign */ role: RepositoryRole; + /** + * Optional repository DID to target (defaults to current repo). + */ + repoDid?: string; } export interface CollaboratorOperations { @@ /** * Lists all collaborators on the repository. @@ - * `@param` params.repoDid- Pagination cursor from previous response + * `@param` params.repoDid - Optional repository DID to list collaborators for (defaults to current repo) * `@returns` Promise resolving to collaborators and optional cursor */ - list(params?: { limit?: number; cursor?: string }): Promise<{ + list(params?: { limit?: number; cursor?: string; repoDid?: string }): Promise<{ collaborators: RepositoryAccessGrant[]; cursor?: string; }>;packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (2)
132-176:grant()ignoresparams.repoDid(feature is non-functional).
Docs sayparams.repoDidcan override the target repo, but the request always usesthis.repoDid(Line 167).Proposed fix (use override when provided)
async grant(params: GrantAccessParams): Promise<void> { const permissions = this.roleToPermissions(params.role); + const repoDid = params.repoDid ?? this.repoDid; const response = await this.session.fetchHandler(`${this.serverUrl}/xrpc/com.sds.repo.grantAccess`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ - repo: this.repoDid, + repo: repoDid, userDid: params.userDid, permissions, }), });
211-246:list()docs mentionrepoDid, but signature/impl don’t support it.
Either remove the doc param, or (preferably, given the PR objective direction) addrepoDid?: stringand use it in the query.Proposed fix (accept repoDid override)
-async list(params?: { limit?: number; cursor?: string }): Promise<{ +async list(params?: { limit?: number; cursor?: string; repoDid?: string }): Promise<{ collaborators: RepositoryAccessGrant[]; cursor?: string; }> { const queryParams = new URLSearchParams({ - repo: this.repoDid, + repo: params?.repoDid ?? this.repoDid, });packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
77-123: Update org-create JSDoc: still referenceshandleand “optional handle”.
The method now requireshandlePrefix(and validates it), but the doc block still documentsparams.handleand shows examples usinghandle.Proposed doc fix
- * `@param` params.handle - Optional custom handle. If not provided, one is auto-generated. + * `@param` params.handlePrefix - Required handle prefix (without SDS domain), e.g. "gainforest". @@ - * `@example` With description and custom handle + * `@example` With description and handle prefix * ```typescript * const org = await repo.organizations.create({ * name: "Reforestation Initiative", * description: "Coordinating tree planting projects worldwide", - * handle: "reforestation", + * handlePrefix: "reforestation", * }); * ```packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
16-74: Good update tohandlePrefix; add coverage for new validation errors.
The tests now passhandlePrefix, but there’s no test assertingValidationErrorwhenhandlePrefixornameis missing/empty (new behavior).Suggested additional tests
+ it("should throw ValidationError when handlePrefix is missing", async () => { + await expect(orgOps.create({ name: "Org" } as any)).rejects.toThrow(/Missing handlePrefix/); + }); + + it("should throw ValidationError when name is missing", async () => { + await expect(orgOps.create({ handlePrefix: "org" } as any)).rejects.toThrow(/Missing name/); + });
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
111-123: Prefer Zod-based runtime validation to match repo guidelines.
You’re doing manual checks +ValidationError; guidelines ask for Zod schemas for runtime validation. Suggest replacing theif (!params.handlePrefix/name)block with a Zod schema parse and passing the ZodError ascause.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-react/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/sdk-react/src/types.ts
- .changeset/fix-create-org-params.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.ts
**/tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/tests/**/*.test.ts: Test files should mirror source structure and be placed in tests/ directory with *.test.ts extension
Minimum test coverage: 70% for lines, functions, and statements; 60% for branches
Use mocks from src/testing/ for unit tests
Use fixtures from tests/utils/fixtures.ts in unit tests
Files:
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts
🧠 Learnings (3)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Applied to files:
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces should use PascalCase with descriptive names
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: One record type requires: types in services/<domain>/types.ts, Zod schemas in services/<domain>/schemas.ts, lexicon registration in lexicons/<domain>/index.ts, operations in repository/<Domain>OperationsImpl.ts, tests in tests/repository/, and optional React hook in sdk-react/src/hooks/
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
🧬 Code graph analysis (4)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (1)
packages/sdk-core/src/repository/interfaces.ts (1)
GrantAccessParams(820-829)
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
packages/sdk-core/src/index.ts (1)
NetworkError(114-114)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (3)
packages/sdk-core/src/repository/interfaces.ts (1)
CreateOrganizationParams(249-264)packages/sdk-core/src/repository/types.ts (1)
OrganizationInfo(82-97)packages/sdk-core/src/core/errors.ts (2)
NetworkError(209-220)ValidationError(170-181)
packages/sdk-core/src/repository/interfaces.ts (2)
packages/sdk-react/src/types.ts (3)
CreateOrganizationParams(207-222)RepositoryRole(439-439)OrganizationInfo(437-437)packages/sdk-core/src/types.ts (2)
RepositoryRole(50-50)OrganizationInfo(52-52)
🔇 Additional comments (2)
packages/sdk-core/src/repository/interfaces.ts (2)
249-265:CreateOrganizationParamslooks aligned with SDS (handlePrefixrequired).
Interface matches the stated{ handlePrefix, name, description? }shape.
917-925:OrganizationOperations.create(params: CreateOrganizationParams)is the right direction.
Nice cleanup vs inline param types, and it enforces the updated contract at the interface level.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
102-109: Update JSDoc example to usehandlePrefixinstead ofhandle.The example still references the old
handleparameter, but the implementation now expectshandlePrefix. This documentation mismatch will confuse developers.📝 Proposed fix
* `@example` With description and custom handle * ```typescript * const org = await repo.organizations.create({ * name: "Reforestation Initiative", * description: "Coordinating tree planting projects worldwide", - * handle: "reforestation", + * handlePrefix: "reforestation", * }); * ```
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-react/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts
- packages/sdk-react/src/types.ts
- .changeset/fix-create-org-params.md
- packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.ts
🧠 Learnings (2)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Applied to files:
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces should use PascalCase with descriptive names
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
🔇 Additional comments (5)
packages/sdk-core/src/repository/interfaces.ts (3)
249-264: LGTM! Interface correctly defines organization creation parameters.The
CreateOrganizationParamsinterface properly marksnameandhandlePrefixas required while keepingdescriptionoptional. The JSDoc comments provide clear documentation, including the example for the handle format.
820-829: LGTM! Clean parameter interface for access grants.The
GrantAccessParamsinterface correctly extracts the inline parameter object into a reusable, well-documented type.
838-838: LGTM! Method signatures now use the typed parameter interfaces.Both
grantandcreatemethods correctly reference the new parameter interfaces, ensuring consistency between the interface contracts and their implementations.Also applies to: 920-923
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (2)
10-13: LGTM! Imports correctly updated.The addition of
ValidationErrorandCreateOrganizationParamsimports aligns with the coding guidelines for using the established error hierarchy and typed parameters.
111-122: LGTM! Validation logic correctly enforces required parameters.The runtime validation for
handlePrefixandnameappropriately throwsValidationError(per the coding guidelines' error hierarchy) before making the API call. This provides clear, early feedback when required fields are missing.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Kzoeps
force-pushed
the
fix/minor-bugs
branch
from
7389e2b to
4997dd2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
77-109: Update JSDoc to reflecthandlePrefixinstead ofhandle.The JSDoc at line 83 still references
@param params.handlebut the parameter is nowhandlePrefix. The example at lines 107-108 also useshandle: "reforestation"instead ofhandlePrefix: "reforestation".📝 Suggested JSDoc update
* `@param` params - Organization parameters * `@param` params.name - Display name for the organization * `@param` params.description - Optional description of the organization's purpose - * `@param` params.handle - Optional custom handle. If not provided, one is auto-generated. + * `@param` params.handlePrefix - Handle prefix for the organization (e.g., "myorg" for "myorg.sds.hypercerts.org") * `@returns` Promise resolving to the created organization info - * `@throws` {`@link` NetworkError} if organization creation fails + * `@throws` {`@link` ValidationError} if required parameters are missing + * `@throws` {`@link` NetworkError} if organization creation failsAnd update the example:
* `@example` With description and custom handle * ```typescript * const org = await repo.organizations.create({ * name: "Reforestation Initiative", * description: "Coordinating tree planting projects worldwide", - * handle: "reforestation", + * handlePrefix: "reforestation", * }); * ```
🧹 Nitpick comments (1)
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
66-73: Consider adding tests forValidationErrorwhen required params are missing.The new validation logic in
OrganizationOperationsImpl.createthrowsValidationErrorwhenhandlePrefixornameis missing. Adding tests for these error paths would improve coverage.📝 Suggested test additions
it("should throw ValidationError when handlePrefix is missing", async () => { // `@ts-expect-error` - Testing runtime validation await expect(orgOps.create({ name: "Test Org" })).rejects.toThrow(ValidationError); }); it("should throw ValidationError when name is missing", async () => { // `@ts-expect-error` - Testing runtime validation await expect(orgOps.create({ handlePrefix: "test" })).rejects.toThrow(ValidationError); }); it("should throw ValidationError when handlePrefix is empty string", async () => { await expect(orgOps.create({ name: "Test Org", handlePrefix: "" })).rejects.toThrow(ValidationError); });Note: You'll need to import
ValidationErrorat the top of the file:import { NetworkError, ValidationError } from "../../src/core/errors.js";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-react/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk-react/src/types.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/src/repository/CollaboratorOperationsImpl.ts
**/tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/tests/**/*.test.ts: Test files should mirror source structure and be placed in tests/ directory with *.test.ts extension
Minimum test coverage: 70% for lines, functions, and statements; 60% for branches
Use mocks from src/testing/ for unit tests
Use fixtures from tests/utils/fixtures.ts in unit tests
Files:
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts
🧠 Learnings (2)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Applied to files:
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces should use PascalCase with descriptive names
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
🧬 Code graph analysis (4)
packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (1)
packages/sdk-core/src/index.ts (1)
NetworkError(114-114)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (3)
packages/sdk-core/src/repository/interfaces.ts (1)
CreateOrganizationParams(249-264)packages/sdk-core/src/repository/types.ts (1)
OrganizationInfo(82-97)packages/sdk-core/src/core/errors.ts (2)
NetworkError(209-220)ValidationError(170-181)
packages/sdk-core/src/repository/interfaces.ts (2)
packages/sdk-core/src/repository/Repository.ts (2)
RepositoryRole(23-23)OrganizationInfo(25-25)packages/sdk-core/src/types.ts (2)
RepositoryRole(50-50)OrganizationInfo(52-52)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (1)
packages/sdk-core/src/repository/interfaces.ts (1)
GrantAccessParams(820-829)
🔇 Additional comments (11)
packages/sdk-core/src/repository/interfaces.ts (4)
820-829: LGTM!The
GrantAccessParamsinterface is well-defined with proper JSDoc documentation, following PascalCase naming conventions per coding guidelines.
838-838: LGTM!The
grantmethod signature correctly references the newGrantAccessParamstype, maintaining consistency between the interface and implementation.
920-923: LGTM!The
OrganizationOperations.createmethod now correctly usesCreateOrganizationParams, resolving the previous inconsistency where inline params usedhandleinstead ofhandlePrefix.
249-264: Well-structuredCreateOrganizationParamsinterface with proper documentation and consistent implementation across packages.The interface correctly defines the required fields (
name,handlePrefix) and optional field (description) with clear JSDoc comments. Both sdk-core and sdk-react implementations are consistent, usinghandlePrefixas the property name for the organization handle.packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (2)
12-13: LGTM!Import updates correctly bring in
GrantAccessParamsfrom the interfaces module and separate the type imports appropriately.
159-159: LGTM!The
grantmethod signature is updated to useGrantAccessParams, maintaining consistency with the interface definition. The implementation logic remains unchanged and correct.packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (2)
10-14: LGTM!Import updates correctly bring in
ValidationErrorfor runtime validation andCreateOrganizationParamsfor type safety. This aligns with the coding guidelines specifying use of the error hierarchy fromsdk-core/src/core/errors.ts.
111-122: LGTM - Validation logic is correct and uses the appropriate error type.The runtime validation for
handlePrefixandnamecorrectly throwsValidationErrorwhen required fields are missing. The falsy check handles bothundefinedand empty string cases appropriately. Based on learnings, this correctly uses the error hierarchy fromsdk-core/src/core/errors.ts.packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts (2)
29-33: LGTM!Test correctly updated to use
handlePrefixinstead ofhandle, aligning with the newCreateOrganizationParamsinterface.
60-60: LGTM!Minimal params test correctly includes both required fields (
nameandhandlePrefix)..changeset/fix-create-org-params.md (1)
1-7: LGTM!The changeset accurately documents the breaking change (parameter rename from
handletohandlePrefix) with aminorversion bump, which is appropriate for the 0.x series. The description clearly explains the motivation and scope of changes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Kzoeps
force-pushed
the
fix/minor-bugs
branch
from
4997dd2 to
3419471
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
77-110: Documentation mismatch: JSDoc referenceshandlebut implementation useshandlePrefix.The JSDoc block references
@param params.handle - Optional custom handle(line 83), but the implementation now useshandlePrefixas a required parameter. The example at lines 107-109 also showshandle: "reforestation"instead ofhandlePrefix.📝 Suggested fix for JSDoc
* `@param` params - Organization parameters * `@param` params.name - Display name for the organization * `@param` params.description - Optional description of the organization's purpose - * `@param` params.handle - Optional custom handle. If not provided, one is auto-generated. + * `@param` params.handlePrefix - Required handle prefix (e.g., "my-team" for "my-team.sds.hypercerts.org") * `@returns` Promise resolving to the created organization info - * `@throws` {`@link` NetworkError} if organization creation fails + * `@throws` {`@link` ValidationError} if handlePrefix or name is missing + * `@throws` {`@link` NetworkError} if organization creation failsAnd update the example:
* `@example` With description and custom handle * ```typescript * const org = await repo.organizations.create({ * name: "Reforestation Initiative", * description: "Coordinating tree planting projects worldwide", - * handle: "reforestation", + * handlePrefix: "reforestation", * }); * ```
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
111-122: Validation logic is correct; consider Zod for consistency.The runtime validation for
handlePrefixandnameis correctly implemented and throwsValidationErroras per the error hierarchy. However, the coding guidelines recommend using Zod schemas for runtime validation.For consistency with the codebase guidelines, consider using a Zod schema:
♻️ Optional: Use Zod schema for validation
import { z } from "zod"; const CreateOrganizationSchema = z.object({ name: z.string().min(1, "Missing name"), handlePrefix: z.string().min(1, "Missing handlePrefix"), description: z.string().optional(), }); // In create method: async create(params: CreateOrganizationParams): Promise<OrganizationInfo> { const userDid = this.session.did || this.session.sub; if (!userDid) { throw new NetworkError("No authenticated user found"); } const result = CreateOrganizationSchema.safeParse(params); if (!result.success) { throw new ValidationError(result.error.errors[0].message, result.error); } // ... rest of implementation }Based on coding guidelines, Zod is preferred for runtime validation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/fix-create-org-params.mdpackages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.tspackages/sdk-core/tests/repository/OrganizationOperationsImpl.test.tspackages/sdk-react/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/sdk-react/src/types.ts
- packages/sdk-core/tests/repository/OrganizationOperationsImpl.test.ts
- .changeset/fix-create-org-params.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript with strict: true configuration
Avoid any types (warns in ESLint)
Prefix unused variables with underscore (_)
Use Zod schemas for runtime validation
Use one class/module per file
Classes should use PascalCase naming (e.g., OAuthClient, Repository)
Files should use PascalCase for classes and camelCase for utilities
Types and Interfaces should use PascalCase with descriptive names
Constants should use UPPER_SNAKE_CASE
Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Files:
packages/sdk-core/src/repository/CollaboratorOperationsImpl.tspackages/sdk-core/src/repository/OrganizationOperationsImpl.tspackages/sdk-core/src/repository/interfaces.ts
🧠 Learnings (2)
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Use the error hierarchy from sdk-core/src/core/errors.ts with base HypercertsError and specific error classes (ValidationError, AuthenticationError, NetworkError, NotFoundError, SDSRequiredError)
Applied to files:
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts
📚 Learning: 2026-01-06T17:56:38.894Z
Learnt from: CR
Repo: hypercerts-org/hypercerts-sdk PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T17:56:38.894Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces should use PascalCase with descriptive names
Applied to files:
packages/sdk-core/src/repository/interfaces.ts
🧬 Code graph analysis (2)
packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (1)
packages/sdk-core/src/repository/interfaces.ts (1)
GrantAccessParams(820-829)
packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (3)
packages/sdk-core/src/repository/interfaces.ts (1)
CreateOrganizationParams(249-264)packages/sdk-core/src/repository/types.ts (1)
OrganizationInfo(82-97)packages/sdk-core/src/core/errors.ts (2)
NetworkError(209-220)ValidationError(170-181)
🔇 Additional comments (6)
packages/sdk-core/src/repository/interfaces.ts (3)
249-264: LGTM! CreateOrganizationParams interface is well-defined.The interface correctly defines:
nameas requiredhandlePrefixas required (with clear documentation example)descriptionas optionalThis addresses the previous review concern about the mismatch between the interface and
OrganizationOperations.createsignature.
819-838: LGTM! GrantAccessParams interface and its usage are correct.The interface is properly typed and the
CollaboratorOperations.grantmethod signature correctly uses the exportedGrantAccessParamstype.
920-923: LGTM! OrganizationOperations.create signature is now consistent.The method signature correctly uses
CreateOrganizationParams, resolving the previous inconsistency where an inline type withhandlewas used instead of the typed interface withhandlePrefix.packages/sdk-core/src/repository/CollaboratorOperationsImpl.ts (2)
12-13: LGTM! Import statements are correctly updated.The imports now properly reference
GrantAccessParamsfrom the interfaces module and separate the type imports from./types.js.
159-175: LGTM! Grant method signature is correctly aligned with the interface.The implementation correctly uses
GrantAccessParamsand accessesparams.roleandparams.userDidas expected.packages/sdk-core/src/repository/OrganizationOperationsImpl.ts (1)
10-14: LGTM! Import statements are correctly updated.The imports now include
ValidationErrorfrom the error hierarchy andCreateOrganizationParamsfrom the interfaces module, aligning with the PR objectives.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
The actual
sds.organizations.createrequires the params to be:However the sdk expected the params as
handlerather thanhandlePrefix. It also wasn't upto date with the sds code wherehandlePrefixis now a requirement and not optional.Also added in validation to check for
handlePrefixandnameand also updated test cases to match the new params.The current setup forrepo.collaborators.grantexpects onlyuserDidandrole. However there is no option to override therepoDidor provide it. Instead it uses the defaultthis.repoDidwhich is just thesession.did. This means that we cant choose which repo/org we want to give the actual perms to.I was using the sdk wrong so each
repositoryclass needs to be scoped for atargetDidrather than sendingrepoDidthrough params. Removed all code related to thatUpdated the test cases for this and the params as well
Summary by CodeRabbit
New Features
Breaking Changes
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.