Fix unused imports, restore email provider API backward compat, add authBehavior/email config tests - #124
Conversation
…compat, add config tests Co-authored-by: thinkdj <688055+thinkdj@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses follow-up issues from the config migration/package toggles work by fixing a lint-breaking unused import, restoring backward compatibility in the email provider metadata API, and adding tests for newly introduced config sections.
Changes:
- Remove unused type imports in
createAppConfig.tsto satisfy@typescript-eslint/no-unused-vars. - Restore
optionalmetadata fields in/api/email/providersresponse while keeping the newconfigNote. - Add unit tests for
emailandfeatures.authBehaviordefaults and env var overrides.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/config/src/createAppConfig.ts | Cleans up unused imports in config creation module. |
| packages/config/src/tests/config.test.ts | Adds test coverage for email + authBehavior config defaults and env overrides. |
| apps/ottabase-template-app-tanstack/worker/routes/email.ts | Restores optional fields in provider metadata for API backward compatibility. |
Comments suppressed due to low confidence (1)
packages/config/src/tests/config.test.ts:223
- The outer
describe('Configuration Utilities', ...)block is not closed at end-of-file after adding these new test suites. This leaves the file with unbalanced braces/parentheses and will cause the test file to fail to parse/compile. Add the missing closing});at the end.
});
});
| process.env['EMAIL_FROM'] = 'override@env.com'; | ||
| try { | ||
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | ||
| expect(appConfig.email.from).toBe('override@env.com'); | ||
| } finally { | ||
| delete process.env['EMAIL_FROM']; | ||
| } | ||
| }); | ||
|
|
||
| it('should override email.sesRegion with AWS_REGION env var', () => { | ||
| process.env['AWS_REGION'] = 'ap-southeast-1'; | ||
| try { | ||
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | ||
| expect(appConfig.email.sesRegion).toBe('ap-southeast-1'); | ||
| } finally { | ||
| delete process.env['AWS_REGION']; |
There was a problem hiding this comment.
These tests mutate process.env and then delete the keys in finally. If the env var was already set in the test runner/CI, this will wipe the original value and can make other tests flaky. Save the previous value before overriding and restore it in finally (set it back if it existed, otherwise delete).
| process.env['EMAIL_FROM'] = 'override@env.com'; | |
| try { | |
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | |
| expect(appConfig.email.from).toBe('override@env.com'); | |
| } finally { | |
| delete process.env['EMAIL_FROM']; | |
| } | |
| }); | |
| it('should override email.sesRegion with AWS_REGION env var', () => { | |
| process.env['AWS_REGION'] = 'ap-southeast-1'; | |
| try { | |
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | |
| expect(appConfig.email.sesRegion).toBe('ap-southeast-1'); | |
| } finally { | |
| delete process.env['AWS_REGION']; | |
| const previousEmailFrom = process.env['EMAIL_FROM']; | |
| process.env['EMAIL_FROM'] = 'override@env.com'; | |
| try { | |
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | |
| expect(appConfig.email.from).toBe('override@env.com'); | |
| } finally { | |
| if (previousEmailFrom === undefined) { | |
| delete process.env['EMAIL_FROM']; | |
| } else { | |
| process.env['EMAIL_FROM'] = previousEmailFrom; | |
| } | |
| } | |
| }); | |
| it('should override email.sesRegion with AWS_REGION env var', () => { | |
| const previousAwsRegion = process.env['AWS_REGION']; | |
| process.env['AWS_REGION'] = 'ap-southeast-1'; | |
| try { | |
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | |
| expect(appConfig.email.sesRegion).toBe('ap-southeast-1'); | |
| } finally { | |
| if (previousAwsRegion === undefined) { | |
| delete process.env['AWS_REGION']; | |
| } else { | |
| process.env['AWS_REGION'] = previousAwsRegion; | |
| } |
| it('should override sessionMaxAge with AUTH_SESSION_MAX_AGE env var', () => { | ||
| process.env['AUTH_SESSION_MAX_AGE'] = '3600'; | ||
| try { | ||
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | ||
| expect(appConfig.features.authBehavior.sessionMaxAge).toBe(3600); | ||
| } finally { | ||
| delete process.env['AUTH_SESSION_MAX_AGE']; | ||
| } | ||
| }); | ||
|
|
||
| it('should override requireEmailVerified with AUTH_REQUIRE_EMAIL_VERIFIED env var', () => { | ||
| process.env['AUTH_REQUIRE_EMAIL_VERIFIED'] = 'true'; | ||
| try { | ||
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | ||
| expect(appConfig.features.authBehavior.requireEmailVerified).toBe(true); | ||
| } finally { | ||
| delete process.env['AUTH_REQUIRE_EMAIL_VERIFIED']; | ||
| } | ||
| }); | ||
|
|
||
| it('should override disableCredentials and verbose with env vars', () => { | ||
| process.env['AUTH_DISABLE_CREDENTIALS'] = 'true'; | ||
| process.env['AUTH_VERBOSE'] = 'true'; | ||
| try { | ||
| const appConfig = createAppConfig({ appId: 'test', appName: 'Test' }); | ||
| expect(appConfig.features.authBehavior.disableCredentials).toBe(true); | ||
| expect(appConfig.features.authBehavior.verbose).toBe(true); | ||
| } finally { | ||
| delete process.env['AUTH_DISABLE_CREDENTIALS']; | ||
| delete process.env['AUTH_VERBOSE']; | ||
| } |
There was a problem hiding this comment.
Same process.env cleanup issue here: deleting the env vars in finally can remove values that were already set before the test ran. Prefer restoring the previous values to avoid leaking state across tests.
* fix: complete config pipeline for email/authBehavior + add package route guards The shareable monorepo config migration was incomplete in two key areas: 1. Config pipeline gap: email and authBehavior settings defined in ottabase.config.ts were silently ignored because AppConfig, ConfigOptions, createAppConfig(), and userConfigToOptions() all lacked support for these fields. Now the full chain works: ottabase.config.ts → userConfigToOptions → createAppConfig → AppConfig with env var overrides for backward compat. 2. Package route guarding: disabling a package via packages.ottablog: false only removed its DB tables from migrations but left API routes active, causing 500 errors on access. Now all package-specific routes in the worker router check PACKAGES toggles and return a clear 404 with PACKAGE_DISABLED code when the package is off. Additional fixes: - Migrate queue/handlers.ts from raw env vars to worker-config imports - Update bootstrap wizard to reflect EMAIL_FROM moving to ottabase.config.ts - Update email route provider info to reference config instead of env vars - Add DEFAULT_EMAIL_CONFIG, DEFAULT_AUTH_BEHAVIOR_CONFIG constants - Export AUTH_BEHAVIOR_CONFIG and EMAIL_CONFIG from app.config.ts - Add PACKAGES toggle export to worker-config.ts https://claude.ai/code/session_01VrsgTM282BW4TFws7ZfnkE * fix: replace hardcoded app branding with config-driven values Framework files contained 'Ottabase' and 'ottabase-template-app-tanstack' literals that forced users to hand-edit framework code to rebrand — exactly what the shareable monorepo design was meant to avoid. Changes: - log.config.ts: 3 occurrences of hardcoded app name → APP_ID from config - appState.ts: hardcoded appName 'Ottabase' → APP_NAME from config - router.ts: /api/health name + analytics defaultAppId → APP_NAME/APP_ID - BrandFooter.tsx: 'Built with Ottabase' → 'Built with {APP_NAME}' - docs.config.ts: 'Ottabase Docs' → '{APP_NAME} Docs' Now users only edit ottabase.config.ts to rebrand the entire app. https://claude.ai/code/session_01VrsgTM282BW4TFws7ZfnkE * Fix unused imports, restore email provider API backward compat, add authBehavior/email config tests (#124) * fix: address PR review comments - unused imports, email API backward compat, add config tests --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
…uthBehavior/email config tests (#124) * fix: address PR review comments - unused imports, email API backward compat, add config tests
…dation (#125) * fix: complete config pipeline for email/authBehavior + add package route guards The shareable monorepo config migration was incomplete in two key areas: 1. Config pipeline gap: email and authBehavior settings defined in ottabase.config.ts were silently ignored because AppConfig, ConfigOptions, createAppConfig(), and userConfigToOptions() all lacked support for these fields. Now the full chain works: ottabase.config.ts → userConfigToOptions → createAppConfig → AppConfig with env var overrides for backward compat. 2. Package route guarding: disabling a package via packages.ottablog: false only removed its DB tables from migrations but left API routes active, causing 500 errors on access. Now all package-specific routes in the worker router check PACKAGES toggles and return a clear 404 with PACKAGE_DISABLED code when the package is off. Additional fixes: - Migrate queue/handlers.ts from raw env vars to worker-config imports - Update bootstrap wizard to reflect EMAIL_FROM moving to ottabase.config.ts - Update email route provider info to reference config instead of env vars - Add DEFAULT_EMAIL_CONFIG, DEFAULT_AUTH_BEHAVIOR_CONFIG constants - Export AUTH_BEHAVIOR_CONFIG and EMAIL_CONFIG from app.config.ts - Add PACKAGES toggle export to worker-config.ts https://claude.ai/code/session_01VrsgTM282BW4TFws7ZfnkE * Fix unused imports, restore email provider API backward compat, add authBehavior/email config tests (#124) * fix: address PR review comments - unused imports, email API backward compat, add config tests * feat: add .example/.template pattern for user-owned files Separate framework-tracked files from user-owned files so `git pull` never overwrites user customisations: - ottabase.config.ts → ottabase.config.example.ts (tracked template) - wrangler.jsonc → wrangler.example.jsonc (tracked; CI uses this) - ottabase/ → ottabase.template/ (tracked reference copy) User working copies (ottabase.config.ts, wrangler.jsonc, ottabase/) are gitignored. First-time setup: copy from .example/.template. Updates CI workflows (deploy.yml, pr-preview.yml), cloudflare-config.json, and README with new setup instructions and directory structure. https://claude.ai/code/session_01VrsgTM282BW4TFws7ZfnkE * feat: add route registration hook for custom/premium packages Custom packages can now register API routes without editing the framework-owned router.ts. Users add handlers in the user-zone file `ottabase/config.routes.ts` (same pattern as config.migrations.ts). - Extract ApiRouteContext to worker/routes/types.ts (shared type) - Add ottabase.template/config.routes.ts with scaffold + docs - Wire handleCustomRoutes() into resolveApiRoute() as fallback after all built-in routes - Update README with route registration in premium package workflow https://claude.ai/code/session_01VrsgTM282BW4TFws7ZfnkE * docs: improve route registration hook documentation - types.ts: add field-level JSDoc on ApiRouteContext with usage example - ottabase.config.example.ts: mention config.routes.ts in premium package steps - config.migrations.ts: cross-reference config.routes.ts in setup steps - README: add dedicated "Custom Routes" section with example + resolution order - README: update File Ownership table to include routes https://claude.ai/code/session_01VrsgTM282BW4TFws7ZfnkE * feat: add runtime config validation to defineOttabaseConfig Typos in ottabase.config.ts no longer silently fall to defaults. defineOttabaseConfig() now validates at startup: - Throws on missing required fields (appId, appName) - Warns on unrecognised keys at all nesting levels (top-level, packages, features, features.authBehavior, email, etc.) Adds validateOttabaseConfig() as a standalone export for testing. 12 new tests covering required fields, typo detection at every nesting level, multiple warnings, and integration with defineOttabaseConfig. https://claude.ai/code/session_01VrsgTM282BW4TFws7ZfnkE * Setup user-zone, dynamic routes & config fixes Add a setup script and npm hooks to ensure user-zone files exist before dev/build. Introduce scripts/setup-user-zone.js which copies ottabase.template → ottabase and wrangler.example.jsonc → wrangler.jsonc when targets are missing, and wire it into package.json (setup, predev, prebuild). Make worker/router load user custom routes via dynamic import with a no-op fallback to avoid crashes when the user-zone is not present. Tighten createAppConfig validation to require non-empty (trimmed) appId and appName. Update tests to centralize console.warn spying with beforeEach/afterEach and adjust import ordering. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Three issues flagged in PR review on the config migration and package toggles work.
Unused imports —
createAppConfig.tsAuthBehaviorConfigandEmailConfigwere imported but never referenced, breaking@typescript-eslint/no-unused-vars.Breaking API change —
email.tshandleEmailProviders()dropped theoptionalfield and replaced it withconfigNote, breaking clients reading provider metadata. Both fields now coexist.Missing tests —
config.test.tsAdded coverage for the new
emailandfeatures.authBehaviorconfig sections introduced in the base PR:email.from,email.sesRegion, and all fourauthBehaviorflagsEMAIL_FROM,AWS_REGION,AUTH_SESSION_MAX_AGE,AUTH_REQUIRE_EMAIL_VERIFIED,AUTH_DISABLE_CREDENTIALS,AUTH_VERBOSE✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.