Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/lib/authkit-application-setup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,14 @@ beforeEach(() => {
}
if (name === 'setRedirectUris') {
const input = options.variables!.input as {
applicationId: string;
environmentId: string;
redirectUris: typeof application.redirectUris;
dryRun: boolean;
};
expect(input.applicationId).toBe('app_1');
// applicationId here resolves an AuthKit IDP application, not the userland
// app returned by defaultAuthkitApplication. Match the live API contract.
if ('applicationId' in input) throw new Error("Application not found: 'app_1'.");
expect(input.environmentId).toBe('env_app');
if (!input.dryRun) application.redirectUris = input.redirectUris;
return { setRedirectUris: { __typename: 'RedirectUrisSet' } };
}
Expand Down Expand Up @@ -814,9 +817,16 @@ describe('native application URL setup', () => {
for (const [, options] of vi.mocked(dashboardGraphqlRequest).mock.calls) {
expect(options.environmentId).toBe('env_app');
}
for (const [, options] of writes()) {
expect(options.variables?.input).toMatchObject({ applicationId: 'app_1' });
for (const [name, options] of writes()) {
expect(options.variables?.input).toMatchObject(
name === 'setRedirectUris' ? { environmentId: 'env_app' } : { applicationId: 'app_1' },
);
}
const redirectCalls = vi.mocked(dashboardGraphqlRequest).mock.calls.filter(([name]) => name === 'setRedirectUris');
expect(redirectCalls.map(([, options]) => options.variables?.input)).toEqual([
{ environmentId: 'env_app', redirectUris: [{ uri: setup.redirectUri, isDefault: true }], dryRun: true },
{ environmentId: 'env_app', redirectUris: [{ uri: setup.redirectUri, isDefault: true }], dryRun: false },
]);
});

it('fails if the callback write reports success but read-back is missing it', async () => {
Expand Down
4 changes: 3 additions & 1 deletion src/lib/authkit-application-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ export async function configureAuthkitApplication(
// other apps may block their own updates, but must not block basic sign-in.
if (!original.redirectUris.some((uri) => uri.uri === setup.redirectUri)) {
const input = {
applicationId: original.id,
// This mutation's applicationId targets an IDP application. The matched
// environment targets the default userland app read and verified above.
environmentId,
redirectUris: [
...original.redirectUris,
{ uri: setup.redirectUri, isDefault: original.redirectUris.length === 0 },
Expand Down
4 changes: 2 additions & 2 deletions src/lib/validation/rules/tanstack-start.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
],
"files": [
{
"path": "app/routes/**/*callback*.{ts,tsx}",
"path": "{app,src}/routes/**/{*callback*,*callback*/index,*callback*/route}.{ts,tsx,js,jsx}",
"mustContainAny": ["handleAuth", "handleCallback", "@workos/authkit", "@workos-inc/authkit"]
},
{
"path": "app/**/*.{ts,tsx}",
"path": "{app,src}/**/*.{ts,tsx,js,jsx}",
Comment thread
greptile-apps[bot] marked this conversation as resolved.
"mustContainAny": ["getAuth", "getUser", "getSignInUrl", "signOut", "authkitMiddleware"]
}
]
Expand Down
1 change: 1 addition & 0 deletions src/lib/validation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface ValidationResult {
// Rule definitions (matches JSON schema)
export interface PackageRule {
name: string;
alternates?: string[]; // accepted package names in the same dependency location
location?: 'dependencies' | 'devDependencies' | 'any'; // default: 'any'
}

Expand Down
146 changes: 145 additions & 1 deletion src/lib/validation/validator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { validateInstallation } from './validator.js';
import { validateFiles, validateInstallation, validatePackages } from './validator.js';
import type { FileRule, PackageRule } from './types.js';
import tanstackRules from './rules/tanstack-start.json' with { type: 'json' };

describe('validateInstallation', () => {
let testDir: string;
Expand Down Expand Up @@ -273,6 +275,59 @@ describe('validateInstallation', () => {
});

describe('pattern validation', () => {
it.each(['a', 'z'])('checks all TanStack source and callback matches (valid file: %s)', async (valid) => {
mkdirSync(join(testDir, 'src/routes'), { recursive: true });
for (const name of ['a', 'z']) {
writeFileSync(join(testDir, `src/${name}.ts`), name === valid ? 'authkitMiddleware()' : 'export {};');
writeFileSync(
join(testDir, `src/routes/${name}.callback.tsx`),
name === valid ? 'handleCallbackRoute()' : 'export {};',
);
}
expect(await validateFiles(tanstackRules, testDir)).toEqual([]);
});

it.each(['a', 'z'])('accepts a later file satisfying both pattern constraints (valid file: %s)', async (valid) => {
mkdirSync(join(testDir, 'src'), { recursive: true });
for (const name of ['a', 'z'])
writeFileSync(join(testDir, `src/${name}.ts`), name === valid ? 'getSignInUrl redirect GET' : 'export {};');
const files: FileRule[] = [
{
path: 'src/*.ts',
mustContain: ['getSignInUrl', 'redirect'],
mustContainAny: ['GET', 'POST'],
},
];
expect(await validateFiles({ framework: 'test', packages: [], envVars: [], files }, testDir)).toEqual([]);
});

it('does not combine incomplete files into one valid handler', async () => {
mkdirSync(join(testDir, 'src'), { recursive: true });
writeFileSync(join(testDir, 'src/a.ts'), 'getSignInUrl redirect');
writeFileSync(join(testDir, 'src/z.ts'), 'GET');
const files: FileRule[] = [
{
path: 'src/*.ts',
mustContain: ['getSignInUrl', 'redirect'],
mustContainAny: ['GET', 'POST'],
severity: 'error',
},
];
const issues = await validateFiles({ framework: 'test', packages: [], envVars: [], files }, testDir);
expect(issues.length).toBeGreaterThan(0);
expect(issues.every((issue) => issue.type === 'pattern' && issue.severity === 'error')).toBe(true);
});

it('still warns when no TanStack match contains the expected calls', async () => {
mkdirSync(join(testDir, 'src/routes'), { recursive: true });
writeFileSync(join(testDir, 'src/a.ts'), 'export {};');
writeFileSync(join(testDir, 'src/routes/a.callback.tsx'), 'export {};');
writeFileSync(join(testDir, 'src/routes/z.callback.tsx'), 'export {};');
const issues = await validateFiles(tanstackRules, testDir);
expect(issues).toHaveLength(2);
expect(issues.every((issue) => issue.type === 'pattern' && issue.severity === 'warning')).toBe(true);
});

it('detects missing pattern in file', async () => {
writeFileSync(
join(testDir, 'package.json'),
Expand Down Expand Up @@ -482,6 +537,95 @@ describe('validateInstallation', () => {
});
});

describe('TanStack validation regressions', () => {
function fixture(root: string, route: string, extension: string) {
writeFileSync(
join(testDir, 'package.json'),
JSON.stringify({
dependencies: {
'@workos/authkit-tanstack-react-start': '^1.0.0',
'@tanstack/react-start': '^1.0.0',
},
}),
);
writeFileSync(
join(testDir, '.env.local'),
`WORKOS_API_KEY=sk_test\nWORKOS_CLIENT_ID=client_test\nWORKOS_REDIRECT_URI=https://example.com/api/auth/callback\nWORKOS_COOKIE_PASSWORD=${'x'.repeat(32)}\n`,
);
mkdirSync(join(testDir, root, 'routes', route, '..'), { recursive: true });
writeFileSync(join(testDir, root, 'routes', `${route}.${extension}`), 'handleCallback(); getAuth();');
writeFileSync(join(testDir, root, `start.${extension}`), 'authkitMiddleware();');
}

const layouts = [
'api/auth/callback',
'api.auth.callback',
'api/auth/callback/index',
'api/auth/callback/route',
'api.auth.callback.index',
];
const cases = ['app', 'src'].flatMap((root) =>
layouts.flatMap((route) => ['ts', 'tsx', 'js', 'jsx'].map((ext) => [root, route, ext])),
);
it.each(cases)('accepts %s/routes/%s.%s', async (root, route, ext) => {
fixture(root, route, ext);
const result = await validateInstallation('tanstack-start', testDir, { runBuild: false });
expect(result.issues).toEqual([]);
expect(result.passed).toBe(true);
});

it.each(['app', 'src'].flatMap((root) => layouts.map((route) => [root, route])))(
'gives a useful mismatch hint for %s/routes/%s',
async (root, route) => {
fixture(root, route, 'tsx');
writeFileSync(join(testDir, '.env.local'), 'WORKOS_REDIRECT_URI=https://example.com/wrong/callback\n');
const result = await validateInstallation('tanstack-start', testDir, { runBuild: false });
const issue = result.issues.find((i) => i.message.includes('no matching route file'));
expect(issue?.severity).toBe('error');
expect(issue?.hint).toContain(`${root}/routes/${route}.tsx`);
expect(issue?.hint).toContain('https://example.com/api/auth/callback');
},
);

it('still rejects genuinely missing packages and callback files', async () => {
fixture('src', 'api/auth/callback', 'tsx');
rmSync(join(testDir, 'src/routes'), { recursive: true });
writeFileSync(join(testDir, 'package.json'), JSON.stringify({ dependencies: {} }));
const result = await validateInstallation('tanstack-start', testDir, { runBuild: false });
expect(result.passed).toBe(false);
expect(result.issues.filter((i) => i.type === 'package')).toHaveLength(2);
expect(result.issues.some((i) => i.message.startsWith('Missing file:') && i.message.includes('callback'))).toBe(
true,
);
expect(result.issues.some((i) => i.message.includes('no matching route file'))).toBe(true);
});
});

describe('generic package alternates', () => {
const locations = [undefined, 'any', 'dependencies', 'devDependencies'] as const;
it.each(
locations.flatMap((location) =>
['dependencies', 'devDependencies'].flatMap((installedIn) =>
['primary', 'alternate'].map((name) => ({ location, installedIn, name })),
),
),
)('respects $location for $name in $installedIn', async ({ location, installedIn, name }) => {
writeFileSync(join(testDir, 'package.json'), JSON.stringify({ [installedIn]: { [name]: '1.0.0' } }));
const rule = { name: 'primary', alternates: ['alternate'], location };
const issues = await validatePackages({ framework: 'test', packages: [rule], files: [], envVars: [] }, testDir);
expect(issues).toHaveLength(!location || location === 'any' || location === installedIn ? 0 : 1);
});

it('reports a missing package with alternate installation guidance', async () => {
writeFileSync(join(testDir, 'package.json'), '{}');
const rule: PackageRule = { name: 'primary', alternates: ['alternate'], location: 'devDependencies' };
const issues = await validatePackages({ framework: 'test', packages: [rule], files: [], envVars: [] }, testDir);
expect(issues[0]).toMatchObject({ type: 'package', severity: 'error', message: 'Missing package: primary' });
expect(issues[0].hint).toContain('alternate');
expect(issues[0].hint).toContain('--save-dev');
});
});

describe('redirect URI validation (TanStack Start)', () => {
it('detects redirect URI path mismatch with callback route', async () => {
writeFileSync(
Expand Down
98 changes: 53 additions & 45 deletions src/lib/validation/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,14 @@ export async function validatePackages(rules: ValidationRules, projectDir: strin
const location = rule.location || 'any';
const searchIn = location === 'any' ? allDeps : location === 'dependencies' ? deps : devDeps;

if (!searchIn[rule.name]) {
if (![rule.name, ...(rule.alternates || [])].some((name) => searchIn[name])) {
issues.push({
type: 'package',
severity: 'error',
message: `Missing package: ${rule.name}`,
hint: `Run: npm install ${rule.name}`,
hint: `Run: npm install ${location === 'devDependencies' ? '--save-dev ' : ''}${rule.name}${
rule.alternates?.length ? ` (or one of: ${rule.alternates.join(', ')})` : ''
}`,
});
}
}
Expand Down Expand Up @@ -187,43 +189,46 @@ export async function validateFiles(rules: ValidationRules, projectDir: string):
continue;
}

// Check content patterns
// Any matching file may satisfy the rule, but its patterns must occur together.
if (rule.mustContain || rule.mustContainAny) {
const filePath = join(projectDir, matches[0]);
let content: string;
try {
content = await readFile(filePath, 'utf-8');
} catch {
// File read error - skip content checks
continue;
}
let patternIssues: ValidationIssue[] | undefined;
for (const file of matches) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Broad globs cause many reads
When no file contains the expected pattern, this loop reads every matching file. The TanStack source rule matches all TS, TSX, JS, and JSX files under app and src, so validation of a large app can spend substantial time reading files just to produce a missing-pattern warning. A bounded or more targeted check would avoid that cost while still checking beyond the first match.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/validation/validator.ts
Line: 195

Comment:
**Broad globs cause many reads**
When no file contains the expected pattern, this loop reads every matching file. The TanStack source rule matches all TS, TSX, JS, and JSX files under `app` and `src`, so validation of a large app can spend substantial time reading files just to produce a missing-pattern warning. A bounded or more targeted check would avoid that cost while still checking beyond the first match.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

let content: string;
try {
content = await readFile(join(projectDir, file), 'utf-8');
} catch {
// A failed read must not hide another matching file.
continue;
}

// All must be present
if (rule.mustContain) {
for (const pattern of rule.mustContain) {
const fileIssues: ValidationIssue[] = [];
for (const pattern of rule.mustContain ?? []) {
if (!content.includes(pattern)) {
issues.push({
fileIssues.push({
type: 'pattern',
severity: rule.severity ?? 'warning',
message: `File ${matches[0]} missing expected pattern: "${pattern}"`,
hint: `Ensure ${matches[0]} contains: ${pattern}`,
message: `File ${file} missing expected pattern: "${pattern}"`,
hint: `Ensure ${file} contains: ${pattern}`,
});
}
}
}

// At least one must be present
if (rule.mustContainAny) {
const hasAny = rule.mustContainAny.some((p) => content.includes(p));
if (!hasAny) {
issues.push({
if (rule.mustContainAny && !rule.mustContainAny.some((pattern) => content.includes(pattern))) {
fileIssues.push({
type: 'pattern',
severity: rule.severity ?? 'warning',
message: `File ${matches[0]} missing one of: ${rule.mustContainAny.join(', ')}`,
hint: `Ensure ${matches[0]} contains one of these patterns`,
message: `File ${file} missing one of: ${rule.mustContainAny.join(', ')}`,
hint: `Ensure ${file} contains one of these patterns`,
});
}

if (fileIssues.length === 0) {
patternIssues = [];
break;
}
// Retain one file's actionable diagnostics only if no match satisfies the rule.
patternIssues ??= fileIssues;
}
issues.push(...(patternIssues ?? []));
}
}

Expand Down Expand Up @@ -595,7 +600,8 @@ async function validateReactRouterRedirectUri(projectDir: string, issues: Valida
* Validates that the TanStack Start redirect URI matches an existing callback route.
*
* TanStack Start uses file-based routing:
* - /auth/callback → app/routes/auth/callback.tsx
* Checks conventional nested, flat, index, and route files under app/routes or src/routes.
* Custom router configuration and runtime handler behavior are not inferred from source.
*/
async function validateTanstackStartRedirectUri(projectDir: string, issues: ValidationIssue[]): Promise<void> {
const envPath = join(projectDir, '.env.local');
Expand Down Expand Up @@ -631,38 +637,40 @@ async function validateTanstackStartRedirectUri(projectDir: string, issues: Vali

const routePath = callbackPath.replace(/^\//, '');

// TanStack Start route patterns
const routePatterns = [
`app/routes/${routePath}.tsx`,
`app/routes/${routePath}.ts`,
`app/routes/${routePath}.jsx`,
`app/routes/${routePath}.js`,
`app/routes/${routePath}/index.tsx`,
`app/routes/${routePath}/index.ts`,
`app/routes/${routePath}/index.jsx`,
`app/routes/${routePath}/index.js`,
];
const dotPath = routePath.replace(/\//g, '.');
const routePatterns = ['app', 'src'].flatMap((root) =>
[routePath, dotPath].flatMap((path) =>
['', '/index', '/route', '.index', '.route'].flatMap((suffix) =>
['ts', 'tsx', 'js', 'jsx'].map((ext) => `${root}/routes/${path}${suffix}.${ext}`),
),
),
);

const routeExists = routePatterns.some((pattern) => existsSync(join(projectDir, pattern)));

if (!routeExists) {
const existingRoutes = await fg(['app/routes/**/*callback*.{ts,tsx,js,jsx}'], {
cwd: projectDir,
});
const existingRoutes = await fg(
[
'{app,src}/routes/**/*callback*.{ts,tsx,js,jsx}',
'{app,src}/routes/**/*callback*/{index,route}.{ts,tsx,js,jsx}',
],
{ cwd: projectDir },
);

let hint = `Create a route at app/routes/${routePath}.tsx`;
let hint = `Create a route at app/routes/${routePath}.tsx or src/routes/${routePath}.tsx`;
if (existingRoutes.length > 0) {
const actualFile = existingRoutes[0];
const actualPath =
'/' +
actualFile
.replace(/^app\/routes\//, '')
.replace(/^(app|src)\/routes\//, '')
.replace(/\.(tsx?|jsx?)$/, '')
.replace(/\/index$/, '');
.replace(/\./g, '/')
.replace(/\/(index|route)$/, '');
hint =
`Found callback route at ${actualFile} but redirect URI points to ${callbackPath}. Either:\n` +
` 1. Change WORKOS_REDIRECT_URI to ${new URL(redirectUri).origin}${actualPath}\n` +
` 2. Move the route to app/routes/${routePath}.tsx`;
` 2. Move the route to app/routes/${routePath}.tsx or src/routes/${routePath}.tsx`;
}

issues.push({
Expand Down
Loading