Chore/port foundation modules - #2
Merged
Merged
Conversation
Integrates global Logger, Supabase, and HTTP modules across hosts. Standardizes config loading with a dedicated subpath, refines environment variable handling for Supabase keys and logging, and removes the temporary demo capability. Ensures explicit wiring of core services as per ADR-007 and 002d plan.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change completes a Foundation migration across API and worker hosts, adding Supabase authentication, user administration, local seed/test workflows, shared infrastructure, and updated configuration. Legacy foundation modules are removed or narrowed, while architecture and migration records document the completed decisions. ChangesFoundation migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthenticationGuard
participant SupabaseStrategy
participant ApiKeyStrategy
participant AuthController
Client->>AuthenticationGuard: request /auth/me
AuthenticationGuard->>SupabaseStrategy: validate bearer JWT
AuthenticationGuard->>ApiKeyStrategy: validate X-API-KEY
SupabaseStrategy->>AuthController: provide AuthenticatedUser
ApiKeyStrategy->>AuthController: provide AuthenticatedUser
AuthController-->>Client: return authenticated identity
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/plans/002-oss-migration/002d-platform-core-import.md (1)
299-309: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
db resetin the seed completion criterion.The “Done when” text says
pnpm supabase start + seed, while the implemented/configured flow runsseed.sqlonpnpm exec supabase db reset. Update the criterion to name the actual command explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/002-oss-migration/002d-platform-core-import.md` around lines 299 - 309, Update the “Done when” criterion in the 002d migration plan to explicitly require running `pnpm exec supabase db reset`, while preserving the requirement for a coherent Foundation dataset and usable test user.
🧹 Nitpick comments (4)
apps/api/src/main.ts (1)
15-20: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
whitelist: trueto the global ValidationPipe.Without
whitelist: true, unknown properties in request bodies are not stripped, creating a potential mass-assignment risk when DTOs map to database writes. Addingwhitelist: true(and optionallyforbidNonWhitelisted: true) ensures only explicitly declared DTO fields are accepted.🛡️ Suggested fix
new ValidationPipe({ transform: true, + whitelist: true, + forbidNonWhitelisted: true, transformOptions: { enableImplicitConversion: false }, }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/main.ts` around lines 15 - 20, Update the global ValidationPipe configuration in main.ts to enable whitelist: true, ensuring unknown request properties are stripped while preserving the existing transformation settings.libs/core/src/modules/supabase/supabase.errors.ts (1)
24-31: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider tightening the Cloudflare HTML error detection.
The
isCloudflareHtmlErrorcheck matches any error message containing the substring"html"(case-insensitive). This could false-positive on legitimate PostgREST error messages that mention HTML (e.g.,"invalid html entity"). A more specific check for actual HTML content like<!doctypeor<htmlwould reduce false positives while still catching Cloudflare error pages.♻️ Suggested refinement
function isCloudflareHtmlError(error: unknown): boolean { const msg = hasMessage(error) ? error.message : undefined; if (typeof msg !== 'string') { return false; } const checkableMsg = msg.trim().toLowerCase(); - return checkableMsg.includes('html') || checkableMsg.includes('internal server error'); + return checkableMsg.includes('<html') || checkableMsg.includes('<!doctype') || checkableMsg.includes('internal server error'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/core/src/modules/supabase/supabase.errors.ts` around lines 24 - 31, Refine isCloudflareHtmlError so it no longer treats any message containing “html” as a Cloudflare page; detect actual HTML markers such as “<!doctype” or “<html” after normalization, while preserving the existing “internal server error” detection.libs/core/src/types/supabase-types-adjusted.ts (1)
16-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using a tuple type for GeoJSON Point coordinates.
coordinates: number[]allows arrays of any length. The GeoJSON Point spec requires exactly 2 (or 3) coordinate values. A tuple type[number, number]would be more precise and catch invalid coordinate arrays at compile time.♻️ Suggested type refinement
location_geom: { type: 'Point'; - coordinates: number[]; + coordinates: [number, number]; } | null;And similarly for
poles:location_geom: { type: 'Point'; - coordinates: number[]; + coordinates: [number, number]; };Also applies to: 24-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/core/src/types/supabase-types-adjusted.ts` around lines 16 - 19, Refine the GeoJSON Point coordinate types in the location_geom and poles definitions from number[] to a tuple enforcing two or three numeric coordinates, such as a union of [number, number] and [number, number, number].apps/api/test/unit/helpers/local-supabase-env.spec.ts (1)
10-15: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest only covers both-missing case, not individual missing vars.
The test name says "when SUPABASE_URL or SUPABASE_SECRET_KEY is missing" but only deletes both. The helper uses
if (!url || !secretKey), so each branch should be tested independently to verify the||logic.🧪 Suggested additional test cases
it('returns null when SUPABASE_URL or SUPABASE_SECRET_KEY is missing', () => { delete process.env.SUPABASE_URL; delete process.env.SUPABASE_SECRET_KEY; expect(getLocalSupabaseEnv()).toBeNull(); }); + it('returns null when only SUPABASE_URL is missing', () => { + process.env.SUPABASE_URL = ''; + process.env.SUPABASE_SECRET_KEY = 'test-secret'; + + expect(getLocalSupabaseEnv()).toBeNull(); + }); + + it('returns null when only SUPABASE_SECRET_KEY is missing', () => { + process.env.SUPABASE_URL = 'http://127.0.0.1:54321'; + process.env.SUPABASE_SECRET_KEY = ''; + + expect(getLocalSupabaseEnv()).toBeNull(); + }); + it('returns trimmed url and secret when both are set', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/test/unit/helpers/local-supabase-env.spec.ts` around lines 10 - 15, Expand the test for getLocalSupabaseEnv to cover each missing-variable case independently: retain the both-missing scenario, then add cases where only SUPABASE_URL is deleted and only SUPABASE_SECRET_KEY is deleted, asserting null each time. Ensure the other environment variable is set so each test validates one side of the helper’s OR condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/.env.example`:
- Around line 19-20: Update the JWT environment-variable comments in the example
configuration to match SupabaseStrategy: document SUPABASE_JWT_SECRET as
applicable only when SUPABASE_JWKS_URL is unset, and state that both variables
may be present with JWKS taking precedence; do not describe fallback on JWKS
endpoint unavailability.
In `@apps/api/src/main.ts`:
- Line 14: Update the CORS configuration at app.enableCors() to use an explicit
allowlist of origins sourced from the application’s existing config or
environment settings, rather than the permissive default. Preserve the API’s
current CORS behavior for approved origins and reject unconfigured origins.
In `@apps/api/src/modules/auth/api-key.strategy.ts`:
- Around line 53-55: The API-key authentication path must not fall through to
the privileged service_role client. Update the strategy and affected handler
flow around the API-key authentication logic to reject user-scoped database
operations unless a tenant-scoped client or explicit authorization boundary is
available; preserve service_role access only for explicitly authorized
non-user-scoped operations.
- Around line 56-64: Update the principal construction in the authentication
strategy to validate account.email, account.full_name, and account.supabase_id
before returning the AuthenticatedUser object. Reject authentication when any
required claim is missing instead of coercing it to an empty string, while
preserving the existing successful return path for complete account records.
In `@apps/api/src/modules/auth/supabase.strategy.ts`:
- Around line 96-99: Update the organization_id validation in the authentication
strategy to require a finite positive value, matching the existing account_id
check. Reject zero and negative organization IDs with the same
UnauthorizedException path while preserving valid organization claims.
In `@apps/api/src/modules/user-admin/user-admin.service.ts`:
- Around line 65-76: Update inviteMember and every related user-admin service
method to perform authorization before obtaining or using the service-role
client: require an authorized administrative role, validate that the target
organization belongs to author.organization_id, and permit cross-tenant targets
only for the explicit platform role. Do not rely on author.validate() alone;
apply these checks to all organization, grid, and entity IDs accepted by the
admin operations.
- Around line 111-120: Update every audit insert flow in the user-admin service,
including the shown call and the referenced locations, so rejected audit writes
are handled rather than detached with void. Prefer awaiting the Supabase insert
and response handling within the surrounding async operation; if the audit
remains best-effort, explicitly catch and log failures while preserving the
primary operation’s behavior.
- Around line 211-213: Update the user-creation methods containing the
fire-and-forget updateUserById calls to await the app_metadata persistence
before returning. Handle and propagate any rejected update, and validate the
update result so account, organization, and grid claims are persisted before the
new user can receive tokens; apply the same change to both create flows.
- Around line 78-109: Update the user-creation workflows surrounding the Auth
calls and database writes, including the analogous member, agent, customer,
restore, and delete flows, to be resumable and idempotent. Track completed
mutations and compensate them when later steps fail, including removing or
disabling orphaned Auth users and reverting stale metadata or database rows;
ensure retries safely reuse or reconcile existing records instead of duplicating
them.
In
`@docs/architecture/004-open-source-architecture-and-capability-modularization.md`:
- Around line 157-160: Move the ADR-005 entry out of the “Out of Scope /
Deferred to Follow-up ADRs” section, or rename that section to “Follow-up ADRs”
so the accepted status is consistent with the document’s roadmap.
In `@docs/architecture/005-inter-host-communication.md`:
- Line 225: Remove the unmatched closing parenthesis after the final bullet in
the Related section, leaving the section properly terminated without altering
its content.
In `@docs/architecture/014-api-key-and-machine-credentials.md`:
- Around line 99-110: Before adding or exposing additional machine routes,
update the API-key authentication flow around ApiKeyStrategy and
AuthenticatedUser.supabase to enforce an explicit route allowlist and scopes,
then attach a principal-specific RLS-bound Supabase client from the JWT claims.
Ensure machine handlers use that client instead of service_role for data access,
retaining the admin client only for genuinely privileged Auth Admin operations.
In `@legacy/apps/tiamat/src/modules/app.module.ts`:
- Line 4: Update the app module imports and module configuration to restore
GlobalHttpModule and GlobalSupabaseModule for GridsModule, ensuring the
HttpService and SupabaseService dependencies used by GridsService are available
during Nest bootstrap; do not remove or migrate the existing service injections.
In `@libs/core/src/modules/supabase/supabase.module.ts`:
- Around line 45-54: Rename the HANDLE_RESPONSE_UNTYPED handler to a camelCase
name consistent with handleResponse and handleSingle, and update every reference
to it throughout the module without changing its behavior.
---
Outside diff comments:
In `@docs/plans/002-oss-migration/002d-platform-core-import.md`:
- Around line 299-309: Update the “Done when” criterion in the 002d migration
plan to explicitly require running `pnpm exec supabase db reset`, while
preserving the requirement for a coherent Foundation dataset and usable test
user.
---
Nitpick comments:
In `@apps/api/src/main.ts`:
- Around line 15-20: Update the global ValidationPipe configuration in main.ts
to enable whitelist: true, ensuring unknown request properties are stripped
while preserving the existing transformation settings.
In `@apps/api/test/unit/helpers/local-supabase-env.spec.ts`:
- Around line 10-15: Expand the test for getLocalSupabaseEnv to cover each
missing-variable case independently: retain the both-missing scenario, then add
cases where only SUPABASE_URL is deleted and only SUPABASE_SECRET_KEY is
deleted, asserting null each time. Ensure the other environment variable is set
so each test validates one side of the helper’s OR condition.
In `@libs/core/src/modules/supabase/supabase.errors.ts`:
- Around line 24-31: Refine isCloudflareHtmlError so it no longer treats any
message containing “html” as a Cloudflare page; detect actual HTML markers such
as “<!doctype” or “<html” after normalization, while preserving the existing
“internal server error” detection.
In `@libs/core/src/types/supabase-types-adjusted.ts`:
- Around line 16-19: Refine the GeoJSON Point coordinate types in the
location_geom and poles definitions from number[] to a tuple enforcing two or
three numeric coordinates, such as a union of [number, number] and [number,
number, number].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f5ffa9a-c37c-4d87-8f2e-263555fb284f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (120)
.env.example.gitignore.nxignore.vscode/extensions.json.vscode/settings.jsonAGENTS.mdREADME.mdapps/api/.env.exampleapps/api/http/.httpyac.jsapps/api/http/README.mdapps/api/http/login.httpapps/api/http/me.httpapps/api/http/user-admin.httpapps/api/jest.config.ctsapps/api/jest.e2e.config.ctsapps/api/jest.integration.config.ctsapps/api/jest.shared.cjsapps/api/package.jsonapps/api/src/main.tsapps/api/src/modules/app.module.tsapps/api/src/modules/auth/api-key.strategy.tsapps/api/src/modules/auth/auth.controller.tsapps/api/src/modules/auth/auth.module.tsapps/api/src/modules/auth/authenticated-user.tsapps/api/src/modules/auth/authentication.guard.tsapps/api/src/modules/auth/supabase.strategy.tsapps/api/src/modules/health/health.service.tsapps/api/src/modules/user-admin/dto/create-agent.dto.tsapps/api/src/modules/user-admin/dto/invite-member.dto.tsapps/api/src/modules/user-admin/dto/update-agent.dto.tsapps/api/src/modules/user-admin/dto/update-customer.dto.tsapps/api/src/modules/user-admin/dto/update-member.dto.tsapps/api/src/modules/user-admin/user-admin.controller.tsapps/api/src/modules/user-admin/user-admin.module.tsapps/api/src/modules/user-admin/user-admin.service.tsapps/api/test/e2e/auth.e2e.spec.tsapps/api/test/helpers/local-supabase-env.tsapps/api/test/integration/auth/api-key.strategy.integration.spec.tsapps/api/test/unit/helpers/local-supabase-env.spec.tsapps/api/tsconfig.app.jsonapps/api/tsconfig.spec.jsonapps/worker/.env.exampleapps/worker/src/main.tsapps/worker/src/modules/app.module.tsapps/worker/tsconfig.app.jsonapps/worker/tsconfig.spec.jsonconfig.default.jsonconfig.example.jsondocs/architecture/004-open-source-architecture-and-capability-modularization.mddocs/architecture/005-inter-host-communication.mddocs/architecture/007-configuration-and-wiring-mechanism.mddocs/architecture/012-company-cutover-strategy.mddocs/architecture/014-api-key-and-machine-credentials.mddocs/deployment/supabase.mddocs/plans/002-oss-migration.mddocs/plans/002-oss-migration/002b-schema-deviation-register.mddocs/plans/002-oss-migration/002d-platform-core-import.mddocs/plans/002-oss-migration/internationalization-and-debrand-register.mdeslint.config.mjslegacy/apps/tiamat/src/modules/accounts/accounts.module.tslegacy/apps/tiamat/src/modules/api-keys/api-keys.module.tslegacy/apps/tiamat/src/modules/app.module.tslegacy/apps/tiamat/src/modules/auth/api-key.strategy.tslegacy/apps/tiamat/src/modules/auth/auth.controller.tslegacy/apps/tiamat/src/modules/auth/auth.module.tslegacy/apps/tiamat/src/modules/auth/auth.service.tslegacy/apps/tiamat/src/modules/auth/authentication.guard.tslegacy/apps/tiamat/src/modules/auth/nxt-supabase-user.tslegacy/apps/tiamat/src/modules/auth/supabase.strategy.tslegacy/apps/tiamat/src/modules/grids/grids.controller.tslegacy/apps/tiamat/src/modules/grids/grids.module.tslegacy/apps/tiamat/src/modules/grids/grids.service.tslegacy/apps/tiamat/src/modules/organizations/organizations.controller.tslegacy/apps/tiamat/src/modules/organizations/organizations.module.tslegacy/apps/tiamat/src/modules/organizations/organizations.service.tslegacy/apps/tiamat/src/modules/user-admin/dto/create-agent.dto.tslegacy/apps/tiamat/src/modules/user-admin/dto/update-agent.dto.tslegacy/apps/tiamat/src/modules/user-admin/user-admin.module.tslegacy/apps/tiamat/src/modules/user-admin/user-admin.service.tslegacy/libs/core/src/index.tslegacy/libs/core/src/modules/accounts/accounts.service.tslegacy/libs/core/src/modules/accounts/entities/account.entity.tslegacy/libs/core/src/modules/api-keys/api-keys.service.tslegacy/libs/core/src/modules/api-keys/entities/api-key.entity.tslegacy/libs/core/src/modules/grids/grids.service.tslegacy/libs/core/src/modules/logger-module.tslegacy/libs/core/src/modules/members/entities/member.entity.tslegacy/libs/core/src/modules/organizations/entities/organization.entity.tslegacy/libs/core/src/modules/supabase.module.tslibs/core/README.mdlibs/core/jest.config.ctslibs/core/package.jsonlibs/core/src/config/index.tslibs/core/src/config/loader.tslibs/core/src/config/require-env.tslibs/core/src/config/schema.tslibs/core/src/constants.tslibs/core/src/index.tslibs/core/src/modules/customers/dto/create-customer.dto.tslibs/core/src/modules/demo/demo-modules.spec.tslibs/core/src/modules/demo/demo-modules.tslibs/core/src/modules/demo/demo.module.tslibs/core/src/modules/demo/demo.schema.tslibs/core/src/modules/demo/demo.service.tslibs/core/src/modules/global-http-module.tslibs/core/src/modules/logger/logger.module.tslibs/core/src/modules/logger/logger.options.tslibs/core/src/modules/platform/package-info.tslibs/core/src/modules/supabase/supabase.errors.tslibs/core/src/modules/supabase/supabase.module.tslibs/core/src/types/supabase-types-adjusted.tslibs/core/test/unit/config/__fixtures__/from-default.config.jsonlibs/core/test/unit/config/__fixtures__/from-path.config.jsonlibs/core/test/unit/config/__fixtures__/invalid-schema-version.config.jsonlibs/core/test/unit/config/index.spec.tslibs/core/test/unit/config/loader.spec.tslibs/core/tsconfig.spec.jsonsupabase/migrations/20260710120000_init.sqlsupabase/seed.sqltsconfig.base.json
💤 Files with no reviewable changes (37)
- legacy/apps/tiamat/src/modules/user-admin/dto/create-agent.dto.ts
- legacy/apps/tiamat/src/modules/accounts/accounts.module.ts
- config.default.json
- legacy/apps/tiamat/src/modules/organizations/organizations.controller.ts
- legacy/libs/core/src/modules/accounts/accounts.service.ts
- libs/core/src/modules/demo/demo-modules.spec.ts
- legacy/apps/tiamat/src/modules/auth/auth.controller.ts
- legacy/apps/tiamat/src/modules/organizations/organizations.module.ts
- libs/core/src/modules/platform/package-info.ts
- legacy/apps/tiamat/src/modules/api-keys/api-keys.module.ts
- libs/core/src/modules/demo/demo.module.ts
- libs/core/test/unit/config/fixtures/invalid-schema-version.config.json
- legacy/apps/tiamat/src/modules/auth/auth.service.ts
- legacy/apps/tiamat/src/modules/auth/supabase.strategy.ts
- legacy/libs/core/src/modules/members/entities/member.entity.ts
- legacy/apps/tiamat/src/modules/auth/api-key.strategy.ts
- apps/worker/tsconfig.app.json
- legacy/libs/core/src/index.ts
- legacy/libs/core/src/modules/api-keys/entities/api-key.entity.ts
- legacy/libs/core/src/modules/logger-module.ts
- legacy/apps/tiamat/src/modules/user-admin/user-admin.service.ts
- legacy/libs/core/src/modules/api-keys/api-keys.service.ts
- legacy/apps/tiamat/src/modules/auth/authentication.guard.ts
- legacy/apps/tiamat/src/modules/organizations/organizations.service.ts
- legacy/apps/tiamat/src/modules/auth/auth.module.ts
- libs/core/src/modules/demo/demo.service.ts
- legacy/apps/tiamat/src/modules/user-admin/dto/update-agent.dto.ts
- libs/core/src/modules/demo/demo.schema.ts
- legacy/apps/tiamat/src/modules/user-admin/user-admin.module.ts
- libs/core/test/unit/config/fixtures/from-default.config.json
- apps/api/tsconfig.app.json
- libs/core/src/modules/demo/demo-modules.ts
- libs/core/test/unit/config/fixtures/from-path.config.json
- legacy/apps/tiamat/src/modules/auth/nxt-supabase-user.ts
- legacy/libs/core/src/modules/supabase.module.ts
- legacy/libs/core/src/modules/accounts/entities/account.entity.ts
- legacy/libs/core/src/modules/organizations/entities/organization.entity.ts
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
legacy/apps/tiamat/src/modules/app.module.ts (1)
4-4: 🩺 Stability & Availability | 🔴 CriticalRestore the providers required by the retained
GridsModule.
GridsModuleis still registered on Line [67], but itsGridsServiceconstructor still injectsHttpServiceandSupabaseServiceinlegacy/apps/tiamat/src/modules/grids/grids.service.ts(constructor block, Lines 30-39). RemovingGlobalHttpModuleandGlobalSupabaseModuleleaves Nest unable to resolve those tokens during bootstrap. Restore both modules or remove/migrateGridsModuleand its remaining consumers together.Proposed fix
-import { CoreTypeOrmModule, CorePgModule } from '`@core`'; +import { + CoreTypeOrmModule, + CorePgModule, + GlobalHttpModule, + GlobalSupabaseModule, +} from '`@core`'; CorePgModule, + GlobalHttpModule, + GlobalSupabaseModule,Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@legacy/apps/tiamat/src/modules/app.module.ts` at line 4, Restore GlobalHttpModule and GlobalSupabaseModule in the module imports/providers required by the retained GridsModule, so GridsService can resolve HttpService and SupabaseService during bootstrap. Alternatively, remove or migrate GridsModule and all remaining consumers together, but do not leave the existing GridsModule registration without its dependencies.
🧹 Nitpick comments (2)
libs/core/src/modules/logger/logger.options.ts (1)
70-77: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winValidate
LOG_LEVELbefore passing it to Pino.Any non-empty string is currently accepted, so a typo such as
LOG_LEVEL=debgucan fail logger initialization when this deferred configuration is restored. Normalize against the configured logger levels or fall back toinfo.Suggested guard
- const level = process.env.LOG_LEVEL?.trim() || DEFAULT_LOG_LEVEL; + const configuredLevel = process.env.LOG_LEVEL?.trim().toLowerCase(); + const allowedLevels = new Set([ 'trace', 'debug', 'info', 'warn', 'error', 'fatal', 'silent' ]); + const level = + configuredLevel && allowedLevels.has(configuredLevel) + ? configuredLevel + : DEFAULT_LOG_LEVEL;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/core/src/modules/logger/logger.options.ts` around lines 70 - 77, Update buildPinoHttpOptions to validate the trimmed LOG_LEVEL against the configured Pino logger levels before assigning level; preserve valid configured values and fall back to the info/default level for empty or unrecognized values, so invalid environment input cannot reach Pino.apps/api/jest.e2e.config.cts (1)
3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not silently pass dedicated test targets with zero collected tests.
passWithNoTests: truecan hide broken globs or accidental test exclusion. If environment gating is intentional for local runs, keep that behavior in a separate optional target and make CI’s integration/e2e targets fail when no tests are collected.
apps/api/jest.e2e.config.cts#L3-L8: make the CI-facing e2e configuration strict, or document the optional-only target.apps/api/jest.integration.config.cts#L3-L8: apply the same policy to integration tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/jest.e2e.config.cts` around lines 3 - 8, Make the CI-facing Jest configurations strict by removing passWithNoTests: true from apps/api/jest.e2e.config.cts and apps/api/jest.integration.config.cts; if local environment-gated runs require allowing zero tests, move that behavior to separately named optional targets and document the distinction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/005-inter-host-communication.md`:
- Around line 16-19: Update the roadmap-status paragraph in ADR-005’s context
section to reflect that ADR-005 was accepted on July 17, 2026. Reword the
reference to Production Monitoring import (002e) to identify it as the first
capability import applying the accepted policy, rather than stating that the ADR
remains open until 002e.
In `@docs/plans/002-oss-migration/002d-platform-core-import.md`:
- Around line 294-297: Update the status entries for Task 5 and Task 6 in the
migration plan to record their actual sign-off status and date, removing the
stale “awaiting sign-off” wording. If either task is not signed off, revise the
plan header, Task 11, and exit bar so the overall completion state remains
pending.
- Line 114: Update the Foundation scope row in the migration plan to remove the
“new pino logging” claim and describe the shipped logging implementation as Nest
Logger plus the no-op GlobalLoggerModule, while retaining the Supabase client
and HTTP entries.
In `@legacy/apps/tiamat/src/modules/app.module.ts`:
- Around line 13-14: Update the Task 11 comment in app.module.ts to remove the
inaccurate “controller gone” claim and accurately state that GridsModule and its
GridsController remain, with only the GET /grids/:id route removed.
In `@libs/core/src/modules/supabase/supabase.errors.ts`:
- Around line 55-56: Remove the raw console.info(error) call from the Supabase
error handling in supabase.errors.ts. Replace it with the configured logger and
a sanitized summary only, avoiding complete upstream error objects or sensitive
details.
- Around line 63-67: Update the error handling around resolveErrorMessage so its
detailed result is used only for server-side logger.error diagnostics. Pass a
stable, generic public message to HttpException while preserving the resolved
status code and existing logging context.
- Around line 24-30: The isCloudflareHtmlError predicate must require a 5xx
response status before classifying an error as a Cloudflare failure. Update it
to read the supplied response status and actual response body, returning false
for non-5xx statuses such as 401, and only match the existing Cloudflare
indicators within that body before the caller overrides the status.
- Around line 35-45: Update resolveErrorMessage so its final serialization path
always returns a string: safely attempt JSON.stringify(error), handle
serialization failures or an undefined result, and fall back to String(error).
Preserve the existing handling for string, Error, and message-bearing values.
---
Duplicate comments:
In `@legacy/apps/tiamat/src/modules/app.module.ts`:
- Line 4: Restore GlobalHttpModule and GlobalSupabaseModule in the module
imports/providers required by the retained GridsModule, so GridsService can
resolve HttpService and SupabaseService during bootstrap. Alternatively, remove
or migrate GridsModule and all remaining consumers together, but do not leave
the existing GridsModule registration without its dependencies.
---
Nitpick comments:
In `@apps/api/jest.e2e.config.cts`:
- Around line 3-8: Make the CI-facing Jest configurations strict by removing
passWithNoTests: true from apps/api/jest.e2e.config.cts and
apps/api/jest.integration.config.cts; if local environment-gated runs require
allowing zero tests, move that behavior to separately named optional targets and
document the distinction.
In `@libs/core/src/modules/logger/logger.options.ts`:
- Around line 70-77: Update buildPinoHttpOptions to validate the trimmed
LOG_LEVEL against the configured Pino logger levels before assigning level;
preserve valid configured values and fall back to the info/default level for
empty or unrecognized values, so invalid environment input cannot reach Pino.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 76439faa-9f89-4126-bfac-090836603e5a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (122)
.coderabbit.yaml.env.example.gitignore.nxignore.vscode/extensions.json.vscode/settings.jsonAGENTS.mdREADME.mdapps/api/.env.exampleapps/api/http/.httpyac.jsapps/api/http/README.mdapps/api/http/login.httpapps/api/http/me.httpapps/api/http/user-admin.httpapps/api/jest.config.ctsapps/api/jest.e2e.config.ctsapps/api/jest.integration.config.ctsapps/api/jest.shared.cjsapps/api/package.jsonapps/api/src/main.tsapps/api/src/modules/app.module.tsapps/api/src/modules/auth/api-key.strategy.tsapps/api/src/modules/auth/auth.controller.tsapps/api/src/modules/auth/auth.module.tsapps/api/src/modules/auth/authenticated-user.tsapps/api/src/modules/auth/authentication.guard.tsapps/api/src/modules/auth/supabase.strategy.tsapps/api/src/modules/health/health.service.tsapps/api/src/modules/user-admin/dto/create-agent.dto.tsapps/api/src/modules/user-admin/dto/invite-member.dto.tsapps/api/src/modules/user-admin/dto/update-agent.dto.tsapps/api/src/modules/user-admin/dto/update-customer.dto.tsapps/api/src/modules/user-admin/dto/update-member.dto.tsapps/api/src/modules/user-admin/user-admin.controller.tsapps/api/src/modules/user-admin/user-admin.module.tsapps/api/src/modules/user-admin/user-admin.service.tsapps/api/test/e2e/auth.e2e.spec.tsapps/api/test/helpers/local-supabase-env.tsapps/api/test/integration/auth/api-key.strategy.integration.spec.tsapps/api/test/unit/helpers/local-supabase-env.spec.tsapps/api/tsconfig.app.jsonapps/api/tsconfig.spec.jsonapps/worker/.env.exampleapps/worker/src/main.tsapps/worker/src/modules/app.module.tsapps/worker/tsconfig.app.jsonapps/worker/tsconfig.spec.jsonconfig.default.jsonconfig.example.jsondocs/architecture/004-open-source-architecture-and-capability-modularization.mddocs/architecture/005-inter-host-communication.mddocs/architecture/007-configuration-and-wiring-mechanism.mddocs/architecture/012-company-cutover-strategy.mddocs/architecture/014-api-key-and-machine-credentials.mddocs/deployment/supabase.mddocs/plans/002-oss-migration.mddocs/plans/002-oss-migration/002b-schema-deviation-register.mddocs/plans/002-oss-migration/002d-platform-core-import.mddocs/plans/002-oss-migration/internationalization-and-debrand-register.mdeslint.config.mjslegacy/apps/tiamat/src/modules/accounts/accounts.module.tslegacy/apps/tiamat/src/modules/api-keys/api-keys.module.tslegacy/apps/tiamat/src/modules/app.module.tslegacy/apps/tiamat/src/modules/auth/api-key.strategy.tslegacy/apps/tiamat/src/modules/auth/auth.controller.tslegacy/apps/tiamat/src/modules/auth/auth.module.tslegacy/apps/tiamat/src/modules/auth/auth.service.tslegacy/apps/tiamat/src/modules/auth/authentication.guard.tslegacy/apps/tiamat/src/modules/auth/nxt-supabase-user.tslegacy/apps/tiamat/src/modules/auth/supabase.strategy.tslegacy/apps/tiamat/src/modules/grids/grids.controller.tslegacy/apps/tiamat/src/modules/grids/grids.module.tslegacy/apps/tiamat/src/modules/grids/grids.service.tslegacy/apps/tiamat/src/modules/organizations/organizations.controller.tslegacy/apps/tiamat/src/modules/organizations/organizations.module.tslegacy/apps/tiamat/src/modules/organizations/organizations.service.tslegacy/apps/tiamat/src/modules/user-admin/dto/create-agent.dto.tslegacy/apps/tiamat/src/modules/user-admin/dto/update-agent.dto.tslegacy/apps/tiamat/src/modules/user-admin/user-admin.module.tslegacy/apps/tiamat/src/modules/user-admin/user-admin.service.tslegacy/libs/core/src/index.tslegacy/libs/core/src/modules/accounts/accounts.service.tslegacy/libs/core/src/modules/accounts/entities/account.entity.tslegacy/libs/core/src/modules/api-keys/api-keys.service.tslegacy/libs/core/src/modules/api-keys/entities/api-key.entity.tslegacy/libs/core/src/modules/grids/grids.service.tslegacy/libs/core/src/modules/logger-module.tslegacy/libs/core/src/modules/members/entities/member.entity.tslegacy/libs/core/src/modules/organizations/entities/organization.entity.tslegacy/libs/core/src/modules/supabase.module.tslibs/core/README.mdlibs/core/jest.config.ctslibs/core/package.jsonlibs/core/src/config/index.tslibs/core/src/config/loader.tslibs/core/src/config/require-env.tslibs/core/src/config/schema.tslibs/core/src/constants.tslibs/core/src/index.tslibs/core/src/modules/customers/dto/create-customer.dto.tslibs/core/src/modules/demo/demo-modules.spec.tslibs/core/src/modules/demo/demo-modules.tslibs/core/src/modules/demo/demo.module.tslibs/core/src/modules/demo/demo.schema.tslibs/core/src/modules/demo/demo.service.tslibs/core/src/modules/global-http-module.tslibs/core/src/modules/logger/logger.module.tslibs/core/src/modules/logger/logger.options.tslibs/core/src/modules/platform/package-info.tslibs/core/src/modules/supabase/supabase.errors.tslibs/core/src/modules/supabase/supabase.module.tslibs/core/src/types/supabase-types-adjusted.tslibs/core/src/types/supabase-types.tslibs/core/test/unit/config/__fixtures__/from-default.config.jsonlibs/core/test/unit/config/__fixtures__/from-path.config.jsonlibs/core/test/unit/config/__fixtures__/invalid-schema-version.config.jsonlibs/core/test/unit/config/index.spec.tslibs/core/test/unit/config/loader.spec.tslibs/core/tsconfig.spec.jsonsupabase/migrations/20260710120000_init.sqlsupabase/seed.sqltsconfig.base.json
💤 Files with no reviewable changes (37)
- libs/core/src/modules/demo/demo.module.ts
- libs/core/src/modules/demo/demo-modules.ts
- legacy/apps/tiamat/src/modules/organizations/organizations.module.ts
- apps/api/tsconfig.app.json
- legacy/libs/core/src/modules/logger-module.ts
- libs/core/src/modules/platform/package-info.ts
- legacy/apps/tiamat/src/modules/auth/api-key.strategy.ts
- legacy/apps/tiamat/src/modules/api-keys/api-keys.module.ts
- libs/core/test/unit/config/fixtures/from-default.config.json
- legacy/apps/tiamat/src/modules/user-admin/dto/create-agent.dto.ts
- legacy/apps/tiamat/src/modules/auth/authentication.guard.ts
- libs/core/test/unit/config/fixtures/from-path.config.json
- legacy/apps/tiamat/src/modules/user-admin/user-admin.module.ts
- legacy/apps/tiamat/src/modules/user-admin/dto/update-agent.dto.ts
- legacy/libs/core/src/modules/api-keys/entities/api-key.entity.ts
- libs/core/src/modules/demo/demo.service.ts
- legacy/apps/tiamat/src/modules/organizations/organizations.controller.ts
- libs/core/src/modules/demo/demo-modules.spec.ts
- config.default.json
- legacy/libs/core/src/modules/organizations/entities/organization.entity.ts
- legacy/apps/tiamat/src/modules/auth/auth.controller.ts
- legacy/apps/tiamat/src/modules/organizations/organizations.service.ts
- legacy/apps/tiamat/src/modules/accounts/accounts.module.ts
- libs/core/src/modules/demo/demo.schema.ts
- legacy/apps/tiamat/src/modules/auth/supabase.strategy.ts
- legacy/libs/core/src/modules/members/entities/member.entity.ts
- legacy/apps/tiamat/src/modules/auth/auth.service.ts
- libs/core/test/unit/config/fixtures/invalid-schema-version.config.json
- legacy/apps/tiamat/src/modules/auth/nxt-supabase-user.ts
- legacy/libs/core/src/index.ts
- legacy/apps/tiamat/src/modules/auth/auth.module.ts
- legacy/libs/core/src/modules/accounts/entities/account.entity.ts
- legacy/libs/core/src/modules/api-keys/api-keys.service.ts
- legacy/libs/core/src/modules/supabase.module.ts
- apps/worker/tsconfig.app.json
- legacy/libs/core/src/modules/accounts/accounts.service.ts
- legacy/apps/tiamat/src/modules/user-admin/user-admin.service.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
GET /auth/mewith JWT andX-API-KEYauthentication plus Supabase-backed principal claims..env.example).