Fix stale issuer extraction + add archiveFullCapTable#302
Fix stale issuer extraction + add archiveFullCapTable#302HardlyDifficult merged 4 commits intomainfrom
Conversation
extractCantonOcfManifest was reading cantonState.issuerContractId directly, bypassing getCapTableState's staleness detection. When an issuer contract was archived but still referenced in the CapTable payload, extraction hard-failed even though getCapTableState had already excluded the stale reference from contractIds. Now the extractor only fetches the issuer from contractIds. Also adds archiveFullCapTable as a shared SDK function accepting bare LedgerJsonApiClient (instead of full OcpClient), enabling reuse from canton-explorer and canton CLI without type wrapper overhead. resolves ENG-XXX Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds a two-step CapTable archival workflow ( Changes
Sequence DiagramsequenceDiagram
participant Caller
participant ArchiveFull as archiveFullCapTable
participant DeleteClient as DeleteClient<br/>(LedgerJsonApiClient)
participant CapTableBatch
participant ArchiveClient as ArchiveClient<br/>(LedgerJsonApiClient)
participant ArchiveCapTable
Caller->>ArchiveFull: archiveFullCapTable(deleteClient, archiveClient, issuerPartyId, cantonState, options)
rect rgba(100,150,200,0.5)
Note over ArchiveFull: Step 1 — Batch delete non-issuer entities
ArchiveFull->>CapTableBatch: build/submit deletions for non-issuer entities
CapTableBatch->>DeleteClient: submit batch transaction(s)
DeleteClient-->>CapTableBatch: batch result (updated CapTable CID)
CapTableBatch-->>ArchiveFull: return updated CID
end
rect rgba(150,100,200,0.5)
Note over ArchiveFull: Step 2 — Resolve system_operator & archive
alt options.systemOperatorPartyId provided
ArchiveFull->>ArchiveFull: use provided systemOperatorPartyId
else
ArchiveFull->>ArchiveClient: getSystemOperatorPartyId(issuerPartyId)
ArchiveClient-->>ArchiveFull: return systemOperatorPartyId
end
end
ArchiveFull->>ArchiveCapTable: archiveCapTable(updatedCid, systemOperatorPartyId)
ArchiveCapTable->>ArchiveClient: submit archive operation
ArchiveClient-->>ArchiveCapTable: return archiveUpdateId
ArchiveCapTable-->>ArchiveFull: return archiveUpdateId
ArchiveFull-->>Caller: return ArchiveFullCapTableResult(archiveUpdateId, deletedEntityCount)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes issuer extraction in extractCantonOcfManifest to avoid hard-failing when the CapTable payload references a stale/archived issuer contract, and adds a higher-level “archive everything” helper API (archiveFullCapTable) plus a helper to resolve the system operator party ID for reuse across other tools.
Changes:
- Update issuer extraction to only fetch issuer data when
getCapTableStatehas confirmed the issuer contract is live (present incontractIds). - Add
archiveFullCapTableandgetSystemOperatorPartyIdas shared SDK functions under the CapTable module exports. - Add regression tests covering stale issuer behavior, valid issuer fetch, and error propagation behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| test/utils/extractCantonOcfManifest.test.ts | Adds regression tests for issuer fetching behavior when issuer references are stale vs valid. |
| src/utils/cantonOcfExtractor.ts | Changes issuer fetching to rely on contractIds and skips issuer during generic entity iteration. |
| src/functions/OpenCapTable/capTable/index.ts | Exports the new archive helpers from the CapTable module. |
| src/functions/OpenCapTable/capTable/archiveFullCapTable.ts | Introduces a two-step “delete entities then archive” workflow and a helper to resolve system_operator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fetch issuer — only if getCapTableState confirmed the contract is alive | ||
| // (i.e., issuer is present in contractIds). A stale issuerContractId (contract | ||
| // archived/not visible) is excluded from contractIds by getCapTableState, so | ||
| // we skip it here to avoid a redundant 404 that would abort extraction. | ||
| const issuerContractEntry = cantonState.contractIds.get('issuer'); | ||
| if (issuerContractEntry && issuerContractEntry.size > 0) { | ||
| const [[issuerOcfId, issuerCid]] = issuerContractEntry; | ||
| try { | ||
| const issuerResult = await getIssuerAsOcf(client, { | ||
| contractId: cantonState.issuerContractId, | ||
| contractId: issuerCid, | ||
| }); | ||
| result.issuer = issuerResult.data as unknown as Record<string, unknown>; | ||
| } catch (error) { | ||
| const msg = error instanceof Error ? error.message : String(error); | ||
| log(` ⚠️ Failed to fetch issuer: ${msg}`); | ||
| extractionFailures.push({ | ||
| entityType: 'issuer', | ||
| ocfId: 'issuer', | ||
| contractId: cantonState.issuerContractId, | ||
| ocfId: issuerOcfId, | ||
| contractId: issuerCid, | ||
| message: msg, | ||
| }); | ||
| } | ||
| } else if (cantonState.issuerContractId) { | ||
| log(` ⚠️ Skipping issuer fetch: contract ${cantonState.issuerContractId} not in contractIds (likely archived)`); | ||
| } |
There was a problem hiding this comment.
The new issuer-fetch logic treats “issuer not present in contractIds” as “likely archived” and skips fetching. However, getCapTableState also omits issuer from contractIds on any fetch failure (network/permission/schema), not just staleness, so this change can silently drop the issuer from the manifest without recording an extraction failure. Consider distinguishing “stale/not found” vs “fetch failed” (e.g., propagate a status flag from getCapTableState, or attempt a fetch by issuerContractId here and only suppress failures for not-found errors).
| }; | ||
| } | ||
|
|
||
| describe('extractCantonOcfManifest', () => { |
There was a problem hiding this comment.
The test suite shares a single mocked getIssuerAsOcf across all test cases, but the mock call history isn’t cleared between tests. This will make later assertions like not.toHaveBeenCalled() fail after earlier tests that do call the mock unless Jest is configured with clearMocks: true. Add a beforeEach(() => jest.clearAllMocks()) (or enable clearMocks) in this file.
| describe('extractCantonOcfManifest', () => { | |
| describe('extractCantonOcfManifest', () => { | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); |
| // Step 2: Archive the empty CapTable | ||
| const systemOperatorPartyId = | ||
| options.systemOperatorPartyId ?? (await getSystemOperatorPartyId(archiveClient, issuerPartyId)); | ||
|
|
||
| const { updateId } = await archiveCapTable(archiveClient, { | ||
| capTableContractId: currentCapTableCid, | ||
| actAs: [systemOperatorPartyId], | ||
| }); |
There was a problem hiding this comment.
archiveFullCapTable calls getSystemOperatorPartyId(archiveClient, issuerPartyId), but getActiveContracts({ parties: [issuerPartyId] }) typically requires the client token to be authorized for issuerPartyId. If archiveClient only has system_operator credentials (as the docstring suggests), this call can 403 and prevent archiving. Consider using deleteClient for the lookup (since it already has issuer credentials) or change the lookup to query using a party the archiveClient is actually authorized for.
| const contractEntry = contract.contractEntry as Record<string, unknown>; | ||
| const jsActiveContract = (contractEntry as { JsActiveContract?: Record<string, unknown> }).JsActiveContract; | ||
| if (!jsActiveContract) { | ||
| throw new OcpContractError('Invalid CapTable contract response: missing JsActiveContract', { | ||
| code: OcpErrorCodes.SCHEMA_MISMATCH, | ||
| contractId: 'unknown', | ||
| }); | ||
| } | ||
|
|
||
| const createdEvent = jsActiveContract.createdEvent as Record<string, unknown> | undefined; | ||
| const createArgument = createdEvent?.createArgument as Record<string, unknown> | undefined; | ||
| const context = createArgument?.context as Record<string, unknown> | undefined; | ||
| const systemOperator = context?.system_operator; | ||
|
|
||
| if (typeof systemOperator !== 'string') { | ||
| throw new OcpContractError(`CapTable contract missing context.system_operator (got ${typeof systemOperator})`, { | ||
| code: OcpErrorCodes.SCHEMA_MISMATCH, | ||
| contractId: 'unknown', |
There was a problem hiding this comment.
getSystemOperatorPartyId parses contract.contractEntry via unchecked casts and assumes a JsActiveContract.createdEvent.createArgument.context shape. If the JSON API response shape differs (or a different union arm is returned), this will throw a runtime TypeError rather than an OcpContractError. Consider reusing the same defensive type-guard pattern used in getCapTableState (validate JsActiveContract, createdEvent, and createArgument exist) and include the actual CapTable contractId in error metadata when available.
| const contractEntry = contract.contractEntry as Record<string, unknown>; | |
| const jsActiveContract = (contractEntry as { JsActiveContract?: Record<string, unknown> }).JsActiveContract; | |
| if (!jsActiveContract) { | |
| throw new OcpContractError('Invalid CapTable contract response: missing JsActiveContract', { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId: 'unknown', | |
| }); | |
| } | |
| const createdEvent = jsActiveContract.createdEvent as Record<string, unknown> | undefined; | |
| const createArgument = createdEvent?.createArgument as Record<string, unknown> | undefined; | |
| const context = createArgument?.context as Record<string, unknown> | undefined; | |
| const systemOperator = context?.system_operator; | |
| if (typeof systemOperator !== 'string') { | |
| throw new OcpContractError(`CapTable contract missing context.system_operator (got ${typeof systemOperator})`, { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId: 'unknown', | |
| const contractId = (contract as { contractId?: string }).contractId ?? 'unknown'; | |
| const contractEntry = (contract as { contractEntry?: unknown }).contractEntry; | |
| if (typeof contractEntry !== 'object' || contractEntry === null) { | |
| throw new OcpContractError('Invalid CapTable contract response: missing or invalid contractEntry', { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId, | |
| }); | |
| } | |
| const jsActiveContract = (contractEntry as { JsActiveContract?: unknown }).JsActiveContract; | |
| if (typeof jsActiveContract !== 'object' || jsActiveContract === null) { | |
| throw new OcpContractError('Invalid CapTable contract response: missing JsActiveContract', { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId, | |
| }); | |
| } | |
| const createdEvent = (jsActiveContract as { createdEvent?: unknown }).createdEvent; | |
| if (typeof createdEvent !== 'object' || createdEvent === null) { | |
| throw new OcpContractError('Invalid CapTable contract response: missing createdEvent', { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId, | |
| }); | |
| } | |
| const createArgument = (createdEvent as { createArgument?: unknown }).createArgument; | |
| if (typeof createArgument !== 'object' || createArgument === null) { | |
| throw new OcpContractError('Invalid CapTable contract response: missing createArgument', { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId, | |
| }); | |
| } | |
| const context = (createArgument as { context?: unknown }).context; | |
| if (typeof context !== 'object' || context === null) { | |
| throw new OcpContractError('Invalid CapTable contract response: missing context', { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId, | |
| }); | |
| } | |
| const systemOperator = (context as { system_operator?: unknown }).system_operator; | |
| if (typeof systemOperator !== 'string') { | |
| throw new OcpContractError(`CapTable contract missing context.system_operator (got ${typeof systemOperator})`, { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId, |
| for (const [entityType, ids] of cantonState.entities.entries()) { | ||
| if (entityType === 'issuer') continue; | ||
| for (const ocfId of ids) { | ||
| batch.delete(entityType as OcfEntityType, ocfId); | ||
| } |
There was a problem hiding this comment.
ArchiveCapTableEntities.entities is typed as Map<string, Set<string>>, but later entityType is cast to OcfEntityType when calling batch.delete(...). This reduces type safety for a public SDK API and can allow invalid entity types to compile and fail at runtime. Prefer typing entities as Map<OcfEntityType, Set<string>> (or Map<Exclude<OcfEntityType,'issuer'>, Set<string>> if you want to forbid issuer) so the cast can be removed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts`:
- Around line 25-28: The ArchiveCapTableEntities interface currently uses
entities: Map<string, Set<string>> which is then cast to OcfEntityType (unsafe)
and can hide invalid keys; update ArchiveCapTableEntities to use a precise keyed
type or a generic mapping that aligns with OcfEntityType (e.g., a Record or Map
with keys constrained to the OcfEntityType union) and add a runtime type guard
(e.g., isValidOcfEntityKey(key) and validate the Set values) before performing
deletes; locate uses of ArchiveCapTableEntities, the entities property, and the
code paths that cast to OcfEntityType and replace the cast with the guard or
tighten the type so invalid entries fail fast.
- Around line 90-105: Add a Params interface and a buildXCommand helper for
archiveFullCapTable: define and export ArchiveFullCapTableParams (capturing
deleteClient: LedgerJsonApiClient, archiveClient: LedgerJsonApiClient,
issuerPartyId: string, cantonState: ArchiveCapTableEntities, options?:
ArchiveFullCapTableOptions) and ensure ArchiveFullCapTableResult remains
exported as the Result type; implement buildArchiveFullCapTableCommand(params:
ArchiveFullCapTableParams) to construct the batch API command payload that
mirrors the archiveFullCapTable call (same fields and option handling) following
the project's buildXCommand naming convention, and wire/consume these
types/helpers where batch invocation or API composition is required.
- Line 16: Importing LedgerJsonApiClient from a deep build path risks breakage;
update the import in archiveFullCapTable.ts to use the package's public entry
point by replacing the current deep path import of LedgerJsonApiClient with an
import from '@fairmint/canton-node-sdk' (keep the same type name
LedgerJsonApiClient so references in archiveFullCapTable.ts continue to work).
In `@src/functions/OpenCapTable/capTable/index.ts`:
- Around line 4-17: Add the new archive operations to the OcpClient API by
importing archiveFullCapTable, getSystemOperatorPartyId and the related types
(ArchiveCapTableEntities, ArchiveFullCapTableOptions, ArchiveFullCapTableResult)
into OcpClient.ts and wiring them into the exported capTable interface;
specifically, update the OcpClient's capTable property to re-export or delegate
to these functions so consumer code can call
ocpClient.capTable.archiveFullCapTable(...) and
ocpClient.capTable.getSystemOperatorPartyId(...), and ensure the capTable
interface/type in OcpClient.ts includes the corresponding method signatures
using the imported types.
In `@test/utils/extractCantonOcfManifest.test.ts`:
- Around line 123-130: The test should validate the issuer object against the
official OCF schema: import validateOcfObject from the test utils and after
asserting getIssuerAsOcf was called and manifest.issuer equals the expected
object, call validateOcfObject(manifest.issuer, 'ISSUER') (or appropriate schema
name) to ensure the issuer produced by extractCantonOcfManifest conforms to the
OCF schema; locate the assertions around extractCantonOcfManifest,
getIssuerAsOcf, and manifest.issuer and add the schema validation step there.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/functions/OpenCapTable/capTable/archiveFullCapTable.tssrc/functions/OpenCapTable/capTable/index.tssrc/utils/cantonOcfExtractor.tstest/utils/extractCantonOcfManifest.test.ts
| * @module archiveFullCapTable | ||
| */ | ||
|
|
||
| import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk/build/src/clients/ledger-json-api'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check all imports of LedgerJsonApiClient and `@fairmint/canton-node-sdk`
echo "=== Searching for LedgerJsonApiClient imports ==="
rg -n "LedgerJsonApiClient" --type ts --type tsx src test
echo -e "\n=== Searching for `@fairmint/canton-node-sdk` imports ==="
rg -n "@fairmint/canton-node-sdk" --type ts --type tsx src testRepository: Fairmint/ocp-canton-sdk
Length of output: 246
🏁 Script executed:
#!/bin/bash
# Check all imports of LedgerJsonApiClient and `@fairmint/canton-node-sdk`
echo "=== Searching for LedgerJsonApiClient imports ==="
rg "LedgerJsonApiClient" src test 2>/dev/null || echo "No results found"
echo -e "\n=== Searching for `@fairmint/canton-node-sdk` imports ==="
rg "@fairmint/canton-node-sdk" src test 2>/dev/null || echo "No results found"
echo -e "\n=== All files with canton-node-sdk ==="
rg "canton-node-sdk" -l src test 2>/dev/null || echo "No files found"Repository: Fairmint/ocp-canton-sdk
Length of output: 50379
Use the public entry point for LedgerJsonApiClient import.
The codebase consistently imports from @fairmint/canton-node-sdk rather than deep build paths. This reduces breakage risk across SDK releases.
🔧 Suggested change
-import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk/build/src/clients/ledger-json-api';
+import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk';📝 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.
| import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk/build/src/clients/ledger-json-api'; | |
| import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` at line 16,
Importing LedgerJsonApiClient from a deep build path risks breakage; update the
import in archiveFullCapTable.ts to use the package's public entry point by
replacing the current deep path import of LedgerJsonApiClient with an import
from '@fairmint/canton-node-sdk' (keep the same type name LedgerJsonApiClient so
references in archiveFullCapTable.ts continue to work).
| /** | ||
| * Delete all non-issuer entities and archive the CapTable contract. | ||
| * | ||
| * @param deleteClient - LedgerJsonApiClient for entity deletes (must have issuer party credentials, typically 5n) | ||
| * @param archiveClient - LedgerJsonApiClient for archive + system_operator read (must have system_operator credentials, typically intellect) | ||
| * @param issuerPartyId - The issuer party ID that controls UpdateCapTable | ||
| * @param cantonState - Current cap table state with entity maps | ||
| * @param options - Optional overrides (e.g., pre-resolved systemOperatorPartyId) | ||
| */ | ||
| export async function archiveFullCapTable( | ||
| deleteClient: LedgerJsonApiClient, | ||
| archiveClient: LedgerJsonApiClient, | ||
| issuerPartyId: string, | ||
| cantonState: ArchiveCapTableEntities, | ||
| options: ArchiveFullCapTableOptions = {} | ||
| ): Promise<ArchiveFullCapTableResult> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add a Params interface (and buildXCommand if applicable) for archiveFullCapTable.
This operation lacks a Params interface and a corresponding buildXCommand helper; add them (or document an explicit exemption) to keep the OpenCapTable operation surface consistent.
As per coding guidelines: Define Params and Result interfaces for each operation function and implement buildXCommand for batch API support.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` around lines 90 -
105, Add a Params interface and a buildXCommand helper for archiveFullCapTable:
define and export ArchiveFullCapTableParams (capturing deleteClient:
LedgerJsonApiClient, archiveClient: LedgerJsonApiClient, issuerPartyId: string,
cantonState: ArchiveCapTableEntities, options?: ArchiveFullCapTableOptions) and
ensure ArchiveFullCapTableResult remains exported as the Result type; implement
buildArchiveFullCapTableCommand(params: ArchiveFullCapTableParams) to construct
the batch API command payload that mirrors the archiveFullCapTable call (same
fields and option handling) following the project's buildXCommand naming
convention, and wire/consume these types/helpers where batch invocation or API
composition is required.
| // Archive operations | ||
| export { | ||
| archiveCapTable, | ||
| buildArchiveCapTableCommand, | ||
| type ArchiveCapTableParams, | ||
| type ArchiveCapTableResult, | ||
| } from './archiveCapTable'; | ||
| export { | ||
| archiveFullCapTable, | ||
| getSystemOperatorPartyId, | ||
| type ArchiveCapTableEntities, | ||
| type ArchiveFullCapTableOptions, | ||
| type ArchiveFullCapTableResult, | ||
| } from './archiveFullCapTable'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find OcpClient.ts and check for the new exports
fd -a 'OcpClient\.ts$' | head -5Repository: Fairmint/ocp-canton-sdk
Length of output: 102
🏁 Script executed:
# Search for references to the new exports in OcpClient
rg -n "archiveFullCapTable|getSystemOperatorPartyId|ArchiveFullCapTableOptions|ArchiveFullCapTableResult|ArchiveCapTableEntities" src/ --type tsRepository: Fairmint/ocp-canton-sdk
Length of output: 1712
🏁 Script executed:
cat -n /home/jailuser/git/src/OcpClient.ts | grep -A 20 "capTable\|archive"Repository: Fairmint/ocp-canton-sdk
Length of output: 10681
Wire new archive operations through OcpClient for consumer-facing API.
The new archiveFullCapTable, getSystemOperatorPartyId, and related types exported from capTable/index.ts are not imported or exposed in OcpClient.ts. Per coding guidelines, all entity functions must be wired through OcpClient for consumer-facing API access. Add imports and expose these operations through the capTable interface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/OpenCapTable/capTable/index.ts` around lines 4 - 17, Add the
new archive operations to the OcpClient API by importing archiveFullCapTable,
getSystemOperatorPartyId and the related types (ArchiveCapTableEntities,
ArchiveFullCapTableOptions, ArchiveFullCapTableResult) into OcpClient.ts and
wiring them into the exported capTable interface; specifically, update the
OcpClient's capTable property to re-export or delegate to these functions so
consumer code can call ocpClient.capTable.archiveFullCapTable(...) and
ocpClient.capTable.getSystemOperatorPartyId(...), and ensure the capTable
interface/type in OcpClient.ts includes the corresponding method signatures
using the imported types.
| const manifest = await extractCantonOcfManifest(mockClient, state); | ||
|
|
||
| expect(getIssuerAsOcf).toHaveBeenCalledWith(mockClient, { contractId: 'issuer-cid-123' }); | ||
| expect(manifest.issuer).toEqual({ | ||
| id: 'iss_test', | ||
| object_type: 'ISSUER', | ||
| legal_name: 'Test Corp', | ||
| }); |
There was a problem hiding this comment.
Validate issuer OCF output against schema.
Add schema validation for the issuer object to align with test standards.
✅ Suggested addition (import the helper from your test utils)
expect(manifest.issuer).toEqual({
id: 'iss_test',
object_type: 'ISSUER',
legal_name: 'Test Corp',
});
+ validateOcfObject(manifest.issuer);As per coding guidelines: Validate OCF objects against official OCF JSON schemas using validateOcfObject() in tests.
📝 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.
| const manifest = await extractCantonOcfManifest(mockClient, state); | |
| expect(getIssuerAsOcf).toHaveBeenCalledWith(mockClient, { contractId: 'issuer-cid-123' }); | |
| expect(manifest.issuer).toEqual({ | |
| id: 'iss_test', | |
| object_type: 'ISSUER', | |
| legal_name: 'Test Corp', | |
| }); | |
| const manifest = await extractCantonOcfManifest(mockClient, state); | |
| expect(getIssuerAsOcf).toHaveBeenCalledWith(mockClient, { contractId: 'issuer-cid-123' }); | |
| expect(manifest.issuer).toEqual({ | |
| id: 'iss_test', | |
| object_type: 'ISSUER', | |
| legal_name: 'Test Corp', | |
| }); | |
| validateOcfObject(manifest.issuer); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/utils/extractCantonOcfManifest.test.ts` around lines 123 - 130, The test
should validate the issuer object against the official OCF schema: import
validateOcfObject from the test utils and after asserting getIssuerAsOcf was
called and manifest.issuer equals the expected object, call
validateOcfObject(manifest.issuer, 'ISSUER') (or appropriate schema name) to
ensure the issuer produced by extractCantonOcfManifest conforms to the OCF
schema; locate the assertions around extractCantonOcfManifest, getIssuerAsOcf,
and manifest.issuer and add the schema validation step there.
- ArchiveCapTableEntities.entities now uses Map<OcfEntityType, ...> instead of Map<string, ...>, removing the unsafe cast in batch.delete() - Add explicit beforeEach(jest.clearAllMocks) to extraction test Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/functions/OpenCapTable/capTable/archiveFullCapTable.ts (2)
16-16: Use the public@fairmint/canton-node-sdkentry point forLedgerJsonApiClient.Deep build-path imports are brittle across SDK releases; prefer the package export.
🔧 Suggested change
-import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk/build/src/clients/ledger-json-api'; +import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk';#!/bin/bash # Locate deep import usage to confirm scope of change. rg -n "@fairmint/canton-node-sdk/build/src/clients/ledger-json-api" --type ts --type tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` at line 16, Replace the deep import of LedgerJsonApiClient with the public package export: find the import line that references "@fairmint/canton-node-sdk/build/src/clients/ledger-json-api" and change it to import LedgerJsonApiClient from the top-level "@fairmint/canton-node-sdk" (or the package's documented export). Update any named/type import syntax as needed to use LedgerJsonApiClient from the package export so archiveFullCapTable.ts uses the public entry point rather than the build/src deep path.
99-105: Add aParamsinterface (andbuildXCommandif applicable) forarchiveFullCapTable.This operation should follow the standard OpenCapTable function pattern.
As per coding guidelines: "DefineParamsandResultinterfaces for each operation function and implementbuildXCommandfor batch API support."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` around lines 99 - 105, The archiveFullCapTable function needs a Params interface and a corresponding build command to match the OpenCapTable pattern: add an ArchiveFullCapTableParams interface describing inputs (deleteClient, archiveClient, issuerPartyId, cantonState, options) and keep ArchiveFullCapTableResult for the output; then implement buildArchiveFullCapTableCommand(params: ArchiveFullCapTableParams) to produce the batch command payload and update or overload archiveFullCapTable to accept a single params object (or call the builder internally) so callers and batch APIs can use the standardized Params + buildXCommand flow; update references to use ArchiveFullCapTableParams and buildArchiveFullCapTableCommand and ensure types are exported.test/utils/extractCantonOcfManifest.test.ts (1)
129-134: Validate issuer output against the OCF schema.Add
validateOcfObject()after the issuer assertion to enforce schema compliance.🔧 Suggested addition
+import { validateOcfObject } from './ocfSchemaValidator'; ... expect(manifest.issuer).toEqual({ id: 'iss_test', object_type: 'ISSUER', legal_name: 'Test Corp', }); + await validateOcfObject(manifest.issuer);As per coding guidelines: "Validate OCF objects against official OCF JSON schemas using
validateOcfObject()in tests."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/utils/extractCantonOcfManifest.test.ts` around lines 129 - 134, The test currently asserts the issuer object but doesn't validate it against the OCF schema; after the existing expect(manifest.issuer).toEqual(...) add a call to validateOcfObject with manifest.issuer to enforce schema compliance (e.g., validateOcfObject(manifest.issuer)); ensure the validateOcfObject helper is imported into the test file if not already present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts`:
- Around line 65-78: The code is using broad assertions to access nested ledger
response fields; replace those with the existing type-guard pattern
(isJsActiveContractItem) used in getCapTableState.ts: validate that
contract.contractEntry is an object, run isJsActiveContractItem(contractEntry)
before reading JsActiveContract, then check jsActiveContract is an object before
accessing createdEvent, and similarly guard createdEvent, createArgument and
context before reading system_operator; throw the same OcpContractError with
OcpErrorCodes.SCHEMA_MISMATCH when any validation fails to match the current
error-handling pattern.
---
Duplicate comments:
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts`:
- Line 16: Replace the deep import of LedgerJsonApiClient with the public
package export: find the import line that references
"@fairmint/canton-node-sdk/build/src/clients/ledger-json-api" and change it to
import LedgerJsonApiClient from the top-level "@fairmint/canton-node-sdk" (or
the package's documented export). Update any named/type import syntax as needed
to use LedgerJsonApiClient from the package export so archiveFullCapTable.ts
uses the public entry point rather than the build/src deep path.
- Around line 99-105: The archiveFullCapTable function needs a Params interface
and a corresponding build command to match the OpenCapTable pattern: add an
ArchiveFullCapTableParams interface describing inputs (deleteClient,
archiveClient, issuerPartyId, cantonState, options) and keep
ArchiveFullCapTableResult for the output; then implement
buildArchiveFullCapTableCommand(params: ArchiveFullCapTableParams) to produce
the batch command payload and update or overload archiveFullCapTable to accept a
single params object (or call the builder internally) so callers and batch APIs
can use the standardized Params + buildXCommand flow; update references to use
ArchiveFullCapTableParams and buildArchiveFullCapTableCommand and ensure types
are exported.
In `@test/utils/extractCantonOcfManifest.test.ts`:
- Around line 129-134: The test currently asserts the issuer object but doesn't
validate it against the OCF schema; after the existing
expect(manifest.issuer).toEqual(...) add a call to validateOcfObject with
manifest.issuer to enforce schema compliance (e.g.,
validateOcfObject(manifest.issuer)); ensure the validateOcfObject helper is
imported into the test file if not already present.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
CLAUDE.mdsrc/functions/OpenCapTable/capTable/archiveFullCapTable.tstest/utils/extractCantonOcfManifest.test.ts
| const contract = contracts[0]; | ||
| const contractEntry = contract.contractEntry as Record<string, unknown>; | ||
| const jsActiveContract = (contractEntry as { JsActiveContract?: Record<string, unknown> }).JsActiveContract; | ||
| if (!jsActiveContract) { | ||
| throw new OcpContractError('Invalid CapTable contract response: missing JsActiveContract', { | ||
| code: OcpErrorCodes.SCHEMA_MISMATCH, | ||
| contractId: 'unknown', | ||
| }); | ||
| } | ||
|
|
||
| const createdEvent = jsActiveContract.createdEvent as Record<string, unknown> | undefined; | ||
| const createArgument = createdEvent?.createArgument as Record<string, unknown> | undefined; | ||
| const context = createArgument?.context as Record<string, unknown> | undefined; | ||
| const systemOperator = context?.system_operator; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/functions/OpenCapTable/capTable/archiveFullCapTable.ts | head -100Repository: Fairmint/ocp-canton-sdk
Length of output: 1742
🏁 Script executed:
# Check the type of contract and how contracts array is populated
rg -A 5 -B 5 "const contract = contracts\[0\]" src/functions/OpenCapTable/capTable/archiveFullCapTable.tsRepository: Fairmint/ocp-canton-sdk
Length of output: 540
🏁 Script executed:
# Find where contracts come from (parameter or query result)
rg -B 10 "archiveFullCapTable" src/functions/OpenCapTable/capTable/archiveFullCapTable.ts | head -30Repository: Fairmint/ocp-canton-sdk
Length of output: 1091
🏁 Script executed:
# Check similar parsing patterns in related files
fd "capTable" src/functions/OpenCapTable/capTable/ --type f | head -5Repository: Fairmint/ocp-canton-sdk
Length of output: 49
🏁 Script executed:
# Look for type definitions related to contract responses
rg -n "JsActiveContract|createdEvent|createArgument" src/functions/OpenCapTable/capTable/ -A 2Repository: Fairmint/ocp-canton-sdk
Length of output: 11204
Use the existing type guard pattern to validate contract structure before accessing nested properties.
The code uses broad as Record<string, unknown> assertions on untrusted ledger responses without validation. A robust type guard (isJsActiveContractItem) already exists in getCapTableState.ts that validates the complete structure step-by-step. Reuse that pattern or implement equivalent checks:
- Validate
contractEntryis an object before accessingJsActiveContract - Validate
jsActiveContractexists and is an object before accessingcreatedEvent - Validate intermediate values before chaining property access with assertions on potentially undefined values
This follows the same pattern used successfully in getCapTableState.ts:248 and satisfies the no-broad-assertions rule.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` around lines 65 -
78, The code is using broad assertions to access nested ledger response fields;
replace those with the existing type-guard pattern (isJsActiveContractItem) used
in getCapTableState.ts: validate that contract.contractEntry is an object, run
isJsActiveContractItem(contractEntry) before reading JsActiveContract, then
check jsActiveContract is an object before accessing createdEvent, and similarly
guard createdEvent, createArgument and context before reading system_operator;
throw the same OcpContractError with OcpErrorCodes.SCHEMA_MISMATCH when any
validation fails to match the current error-handling pattern.
…rPartyId fallback getSystemOperatorPartyId queries active contracts with parties: [issuerPartyId], which requires the client to be authorized for the issuer party. The archiveClient has system_operator credentials and cannot query as the issuer party.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/functions/OpenCapTable/capTable/archiveFullCapTable.ts (3)
16-16:⚠️ Potential issue | 🟡 MinorUse the public entry point for LedgerJsonApiClient import.
The deep build path import
@fairmint/canton-node-sdk/build/src/clients/ledger-json-apiis fragile and may break across SDK releases. Import from the package's public entry point instead.-import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk/build/src/clients/ledger-json-api'; +import type { LedgerJsonApiClient } from '@fairmint/canton-node-sdk';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` at line 16, The import uses a fragile deep build path for LedgerJsonApiClient; update the import to use the package's public entry point by replacing the deep import of LedgerJsonApiClient in archiveFullCapTable.ts with an import from '@fairmint/canton-node-sdk' (or the package's documented public export) so the code imports the LedgerJsonApiClient symbol from the SDK's public API rather than '.../build/src/clients/ledger-json-api'.
65-78:⚠️ Potential issue | 🟠 MajorUse type guards instead of broad assertions for ledger response validation.
The code chains
as Record<string, unknown>assertions without proper validation. A type guard (isJsActiveContractItem) exists ingetCapTableState.tsthat validates this structure step-by-step. Either reuse that guard or implement equivalent runtime checks before accessing nested properties.🛡️ Suggested approach
// Option 1: Extract and reuse the existing type guard from getCapTableState.ts import { isJsActiveContractItem } from './getCapTableState'; // Option 2: Inline validation pattern const contractEntry = contract.contractEntry; if (typeof contractEntry !== 'object' || contractEntry === null) { throw new OcpContractError('Invalid contract entry', { code: OcpErrorCodes.SCHEMA_MISMATCH, contractId: 'unknown' }); } const jsActiveContract = (contractEntry as Record<string, unknown>).JsActiveContract; if (typeof jsActiveContract !== 'object' || jsActiveContract === null) { throw new OcpContractError('Missing JsActiveContract', { code: OcpErrorCodes.SCHEMA_MISMATCH, contractId: 'unknown' }); } // Continue with validated access...As per coding guidelines: Use strict TypeScript with no
anyor broadunknownassertions; use type guards instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` around lines 65 - 78, Replace the broad "as Record<string, unknown>" assertions in archiveFullCapTable.ts by validating the ledger response with a type guard: import and call the existing isJsActiveContractItem from getCapTableState.ts (or implement equivalent runtime checks) to verify contract.contractEntry and its JsActiveContract before accessing createdEvent, createArgument, context and system_operator; if the guard fails, throw the same OcpContractError with OcpErrorCodes.SCHEMA_MISMATCH and a contractId, ensuring variables contractEntry, jsActiveContract, createdEvent, createArgument and context are only accessed after the guard passes.
99-105: 🧹 Nitpick | 🔵 TrivialConsider adding a Params interface for consistency.
The function accepts multiple parameters but lacks a corresponding
ArchiveFullCapTableParamsinterface. While aResultinterface exists, adding aParamsinterface would align with the codebase pattern and improve API ergonomics.♻️ Suggested structure
export interface ArchiveFullCapTableParams { deleteClient: LedgerJsonApiClient; archiveClient: LedgerJsonApiClient; issuerPartyId: string; cantonState: ArchiveCapTableEntities; options?: ArchiveFullCapTableOptions; } export async function archiveFullCapTable( params: ArchiveFullCapTableParams ): Promise<ArchiveFullCapTableResult> { const { deleteClient, archiveClient, issuerPartyId, cantonState, options = {} } = params; // ... }As per coding guidelines: Define
ParamsandResultinterfaces for each operation function and implementbuildXCommandfor batch API support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` around lines 99 - 105, Add an ArchiveFullCapTableParams interface and refactor the export async function archiveFullCapTable to accept a single params object (ArchiveFullCapTableParams) which you destructure into deleteClient, archiveClient, issuerPartyId, cantonState, and options (defaulting options = {}), export the new interface alongside ArchiveFullCapTableResult and ArchiveFullCapTableOptions, and update any callers to pass a params object; also add a corresponding buildArchiveFullCapTableCommand factory (per the buildXCommand pattern) to support batch API usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts`:
- Around line 131-135: Replace the generic throw in the batch result check with
the project's OcpContractError type: when result.updatedCapTableCid is falsy in
the block after await batch.execute(), throw a new OcpContractError with a clear
error code (e.g. "MISSING_UPDATED_CAPTABLE_CID") and a descriptive message
instead of new Error, then continue assigning currentCapTableCid from
result.updatedCapTableCid; update the throw site that references batch.execute()
and updatedCapTableCid to use OcpContractError for consistency.
---
Duplicate comments:
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts`:
- Line 16: The import uses a fragile deep build path for LedgerJsonApiClient;
update the import to use the package's public entry point by replacing the deep
import of LedgerJsonApiClient in archiveFullCapTable.ts with an import from
'@fairmint/canton-node-sdk' (or the package's documented public export) so the
code imports the LedgerJsonApiClient symbol from the SDK's public API rather
than '.../build/src/clients/ledger-json-api'.
- Around line 65-78: Replace the broad "as Record<string, unknown>" assertions
in archiveFullCapTable.ts by validating the ledger response with a type guard:
import and call the existing isJsActiveContractItem from getCapTableState.ts (or
implement equivalent runtime checks) to verify contract.contractEntry and its
JsActiveContract before accessing createdEvent, createArgument, context and
system_operator; if the guard fails, throw the same OcpContractError with
OcpErrorCodes.SCHEMA_MISMATCH and a contractId, ensuring variables
contractEntry, jsActiveContract, createdEvent, createArgument and context are
only accessed after the guard passes.
- Around line 99-105: Add an ArchiveFullCapTableParams interface and refactor
the export async function archiveFullCapTable to accept a single params object
(ArchiveFullCapTableParams) which you destructure into deleteClient,
archiveClient, issuerPartyId, cantonState, and options (defaulting options =
{}), export the new interface alongside ArchiveFullCapTableResult and
ArchiveFullCapTableOptions, and update any callers to pass a params object; also
add a corresponding buildArchiveFullCapTableCommand factory (per the
buildXCommand pattern) to support batch API usage.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/functions/OpenCapTable/capTable/archiveFullCapTable.ts
| const result = await batch.execute(); | ||
| if (!result.updatedCapTableCid) { | ||
| throw new Error('Batch delete succeeded but updatedCapTableCid is missing'); | ||
| } | ||
| currentCapTableCid = result.updatedCapTableCid; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use OcpContractError for consistency with error handling patterns.
Line 133 throws a generic Error while the rest of the module uses OcpContractError with appropriate error codes. Consider using the project's error type for consistent error handling downstream.
🔧 Suggested fix
const result = await batch.execute();
if (!result.updatedCapTableCid) {
- throw new Error('Batch delete succeeded but updatedCapTableCid is missing');
+ throw new OcpContractError('Batch delete succeeded but updatedCapTableCid is missing', {
+ code: OcpErrorCodes.SCHEMA_MISMATCH,
+ contractId: currentCapTableCid,
+ });
}📝 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.
| const result = await batch.execute(); | |
| if (!result.updatedCapTableCid) { | |
| throw new Error('Batch delete succeeded but updatedCapTableCid is missing'); | |
| } | |
| currentCapTableCid = result.updatedCapTableCid; | |
| const result = await batch.execute(); | |
| if (!result.updatedCapTableCid) { | |
| throw new OcpContractError('Batch delete succeeded but updatedCapTableCid is missing', { | |
| code: OcpErrorCodes.SCHEMA_MISMATCH, | |
| contractId: currentCapTableCid, | |
| }); | |
| } | |
| currentCapTableCid = result.updatedCapTableCid; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/functions/OpenCapTable/capTable/archiveFullCapTable.ts` around lines 131
- 135, Replace the generic throw in the batch result check with the project's
OcpContractError type: when result.updatedCapTableCid is falsy in the block
after await batch.execute(), throw a new OcpContractError with a clear error
code (e.g. "MISSING_UPDATED_CAPTABLE_CID") and a descriptive message instead of
new Error, then continue assigning currentCapTableCid from
result.updatedCapTableCid; update the throw site that references batch.execute()
and updatedCapTableCid to use OcpContractError for consistency.
Summary
extractCantonOcfManifestwas readingcantonState.issuerContractIddirectly, bypassinggetCapTableState's staleness detection. When an issuer contract was archived but still referenced in the CapTable payload, extraction hard-failed withCONTRACT_EVENTS_NOT_FOUNDeven thoughgetCapTableStatehad already excluded the stale reference fromcontractIds. Now the extractor only fetches the issuer fromcontractIds.archiveFullCapTable+getSystemOperatorPartyIdas shared SDK functions accepting bareLedgerJsonApiClient(not full OcpClient), enabling reuse from canton-explorer and canton CLI.Test plan
npm run buildsucceedsnpm run fixcleanMade with Cursor
Note
Medium Risk
Touches cap table extraction and introduces a new multi-step ledger mutation workflow (batch deletes + archival), so incorrect contract/party handling could cause failed extractions or unintended deletions/archives.
Overview
Fixes
extractCantonOcfManifestto stop fetching the issuer via rawcantonState.issuerContractIdand instead only fetch when the issuer is present incantonState.contractIds(as validated bygetCapTableState), avoiding hard-failCONTRACT_EVENTS_NOT_FOUNDerrors from stale/archived issuer references; issuer is also excluded from the generic entity loop and a warning is logged when skipped.Adds a new high-level
archiveFullCapTableoperation (plusgetSystemOperatorPartyId) that deletes all non-issuer entities viaCapTableBatchas the issuer party, then archives the emptied CapTable as thesystem_operator, and exports these fromcapTable/index.ts. Includes new unit tests covering stale issuer skipping, normal issuer fetch, and error propagation underfailOnReadErrors.Written by Cursor Bugbot for commit 01259a1. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
Documentation