Skip to content

feat(pq-algorithm-id/ts): phase 2 - implement conversion and x509 mapping apis (ENG-1915)#27

Merged
eacet merged 1 commit intomainfrom
feature/eng-1915
Mar 12, 2026
Merged

feat(pq-algorithm-id/ts): phase 2 - implement conversion and x509 mapping apis (ENG-1915)#27
eacet merged 1 commit intomainfrom
feature/eng-1915

Conversation

@eacet
Copy link
Copy Markdown
Member

@eacet eacet commented Mar 10, 2026

Summary

Package(s)

Languages

  • TypeScript
  • Rust

Checklist

  • Tests pass for all modified packages
  • Linting/formatting passes (biome check, cargo fmt)
  • Both language implementations are consistent (or noted as follow-up)
  • Package README updated if public API changed
  • No unnecessary dependencies added

Related Issues

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 10, 2026

Greptile Summary

This PR implements Phase 2 of the pq-algorithm-id TypeScript package by adding conversion and X.509 mapping APIs. It introduces lookup.ts (OID/JOSE/COSE bidirectional conversion), x509.ts (X.509 AlgorithmIdentifier generation and parsing), and exports them through index.ts, along with comprehensive test coverage for both new modules.

Key changes and observations:

  • lookup.ts: Module-level reverse-lookup Maps (JOSE_TO_NAME, COSE_TO_NAME) are built eagerly at import time from the registry — a clean, efficient design. The isCanonicalOid validator correctly enforces dotted-decimal format, leading-zero rejection, and ITU-T arc constraints.
  • x509.ts: Three interface declarations (X509AlgorithmIdentifier, X509AlgorithmIdentifierInput, X509AlgorithmIdentifierOptions) should use type instead per the project's TypeScript conventions, since none of them use inheritance or class implementation. A redundant input !== null check exists inside normalizeParameters after a prior early return for null. The validateParametersForAlgorithm error paths are currently unreachable because all 18 registered algorithms share a permissive default policy (acceptNull: true, acceptAbsent: true); consider documenting this or adding future-proofing tests.
  • Tests: Both test files copy an identical expectError helper — this should be extracted into a shared tests/helpers.ts to avoid drift.

Confidence Score: 4/5

  • This PR is safe to merge with minor style fixes recommended.
  • The core logic is well-structured and the input validation (OID canonicalization, COSE integer checks, X.509 parameter normalization) is thorough and correctly implemented. No functional bugs were found. The deductions are for: three interface declarations that should be type per project convention, a dead null-check, and duplicated test helpers — all style/maintainability concerns rather than correctness issues.
  • packages/pq-algorithm-id/ts/src/x509.ts has the interface-vs-type violations and a redundant guard. Test files both contain a duplicated helper that should be extracted.

Important Files Changed

Filename Overview
packages/pq-algorithm-id/ts/src/lookup.ts New file implementing OID/JOSE/COSE conversion APIs. Module-level reverse-lookup maps are built eagerly on load. isCanonicalOid is thorough and correctly enforces arc constraints. toOid does a redundant record fetch (harmless). No logic issues found.
packages/pq-algorithm-id/ts/src/x509.ts New file implementing X.509 AlgorithmIdentifier conversion. Three interface declarations should be type per project convention. The input !== null guard on line 51 is dead code after the earlier null check. The validateParametersForAlgorithm error paths are currently unreachable with the default registry policy.
packages/pq-algorithm-id/ts/src/index.ts Re-exports new lookup and x509 APIs cleanly. Mixed value/type export blocks follow existing patterns. No issues found.
packages/pq-algorithm-id/ts/tests/lookup.test.ts Comprehensive tests for OID/JOSE/COSE round-trips, invalid input rejection, and uniqueness invariants. The expectError helper is duplicated from x509.test.ts and could be shared.
packages/pq-algorithm-id/ts/tests/x509.test.ts Good coverage for normalization, round-trip, and error cases. Missing test coverage for validateParametersForAlgorithm error paths (currently unreachable with default registry). expectError duplicated from lookup.test.ts.
.claude/skills/implement-plan-linear/SKILL.md Minor workflow update adding a Biome autofix step and fixing a missing newline at end of file. No code concerns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[AlgorithmName] -->|toOid| B[OID string]
    B -->|fromOid| A
    A -->|toJose| C[JoseIdentifier]
    C -->|fromJose| A
    A -->|toCose| D[CoseIdentifier]
    D -->|fromCose| A
    A -->|toX509AlgorithmIdentifier| E[X509AlgorithmIdentifier]
    E -->|fromX509AlgorithmIdentifier| A

    subgraph lookup.ts
        direction TB
        B
        C
        D
        JOSE_MAP[JOSE_TO_NAME Map]
        COSE_MAP[COSE_TO_NAME Map]
        OID_LIB[pq-oid OID]
        C -->|lookup| JOSE_MAP
        D -->|lookup| COSE_MAP
        B -->|validate| isCanonical[isCanonicalOid]
        isCanonical -->|OID.toName| OID_LIB
    end

    subgraph x509.ts
        direction TB
        E
        NP[normalizeParameters]
        VP[validateParametersForAlgorithm]
        E --> NP
        NP --> VP
        VP -->|policy check| REG[registry x509 policy]
    end

    subgraph registry.ts
        direction TB
        REG
        RECORDS[IdentifierRecord array]
        RECORDS --> JOSE_MAP
        RECORDS --> COSE_MAP
        RECORDS --> REG
    end
Loading

Last reviewed commit: 3f96040

Copy link
Copy Markdown
Member Author

eacet commented Mar 12, 2026

Merge activity

  • Mar 12, 12:35 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 12, 12:36 PM UTC: Graphite rebased this pull request as part of a merge.
  • Mar 12, 12:37 PM UTC: @eacet merged this pull request with Graphite.

@eacet eacet changed the base branch from feature/eng-1914 to graphite-base/27 March 12, 2026 12:35
@eacet eacet changed the base branch from graphite-base/27 to main March 12, 2026 12:35
@eacet eacet force-pushed the feature/eng-1915 branch from 0cd7def to 6a1beb9 Compare March 12, 2026 12:36
@eacet eacet merged commit 0c8a94a into main Mar 12, 2026
6 checks passed
@eacet eacet deleted the feature/eng-1915 branch March 12, 2026 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant