Autogenerate the activity claim's rKey#108
Conversation
🦋 Changeset detectedLatest commit: fb29a89 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 |
📝 WalkthroughWalkthroughIntroduces deterministic rKey generation using SHA-256 hashing of stable content, refactors hypercert creation to process and embed multiple locations and contributors upfront, and updates type signatures to support plural locations with corresponding URI/CID arrays. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Ops as HypercertOperationsImpl
participant Crypto as Crypto
participant Storage as Storage/Repo
Client->>Ops: create(params with locations, contributors)
activate Ops
Ops->>Ops: processLocations(params.locations)
activate Ops
Ops->>Storage: create each location
Storage-->>Ops: locationRefs[]
deactivate Ops
Ops->>Ops: processContributors(params.contributions)
activate Ops
Ops->>Storage: create contribution details (if needed)
Storage-->>Ops: contributorsData[]
deactivate Ops
Ops->>Ops: createHypercertRecord(params, locationRefs, contributorsData)
activate Ops
Ops->>Crypto: stableStringify({imageRef, locationRefs, contributors, ...})
Crypto-->>Ops: contentString
Ops->>Crypto: sha256Hash(contentString)
Crypto-->>Ops: contentHash
Ops->>Ops: rKey = `hc2:{contentHash}`
Ops->>Storage: create hypercert with embedded locations/contributors
Storage-->>Ops: hypercertRecord
deactivate Ops
Ops-->>Client: CreateHypercertResult with locationUris[], locationCids[], rKey
deactivate Ops
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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
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: 3
🤖 Fix all issues with AI agents
In @.changeset/wise-papayas-sing.md:
- Around line 1-5: Update the changeset to include a 0.x breaking-change note
for "@hypercerts-org/sdk-core": add a section under the frontmatter explaining
that the new deterministic rKeys (SHA-256 content hashing) replace
server-assigned random rKeys and may break clients relying on random keys or
duplicate-creation behavior; explicitly mark it as a breaking change for 0.x
consumers and include a short suggested message describing the behavior change
and migration advice.
In `@packages/sdk-core/src/lib/crypto.ts`:
- Around line 12-21: The sha256Hash function is using JSON.stringify with
Object.keys(content).sort() as a replacer which only applies top-level keys
(causing nested property omissions and collisions) and will throw on null;
change it to deterministically stringify by implementing or calling a deep-sort
helper that recursively sorts object keys at all nesting levels before
stringifying (handle null/primitive inputs safely), avoid using
Object.keys(content as object) directly, and in the unsupported-crypto branch
throw the SDK NetworkError (imported from the SDK error hierarchy) instead of a
generic Error so the error follows the SDK's hierarchy.
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 292-295: The rKey is computed from the full hypercertRecord (via
sha256Hash) which includes volatile fields like createdAt and newly minted
rights.uri/cid, so rkey (`rkey`) changes across retries and breaks idempotency;
fix by building a stable, deterministic hash input (e.g., a normalized object
composed of the creation parameters and the static rights payload but explicitly
excluding createdAt and rights.uri/cid), canonicalize it (stable key order JSON
stringify) and pass that to sha256Hash instead of the raw hypercertRecord so
create() produces a consistent rKey across retries.
🧹 Nitpick comments (2)
packages/sdk-core/src/lib/crypto.ts (1)
29-31: Use SDK error hierarchy instead of generic Error.
Throwing a plain Error bypasses the SDK’s error conventions. Prefer HypercertsError (or another SDK error type) for consistency. As per coding guidelines.♻️ Suggested error-type alignment
+import { HypercertsError } from "../core/errors.js"; ... - } catch { - throw new Error("SHA-256 hashing not supported in this environment"); + } catch { + throw new HypercertsError("SHA-256 hashing not supported in this environment"); }packages/sdk-core/tests/repository/HypercertOperationsImpl.hashing.test.ts (1)
1-64: Prefer shared fixtures/mocks for test data.
Guidelines call for using fixtures from tests/utils/fixtures.ts and mocks from src/testing for unit tests; this test builds inline params and uses tests/utils/mocks. If shared fixtures exist, using them keeps test data consistent and reduces duplication. As per coding guidelines.
| --- | ||
| "@hypercerts-org/sdk-core": minor | ||
| --- | ||
|
|
||
| feat: implement pre-generation of rKeys for activity claims using deterministic content hashing (SHA-256). |
There was a problem hiding this comment.
Add a 0.x breaking-change note to the changeset.
Since @hypercerts-org/sdk-core is still 0.x, this behavioral change (deterministic rKeys replacing server-assigned keys) can be breaking for clients depending on random rKeys or duplicate-creation behavior. Please call that out explicitly. Based on learnings.
✍️ Suggested wording update
feat: implement pre-generation of rKeys for activity claims using deterministic content hashing (SHA-256).
+Note: `@hypercerts-org/sdk-core` is 0.x; this changes rKey generation from server-assigned keys to deterministic `hc:<hash>`, which may affect clients relying on random rKeys or duplicate-creation behavior.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --- | |
| "@hypercerts-org/sdk-core": minor | |
| --- | |
| feat: implement pre-generation of rKeys for activity claims using deterministic content hashing (SHA-256). | |
| --- | |
| "@hypercerts-org/sdk-core": minor | |
| --- | |
| feat: implement pre-generation of rKeys for activity claims using deterministic content hashing (SHA-256). | |
| Note: `@hypercerts-org/sdk-core` is 0.x; this changes rKey generation from server-assigned keys to deterministic `hc:<hash>`, which may affect clients relying on random rKeys or duplicate-creation behavior. |
🤖 Prompt for AI Agents
In @.changeset/wise-papayas-sing.md around lines 1 - 5, Update the changeset to
include a 0.x breaking-change note for "@hypercerts-org/sdk-core": add a section
under the frontmatter explaining that the new deterministic rKeys (SHA-256
content hashing) replace server-assigned random rKeys and may break clients
relying on random keys or duplicate-creation behavior; explicitly mark it as a
breaking change for 0.x consumers and include a short suggested message
describing the behavior change and migration advice.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/tests/repository/HypercertOperationsImpl.hashing.test.ts`:
- Line 25: Update the test description string in the it block that currently
reads "should generate rKey with 'hc:' prefix and content hash" to match the
expected prefix used in the assertion ("hc2:"); specifically edit the it("should
generate rKey with 'hc:' prefix and content hash", ...) description to use
"hc2:" so the test name and the assertion on rKey (in the same it block) are
consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 292-315: The rKey hash currently spreads paramsForHash which can
include sidecar fields (location, contributions, evidence, blobs) making rKeys
unstable; replace the hash input to include only the actual claim fields that
are written into the record (use the claim object used to build hypercertRecord,
e.g., hypercertRecord.claim or explicitly list
title/description/contributors/etc.), plus imageBlobRef and the normalized
rightsData (as already done), and remove paramsForHash entirely so the
sha256Hash input is strictly the claim content + imageBlobRef + rightsData to
guarantee stable, deterministic rKeys.
s-adamantine
force-pushed
the
activity-claim-rkey
branch
from
d935cfc to
b949a2c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 539-556: The current catch block around resolveLocation swallows
errors (locationRef remains undefined) which can change the rKey on retries;
instead of silently continuing, rethrow the error after emitting progress and
logging so create() fails and the caller can retry the whole operation; modify
the catch in the create flow that surrounds resolveLocation / locationRef (and
touches result.locationUri and emitProgress) to re-throw the caught error (or
return a rejected Promise) after logging so rKey inputs remain consistent across
retries.
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/HypercertOperationsImpl.ts (1)
402-430: Remove unusedcreateContributionsWithProgressmethod.This private method is not called anywhere in the codebase. With the refactored
create()flow now embedding contributors directly, this helper method has become dead code and should be removed.
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
593-604: Variable shadowing: innerresultshadows outerresult.The
resultvariable at line 594 shadows the outerresult: CreateHypercertResultdeclared at line 538. While this doesn't cause a functional bug (the innerresultis block-scoped), it reduces readability and could lead to confusion during maintenance.♻️ Suggested rename
- const result = await this.addContribution({ + const contributionResult = await this.addContribution({ contributors: contrib.contributors, // Passed for legacy reasons/completeness role: contrib.role, description: contrib.description, }); - detailsRef = { uri: result.uri, cid: result.cid }; + detailsRef = { uri: contributionResult.uri, cid: contributionResult.cid }; this.emitProgress(params.onProgress, { name: "createContribution", status: "success", - data: result, + data: contributionResult, });
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/lib/crypto.ts`:
- Around line 47-50: sha256Hash currently falls back to hashing "" when
stableStringify returns undefined; change sha256Hash so after calling
stableStringify(content) it validates the result and if undefined throws an SDK
validation error (use the SDK error hierarchy, e.g., InvalidInputError or
SdkError) instead of proceeding to TextEncoder; include a clear message that the
content is not serializable and optionally include typeof content to aid
debugging, and update imports to use the appropriate SDK error class referenced.
| const imageBlobRef = params.image ? await this.uploadImageBlob(params.image, params.onProgress) : undefined; | ||
|
|
||
| // Step 2: Create rights record | ||
| // Step 2: Create location record if provided (must be before hypercert) |
There was a problem hiding this comment.
This should be done in a separate method otherwise this function ends up being too long.
| result.rightsCid = rightsCid; | ||
|
|
||
| // Step 3: Create hypercert record | ||
| // Step 4: Build contributors data for embedding (if provided) |
There was a problem hiding this comment.
This should be done in a separate method otherwise this function ends up being too long.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 317-335: The rKey idempotency fails because hashInput in
HypercertOperationsImpl uses generated StrongRef URIs/CIDs (locationRef and
contributorsData) which change when sub-records are created server-side; instead
compute the hash over a canonicalized, deterministic representation of the
underlying payloads (e.g., the raw location parameters and contributor details)
or ensure those sub-records are content-addressed before inclusion. Update the
code that builds hashInput to replace locationRef and contributorsData with the
canonical source objects (e.g., location params and contributor payloads) or
their deterministic content-addressed IDs, canonicalize fields (stable ordering
and serialization) and then hash to produce rKey so retries produce identical
hashes.
- Around line 1128-1170: The contributions "props" field is being accepted by
processContributors but never used; update processContributors to forward
contrib.props into the addContribution call (i.e., include props: contrib.props
in the payload) and ensure the addContribution signature supports and persists
props, or if you prefer not to support props remove it from the contributions
parameter type and all callers; modify the contributions parameter type, the
addContribution invocation inside processContributors, and, if needed, the
addContribution function signature to accept and persist props so the field
isn't silently discarded.
- Around line 293-296: In HypercertOperationsImpl adjust the creation path to
use the same singular field name as updates: replace setting
hypercertRecord.locations with hypercertRecord.location so the create code
aligns with attachLocation and validation; locate the block that currently sets
hypercertRecord.locations (in the create method) and change it to assign a
single embedded StrongRef object to hypercertRecord.location (matching the shape
used by attachLocation and the collection code paths).
| private async processContributors( | ||
| contributions: | ||
| | Array<{ contributors: string[]; role: string; description?: string; props?: Record<string, unknown> }> | ||
| | undefined, | ||
| onProgress?: (step: ProgressStep) => void, | ||
| ): Promise< | ||
| Array<{ contributorIdentity: string; contributionDetails?: string | { uri: string; cid: string } }> | undefined | ||
| > { | ||
| if (!contributions || contributions.length === 0) return undefined; | ||
|
|
||
| const contributorsPromises = contributions.map(async (contrib) => { | ||
| let detailsRef: string | { uri: string; cid: string } = contrib.role; | ||
|
|
||
| // If description is provided, create a detailed record | ||
| if (contrib.description) { | ||
| try { | ||
| this.emitProgress(onProgress, { name: "createContribution", status: "start" }); | ||
| const result = await this.addContribution({ | ||
| contributors: contrib.contributors, // Passed for legacy reasons/completeness | ||
| role: contrib.role, | ||
| description: contrib.description, | ||
| }); | ||
| detailsRef = { uri: result.uri, cid: result.cid }; | ||
| this.emitProgress(onProgress, { | ||
| name: "createContribution", | ||
| status: "success", | ||
| data: result, | ||
| }); | ||
| } catch (error) { | ||
| this.emitProgress(onProgress, { | ||
| name: "createContribution", | ||
| status: "error", | ||
| error: error as Error, | ||
| }); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| // Expand to one entry per contributor DID | ||
| return contrib.contributors.map((did) => ({ | ||
| contributorIdentity: did, | ||
| contributionDetails: detailsRef, | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the contribution-related types and methods
rg -n "processContributors|addContribution" packages/sdk-core/src/repository/HypercertOperationsImpl.ts -A 5 -B 2Repository: hypercerts-org/hypercerts-sdk
Length of output: 2011
🏁 Script executed:
# Search for contribution type definitions and schema
rg -n "type.*Contribution|interface.*Contribution|ContributionDetails" packages/sdk-core/src --type=ts -A 3Repository: hypercerts-org/hypercerts-sdk
Length of output: 2977
🏁 Script executed:
# Check if props is used anywhere in the contribution context
rg -n "contrib\.props|contribution.*props" packages/sdk-core/src --type=tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 55
🏁 Script executed:
# Examine the addContribution method implementation
rg -n "addContribution" packages/sdk-core/src/repository/HypercertOperationsImpl.ts -A 20 | head -50Repository: hypercerts-org/hypercerts-sdk
Length of output: 1869
🏁 Script executed:
# Find the HypercertContributionDetails schema definition
rg -n "OrgHypercertsClaimContributionDetails" packages/sdk-core/src/services/hypercerts/types.ts -A 20Repository: hypercerts-org/hypercerts-sdk
Length of output: 2892
🏁 Script executed:
# Look for the complete addContribution implementation
sed -n '1206,1250p' packages/sdk-core/src/repository/HypercertOperationsImpl.tsRepository: hypercerts-org/hypercerts-sdk
Length of output: 1682
🏁 Script executed:
# Check if there's a schema definition file (likely generated or in a schema folder)
fd -type f -name "*.ts" packages/sdk-core/src/services/hypercerts/ | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 243
🏁 Script executed:
# Search for props field in contribution-related schema or type definitions
rg -n "props" packages/sdk-core/src/services/hypercerts/types.ts -B 2 -A 2Repository: hypercerts-org/hypercerts-sdk
Length of output: 55
🏁 Script executed:
# Check package.json to see if `@hypercerts-org/lexicon` is an external dependency
cat packages/sdk-core/package.json | grep -A 20 "dependencies"Repository: hypercerts-org/hypercerts-sdk
Length of output: 376
🏁 Script executed:
# Look for any schema definition or documentation about contribution structure
fd -type f \( -name "*.json" -o -name "*.md" \) packages/sdk-core/src/services/hypercerts/Repository: hypercerts-org/hypercerts-sdk
Length of output: 243
🏁 Script executed:
# Check if there's a local lexicon file or schema definition
find . -name "*lexicon*" -o -name "*schema*" | grep -E "\.(json|ts)$" | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 1196
🏁 Script executed:
# Check the actual contribution creation to see what fields are validated
rg -n "CONTRIBUTION_DETAILS" packages/sdk-core/src --type=ts -B 2 -A 2Repository: hypercerts-org/hypercerts-sdk
Length of output: 3177
🏁 Script executed:
# Examine the lexicons.ts file to see how CONTRIBUTION_DETAILS_LEXICON_JSON is imported
cat packages/sdk-core/src/lexicons.ts | head -100Repository: hypercerts-org/hypercerts-sdk
Length of output: 2861
🏁 Script executed:
# Look for any imports from `@hypercerts-org/lexicon` related to contribution
rg -n "CONTRIBUTION_DETAILS" packages/sdk-core/src/lexicons.ts -B 5 -A 5Repository: hypercerts-org/hypercerts-sdk
Length of output: 1597
🏁 Script executed:
# Check if there are any examples or tests showing contribution with props
rg -n "props" packages/sdk-core --type=ts -B 3 -A 3 | grep -i contribRepository: hypercerts-org/hypercerts-sdk
Length of output: 434
🏁 Script executed:
# Search for any documentation about the contribution schema structure
find . -name "README*" -o -name "CHANGELOG*" | xargs grep -l "contribution\|props" 2>/dev/null | head -5Repository: hypercerts-org/hypercerts-sdk
Length of output: 349
🏁 Script executed:
# Check the CHANGELOG for any recent changes related to contributions and props
rg -n "contribution|props" packages/sdk-core/CHANGELOG.md -B 2 -A 2 | head -80Repository: hypercerts-org/hypercerts-sdk
Length of output: 507
🏁 Script executed:
# Check if there are any TODOs or FIXMEs related to props or contribution
rg -n "TODO|FIXME" packages/sdk-core/src/repository/HypercertOperationsImpl.ts | grep -i "prop\|contrib"Repository: hypercerts-org/hypercerts-sdk
Length of output: 55
🏁 Script executed:
# Look at the git history of this file to understand when props was added
git log --oneline packages/sdk-core/src/repository/HypercertOperationsImpl.ts | head -20Repository: hypercerts-org/hypercerts-sdk
Length of output: 129
Remove props from the input type or implement support. The processContributors method accepts props?: Record<string, unknown> but never passes it to addContribution, silently discarding it. Either remove it from the input type if it's not supported, or pass it through to the contribution record if it should be persisted.
🤖 Prompt for AI Agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts` around lines
1128 - 1170, The contributions "props" field is being accepted by
processContributors but never used; update processContributors to forward
contrib.props into the addContribution call (i.e., include props: contrib.props
in the payload) and ensure the addContribution signature supports and persists
props, or if you prefer not to support props remove it from the contributions
parameter type and all callers; modify the contributions parameter type, the
addContribution invocation inside processContributors, and, if needed, the
addContribution function signature to accept and persist props so the field
isn't silently discarded.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 317-338: The hashInput used to compute contentHash/rkey can
contain non-serializable or non-canonical values (e.g., a GeoJSON Blob in
params.location and arbitrary props in params.contributions) causing sha256Hash
to throw or produce inconsistent rKeys; before calling sha256Hash(rkey)
normalize hashInput inside HypercertOperationsImpl: replace any Blob or binary
objects (params.location, imageBlobRef) with stable identifiers (content hashes
or StrongRef strings), canonicalize contributors/contributions by stripping or
sorting arbitrary props and converting StrongRef vs URI variants to a single
representative form, and ensure the whole object is JSON-safe and
deterministically serialized; perform this normalization where hashInput is
built so contentHash and rkey are stable and computed before any side-record
creation.
♻️ Duplicate comments (2)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (2)
293-296: Use singularlocationfield to match update path and schema.
create()writeslocations(plural array), whileattachLocation()and updates uselocation(singular). This yields inconsistent records and likely schema validation failures.🛠️ Proposed fix
- if (locationRef) { - hypercertRecord.locations = [{ uri: locationRef.uri, cid: locationRef.cid }]; - } + if (locationRef) { + hypercertRecord.location = { uri: locationRef.uri, cid: locationRef.cid }; + }
1128-1149:propsis accepted but silently dropped.
processContributorsallowsprops, but never forwards or persists it. Either remove it from the input type or implement storage.🛠️ If not supported, remove from the input type
- contributions: - | Array<{ contributors: string[]; role: string; description?: string; props?: Record<string, unknown> }> + contributions: + | Array<{ contributors: string[]; role: string; description?: string }> | undefined,
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 293-296: The create path currently assigns an array to
hypercertRecord.locations which conflicts with update operations
(attachLocation, attachLocationToCollection, attachLocationToProject) that use a
singular location object; change the create logic to set
hypercertRecord.location = { uri: locationRef.uri, cid: locationRef.cid } when
locationRef is provided (and remove or stop setting hypercertRecord.locations)
so the created record matches the existing update schema and the lexicon; ensure
the symbol hypercertRecord and the incoming locationRef are used exactly as in
the diff.
♻️ Duplicate comments (2)
.changeset/wise-papayas-sing.md (1)
1-8: Add a 0.x breaking-change note to the changeset.Since
@hypercerts-org/sdk-coreis 0.x, this behavioral change (deterministichc2:<hash>rKeys replacing server-assigned keys) may be breaking for clients that:
- Depend on random/unique rKeys for duplicate creation
- Parse or validate rKey formats
Based on learnings, please document this explicitly.
✍️ Suggested addition
feat: implement pre-generation of rKeys for activity claims using deterministic content hashing (SHA-256). +**Note (0.x):** This changes rKey generation from server-assigned keys to deterministic `hc2:<contentHash>`. Clients relying on random rKeys or duplicate-creation behavior may need adjustment. + fix: normalize hashInput in `createHypercertRecord()` to use resolved StrongRefs (`locationRef`, `contributorsData`) instead of raw params which may contain non-serializable Blobs or inconsistent formats, ensuring stable rKey generation.packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
1141-1148:propsparameter accepted but silently discarded.The
contributionsparameter type includesprops?: Record<string, unknown>(line 1143), but this field is never forwarded toaddContributionor included in the returned contributor data. This silently drops user-provided data.Either remove
propsfrom the input type signature, or pass it through to the contribution record if it should be persisted.♻️ Option 1: Remove unused props from type
private async processContributors( contributions: - | Array<{ contributors: string[]; role: string; description?: string; props?: Record<string, unknown> }> + | Array<{ contributors: string[]; role: string; description?: string }> | undefined,
…teHypercertRecord JSDoc
…rams
- Use locationRef (resolved StrongRef) instead of params.location which may be Blob
- Use contributorsData (processed canonical format) instead of params.contributions
- Extract image CID string instead of full imageBlobRef object
- Use canonical rights object with only {name, type, description}
This ensures sha256Hash() receives only JSON-serializable values and produces
deterministic rKeys regardless of input format (Blob vs URI vs StrongRef).
aspiers
force-pushed
the
activity-claim-rkey
branch
from
7cf81f5 to
3b58749
Compare
Without this patch, the SDK only allowed a single location per hypercert activity claim, using a singular 'location' field in CreateHypercertParams. This was inconsistent with the lexicon specification (org.hypercerts.claim.activity) which defines 'locations' as an array of StrongRefs. This is a problem because: - The SDK didn't match the underlying lexicon schema - Users couldn't create hypercerts for activities spanning multiple places - Attempting to attach multiple locations would overwrite previous ones This patch solves the problem by: - Renaming 'location' to 'locations' (array) in CreateHypercertParams - Updating CreateHypercertResult to return locationUris/locationCids arrays - Refactoring processLocation() to processLocations() to handle arrays - Modifying attachLocation() to append to existing locations array - Adding comprehensive tests for multiple location support Breaking Changes: - CreateHypercertParams.location → CreateHypercertParams.locations (array) - CreateHypercertResult.locationUri → CreateHypercertResult.locationUris (array) - CreateHypercertResult.locationCid → CreateHypercertResult.locationCids (array) Also fixed: - Documentation examples now use correct field names (startDate/endDate) - README example updated to match current API Co-authored-by: Claude (Anthropic AI) <claude@anthropic.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/sdk-core/src/repository/HypercertOperationsImpl.ts`:
- Around line 918-937: When appending a new location in the update path (around
the code that calls this.get(hypercertUri), this.resolveLocation(location) and
this.update({... updates: { locations: newLocations } }), ensure any legacy
singular "location" field on the record is migrated to the "locations" array
before calling update/putRecord: if a "location" exists, prepend it into the
record's locations array (creating the array if missing) and delete the legacy
"location" key so the update payload only contains "locations" and passes
validation; perform this normalization on the record you pass to update (e.g.,
recordForUpdate / updates) prior to the update call.
In `@packages/sdk-core/src/repository/interfaces.ts`:
- Around line 81-87: The example location object in the docs uses the wrong key
and omits required fields: replace the example that uses "value" with an object
shaped to match CreateLocationParams by using "location" instead of "value" and
adding the required "srs" and "locationType" properties (and optionally
"lpVersion"); update the example near the CreateLocationParams definition
(referenced by the CreateLocationParams type) so it shows e.g. { location:
"...", srs: "...", locationType: "...", lpVersion?: "..." } to prevent
copy‑paste errors.
♻️ Duplicate comments (2)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (2)
293-356: rKey can drift when side records are auto‑created.
hashInputincludeslocationRefsandcontributorsData(StrongRefs). If those records are created on the fly, their URIs/CIDs change across retries, so identical inputs can yield different rKeys. Consider hashing canonical payloads (location params / contribution details) or assigning deterministic rKeys to those side records before hashing.
1157-1199:propsis accepted but never persisted.
Either remove it from the input type or thread it through to contribution details; otherwise callers silently lose data.
| * locations: [ | ||
| * { | ||
| * value: "Amazon Rainforest, Brazil", | ||
| * name: "Amazon Basin", | ||
| * description: "Southern Amazon region", | ||
| * }, | ||
| * ], |
There was a problem hiding this comment.
Location example doesn’t match CreateLocationParams.
The snippet uses value, but the location object requires location plus required srs/locationType (and optionally lpVersion). Updating it prevents copy‑paste errors.
📝 Suggested doc update
- locations: [
- {
- value: "Amazon Rainforest, Brazil",
- name: "Amazon Basin",
- description: "Southern Amazon region",
- },
- ],
+ locations: [
+ {
+ lpVersion: "1.0",
+ srs: "EPSG:4326",
+ locationType: "coordinate-decimal",
+ location: "Amazon Rainforest, Brazil",
+ name: "Amazon Basin",
+ description: "Southern Amazon region",
+ },
+ ],🤖 Prompt for AI Agents
In `@packages/sdk-core/src/repository/interfaces.ts` around lines 81 - 87, The
example location object in the docs uses the wrong key and omits required
fields: replace the example that uses "value" with an object shaped to match
CreateLocationParams by using "location" instead of "value" and adding the
required "srs" and "locationType" properties (and optionally "lpVersion");
update the example near the CreateLocationParams definition (referenced by the
CreateLocationParams type) so it shows e.g. { location: "...", srs: "...",
locationType: "...", lpVersion?: "..." } to prevent copy‑paste errors.
750cd44 to
3bece2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.changeset/wise-rice-create.md:
- Around line 1-6: Update the changeset text to fully describe the user-facing
changes: note that attachLocation now migrates legacy location -> locations
array, describe the rKey hashing and embedding behavior changes (what changed,
how rKeys are hashed/used, and any impacts on indexing/search), and add
compatibility caveats for 0.x minor releases (migration steps, potential
breaking-risk scenarios and how consumers should adapt). Reference the package
entries "@hypercerts-org/sdk-react" and "@hypercerts-org/sdk-core" and the
attachLocation routine and rKey hashing/embedding behavior in your expanded
description so consumers can find the relevant code paths.
♻️ Duplicate comments (1)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
1183-1229:propsis accepted but never persisted.Either forward
contrib.propsinto the contribution details record or remove it from the input type to avoid silent data loss.
🧹 Nitpick comments (1)
packages/sdk-core/src/repository/HypercertOperationsImpl.ts (1)
920-945: Avoidanyin the location-migration path.You can keep strictness and drop the eslint disables with a narrow helper type. As per coding guidelines, avoid
anytypes.♻️ Suggested refactor
- // Use 'any' cast as types might not yet reflect the schema change - // eslint-disable-next-line `@typescript-eslint/no-explicit-any` - const existingLocations: StrongRef[] = (record as any).locations ? [...(record as any).locations] : []; - // eslint-disable-next-line `@typescript-eslint/no-explicit-any` - const legacyLocation = (record as any).location; + type HypercertRecordWithLocations = HypercertClaim & { locations?: StrongRef[]; location?: StrongRef }; + const recordWithLocations = record as HypercertRecordWithLocations; + const existingLocations: StrongRef[] = recordWithLocations.locations ? [...recordWithLocations.locations] : []; + const legacyLocation = recordWithLocations.location;
| --- | ||
| "@hypercerts-org/sdk-react": minor | ||
| "@hypercerts-org/sdk-core": minor | ||
| --- | ||
|
|
||
| fix: migrate legacy location field to locations array in attachLocation |
There was a problem hiding this comment.
Changeset text is too narrow for this PR.
Please expand it to cover rKey hashing/embedding behavior changes and note any compatibility caveats for 0.x minors. Based on learnings, changesets should fully describe user-facing behavior and breaking-risk notes.
📝 Example update
-fix: migrate legacy location field to locations array in attachLocation
+fix(sdk-core): deterministically hash claim content for hc2 rKeys and embed resolved
+locations/contributionDetails in claim creation; attachLocation migrates legacy
+location -> locations
+
+note: sdk-core is pre-1.0; minor releases may include behavior changes that
+affect idempotency and claim embedding.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --- | |
| "@hypercerts-org/sdk-react": minor | |
| "@hypercerts-org/sdk-core": minor | |
| --- | |
| fix: migrate legacy location field to locations array in attachLocation | |
| --- | |
| "@hypercerts-org/sdk-react": minor | |
| "@hypercerts-org/sdk-core": minor | |
| --- | |
| fix(sdk-core): deterministically hash claim content for hc2 rKeys and embed resolved | |
| locations/contributionDetails in claim creation; attachLocation migrates legacy | |
| location -> locations | |
| note: sdk-core is pre-1.0; minor releases may include behavior changes that | |
| affect idempotency and claim embedding. |
🤖 Prompt for AI Agents
In @.changeset/wise-rice-create.md around lines 1 - 6, Update the changeset text
to fully describe the user-facing changes: note that attachLocation now migrates
legacy location -> locations array, describe the rKey hashing and embedding
behavior changes (what changed, how rKeys are hashed/used, and any impacts on
indexing/search), and add compatibility caveats for 0.x minor releases
(migration steps, potential breaking-risk scenarios and how consumers should
adapt). Reference the package entries "@hypercerts-org/sdk-react" and
"@hypercerts-org/sdk-core" and the attachLocation routine and rKey
hashing/embedding behavior in your expanded description so consumers can find
the relevant code paths.
…etadata in Project type Without this patch, the Project type defined the record property as HypercertProjectWithMetadata, but the useProjects and useProject hooks were creating objects with record: HypercertProject. This caused TypeScript errors because HypercertProjectWithMetadata is not a nested structure with uri/cid/record properties. This was a problem because the type mismatch prevented the sdk-react package from passing typecheck, blocking development and builds. This patch solves the problem by: - Importing HypercertProject from @hypercerts-org/sdk-core - Changing the Project type to use record: HypercertProject - Removing the unused HypercertProjectWithMetadata import and re-export The HypercertProject type correctly represents a project record without the resolved metadata fields, matching what the hooks actually return. Co-authored-by: Claude Code <claude-code@noreply.anthropic.com>
aspiers
force-pushed
the
activity-claim-rkey
branch
from
3bece2e to
fb29a89
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| rightsCid, | ||
| imageBlobRef, | ||
| locationRefs, | ||
| contributorsData, |
There was a problem hiding this comment.
Contribution record URIs not returned in result
Medium Severity
When contributions with descriptions are provided, processContributors creates contribution details records via addContribution but the URIs are never collected and returned in result.contributionUris. The CreateHypercertResult interface documents contributionUris as "AT-URIs of contribution records, if contributions were provided" but this field is never populated, breaking the API contract.
Additional Locations (1)
| repo: this.repoDid, | ||
| collection: HYPERCERT_COLLECTIONS.CLAIM, | ||
| record: hypercertRecord, | ||
| rkey, |
There was a problem hiding this comment.
Unused private methods after refactoring
Low Severity
The private methods attachLocationWithProgress and createContributionsWithProgress are never called after the refactoring. The diff removes their invocations from create() but leaves the method definitions in place, creating dead code.
Additional Locations (1)
| locationRefs: locationRefs?.map((ref) => ({ uri: ref.uri, cid: ref.cid })), | ||
| // Contributors: use already-processed canonical format from processContributors() | ||
| contributors: contributorsData, | ||
| }; |
There was a problem hiding this comment.
Facets excluded from rKey hash causing collisions
Medium Severity
The hashInput used to generate the deterministic rKey includes most record fields but omits shortDescriptionFacets and descriptionFacets, even though these fields are stored in the hypercertRecord (lines 298-304). This means two claims with identical text but different rich text annotations (mentions, links, hashtags) would generate the same rKey, causing the second creation to fail with a duplicate key error or unexpectedly overwrite the first record.
| const resolvedLocation = await this.resolveLocation(location); | ||
|
|
||
| // Build new locations array: existing + new location | ||
| const existingLocations = existing.record.locations || []; |
There was a problem hiding this comment.
Legacy location field ignored in attachLocation migration
Medium Severity
The attachLocation method reads only from existing.record.locations (plural), but records created with the previous SDK stored location data in a location field (singular). When attachLocation is called on such records, the old location is silently ignored, and the new locations array starts empty. The old location data remains in the record but is orphaned and no longer part of the canonical locations array. This contradicts the documented behavior that "attachLocation() now appends to existing locations array."
2 participants
Feature: Deterministic rKey Generation for Activity Claims
Closes #102
Feature: Deterministic rKey Generation & Enhanced Contribution Support
Deterministic rKeys
The SDK now pre-generates the
rKeybased on the SHA-256 hash of the complete claim content.Format:
hc2:{sha256Hash}Detailed Contributions
Added support for the full
contributionDetailsunion type in the lexicon.org.hypercerts.claim.contributionDetailsrecord and embeds the StrongRef.Motivation
Changes
HypercertOperationsImpl.createto:locationStrongRefs before the claim.contributionDetailsStrongRefs before the claim.Verification
pnpm testinpackages/sdk-core-> All 618 tests passed.pnpm typecheckpasses.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Breaking Changes
locationparameter renamed tolocations(array)locationUri/locationCidrenamed tolocationUris/locationCids(arrays)Documentation
✏️ Tip: You can customize this high-level summary in your review settings.
Note
Introduces multi-location claims and deterministic record keys, refactoring creation flow to embed normalized references for idempotency and consistency.
CreateHypercertParams.location→CreateHypercertParams.locations[];CreateHypercertResult.locationUri/Cid→locationUris/Cids;attachLocation()now appends tolocationsrKeyashc2:{sha256}usingsrc/lib/crypto.ts(stableStringify,sha256Hash) over normalized inputs (image CID, rights,locationsStrongRefs, contributors data)locationsand contribution details first, then create rights and the claim with embeddedlocationsandcontributorsstartDate/endDateandlocationsHypercertProjectWithMetadatawithHypercertProjectre-exportsWritten by Cursor Bugbot for commit fb29a89. This will update automatically on new commits. Configure here.