promote: dev history slice 1 of 4 - #29
Conversation
…y-completion Foundation and IAM completion
📝 WalkthroughWalkthroughThis PR adds Luna Max delivery orchestration, PKCE and CSRF domain utilities, IAM session and MFA persistence, tenant-aware HTTP security, audit and entitlement modules, expanded OpenAPI contracts, and comprehensive validation tests. ChangesPlatform foundation and delivery
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/api/src/bootstrap.ts (1)
27-35: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExtend
ApiApplicationOptionswith the audit and entitlement module options.
AppModule.registerat line 43 acceptsAppModuleOptions, which now includesAudModuleOptionsandBuaModuleOptions(seeservices/api/src/app.module.tslines 12-18).ApiApplicationOptionsdoes not extend those two interfaces. A caller ofcreateApiApplicationcannot passauditDatabaseorentitlementDatabase.AudModulethen selectsInMemoryAuditRepositoryAdapter(seeservices/api/src/features/aud/aud.module.tslines 33-37), so audit records are not durable in production composition.🐛 Proposed fix to expose the new module options
export interface ApiApplicationOptions extends IamModuleOptions, IaeModuleOptions, DsmModuleOptions, - DsoModuleOptions { + DsoModuleOptions, + AudModuleOptions, + BuaModuleOptions { readonly compatibilityPort?: ClientCompatibilityPort; readonly readinessPort?: ReadinessPort; readonly requestContext?: RequestContextOptions; }Add the imports:
+import type { AudModuleOptions } from './features/aud/aud.module.js'; +import type { BuaModuleOptions } from './features/bua/bua.module.js';🤖 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 `@services/api/src/bootstrap.ts` around lines 27 - 35, Extend ApiApplicationOptions with AudModuleOptions and BuaModuleOptions, adding the corresponding imports and preserving the existing module-option inheritance. This exposes auditDatabase and entitlementDatabase to createApiApplication and ensures AppModule.register receives the production persistence configuration.
🟠 Major comments (29)
services/api/src/features/bua/adapter/prisma-entitlement-repository.adapter.ts-165-178 (1)
165-178: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
databaseScopedropsprojectId, which makes a project-scoped row unreadable.
databaseScopepersists onlyscopeType,organizationId, andworkspaceId.scopeKeyat line 177 still handles aprojectscope, andentryCreateData(line 392) andreservationCreateData(line 409) both pass a caller-suppliedTenantScopeV1, which can be project-scoped.For a project-scoped entry the write succeeds and stores
scopeType: 'project'with no project identifier. Every later read callspersistedScope({ ...row, projectId: null }), which builds aprojectscope withoutprojectId.parseTenantScopeV1rejects it and the adapter throwsBUA_PERSISTED_SCOPE_INVALID.listUsageStatemaps all rows for the organization, so one such row makes the whole usage read fail permanently.Reject a project-scoped tenant scope at the write boundary, or persist
projectIdand restore it on read. ThescopeKeyproject branch anddatabaseScopemust agree.Run the following script to check whether project-scoped usage reaches this adapter:
#!/bin/bash # Description: Check the entitlement tenant scope surface and the Prisma schema columns. set -euo pipefail rg -n -C 4 'tenantScope' packages/domain/src/entitlements rg -n -C 6 "scopeType: 'project'|scopeType === 'project'" services/api/src/features/bua fd -e prisma . services/api/prisma --exec rg -n -C 25 'UsageLedgerEntry|UsageReservation|EntitlementSnapshot' {}🤖 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 `@services/api/src/features/bua/adapter/prisma-entitlement-repository.adapter.ts` around lines 165 - 178, Update databaseScope and the corresponding persisted-scope read path to preserve projectId for project-scoped TenantScopeV1 values, ensuring project scopes remain parseable and consistent with scopeKey. If the Prisma models cannot persist projectId, instead reject project-scoped scopes at the entryCreateData and reservationCreateData write boundaries.services/api/src/features/aud/adapter/prisma-audit-repository.adapter.ts-307-311 (1)
307-311: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftBound the audit list reads.
listEventsandlistSealsselect every row for the organization with no limit and no pagination.AuditControllerexposes both throughGET /v1/audit/eventsandGET /v1/audit/seals. The audit tables are append-only, so the result size grows for the lifetime of the tenant. A large organization will exhaust request memory and exceed request timeouts.listEventsalso hashes every returned event on each call.Add a bounded page size and a cursor to the port, the adapter, and the controller.
Also applies to: 338-342
🤖 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 `@services/api/src/features/aud/adapter/prisma-audit-repository.adapter.ts` around lines 307 - 311, Update the audit listing contract across the repository port, Prisma adapter methods listEvents and listSeals, and AuditController endpoints to accept a bounded page size and cursor, returning only that page plus continuation metadata as needed. Apply the limit and cursor to both database queries, preserving ascending sequence ordering, and update controller request parsing and response handling so callers can paginate instead of loading all tenant records.services/api/src/features/bua/adapter/prisma-entitlement-repository.adapter.ts-605-611 (1)
605-611: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe direct
persistUsageStatepath performs many writes without a transaction.
PrismaEntitlementRepositoryAdapter.persistUsageStatebuilds aPrismaEntitlementTransactionAdapterover the non-transactional client.persistUsageStatethen writes every ledger entry and every reservation in sequence. A failure in the middle leaves the entries committed and the reservations unwritten.Usage entries and reservation state must move together. Route this method through
this.client.$transaction, the same waywithTransactiondoes at line 581.🔒 Proposed fix
public persistUsageState(context: IamTenantContextV1, state: UsageLedgerStateV1): Promise<void> { - return new PrismaEntitlementTransactionAdapter(this.client).persistUsageState(context, state); + return this.client.$transaction((transaction) => + new PrismaEntitlementTransactionAdapter(transaction).persistUsageState(context, state), + ); }🤖 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 `@services/api/src/features/bua/adapter/prisma-entitlement-repository.adapter.ts` around lines 605 - 611, Update PrismaEntitlementRepositoryAdapter.persistUsageState to execute the operation inside this.client.$transaction, matching the transaction handling used by withTransaction. Construct PrismaEntitlementTransactionAdapter with the transaction client and invoke persistUsageState there so usage entries and reservations commit or roll back together.services/api/src/features/aud/adapter/prisma-audit-repository.adapter.ts-290-302 (1)
290-302: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winAvoid loading the full scope history on every append.
appendEventloads every audit event row for the scope, but uses only two facts: the presence of a matchingidempotencyKey, and the latest row. The audit table is append-only and grows without bound per tenant scope. Each write therefore costs O(n) rows transferred and materialized, and the cost grows for the lifetime of the tenant.Replace the full scan with two bounded queries: a keyed lookup on
(scopeKey, idempotencyKey), and a single-row lookup for the latest sequence. Both require extendingAuditEventDelegateV1withfindFirstandtake.⚡ Proposed change
- const siblings = await this.client.auditEventRecord.findMany({ - where: { scopeKey: scopeKey(event.tenantScope) }, - orderBy: { sequence: 'desc' }, - }); - const duplicate = siblings.find((row) => row.idempotencyKey === event.idempotencyKey); - if (duplicate !== undefined) throw new Error('AUD_IDEMPOTENCY_CONFLICT'); - const latest = siblings[0]; + const key = scopeKey(event.tenantScope); + const duplicate = await this.client.auditEventRecord.findFirst({ + where: { scopeKey: key, idempotencyKey: event.idempotencyKey }, + }); + if (duplicate !== null) throw new Error('AUD_IDEMPOTENCY_CONFLICT'); + const [latest] = await this.client.auditEventRecord.findMany({ + where: { scopeKey: key }, + orderBy: { sequence: 'desc' }, + take: 1, + });Extend the delegate interface accordingly:
interface AuditEventDelegateV1 { create(input: { readonly data: AuditEventCreateDataV1 }): Promise<AuditEventDatabaseRowV1>; findUnique(input: { readonly where: { readonly id: string }; }): Promise<AuditEventDatabaseRowV1 | null>; findFirst(input: { readonly where: Readonly<Record<string, unknown>>; }): Promise<AuditEventDatabaseRowV1 | null>; findMany(input: { readonly where: Readonly<Record<string, unknown>>; readonly orderBy: { readonly sequence: 'asc' | 'desc' }; readonly take?: number; }): Promise<readonly AuditEventDatabaseRowV1[]>; }🤖 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 `@services/api/src/features/aud/adapter/prisma-audit-repository.adapter.ts` around lines 290 - 302, Update appendEvent to replace the full-scope findMany scan with a bounded idempotency lookup using findFirst and a latest-event lookup using findMany with take: 1, preserving the existing AUD_IDEMPOTENCY_CONFLICT and AUD_SEQUENCE_CONFLICT checks. Extend AuditEventDelegateV1 with findFirst and an optional take property on findMany, and ensure the Prisma delegate implementation supports both inputs.services/api/src/features/aud/adapter/prisma-audit-repository.adapter.ts-312-317 (1)
312-317: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winChain verification groups events across different
scopeKeyvalues.
appendEventmaintains sequence andpreviousDigestcontinuity perscopeKey(line 291).listEventsselects every row for the organization, filters withvisible, and verifies the result as one chain.
visibleadmits both ancestor and descendant scopes. A tenant that has events at the organization scope and at a workspace scope therefore produces a list that interleaves two independent chains, ordered bysequence. ThepreviousDigestlinks do not match across that interleave, soverifyAuditChainV1rejects andlistEventsthrowsAUD_CHAIN_INVALIDfor a valid database.Group the events by
scopeKeyand verify each chain on its own.🐛 Proposed fix
const events = rows .filter((row) => visible(context.tenantScope, persistedScope(row))) .map(persistedEvent); - const verified = verifyAuditChainV1(events, this.digestPort); - if (!verified.accepted) throw new Error('AUD_CHAIN_INVALID'); + const chains = new Map<string, AuditEventV1[]>(); + for (const event of events) { + const key = scopeKey(event.tenantScope); + const chain = chains.get(key); + if (chain === undefined) chains.set(key, [event]); + else chain.push(event); + } + for (const chain of chains.values()) { + const verified = verifyAuditChainV1(chain, this.digestPort); + if (!verified.accepted) throw new Error('AUD_CHAIN_INVALID'); + } return events;🤖 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 `@services/api/src/features/aud/adapter/prisma-audit-repository.adapter.ts` around lines 312 - 317, Update listEvents around persistedEvent and verifyAuditChainV1 to group visible rows by scopeKey, then verify each scope-specific event chain independently using this.digestPort. Preserve returning the combined events and throw AUD_CHAIN_INVALID if any individual chain is rejected.services/api/src/platform/http/csrf-protection.ts-50-76 (1)
50-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winWiden the cookie-name validation to avoid false CSRF rejections.
parseCookiesmarks the whole cookie header asmalformedwhen any cookie name fails/^[A-Za-z0-9_]+$/u. Line 120 then rejects the whole request withCSRF_INVALIDwhenevermalformedis true, even though this check only runs when adatabreeze_*auth cookie is present.Real cookie names use characters outside
[A-Za-z0-9_](for example hyphens and dots) per the RFC 2616 token grammar that RFC 6265 references. If a browser sends any such cookie alongside the app's auth cookie, on this call the whole request is rejected, not just the offending cookie. This can break legitimate authenticated mutation requests whenever an unrelated cookie (analytics, another product on the same domain, etc.) uses a hyphenated or dotted name.Widen the accepted name character set to the standard cookie-name token grammar. Keep the CR/LF checks on the value, since those catch real injection attempts.
🍪 Proposed fix to widen the accepted cookie-name charset
- if (!/^[A-Za-z0-9_]+$/u.test(name) || value.includes('\r') || value.includes('\n')) { + if (!/^[!#$%&'*+\-.^_`|~A-Za-z0-9]+$/u.test(name) || value.includes('\r') || value.includes('\n')) { malformed = true; continue; }Also applies to: 120-122
🤖 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 `@services/api/src/platform/http/csrf-protection.ts` around lines 50 - 76, Update the cookie-name validation in parseCookies to accept the RFC token character set, including valid characters such as hyphens and dots, instead of restricting names to alphanumerics and underscores. Preserve the existing malformed handling and duplicate tracking, and keep the value CR/LF rejection unchanged.services/api/src/platform/http/request-context.ts-83-86 (1)
83-86: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winConfigure production CSRF origins.
services/api/src/main.tsstarts the API withoutrequestContextoptions. Production therefore uses the localhost defaults, and browser mutations from deployed frontend origins returnORIGIN_INVALID. Supplyoptions.requestContext.csrf.allowedOriginsfrom production configuration.🤖 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 `@services/api/src/platform/http/request-context.ts` around lines 83 - 86, Update the API startup configuration in main.ts to pass production-configured CSRF origins through requestContext.csrf.allowedOrigins, instead of allowing request-context defaults to select localhost origins. Reuse the existing production configuration source and preserve DEFAULT_CSRF_ALLOWED_ORIGINS_V1 only as the fallback when no production value is configured.services/api/openapi/v1.json-355-424 (1)
355-424: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
GET /v1/auth/medeclares no bearer security, and the contract test cannot detect it. Every other authenticated operation in the snapshot declares"security": [{ "bearer": [] }]./v1/auth/mereturns the authenticated session identity and resolves the caller throughSessionRequestTenantContextAdapter, which requires a bearer token. The contract test asserts path presence only, so the omission passes CI.
services/api/openapi/v1.json#L355-L424: add@ApiBearerAuth()toAuthenticationController.meand regenerate this snapshot so the operation declares"security": [{ "bearer": [] }].services/api/test/openapi.test.ts#L69-L77: add an assertion that every operation outside the public set (/health/*,/v1/system/*,/v1/auth/sign-in,/v1/auth/refresh) declares a bearer security requirement.🤖 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 `@services/api/openapi/v1.json` around lines 355 - 424, Add `@ApiBearerAuth`() to AuthenticationController.me and regenerate services/api/openapi/v1.json so GET /v1/auth/me declares security [{ "bearer": [] }]. In services/api/test/openapi.test.ts, extend the contract test to require bearer security for every operation except /health/*, /v1/system/*, /v1/auth/sign-in, and /v1/auth/refresh.services/api/openapi/v1.json-3935-3949 (1)
3935-3949: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove
writeOnlyfromrefreshTokeninSessionRefreshResponseDto.
SessionRefreshResponseDtois used only as the200response ofPOST /v1/auth/refresh(line 550). In OpenAPI,writeOnlydeclares that a property appears in requests and must not appear in responses. The current schema therefore documents a property that can never be returned.Choose one behavior and align the DTO:
- If the rotated refresh token is returned in the body, remove the
writeOnlyflag.- If the rotated refresh token is returned only through the
Set-Cookieheader, remove the property from the response DTO.The refresh-token rotation flow is in
services/api/src/features/iam/api/session-refresh-response.dto.tsandservices/api/src/features/iam/api/session-cookies.ts.🤖 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 `@services/api/openapi/v1.json` around lines 3935 - 3949, Align SessionRefreshResponseDto with the refresh-token delivery behavior by inspecting the rotation flow in the DTO and session-cookies implementation. If the rotated token is returned in the response body, remove writeOnly from refreshToken; if it is delivered only via Set-Cookie, remove refreshToken from the response DTO and its schema, keeping the OpenAPI response consistent.services/api/src/app.module.ts-23-32 (1)
23-32: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDerive the tenant context also when only
sessionDatabaseis supplied.
AppModule.registerinspects onlyoptions.sessions.IamModule.registeralso builds a session adapter fromoptions.sessionDatabase(seeservices/api/src/features/iam/iam.module.tslines 118-122). If production composition passessessionDatabaseinstead of a pre-builtsessionsport,requestTenantContextstaysundefined. Every feature module then receivesUnavailableRequestTenantContextAdapterand all tenant-scoped routes fail closed.Mirror the IamModule fallback so both composition paths produce the same adapter.
🐛 Proposed fix to cover the database-backed composition path
- const sessions = options.sessions; + const sessions = + options.sessions ?? + (options.sessionDatabase === undefined + ? undefined + : new PrismaSessionLifecycleAdapter(options.sessionDatabase)); const requestTenantContext = options.requestTenantContext ?? (typeof sessions?.findPrincipalByAccessToken === 'function' ? new SessionRequestTenantContextAdapter({ findPrincipalByAccessToken: sessions.findPrincipalByAccessToken.bind(sessions), }) : undefined); const composedOptions = - requestTenantContext === undefined ? options : { ...options, requestTenantContext }; + requestTenantContext === undefined + ? options + : { ...options, requestTenantContext, sessions };Add the import:
+import { PrismaSessionLifecycleAdapter } from './features/iam/adapter/prisma-session-lifecycle.adapter.js';Passing the resolved
sessionsintocomposedOptionsalso preventsIamModulefrom constructing a second adapter over the same client.🤖 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 `@services/api/src/app.module.ts` around lines 23 - 32, Update AppModule.register to derive the sessions port from options.sessionDatabase when options.sessions is absent, using the same adapter/fallback construction as IamModule.register. Use the resolved sessions when building composedOptions so downstream modules, including IamModule, reuse the same client and requestTenantContext adapter instead of constructing another one.services/api/openapi/v1.json-4016-4020 (1)
4016-4020: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire factor proof before activating a factor.
VerifyMfaFactorDtocontains onlyat. The controller passes onlyinput.at, andMfaService.verifyFactoractivates the pending factor without validating a TOTP code or WebAuthn assertion. Add method-specific proof validation before saving the transition.🤖 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 `@services/api/openapi/v1.json` around lines 4016 - 4020, Update VerifyMfaFactorDto and the factor-verification flow around the controller and MfaService.verifyFactor to require method-specific proof in addition to at: validate a TOTP code for TOTP factors or a WebAuthn assertion for WebAuthn factors before activating or saving the pending factor transition, and reject missing or invalid proof.services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts-137-144 (1)
137-144: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winPush the tenant filter into the query instead of loading every membership row.
listMembershipscallsfindMany({ where: {} }). This loads all membership rows for all tenants, then discards most of them in memory. The row count grows with total platform usage, not with the caller's tenant. Every audit, entitlement, and IAM read that reaches this path pays the full table scan.
context.tenantScope.organizationIdis always available. Filter on it in the database, then keepvisibleInScopefor the workspace and project refinement.⚡ Proposed fix to narrow the query
public async listMemberships( context: IamTenantContextV1, ): Promise<readonly IamMembershipRecordV1[]> { - const rows = await this.client.membershipIdentity.findMany({ where: {} }); + const rows = await this.client.membershipIdentity.findMany({ + where: { organizationId: context.tenantScope.organizationId }, + }); return rows .map(membershipFromRow) .filter((membership) => visibleInScope(context.tenantScope, membership.scope)); }Apply the same narrowing to
findMembershipat Line 126:const rows = await this.client.membershipIdentity.findMany({ where: { principalId, organizationId: context.tenantScope.organizationId }, });🤖 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 `@services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts` around lines 137 - 144, Update listMemberships and findMembership in the Prisma IAM repository to include context.tenantScope.organizationId in the membershipIdentity.findMany where clause, restricting rows at the database level. Preserve visibleInScope filtering in listMemberships for workspace and project refinement, and retain the existing principalId condition in findMembership.services/api/test/http-contract.test.ts-569-589 (1)
569-589: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftThe entitlement endpoints break the problem-details convention used elsewhere in this contract.
Both cases assert HTTP 200 with an
accepted: falseenvelope. A missing snapshot returns 200. A malformed identifier returns 200. The same test file asserts RFC 9457 problem responses with accurate status codes everywhere else: 403 for CSRF at Line 201, 401 for an invalid session at Line 438, 400 for a rejected MFA request at Line 647.The behavior originates in
services/api/src/features/bua/api/entitlement.controller.ts(Lines 30-47), which returns the envelope instead of raising a problem. Two consequences follow. A client cannot rely on the status code to detect failure and must branch on the body shape for these two routes only. Caches and proxies treat the 200 "not found" as a successful response.A malformed identifier fits 400. A missing snapshot fits 404.
🤖 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 `@services/api/test/http-contract.test.ts` around lines 569 - 589, Update the entitlement snapshot GET handler in entitlement.controller.ts to use RFC 9457 problem responses instead of returning accepted:false envelopes: raise a 400 problem for malformed identifiers and a 404 problem when the snapshot is missing, while preserving successful responses for valid snapshots. Update the corresponding assertions in the entitlement contract test to verify the status codes and problem-details response shape.services/api/test/platform/http/session-tenant-context.test.ts-69-80 (1)
69-80: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThis test passes for the wrong reason and asserts the wrong code.
'Bearer token'carries a 5-character token. The regex at Line 65 ofservices/api/src/platform/http/session-tenant-context.adapter.tsrequires 20 to 4096 characters.bearerTokenreturnsundefined, and the adapter throwsAUTHENTICATION_FAILEDat Line 77 beforefindPrincipalByAccessTokenruns.Two consequences follow. The
securityEpoch: 0stub is never evaluated, so the "unsafe principal state" behavior is untested. Therequest.idfallback for the idempotency key is never reached, so the "uses the request id for read-only calls" behavior in the test name is also untested.The expected code is wrong for the intended path. A valid token with
securityEpoch: 0reachescreateIamTenantContextV1, which rejects it withINVALID_EPOCH, so the adapter throwsCONTEXT_INVALID.💚 Proposed fix
void test('uses the request id for read-only calls and rejects unsafe principal state', async () => { - const adapter = new SessionRequestTenantContextAdapter({ - findPrincipalByAccessToken: () => Promise.resolve({ ...principal, securityEpoch: 0 }), - }); - await assert.rejects( - adapter.resolve({ id: 'request-read-001', headers: { authorization: 'Bearer token' } }), - (error: unknown) => { - assert.equal((error as { code?: unknown }).code, 'AUTHENTICATION_FAILED'); - return true; - }, - ); + const unsafe = new SessionRequestTenantContextAdapter({ + findPrincipalByAccessToken: () => Promise.resolve({ ...principal, securityEpoch: 0 }), + }); + await assert.rejects( + unsafe.resolve({ + id: 'request-read-001', + headers: { authorization: 'Bearer valid-looking-access-token-1' }, + }), + (error: unknown) => { + assert.equal((error as { code?: unknown }).code, 'CONTEXT_INVALID'); + return true; + }, + ); + + const healthy = new SessionRequestTenantContextAdapter({ + findPrincipalByAccessToken: () => Promise.resolve(principal), + }); + const context = await healthy.resolve({ + id: 'request-read-001', + headers: { authorization: 'Bearer valid-looking-access-token-1' }, + }); + assert.equal(context.idempotencyKey, 'request-read-001'); });🤖 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 `@services/api/test/platform/http/session-tenant-context.test.ts` around lines 69 - 80, Update the test around SessionRequestTenantContextAdapter.resolve to use an authorization token meeting the adapter’s minimum length, allowing findPrincipalByAccessToken to execute with securityEpoch: 0 and the request.id fallback to be exercised. Assert the resulting error code is CONTEXT_INVALID, reflecting createIamTenantContextV1’s rejection of the unsafe principal state.services/api/src/platform/http/session-tenant-context.adapter.ts-79-86 (1)
79-86: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe catch reports infrastructure failures as authentication failures.
findPrincipalByAccessTokenreaches the session store. The catch at Lines 82-84 converts every thrown error intoAUTHENTICATION_FAILED. A database outage, a connection-pool exhaustion, and a rejected token all produce the same 401.Two consequences follow. Clients treat a transient outage as a credential problem and re-authenticate, which cannot succeed. Operators lose the signal, because the original error is discarded without being logged or rethrown.
Map a lookup failure to a distinct problem code, and keep
AUTHENTICATION_FAILEDfor theundefinedprincipal at Line 85. Preserve the original error as thecauseso it reaches the logs.🔧 Proposed fix
-export type RequestTenantContextProblemCodeV1 = 'AUTHENTICATION_FAILED' | 'CONTEXT_INVALID'; +export type RequestTenantContextProblemCodeV1 = + | 'AUTHENTICATION_FAILED' + | 'CONTEXT_INVALID' + | 'SESSION_LOOKUP_UNAVAILABLE'; export class RequestTenantContextProblemError extends Error { - constructor(readonly code: RequestTenantContextProblemCodeV1) { - super(code); + constructor( + readonly code: RequestTenantContextProblemCodeV1, + options?: { readonly cause?: unknown }, + ) { + super(code, options); this.name = 'RequestTenantContextProblemError'; } }let principal: AuthenticatedPrincipalV1 | undefined; try { principal = await this.sessions.findPrincipalByAccessToken(token); - } catch { - throw new RequestTenantContextProblemError('AUTHENTICATION_FAILED'); + } catch (error) { + throw new RequestTenantContextProblemError('SESSION_LOOKUP_UNAVAILABLE', { cause: error }); }Map the new code to a 503 in the problem-details filter.
🤖 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 `@services/api/src/platform/http/session-tenant-context.adapter.ts` around lines 79 - 86, Update the error handling around SessionTenantContextAdapter’s findPrincipalByAccessToken call to throw a distinct lookup/infrastructure problem code while preserving the caught error as its cause. Keep AUTHENTICATION_FAILED exclusively for an undefined principal, and add the new problem code to the problem-details filter with a 503 response mapping.services/api/src/platform/http/session-tenant-context.adapter.ts-55-60 (1)
55-60: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFabricating an idempotency key defeats idempotency for mutations.
When no
idempotency-keyheader is present andrequest.idis absent,idempotencyKeyreturnsrandomUUID(). Each retry of the same mutation then carries a different key. A downstream deduplication check keyed on this value never matches, so a retried write executes twice.
RequestLikeV1declaresmethodat Line 21, and no code reads it. The test at Line 69 ofservices/api/test/platform/http/session-tenant-context.test.tsis named "uses the request id for read-only calls", which suggests that method-aware handling was intended and not completed.For unsafe methods, require a caller-supplied key and reject the request when it is missing. Keep the generated fallback for safe methods, where the key is only correlation metadata.
🔧 Proposed fix
+const SAFE_METHODS_V1 = new Set(['GET', 'HEAD', 'OPTIONS']); + +function isSafeMethod(request: RequestLikeV1): boolean { + return typeof request.method === 'string' && SAFE_METHODS_V1.has(request.method.toUpperCase()); +} + function idempotencyKey(request: RequestLikeV1): string { const header = oneHeader(request, 'idempotency-key'); if (header !== undefined) return header; if (typeof request.id === 'string' && request.id.length > 0) return request.id; + if (!isSafeMethod(request)) throw new RequestTenantContextProblemError('CONTEXT_INVALID'); return randomUUID(); }A dedicated
IDEMPOTENCY_KEY_REQUIREDproblem code would describe the failure better thanCONTEXT_INVALID.🤖 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 `@services/api/src/platform/http/session-tenant-context.adapter.ts` around lines 55 - 60, Update idempotencyKey to branch on RequestLikeV1.method: preserve the header and non-empty request.id behavior, retain randomUUID() only for safe methods, and reject unsafe-method requests when neither caller-supplied value exists. Use the existing request-validation mechanism, preferably introducing the proposed IDEMPOTENCY_KEY_REQUIRED problem code instead of CONTEXT_INVALID.services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts-122-144 (1)
122-144: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winA single unparseable row makes both read paths throw for unrelated callers.
findMembershipandlistMembershipsmap every fetched row throughmembershipFromRow, which throwsIAM_PERSISTED_MEMBERSHIP_INVALIDwhen validation fails. One corrupt, legacy, or partially migrated row therefore fails the whole read for callers who have no interest in that row.The credential lookup adapter takes the opposite approach. In
services/api/src/features/iam/adapter/prisma-credential-lookup.adapter.ts(Lines 84-99),activeMembershipreturnsundefinedfor rows it cannot interpret. Align the read paths with that behavior and keep the throw insaveMembership, where a corrupt existing row must block the write.🛡️ Proposed fix to skip unusable rows on reads
+function membershipFromRowOrSkip( + row: IamMembershipDatabaseRowV1, +): IamMembershipRecordV1 | undefined { + const scope = scopeFromRow(row); + const validated = validateMembershipV1({ + id: row.id, + principalType: row.principalType, + principalId: row.principalId, + scope, + roleId: row.roleId, + status: row.status, + ...(row.startsAt ? { startsAt: timestamp(row.startsAt) } : {}), + ...(row.expiresAt ? { expiresAt: timestamp(row.expiresAt) } : {}), + revision: row.revision, + }); + return validated.accepted ? validated.value : undefined; +}Then use it in the read paths:
return rows - .map(membershipFromRow) + .flatMap((row) => membershipFromRowOrSkip(row) ?? []) .filter((membership) => visibleInScope(context.tenantScope, membership.scope));🤖 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 `@services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts` around lines 122 - 144, Update findMembership and listMemberships to skip rows that membershipFromRow cannot parse, matching the credential lookup adapter’s activeMembership behavior. Preserve filtering for principal, ACTIVE status, and visible scope on successfully parsed rows, while leaving saveMembership’s validation failure behavior unchanged.services/api/src/features/iam/application/tenant-context.ts-14-14 (1)
14-14: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftEnforce
mfaRequiredbefore protected operations.
SessionRequestTenantContextAdapteronly propagatesprincipal.mfaRequired.MfaService.requireStepUphas no production call sites. Protected controllers pass the context to repositories and services without checking the flag. Add a centralized guard or interceptor that blocks sensitive operations until step-up succeeds.🤖 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 `@services/api/src/features/iam/application/tenant-context.ts` at line 14, Update SessionRequestTenantContextAdapter and the protected-operation request path to enforce tenantContext.mfaRequired before sensitive controllers, repositories, or services execute. Add a centralized guard or interceptor that blocks requests requiring MFA until MfaService.requireStepUp succeeds, while allowing operations when the flag is absent or false.services/api/test/http-contract.test.ts-459-495 (1)
459-495: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAuthorize sign-out against the caller's session.
signOutdirectly revokes the body-suppliedsessionIdwithout authentication or ownership checks. Require the caller to own that session, and test rejection of another user'ssessionId.🤖 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 `@services/api/test/http-contract.test.ts` around lines 459 - 495, Update the sign-out flow exercised by the web and native requests to authenticate the caller and verify ownership of the supplied sessionId before revoking it. Preserve successful revocation for the caller’s own sessions, and extend the sign-out contract test to submit another user’s sessionId and assert the request is rejected without revoking that session.services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts-32-51 (1)
32-51: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse a transaction-specific client type.
PrismaClientis not assignable toIamDatabaseClientV1because Prisma’sTransactionClientdoes not expose$transaction. Define a separate transaction client interface and use it for the callback parameter. ThemembershipIdentitydelegate name matches theMembershipIdentitymodel.🤖 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 `@services/api/src/features/iam/adapter/prisma-iam-repository.adapter.ts` around lines 32 - 51, Define a separate transaction-specific interface for the client passed to `$transaction`, containing only the `membershipIdentity` delegate and no `$transaction` method. Update the `$transaction` callback parameter in `IamDatabaseClientV1` to use that interface while retaining `IamDatabaseClientV1` as the root client type and preserving the existing delegate name.services/api/src/features/iam/adapter/prisma-identity-bootstrap-repository.adapter.ts-190-215 (1)
190-215: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftSelect the bootstrap records with stable discriminators instead of first match.
findByUserIdresolves three records by taking the first candidate that matches a loose predicate:
- Line 190: the first
ownerorganization-scope membership. A user can own more than one organization. The selected membership is then not necessarily the personal organization.- Line 209: the first workspace whose name equals the literal
'Personal workspace'. If the workspace is renamed, the method throwsIAM_PERSISTED_WORKSPACE_INVALIDinstead of returning the bootstrap.- Line 214: the first project with
kind === 'INTERNAL'.None of the
findManycalls setsorderBy, so PostgreSQL does not guarantee a stable order for the remaining candidates.Filter the membership by the personal organization (
organization.personal === true) and select the workspace and project by a persisted marker or a deterministic order rather than by display name.🤖 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 `@services/api/src/features/iam/adapter/prisma-identity-bootstrap-repository.adapter.ts` around lines 190 - 215, Update findByUserId to resolve bootstrap records using stable discriminators: identify the owner membership whose organization is personal rather than selecting the first owner membership, and select the personal workspace via its persisted marker or deterministic ordering instead of the literal name. Replace the project’s first INTERNAL match with a persisted marker or deterministic ordering as well, preserving the existing invalid-record errors when no valid bootstrap record exists.services/api/src/features/iam/adapter/prisma-mfa-repository.adapter.ts-230-248 (1)
230-248: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftEnforce the revision check in the
UPDATEpredicate.
saveStatereads the current state at Line 230, validates revisions in memory at Line 231, then updates each row byidalone. The read and the write are separated, so the check does not hold under concurrency. On PostgreSQL, Prisma runs$transactionat the database default isolation level, which isREAD COMMITTED. Two concurrentsaveStatecalls for the same user can both read revision1, both passimmutableState, and both write revision2. One update is then lost, and a revoked factor can be resurrected as active.Include the expected revision in the
whereclause and verify that exactly one row changed.MfaFactorDelegateV1.updateandMfaRecoveryCodeDelegateV1.updatecurrently accept only{ id }, so the delegate types need a matching change (or useupdateMany).Line 195 relaxes the check further: it accepts
factor.revision === prior.revisionwhile other fields change, so a status change can be persisted without a revision bump. Require a strict increment for every modified record.🤖 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 `@services/api/src/features/iam/adapter/prisma-mfa-repository.adapter.ts` around lines 230 - 248, Update saveState and the MfaFactorDelegateV1/MfaRecoveryCodeDelegateV1 update path to enforce optimistic concurrency: require every modified factor’s revision to equal prior.revision + 1, include prior.revision in the database update predicate, and verify exactly one row was updated, raising IAM_MFA_REVISION_CONFLICT otherwise. Preserve immutableState validation while preventing concurrent writes from silently overwriting each other.services/api/src/features/iam/adapter/prisma-identity-bootstrap-repository.adapter.ts-300-318 (1)
300-318: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRun
saveinside a transaction.
PrismaIdentityBootstrapRepositoryAdapter.savecalls the transaction adapter withthis.client, which is the base client.PrismaIdentityBootstrapTransactionAdapter.savethen performs four separate writes (Lines 280-283). If one write fails, the earlier writes stay committed. The user is left with a partial personal-organization graph: for example an organization and workspace without a project or an owner membership. A later retry then hitssaveImmutableon the already-created rows.Wrap the write path in
$transaction, asPrismaMfaRepositoryAdapter.saveStatedoes inservices/api/src/features/iam/adapter/prisma-mfa-repository.adapter.ts(Lines 275-279).🐛 Proposed fix to make bootstrap persistence atomic
public save(bootstrap: PersonalOrganizationBootstrapV1) { - return new PrismaIdentityBootstrapTransactionAdapter(this.client).save(bootstrap); + return this.client.$transaction((transaction) => + new PrismaIdentityBootstrapTransactionAdapter(transaction).save(bootstrap), + ); }🤖 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 `@services/api/src/features/iam/adapter/prisma-identity-bootstrap-repository.adapter.ts` around lines 300 - 318, Update PrismaIdentityBootstrapRepositoryAdapter.save to execute the PrismaIdentityBootstrapTransactionAdapter.save operation inside this.client.$transaction, passing the transaction-scoped client to the adapter so all four writes commit or roll back atomically. Keep findByUserId and withTransaction unchanged.services/api/src/features/iam/adapter/prisma-credential-lookup.adapter.ts-150-162 (1)
150-162: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the fallback workspace selection deterministic.
For an
ORGANIZATION-scope membership, the adapter picks the first row thatworkspaceIdentity.findManyreturns. The query has noorderBy, so PostgreSQL does not guarantee a stable row order. If the organization has more than one active workspace, the resolvedworkspaceIdcan change between sign-ins for the same user. The principal's tenant scope then becomes unstable.Add an explicit
orderBy(for example the workspace creation timestamp) so the canonical workspace resolves the same way on every lookup. The membership query on Line 126 already appliesorderBy: { createdAt: 'asc' }, so apply the same rule here.Note that
WorkspaceLookupDelegateV1.findMany(Lines 52-56) does not acceptorderBytoday, so the delegate type also needs the field.🐛 Proposed fix for deterministic workspace selection
interface WorkspaceLookupDelegateV1 extends UniqueDelegateV1<WorkspaceIdentityDatabaseRowV1> { readonly findMany?: (input: { readonly where: Readonly<Record<string, unknown>>; + readonly orderBy?: Readonly<Record<string, 'asc' | 'desc'>>; }) => Promise<readonly WorkspaceIdentityDatabaseRowV1[]>; }const workspaces = await this.client.workspaceIdentity.findMany({ where: { organizationId: selected.organizationId, status: 'ACTIVE' }, + orderBy: { createdAt: 'asc' }, });🤖 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 `@services/api/src/features/iam/adapter/prisma-credential-lookup.adapter.ts` around lines 150 - 162, Make the fallback workspace lookup deterministic by adding ascending createdAt ordering to the workspaceIdentity.findMany call in the ORGANIZATION-scope resolution path, matching the existing membership query. Extend the WorkspaceLookupDelegateV1.findMany type to accept the orderBy field and ensure the adapter forwards it, preserving selection of the first active workspace after ordering.services/api/src/features/iam/adapter/prisma-session-lifecycle.adapter.ts-325-336 (1)
325-336: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not fall back to the presented token as the active token.
Line 332 uses
activeToken?.id ?? token.id. If the family has noACTIVErefresh token, the adapter treats the presented token as the active one, androtateRefreshFamilyV1cannot reportREUSE_DETECTED. The rotation then depends only onfamilyStatusandtokenExpiresAtfor safety. That is a fail-open default in the reuse-detection path.Line 325 also reads
active[0]from afindManywithoutorderBy. If more than one row isACTIVEfor the family, the selected row is not deterministic.Treat an empty active set as reuse and fail closed.
🔒️ Proposed fail-closed handling
const activeToken = active[0] ? tokenFromRow(active[0]) : undefined; + if (!activeToken) return { accepted: false, code: 'REUSE_DETECTED' }; const rotated = rotateRefreshFamilyV1({ now: now.toISOString(), presentedTokenId: token.id, - activeTokenId: activeToken?.id ?? token.id, + activeTokenId: activeToken.id,🤖 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 `@services/api/src/features/iam/adapter/prisma-session-lifecycle.adapter.ts` around lines 325 - 336, Update the active-token selection in the refresh-family rotation flow around rotateRefreshFamilyV1: make the ACTIVE query deterministic with an explicit orderBy, and do not substitute token.id when no ACTIVE row exists. Represent the missing active token so rotateRefreshFamilyV1 can classify the request as reuse (REUSE_DETECTED) and fail closed, while preserving the existing family-status and expiry inputs.services/api/src/features/iam/adapter/prisma-session-lifecycle.adapter.ts-320-324 (1)
320-324: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
refreshdoes not enforce the inactivity window.
refreshloads the session and then rotates the token family. It checks the refresh-token expiry and the session status, but it never comparessession.inactivityExpiresAtwithnow. Line 379 then extendsinactivityExpiresAtunconditionally. An idle session can therefore be refreshed after the inactivity window has passed, and the window restarts. The inactivity control never terminates a session while the refresh token remains valid, which leaves the absolute window as the only bound.
findPrincipalenforces the same field at Line 474, so the two paths disagree about session validity.Reject the refresh and mark the session
EXPIREDwhennowis at or aftersession.inactivityExpiresAt.🔒️ Proposed inactivity check
const session = sessionFromRow(sessionRow); + if (now.getTime() >= Date.parse(session.inactivityExpiresAt)) { + await transaction.refreshTokenRecord.updateMany({ + where: { familyId: token.familyId, status: 'ACTIVE' }, + data: { status: 'EXPIRED' }, + }); + await transaction.sessionRecord.update({ + where: { id: token.sessionId }, + data: { status: 'EXPIRED' }, + }); + return { accepted: false, code: 'EXPIRED' }; + }🤖 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 `@services/api/src/features/iam/adapter/prisma-session-lifecycle.adapter.ts` around lines 320 - 324, Update the refresh flow after sessionFromRow in the refresh method to reject sessions when now is at or after session.inactivityExpiresAt, marking the session status as EXPIRED before returning INVALID_REFRESH_TOKEN. Keep valid sessions on the existing token-rotation path, while preserving the corresponding behavior enforced by findPrincipal.services/api/src/features/iam/api/mfa.controller.ts-25-34 (1)
25-34: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse server time for MFA timestamps
The domain functions only validate timestamp syntax. They do not compare timestamps with trusted server state. Client input can therefore forge
enrolledAt,verifiedAt,revokedAt, andusedAt. Recovery-code single-use enforcement uses status, not timestamps. Remove these writable timestamp fields and derive them from an injected server clock.🤖 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 `@services/api/src/features/iam/api/mfa.controller.ts` around lines 25 - 34, Update the MFA enrollment flow centered on enroll and the underlying MFA domain APIs to stop accepting client-provided enrolledAt, verifiedAt, revokedAt, and usedAt values; derive each timestamp from the injected server clock at the corresponding state transition, while preserving recovery-code single-use enforcement through status.services/api/src/features/iam/api/session-refresh-response.dto.ts-15-20 (1)
15-20: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove
writeOnly: true; this DTO is a response withrefreshTokenactually populated.
writeOnly: truetells OpenAPI consumers that a field is accepted in requests but never returned in responses. This is a response DTO, andauthentication.controller.ts'srefreshhandler explicitly returnsrefreshTokenin the body for non-web clients. The siblingAuthSessionDto.refreshTokenproperty (auth-session.dto.ts) has the same shape withoutwriteOnly, which confirms the inconsistency here. Generated API clients that honorwriteOnlymay drop this field and break token persistence for non-web clients.🛠️ Proposed fix
- `@ApiProperty`({ minLength: 1, maxLength: 4096, required: false, writeOnly: true }) + `@ApiProperty`({ minLength: 1, maxLength: 4096, required: false }) `@IsOptional`() `@IsString`() `@MinLength`(1) `@MaxLength`(4096) refreshToken?: string;#!/bin/bash rg -n "writeOnly" services/api/openapi/v1.json -C3🤖 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 `@services/api/src/features/iam/api/session-refresh-response.dto.ts` around lines 15 - 20, Remove the writeOnly: true option from the ApiProperty decorator on the refreshToken property in the session refresh response DTO. Preserve the existing validation constraints and optionality so OpenAPI clients recognize refreshToken as a returned response field.docs/plans/004-luna-max-execution-plan.md-65-82 (1)
65-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCommit-budget minimums disagree between the plan table and the JSON ledger for every batch.
docs/plans/004-luna-max-execution-plan.md's "Commit budget" column documents batch-specific minimums (for example,50for B01,65for B12,35for B13), whiledocs/plans/execution-orchestration.jsonsetscommitBudget.minimum: 30uniformly for all 15 batches.targetandmaximummatch exactly between the two files in every batch; onlyminimumdiverges. Neither the checker (check-execution-orchestration.mjs, which only assertsminimum >= 30) nor the tests catch this drift.
docs/plans/004-luna-max-execution-plan.md#L65-L82: Update the "Commit budget" column to state30as the minimum for every batch to match the enforced ledger, or state explicitly that these minimums are non-binding planning guidance distinct from the enforced floor.docs/plans/execution-orchestration.json#L143-L170: Set each batch'scommitBudget.minimumto the value documented in the 004 plan's table (repeat for every batch through line 325), so the machine-enforced floor matches the documented per-batch policy.🤖 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/004-luna-max-execution-plan.md` around lines 65 - 82, Align the commit-budget minimum policy across both sources: in docs/plans/004-luna-max-execution-plan.md lines 65-82, retain the documented per-batch minimums and explicitly identify them as the enforced policy, then update every batch’s commitBudget.minimum in docs/plans/execution-orchestration.json lines 143-170 and through line 325 to match those values; update checker/tests if needed to validate the alignment.
|
CodeRabbit review disposition: this PR received exactly one automatic full review, and no rerun was requested. All four inline findings were reproduced, accepted, fixed on dev, and merged through PR #30; the fixing commits are linked in the inline replies. The broader review also produced two rejected suggestions: the alleged multi-scope audit-chain break is not reproducible because verification groups entries by scope, and a proposed central MFA-required guard would lock out every enrolled user before step-up proof is established. Full accepted/rejected rationale and verification evidence: https://github.com/DatabreezeService/databreeze-platform/blob/dev/docs/operations/coderabbit-pr-29-disposition.md. This is an ordered historical promotion slice only; main is not releaseable until all bounded promotion slices have landed and the coordinated release gate passes. |
1 participant
Promotion slice 1 of 4
This supersedes oversized historical promotion PR #25 and advances
mainto the firstdevfirst-parent boundary.3ed3d77d..86f25c85The four promotion slices contain 33, 75, 36, and 37 commits. Each keeps original atomic commits and merge history intact while remaining within the 30-50 normal target or 79-commit exceptional ceiling.
Review policy
mainSummary by CodeRabbit