Skip to content

ADR β€” PR #1333

2026-05-30 12:20 β€” Fix API route pattern: convert NextRequest/NextResponse imports to Astro APIRoute

Section titled β€œ2026-05-30 12:20 β€” Fix API route pattern: convert NextRequest/NextResponse imports to Astro APIRoute”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381234567
  • Priority: must-fix
  • Decision: accept
  • Rationale: The NextRequest/NextResponse imports from β€˜next’ do not exist in this Astro app, causing runtime failures. Converted both [...path].ts and [id]/revoke.ts to Astro APIRoute pattern with proper { request, locals } context and authenticateRequest auth checks.

2026-05-30 12:25 β€” Replace weak base64 hashToken with SHA-256

Section titled β€œ2026-05-30 12:25 β€” Replace weak base64 hashToken with SHA-256”
Section titled β€œ2026-05-30 12:30 β€” Replace cookie-only session checks with proper Clerk auth”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381245000
  • Priority: must-fix
  • Decision: accept
  • Rationale: Checking only request.cookies.get('session')?.value trusts the client without validating the session token. Replaced with authenticateRequest(request) from @repo/auth which performs local JWT verification and falls back to Clerk remote validation. Also added getRolesFromPublicMetadata + hasPlatformSysAdminRole checks for 403 enforcement.

2026-05-30 12:35 β€” Fix issuedBy/revokedBy fields to store Clerk user IDs, not organization IDs

Section titled β€œ2026-05-30 12:35 β€” Fix issuedBy/revokedBy fields to store Clerk user IDs, not organization IDs”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381250000
  • Priority: must-fix
  • Decision: accept
  • Rationale: issuedBy was set to org.id and revokedBy to the literal string 'system', both of which break FK constraints and produce incorrect audit trails. Changed schema issuedBy/revokedBy from uuid (FK to organizations.id) to text (Clerk user ID), matching sysAdminAuditLog.adminUserId. Updated API handlers to pass auth.userId instead. Generated migration 0067_bent_moondragon.sql to drop the old FK constraints and alter column types.

2026-05-30 12:40 β€” Remove scopes JSON.stringify and fix insertMcpTokenSchema

Section titled β€œ2026-05-30 12:40 β€” Remove scopes JSON.stringify and fix insertMcpTokenSchema”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381255000
  • Priority: must-fix
  • Decision: accept
  • Rationale: The insertMcpTokenSchema used .transform() on a string input which silently swallowed parse errors. The schema column is text (for storage compatibility), but the API passes JSON.stringify(validatedData.scopes) at the boundary. The insert schema is kept as z.array(z.string()) to validate shape correctly.

2026-05-30 12:45 β€” Remove unused boolean import from mcp-tokens schema

Section titled β€œ2026-05-30 12:45 β€” Remove unused boolean import from mcp-tokens schema”

2026-05-30 12:50 β€” Use shared MCP_SCOPES constant instead of local SCOPE_OPTIONS

Section titled β€œ2026-05-30 12:50 β€” Use shared MCP_SCOPES constant instead of local SCOPE_OPTIONS”

2026-05-30 12:55 β€” Add safe date formatting helper to prevent runtime crashes

Section titled β€œ2026-05-30 12:55 β€” Add safe date formatting helper to prevent runtime crashes”

2026-05-30 13:00 β€” Remove token visibility toggle to enforce β€œshow once” security model

Section titled β€œ2026-05-30 13:00 β€” Remove token visibility toggle to enforce β€œshow once” security model”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381275000
  • Priority: must-fix
  • Decision: accept
  • Rationale: The UI allowed toggling token visibility, contradicting the β€œshow once” warning. Removed the toggle button, changed showToken state to a plain string, and added a useEffect timer to auto-clear the raw token from the UI after 60 seconds.

2026-05-30 13:05 β€” Skip request for CSRF token addition

Section titled β€œ2026-05-30 13:05 β€” Skip request for CSRF token addition”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381280000
  • Priority: skip
  • Decision: reject
  • Rationale: The API routes now use authenticateRequest(request) from @repo/auth, which validates the Clerk JWT/session token on every request. This provides stateless, token-based authentication that is not vulnerable to CSRF attacks via simple cross-origin POSTs because the session token is SameSite and validated server-side. Adding an additional CSRF token would deviate from the existing sys-admin API pattern used across all other endpoints in the app.

2026-05-30 13:10 β€” Skip request for splitting combined migration

Section titled β€œ2026-05-30 13:10 β€” Skip request for splitting combined migration”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381285000
  • Priority: skip
  • Decision: reject
  • Rationale: The combined migration (0067_bent_moondragon.sql) has already been generated, committed, and potentially applied in some environments. Splitting it retroactively would create a new divergence in migration history without improving runtime behavior. Future migrations should be generated incrementally via pnpm --filter @repo/db db:generate as the existing convention requires.

2026-05-30 13:15 β€” Fix revoke.ts to use Astro APIRoute and proper Clerk auth

Section titled β€œ2026-05-30 13:15 β€” Fix revoke.ts to use Astro APIRoute and proper Clerk auth”

2026-05-30 13:20 β€” Fix migration FK constraints referencing organizations.id for audit fields

Section titled β€œ2026-05-30 13:20 β€” Fix migration FK constraints referencing organizations.id for audit fields”
  • Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381295000 (duplicate of issuedBy/revokedBy finding)
  • Priority: must-fix
  • Decision: accept
  • Rationale: The FK constraints mcp_tokens_issued_by_organizations_id_fk and mcp_tokens_revoked_by_organizations_id_fk were dropped and the columns converted to text in migration 0067_bent_moondragon.sql. We do not recreate FKs to a users table because Clerk user IDs are external to Postgres and we do not have a synced users table to reference.