Skip to content

fix(demo): resolve type error in skyfire-kya token creation#66

Open
ak68a wants to merge 1 commit intoagentcommercekit:mainfrom
ak68a:fix/skyfire-kya-type-error
Open

fix(demo): resolve type error in skyfire-kya token creation#66
ak68a wants to merge 1 commit intoagentcommercekit:mainfrom
ak68a:fix/skyfire-kya-type-error

Conversation

@ak68a
Copy link

@ak68a ak68a commented Mar 25, 2026

Summary

  • Replace @ts-expect-error suppression with an explicit type assertion for the non-standard "kya+JWT" typ header

The Skyfire KYA spec uses a custom media type (kya+JWT) per RFC 7515 structured syntax suffixes. The assertion documents this intent rather than silencing the type error with a TODO comment.

Test plan

  • Demo compiles without type errors
  • pnpm demo:skyfire-kya runs successfully

Summary by CodeRabbit

  • Refactor
    • Improved TypeScript type handling by replacing error suppression with an explicit type assertion in token creation logic. No functional changes.

Replace @ts-expect-error suppression with an explicit type assertion
for the non-standard "kya+JWT" typ header. KYA tokens intentionally
use a custom media type per the Skyfire spec, so the assertion
documents the intent rather than silencing the error.
@coderabbitai
Copy link

coderabbitai bot commented Mar 25, 2026

Walkthrough

Updated JWT header construction in createMockSkyfireKyaToken by replacing a TypeScript error suppression comment with a type assertion. The typ field value "kya+JWT" is now asserted as "JWT" type instead of suppressing type errors. No logic or control flow changes.

Changes

Cohort / File(s) Summary
JWT Header Type Assertion
demos/skyfire-kya/src/kya-token.ts
Replaced // @ts-expect-error`` workaround with type assertion for JWT header typ field, converting `"kya+JWT"` to asserted `"JWT"` type.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing a TypeScript type error in the Skyfire KYA token creation by replacing a @ts-expect-error suppression with an explicit type assertion.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ak68a
Copy link
Author

ak68a commented Mar 25, 2026

Worth noting.. the underlying issue is that JwtHeader.typ in the jwt package is typed as the literal "JWT", so there's no way to pass a custom media type like "kya+JWT" without an assertion.

If custom typ values are expected to be a supported use case (Skyfire KYA uses it per RFC 7515), it might be worth widening the type to something like "JWT" | (string & {}) in the jwt package itself. Happy to open a separate PR for that if it makes sense.

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