Skip to content

Add MCP auth helper tests and remove redundant request proof initiali…#354

Merged
calvadev merged 3 commits intoshopstr-eng:mainfrom
YASH-ai-bit:fix/mcp-auth
Apr 7, 2026
Merged

Add MCP auth helper tests and remove redundant request proof initiali…#354
calvadev merged 3 commits intoshopstr-eng:mainfrom
YASH-ai-bit:fix/mcp-auth

Conversation

@YASH-ai-bit
Copy link
Copy Markdown
Contributor

Summary

This PR adds direct test coverage for the MCP auth helpers in utils/mcp/auth.ts and removes a redundant mcp_request_proofs initialization block from the same file.

What changed

  • added utils/mcp/__tests__/auth.test.ts
  • added coverage for:
    • generateApiKey()
    • hashApiKey()
    • verifyApiKey()
    • extractBearerToken()
    • verifyNostrAuth()
  • removed the second CREATE TABLE IF NOT EXISTS mcp_request_proofs ... block from initializeApiKeysTable() in utils/mcp/auth.ts

Why

utils/mcp/__tests__/request-proof-server.test.ts already covers the request-proof flow, but the helper logic in utils/mcp/auth.ts did not have direct regression coverage yet.

This PR fills that gap and also cleans up the redundant mcp_request_proofs table/index initialization in utils/mcp/auth.ts while keeping the intended behavior unchanged.

Testing

Ran:

  • npm test -- utils/mcp/__tests__/auth.test.ts
  • npm test -- utils/mcp/__tests__/request-proof-server.test.ts

Notes

The schema cleanup in utils/mcp/auth.ts is meant to be behavior-preserving. The same mcp_request_proofs table is already initialized earlier in initializeApiKeysTable(), so this just keeps a single initialization block there.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 6, 2026

@YASH-ai-bit is attempting to deploy a commit to the shopstr-eng Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Copy Markdown
Contributor

@arnavkirti arnavkirti left a comment

Choose a reason for hiding this comment

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

  • Use Date.now = jest.fn() or jest.useFakeTimers() to freeze time, so tests on created_at are not flaky over long durations. Right now they rely on Date.now() inline and arithmetic (e.g., - 3600) which is OK but more brittle.
  • Consider grouping Nostr auth constants to improve readability, e.g. reuse a base event object and spread it in each test instead of repeating kind: 27235 and the created_at calculation.
  • Add a test for the case where verifyEvent throws (if that’s a realistic failure mode) to ensure the helper either catches or propagates as intended.

Also, Is there a dedicated migration for mcp_request_proofs now? If not, can you clarify why it’s safe to remove this CREATE TABLE from initializeApiKeysTable?

@YASH-ai-bit
Copy link
Copy Markdown
Contributor Author

YASH-ai-bit commented Apr 6, 2026

Thanks for the tip!
Updated the tests in utils/mcp/__tests__/auth.test.ts to freeze time via Date.now, reuse a shared base auth event, and add an explicit test for the case where verifyEvent throws.

There isn’t a separate migration for mcp_request_proofs right now. The intent of this change is not to remove table creation for request proofs, only to remove the redundant second initialization block from initializeApiKeysTable() in utils/mcp/auth.ts.

There were two mcp_request_proofs initialization blocks in utils/mcp/auth.ts.
I kept the earlier one at utils/mcp/auth.ts:98 and utils/mcp/auth.ts:104, and removed the later duplicate block. So the table creation path is still present.

I might be missing something in SQL part, but by functionality, the two looked duplicates.

@arnavkirti
Copy link
Copy Markdown
Contributor

Thanks for the tip! Updated the tests in utils/mcp/__tests__/auth.test.ts to freeze time via Date.now, reuse a shared base auth event, and add an explicit test for the case where verifyEvent throws.

There isn’t a separate migration for mcp_request_proofs right now. The intent of this change is not to remove table creation for request proofs, only to remove the redundant second initialization block from initializeApiKeysTable() in utils/mcp/auth.ts.

There were two mcp_request_proofs initialization blocks in utils/mcp/auth.ts. I kept the earlier one at utils/mcp/auth.ts:98 and utils/mcp/auth.ts:104, and removed the later duplicate block. So the table creation path is still present.

I might be missing something in SQL part, but by functionality, the two looked duplicates.

Please verify the SQL part once, otherwise Looks good!

@YASH-ai-bit
Copy link
Copy Markdown
Contributor Author

Thanks!
SQL part might be an overdo, but let's wait for confirmation.

@GautamBytes
Copy link
Copy Markdown
Contributor

LGTM!!

@calvadev calvadev merged commit 4101a3c into shopstr-eng:main Apr 7, 2026
1 check failed
@YASH-ai-bit
Copy link
Copy Markdown
Contributor Author

Hey @calvadev, just a quick question about the SQL syntax you fixed. After the fix it has two PRIMARY KEY declarations, one inline and other table-level constraint. It would create error on table creation, please look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants