chore(deps): update dependencies, expand catalogs, add knip + remove dead code#114
Merged
Conversation
Bump non-breaking dependency versions across the catalog and workspace: - catalog: hono 4.12.2->4.12.26, jose 6.1.3->6.2.3, valibot 1.2.0->1.4.1, viem 2.46.3->2.52.2, @hono/standard-validator 0.1.5->0.2.2, standard-parse 0.4.0->0.5.0 - root: @changesets/cli 2.29.8->2.31.0, @changesets/changelog-github 0.5.2->0.7.0, tsx 4.21.0->4.22.4, turbo 2.8.10->2.9.18, vitest 4.0.18->4.1.9 - cli-tools: figlet 1.10.0->1.11.0, strip-ansi 7.1.2->7.2.0 Annotate demos/payments publicClient with an explicit PublicClient type to avoid TS7056 (inferred type too large to serialize) introduced by the viem bump. oxlint/oxlint-tsgolint intentionally held back: the latest versions enable new default rules (vitest plugin + type-aware) that surface 75 errors and 121 warnings against the repo's existing intentional patterns (branded-type assertions, established test style). Adopting them belongs in a dedicated PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bump @agentcommercekit/keys cryptographic dependencies:
- @noble/curves 1.9.1 -> 2.2.0
- @solana/codecs-strings 2.1.1 -> 6.10.0
- multiformats 13.4.2 -> 14.0.0
- uint8arrays 5.1.0 -> 6.1.1
Migrate the curve modules to the @noble/curves v2 API:
- subpath imports now require the .js extension
(@noble/curves/ed25519 -> @noble/curves/ed25519.js)
- the P-256 curve moved to the nist entry point
(@noble/curves/p256 -> import { p256 } from "@noble/curves/nist.js")
- utils.randomPrivateKey() -> utils.randomSecretKey()
- ExtendedPoint/ProjectivePoint -> Point, and .fromHex(bytes) ->
.fromBytes(bytes) for public-key validation
All 111 keys tests pass; full monorepo check is green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- catalog: uuid 11.1.0 -> 14.0.0 (v4 API unchanged) - catalog: @hono/node-server 1.19.9 -> 2.0.5 (serve API compatible) Full check is green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- examples/issuer: drizzle-orm 0.43.1->0.45.2, drizzle-kit 0.31.9->0.31.10, @libsql/client 0.15.15->0.17.4, vite-tsconfig-paths 5.1.4->6.1.1 - demos/identity-a2a: express 4.21.2->5.2.1 (aligns with already-v5 @types/express) - tools/cli-tools: @inquirer/prompts 7.5.1->8.5.2, wrap-ansi 9.0.0->10.0.0 Full monorepo check (build + types + lint + test) is green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add knip (with a monorepo-aware knip.json) and a `pnpm knip` script, then act on its findings: Unused dependencies removed: - demos/identity-a2a: jose, safe-stable-stringify (code uses native JSON.stringify) - demos/payments: @solana-program/system (only @solana-program/token is used) - tools/cli-tools: @types/figlet (figlet 1.11 ships its own types) Dead code removed: - demos/identity-a2a: unused isRpcSuccessResponse helper - demos/skyfire-kya: unused SkyFireKYAPayload interface (+ now-unused JwtPayload import) Unnecessary exports demoted to local (used only within their own file): - BankClientAgent, issuerDidDocument (identity-a2a), skyfireKyaJwtPayloadSchema (skyfire-kya) Vestigial broken package.json "main" fields removed (pointed at nonexistent files; packages are consumed via subpath exports / tsconfig extends): - tools/api-utils (main -> ./src/index.ts), tools/typescript-config (main -> ./base.js) The two remaining knip "duplicate exports" are intentional @deprecated public aliases in @agentcommercekit/keys (bytesToJwk, jwkToBytes); the duplicates rule is set to "warn" so knip still exits 0. Full check + manypkg are green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- demos/identity: pin zod to the catalog (3.25.4). The viem 2.46->2.52 bump
introduced an optional viem<->zod peer that pnpm resolved to zod 4.4.3, which
the AI SDK packages (zod ^3 peers) were then grouped onto — a peer violation.
Pinning zod 3 in the demo makes @ai-sdk/* resolve their zod peer correctly.
(flagged by codex [MEDIUM] + claude [LOW])
- root: raise engines.node ">=22" -> ">=22.13.0" to reflect the real floor
introduced by @inquirer/prompts 8 (^22.13.0) and knip (>=22.12.0).
(flagged by codex [MEDIUM])
- keys: fix copy-paste JSDoc on secp256r1 isValidPublicKey ("secp256k1" ->
"secp256r1"). (flagged by opencode [LOW])
Foregone: opencode's "knip only covers examples/*" — a misread; knip scans all
workspaces by default, the config only customizes entry points for examples
(that is how it found the unused demo exports).
knip exits 0; full check green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Panel review round 2 flagged that bumping the identity-a2a demo to express 5 (done in 14f4195) is unsafe: @a2a-js/sdk@0.2.2 declares `express ^4.21.2` and its `A2AExpressApp.setupRoutes` registers `app.post(baseUrl, ...)` — with the demo's empty baseUrl that is `app.post("", ...)`, an empty-path route that express 5's path-to-regexp v8 handles differently and can throw on at startup, leaving `pnpm demo:identity-a2a` dead on arrival. The express 5 bump adds no value to a demo running against an express-4-pinned SDK, so revert to 4.21.2 (restoring the known-green baseline). express 5 for this demo should wait until @a2a-js/sdk supports it. (flagged by claude [MEDIUM]; opencode NO_FINDINGS; codex timed out mid-review) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Security hardening (MEDIUM): the signed-payload validator's dev escape hatch — accept an unsigned payload with an attacker-supplied `X-Payload-Issuer` header, skipping signature verification — was gated on `NODE_ENV === "development"`. `examples/issuer/.env.example` ships `NODE_ENV="development"`, so the reference issuer ran with this authentication bypass enabled by default, and any deploy that sets/defaults `NODE_ENV` to "development" would be fully bypassable. Decouple the bypass from `NODE_ENV` and require an explicit, default-off `ALLOW_UNSIGNED_PAYLOADS="true"` flag, and log a loud SECURITY warning whenever the bypass path is taken. Nothing in the repo sends `X-Payload-Issuer`, so no automated flow or test depends on it. Documented the flag (off) in the issuer .env.example. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR migrates Changes
ALLOW_UNSIGNED_PAYLOADS bypass flag
Knip tooling, export cleanup, and dependency updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/keys/package.json`:
- Around line 59-62: The dependencies "`@noble/curves`", "`@solana/codecs-strings`",
"multiformats", and "uint8arrays" in packages/keys/package.json are using pinned
version numbers instead of the workspace catalog specifier. Replace each raw
version string (e.g., "2.2.0", "6.10.0", "14.0.0", "6.1.1") with the "catalog:"
prefix to ensure version consistency across the monorepo and maintain the
workspace dependency management flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6c00c4b-6afe-4421-a9e0-d84b52375f75
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
.changeset/keys-noble-curves-v2.mddemos/identity-a2a/package.jsondemos/identity-a2a/src/bank-client-agent.tsdemos/identity-a2a/src/issuer.tsdemos/identity-a2a/src/utils/response-parsers.tsdemos/identity/package.jsondemos/payments/package.jsondemos/payments/src/constants.tsdemos/skyfire-kya/src/kya-token.tsdemos/skyfire-kya/src/skyfire-kya-ack-id.tsexamples/issuer/.env.exampleexamples/issuer/README.mdexamples/issuer/package.jsonknip.jsonpackage.jsonpackages/keys/package.jsonpackages/keys/src/curves/ed25519.tspackages/keys/src/curves/secp256k1.tspackages/keys/src/curves/secp256r1.tspnpm-workspace.yamltools/api-utils/package.jsontools/api-utils/src/middleware/signed-payload-validator.tstools/cli-tools/package.jsontools/typescript-config/package.json
💤 Files with no reviewable changes (5)
- demos/identity-a2a/src/utils/response-parsers.ts
- demos/payments/package.json
- demos/identity-a2a/package.json
- tools/api-utils/package.json
- demos/skyfire-kya/src/skyfire-kya-ack-id.ts
Swapping the bundled crypto implementation across four dependency majors warrants a minor bump even though the public API is unchanged, so downstream consumers notice. (panel review #114) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dependency/maintenance pass across the monorepo. (The CRITICAL credential-forgery security fix that came out of this work is in a separate PR (#113) for focused review.)
Dependencies
@agentcommercekit/keyscrypto majors (the one published-package change):@noble/curves1→2 (real API migration —.jssubpaths,p256fromnist.js,randomSecretKey,Point.fromBytes),@solana/codecs-strings2→6,multiformats13→14,uint8arrays5→6. Public API unchanged; 111 keys tests pass.@hono/node-server1→2, drizzle 0.45,@libsql/client0.17, vite-tsconfig-paths 6,@inquirer/prompts8, wrap-ansi 10, plus all safe minor/patch bumps (hono, jose, valibot, viem, turbo, vitest, changesets, tsx…). Bumped the rootengines.nodeto>=22.13.0to match the new floor.^4),@a2a-js/sdk0.3 + express 5 (the SDK pins express 4 — express 5's path-to-regexp v8 breaks the demo's empty-path route), AI SDK v6 / Solana kit v6 (major rewrites), TS 6 / oxlint 1.70 / oxfmt /@types/node26 (tooling churn / lint-rule churn / lockfile instability). Each can be its own follow-up.Catalogs
Already comprehensive — every cross-package dependency is cataloged. Refreshed all catalog versions.
knip
Added
knip+knip.json+ apnpm knipscript, then removed what it found: unused deps (jose, safe-stable-stringify,@solana-program/system,@types/figlet), dead code, redundant exports, and two vestigial brokenmainfields. Exits clean.fix(api-utils): gate unsigned-payload bypass behind explicit opt-in flag— the signed-payload validator's unsigned-payload dev bypass was gated onNODE_ENV=development(which the issuer example ships by default), so the reference issuer ran with an auth bypass enabled by default. It now requires an explicit, default-offALLOW_UNSIGNED_PAYLOADSflag and logs a loud warning when used. Nothing in the repo sendsX-Payload-Issuer, so no flow/test depends on it.Verification
Full
pnpm run check(build + types + lint + test, 47 tasks) is green;knipandmanypkg checkare clean.🤖 Generated with Claude Code
Summary by CodeRabbit
ALLOW_UNSIGNED_PAYLOADS="true"to allow unsigned request payloads for local testing, usingX-Payload-Issuer; signature verification is only bypassed when explicitly enabled..env.exampleto reflect the new flag-based unsigned-payload flow.