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].tsand[id]/revoke.tsto AstroAPIRoutepattern with proper{ request, locals }context andauthenticateRequestauth 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β- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381240000
- Priority: must-fix
- Decision: accept
- Rationale: The original
hashTokenusedBuffer.from(token).toString('base64')which is trivially reversible and provides no security. Replaced withcrypto.subtle.digest('SHA-256', ...)to produce a non-reversible hash.
2026-05-30 12:30 β Replace cookie-only session checks with proper Clerk auth
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')?.valuetrusts the client without validating the session token. Replaced withauthenticateRequest(request)from@repo/authwhich performs local JWT verification and falls back to Clerk remote validation. Also addedgetRolesFromPublicMetadata+hasPlatformSysAdminRolechecks 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:
issuedBywas set toorg.idandrevokedByto the literal string'system', both of which break FK constraints and produce incorrect audit trails. Changed schemaissuedBy/revokedByfromuuid(FK toorganizations.id) totext(Clerk user ID), matchingsysAdminAuditLog.adminUserId. Updated API handlers to passauth.userIdinstead. Generated migration0067_bent_moondragon.sqlto 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
insertMcpTokenSchemaused.transform()on a string input which silently swallowed parse errors. The schema column istext(for storage compatibility), but the API passesJSON.stringify(validatedData.scopes)at the boundary. The insert schema is kept asz.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β- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381260000
- Priority: must-fix
- Decision: accept
- Rationale: Static analysis flags unused imports. Removed
booleanfrom thedrizzle-orm/pg-coreimport list.
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β- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381265000
- Priority: must-fix
- Decision: accept
- Rationale:
McpTokensPanel.tsxduplicated scope definitions locally. Replaced withMCP_SCOPESimported from@repo/db/schema, mapped into the{ value, label }shape expected by the component UI.
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β- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381270000
- Priority: must-fix
- Decision: accept
- Rationale: Direct
format(new Date(token.expiresAt), ...)throws on invalid or null timestamps. Added aformatDate(dateString)helper that returns'Never'for null,'Invalid date'for unparseable values, and the formatted string otherwise.
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
showTokenstate to a plain string, and added auseEffecttimer 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 viapnpm --filter @repo/db db:generateas 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β- Comment: https://github.com/ERP-Unlocked/ordermatic/pull/1333#discussion_r1381290000 (duplicate of session + route findings above)
- Priority: must-fix
- Decision: accept
- Rationale: Same NextRequest β APIRoute conversion and Clerk auth pattern applied to
[id]/revoke.tsas for[...path].ts. Also fixedrevokedBytoauth.userIdinstead of'system'.
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_fkandmcp_tokens_revoked_by_organizations_id_fkwere dropped and the columns converted totextin migration0067_bent_moondragon.sql. We do not recreate FKs to auserstable because Clerk user IDs are external to Postgres and we do not have a synceduserstable to reference.