Skip to content

feat(pq-jws/ts): phase 1 - define public contracts (ENG-1640)#18

Open
eacet wants to merge 1 commit intomainfrom
feature/eng-1640
Open

feat(pq-jws/ts): phase 1 - define public contracts (ENG-1640)#18
eacet wants to merge 1 commit intomainfrom
feature/eng-1640

Conversation

@eacet
Copy link
Member

@eacet eacet commented Feb 26, 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

@eacet
Copy link
Member Author

eacet commented Feb 26, 2026

@codex review

@greptile-apps
Copy link

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR establishes the TypeScript type contracts and function signatures for the pq-jws (post-quantum JSON Web Signature) package as part of phase 1 implementation.

Key changes:

  • Defines public API contracts including JwsProtectedHeader, ParsedCompactJws, JwsSigner, and JwsVerifier types
  • Creates stub implementations for signJwsCompact, verifyJwsCompact, parseJwsCompact, and base64url encoding functions (all throw "not implemented" as expected)
  • Establishes custom error hierarchy with JwsError, JwsFormatError, and JwsValidationError
  • Adds pq-oid dependency and TypeScript project reference for future use
  • Removes several Claude skill configuration files

Issues found:

  • All type definitions use interface instead of type, violating project conventions
  • TypeScript configuration missing composite: true flag needed for proper project references
  • Unused pq-oid dependency (likely intentional for phase 2)
  • PR description template not filled out

Confidence Score: 3/5

  • Safe to merge after addressing TypeScript configuration and style issues
  • The implementation correctly defines phase 1 contracts with stub functions. However, there are style convention violations (interface vs type) throughout and a missing TypeScript configuration flag that could cause build issues when other packages reference this one. These are non-critical but should be fixed for consistency.
  • Focus on packages/pq-jws/ts/src/types.ts for style fixes and packages/pq-jws/ts/tsconfig.json for the missing composite flag

Important Files Changed

Filename Overview
packages/pq-jws/ts/src/types.ts Defines JWS type contracts, but uses interface instead of preferred type keyword per project conventions
packages/pq-jws/ts/package.json Adds pq-oid dependency (not yet used) and changes build script to tsc -b
packages/pq-jws/ts/tsconfig.json Adds project reference to pq-oid, but missing composite: true flag for proper TypeScript project references

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class JwsProtectedHeader {
        +string alg
        +string? kid
        +string? typ
        +string? cty
        +string[]? crit
        +boolean? b64
    }
    
    class CompactJwsSegments {
        +string protectedHeader
        +string payload
        +string signature
    }
    
    class ParsedCompactJws {
        +string compact
        +CompactJwsSegments segments
        +JwsProtectedHeader protectedHeader
        +string encodedProtectedHeader
        +string encodedPayload
        +Uint8Array payload
        +Uint8Array signature
        +Uint8Array signingInput
    }
    
    class JwsSignerContext {
        +JwsProtectedHeader protectedHeader
        +Uint8Array payload
        +string encodedProtectedHeader
        +string encodedPayload
    }
    
    class JwsVerifierContext {
        Extends JwsSignerContext
    }
    
    class SignJwsCompactInput {
        +JwsProtectedHeader protectedHeader
        +Uint8Array|string payload
        +JwsSigner signer
    }
    
    class JwsError {
        +string name
        +string message
    }
    
    class JwsFormatError {
        Extends JwsError
    }
    
    class JwsValidationError {
        Extends JwsError
    }
    
    ParsedCompactJws --> CompactJwsSegments
    ParsedCompactJws --> JwsProtectedHeader
    SignJwsCompactInput --> JwsProtectedHeader
    JwsSignerContext --> JwsProtectedHeader
    JwsVerifierContext --|> JwsSignerContext
    JwsFormatError --|> JwsError
    JwsValidationError --|> JwsError
Loading

Last reviewed commit: 04a7709

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

15 files reviewed, 8 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +1 to +9
export interface JwsProtectedHeader {
alg: string;
kid?: string;
typ?: string;
cty?: string;
crit?: string[];
b64?: boolean;
[name: string]: unknown;
}
Copy link

Choose a reason for hiding this comment

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

Use type instead of interface per project convention. These definitions don't use interface-specific features like extension or implementation.

Suggested change
export interface JwsProtectedHeader {
alg: string;
kid?: string;
typ?: string;
cty?: string;
crit?: string[];
b64?: boolean;
[name: string]: unknown;
}
export type JwsProtectedHeader = {
alg: string;
kid?: string;
typ?: string;
cty?: string;
crit?: string[];
b64?: boolean;
[name: string]: unknown;
};

Context Used: Rule from dashboard - Use type by default in TypeScript unless you specifically need interface features like extension... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/src/types.ts
Line: 1-9

Comment:
Use `type` instead of `interface` per project convention. These definitions don't use interface-specific features like extension or implementation.

```suggestion
export type JwsProtectedHeader = {
  alg: string;
  kid?: string;
  typ?: string;
  cty?: string;
  crit?: string[];
  b64?: boolean;
  [name: string]: unknown;
};
```

**Context Used:** Rule from `dashboard` - Use `type` by default in TypeScript unless you specifically need `interface` features like extension... ([source](https://app.greptile.com/review/custom-context?memory=c862f053-5655-4b41-be69-c840e3c9f280))

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +11 to +15
export interface CompactJwsSegments {
protectedHeader: string;
payload: string;
signature: string;
}
Copy link

Choose a reason for hiding this comment

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

Use type instead of interface per project convention.

Suggested change
export interface CompactJwsSegments {
protectedHeader: string;
payload: string;
signature: string;
}
export type CompactJwsSegments = {
protectedHeader: string;
payload: string;
signature: string;
};

Context Used: Rule from dashboard - Use type by default in TypeScript unless you specifically need interface features like extension... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/src/types.ts
Line: 11-15

Comment:
Use `type` instead of `interface` per project convention.

```suggestion
export type CompactJwsSegments = {
  protectedHeader: string;
  payload: string;
  signature: string;
};
```

**Context Used:** Rule from `dashboard` - Use `type` by default in TypeScript unless you specifically need `interface` features like extension... ([source](https://app.greptile.com/review/custom-context?memory=c862f053-5655-4b41-be69-c840e3c9f280))

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +17 to +26
export interface ParsedCompactJws {
compact: string;
segments: CompactJwsSegments;
protectedHeader: JwsProtectedHeader;
encodedProtectedHeader: string;
encodedPayload: string;
payload: Uint8Array;
signature: Uint8Array;
signingInput: Uint8Array;
}
Copy link

Choose a reason for hiding this comment

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

Use type instead of interface per project convention.

Suggested change
export interface ParsedCompactJws {
compact: string;
segments: CompactJwsSegments;
protectedHeader: JwsProtectedHeader;
encodedProtectedHeader: string;
encodedPayload: string;
payload: Uint8Array;
signature: Uint8Array;
signingInput: Uint8Array;
}
export type ParsedCompactJws = {
compact: string;
segments: CompactJwsSegments;
protectedHeader: JwsProtectedHeader;
encodedProtectedHeader: string;
encodedPayload: string;
payload: Uint8Array;
signature: Uint8Array;
signingInput: Uint8Array;
};

Context Used: Rule from dashboard - Use type by default in TypeScript unless you specifically need interface features like extension... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/src/types.ts
Line: 17-26

Comment:
Use `type` instead of `interface` per project convention.

```suggestion
export type ParsedCompactJws = {
  compact: string;
  segments: CompactJwsSegments;
  protectedHeader: JwsProtectedHeader;
  encodedProtectedHeader: string;
  encodedPayload: string;
  payload: Uint8Array;
  signature: Uint8Array;
  signingInput: Uint8Array;
};
```

**Context Used:** Rule from `dashboard` - Use `type` by default in TypeScript unless you specifically need `interface` features like extension... ([source](https://app.greptile.com/review/custom-context?memory=c862f053-5655-4b41-be69-c840e3c9f280))

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +28 to +33
export interface JwsSignerContext {
protectedHeader: JwsProtectedHeader;
payload: Uint8Array;
encodedProtectedHeader: string;
encodedPayload: string;
}
Copy link

Choose a reason for hiding this comment

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

Use type instead of interface per project convention.

Suggested change
export interface JwsSignerContext {
protectedHeader: JwsProtectedHeader;
payload: Uint8Array;
encodedProtectedHeader: string;
encodedPayload: string;
}
export type JwsSignerContext = {
protectedHeader: JwsProtectedHeader;
payload: Uint8Array;
encodedProtectedHeader: string;
encodedPayload: string;
};

Context Used: Rule from dashboard - Use type by default in TypeScript unless you specifically need interface features like extension... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/src/types.ts
Line: 28-33

Comment:
Use `type` instead of `interface` per project convention.

```suggestion
export type JwsSignerContext = {
  protectedHeader: JwsProtectedHeader;
  payload: Uint8Array;
  encodedProtectedHeader: string;
  encodedPayload: string;
};
```

**Context Used:** Rule from `dashboard` - Use `type` by default in TypeScript unless you specifically need `interface` features like extension... ([source](https://app.greptile.com/review/custom-context?memory=c862f053-5655-4b41-be69-c840e3c9f280))

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

encodedPayload: string;
}

export interface JwsVerifierContext extends JwsSignerContext {}
Copy link

Choose a reason for hiding this comment

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

Since JwsVerifierContext is identical to JwsSignerContext, just use a type alias instead of interface extension.

Suggested change
export interface JwsVerifierContext extends JwsSignerContext {}
export type JwsVerifierContext = JwsSignerContext;

Context Used: Rule from dashboard - Use type by default in TypeScript unless you specifically need interface features like extension... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/src/types.ts
Line: 35

Comment:
Since `JwsVerifierContext` is identical to `JwsSignerContext`, just use a type alias instead of interface extension.

```suggestion
export type JwsVerifierContext = JwsSignerContext;
```

**Context Used:** Rule from `dashboard` - Use `type` by default in TypeScript unless you specifically need `interface` features like extension... ([source](https://app.greptile.com/review/custom-context?memory=c862f053-5655-4b41-be69-c840e3c9f280))

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +48 to +52
export interface SignJwsCompactInput {
protectedHeader: JwsProtectedHeader;
payload: Uint8Array | string;
signer: JwsSigner;
}
Copy link

Choose a reason for hiding this comment

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

Use type instead of interface per project convention.

Suggested change
export interface SignJwsCompactInput {
protectedHeader: JwsProtectedHeader;
payload: Uint8Array | string;
signer: JwsSigner;
}
export type SignJwsCompactInput = {
protectedHeader: JwsProtectedHeader;
payload: Uint8Array | string;
signer: JwsSigner;
};

Context Used: Rule from dashboard - Use type by default in TypeScript unless you specifically need interface features like extension... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/src/types.ts
Line: 48-52

Comment:
Use `type` instead of `interface` per project convention.

```suggestion
export type SignJwsCompactInput = {
  protectedHeader: JwsProtectedHeader;
  payload: Uint8Array | string;
  signer: JwsSigner;
};
```

**Context Used:** Rule from `dashboard` - Use `type` by default in TypeScript unless you specifically need `interface` features like extension... ([source](https://app.greptile.com/review/custom-context?memory=c862f053-5655-4b41-be69-c840e3c9f280))

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +24 to +26
"dependencies": {
"pq-oid": "^1.0.1"
},
Copy link

Choose a reason for hiding this comment

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

The pq-oid dependency is added but not yet imported or used in any source files. Consider adding this dependency when it's actually needed in phase 2, or document why it's added now.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/package.json
Line: 24-26

Comment:
The `pq-oid` dependency is added but not yet imported or used in any source files. Consider adding this dependency when it's actually needed in phase 2, or document why it's added now.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

@greptile-apps
Copy link

greptile-apps bot commented Feb 26, 2026

Additional Comments (1)

packages/pq-jws/ts/tsconfig.json
Add "composite": true to enable proper TypeScript project references. This is required when using project references and allows other packages to reference this one.

  "compilerOptions": {
    "target": "ES2022",
    "module": "ESNext",
    "moduleResolution": "bundler",
    "declaration": true,
    "composite": true,
    "outDir": "./dist",
    "rootDir": "./src",
    "strict": true,
    "esModuleInterop": true,
    "skipLibCheck": true
  },
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-jws/ts/tsconfig.json
Line: 2-12

Comment:
Add `"composite": true` to enable proper TypeScript project references. This is required when using project references and allows other packages to reference this one.

```suggestion
  "compilerOptions": {
    "target": "ES2022",
    "module": "ESNext",
    "moduleResolution": "bundler",
    "declaration": true,
    "composite": true,
    "outDir": "./dist",
    "rootDir": "./src",
    "strict": true,
    "esModuleInterop": true,
    "skipLibCheck": true
  },
```

How can I resolve this? If you propose a fix, please make it concise.

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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