fix/batch-dedup

job.rpu is string|null in SFN input without a null guard

apps/platform/src/api/handlers/cfe-jobs.handler.ts

insertBatchSkippingActive returns Array<{publicId: string; rpu: string|null}>. In dispatch.map(({job, row}) => ({rpu: job.rpu, ...})), job.rpu is typed string|null. In practice safe (all insertRows supply rpu: string from BatchInsertRow), but the type system does not enforce this. If the SFN state machine requires rpu as a required string, passing null fails at runtime. Fix: rpu: job.rpu ?? row.rpu.

Intent records committed for TOCTOU-raced RPUs — idempotent tradeoff, undocumented

apps/platform/src/api/handlers/cfe-jobs.handler.ts

commitCfeJobIntentBatchShell runs before insertBatchSkippingActive. When a row loses the TOCTOU race, its site/SUC intent is already committed. Idempotent and desirable, but the code doesn't document that this is an accepted tradeoff (site/SUC creation is intentionally unconditional).

Partial index target and predicate exactly match the index definition

domains/utility/src/cfe-job/cfe-job.queries.ts

Index: UNIQUE ON (rpu, org_id, pipeline) WHERE status IN ('queued', 'running'). onConflictDoNothing target: [cfeJobs.rpu, cfeJobs.orgId, cfeJobs.pipeline] with where: inArray(cfeJobs.status, ['queued', 'running']). Exact match. Integration test confirms unrelated publicId collision still throws 23505.

RETURNING semantics correct — only inserted rows returned, pairing logic sound

domains/utility/src/cfe-job/cfe-job.queries.ts

PostgreSQL ON CONFLICT DO NOTHING with RETURNING only returns rows that were actually inserted. pairBatchInsertResults correctly exploits this: maps by publicId, reports every input row not in the returned set as skipped. Semantics validated by unit tests.

Analytics rpu_count counts DB-inserted jobs — not emitted on SFN failure (no overcounting)

apps/platform/src/api/handlers/cfe-jobs.handler.ts

trackServer fires after the SFN block, so the 503 early-return bypasses it. No overcounting. However SFN-error path leaves jobs orphaned in queued state with no SFN execution (pre-existing, unrelated to TOCTOU fix).

Pre-existing: single-job 503 still echoes raw SFN error to API client

apps/platform/src/api/handlers/cfe-jobs.handler.ts

The batch path correctly strips the SFN error message (this PR). The single-job path (createCfeJobHandler) still passes cause.message via runPostCreateChoreography → 'Dispatch failed: ${cause.message}'. AWS SDK error messages can contain ARNs/account IDs. Not introduced by this PR but highlighted by the contrast.

Service-role bypass correctly justified — org isolation via JWT membership

apps/platform/src/api/handlers/cfe-jobs.handler.ts

orgId/orgPublicId come exclusively from pickCurrentMembership(authReq.auth.memberships) — JWT-resolved, not user-input. The conflict index (rpu, orgId, pipeline) is org-scoped, so cross-org RPU collisions cannot trigger DO NOTHING for a different org's row. Three independent guards: JWT resolution, dedup query scoped to orgId, and index-key structure. The comment accurately describes the invariant.

pairBatchInsertResults does not leak cross-org data

apps/platform/src/api/utils/cfe-batch-dispatch.ts

RETURNING in Postgres only returns rows from the current INSERT statement — never pre-existing rows. The ghost-row defensive drop silently discards unexpected returns rather than surfacing them. No cross-org information is disclosed.

Handler calls queries directly — FCIS violation (pre-existing, extended)

apps/platform/src/api/handlers/cfe-jobs.handler.ts

The batch handler calls jobQueries.insertBatchSkippingActive(database, ...) directly, not through a shell. Canonical form: 'Handlers never call queries directly — always through shells.' The pre-existing pattern is extended by the new insert call. Consequence: no outbox event is written atomically with the batch insert — future per-job CDC events (e.g. utility.cfe_job.created) would be missed for batch-created jobs.

Query name insertBatchSkippingActive departs from canonical scheme

domains/utility/src/cfe-job/cfe-job.queries.ts

Canonical query naming: findBy{Field} / insert / updateWithVersion. insertBatchSkippingActive encodes behaviour in the name rather than the operation. A name like insertBatch with conflict semantics in JSDoc only would be more consistent with the existing convention.

Pure dispatch helper in api/utils instead of domain decisions layer

apps/platform/src/api/utils/cfe-batch-dispatch.ts

pairBatchInsertResults is a pure function containing domain-level semantics. By canonical form, pure business logic belongs in {entity}.decisions.ts inside domains/utility/src/cfe-job/. Its current location in apps/platform/src/api/utils/ ties domain logic to the app layer.

PR references in JSDoc comments should be removed

domains/utility/src/cfe-job/cfe-job.queries.ts

CLAUDE.md: 'Don't reference the current task, fix, or callers... since those belong in the PR description and rot as the codebase evolves.' insertBatchSkippingActive JSDoc references 'PR #288' and cfe-batch-dispatch.ts module JSDoc references 'see PR #288'. Both should be removed.

admin-required comment reads as a task-annotation, not a durable code comment

apps/platform/src/api/handlers/cfe-jobs.handler.ts

The incremental diff adds '// admin-required: bulk job insert sets orgId/orgPublicId explicitly / from the JWT-resolved membership...' The 'admin-required:' prefix appears to be a review-annotation tag. CLAUDE.md: comments should explain WHY (a hidden constraint, subtle invariant) not cross-reference a code path that may diverge. Pre-existing uses of admin-required throughout the file suggest an established team convention, but the comment could be trimmed.

No handler-level test: dispatch.length === 0 → SFN not called

apps/platform/src/api/handlers/cfe-jobs.handler.ts:825

The handler guards `if (dispatch.length > 0)` before SFN dispatch. No test asserts that a 100%-conflict batch (all rows skipped) returns 201 with created:0 and never calls StartExecution. A regression could silently fire SFN with an empty jobs array, triggering an infinite Map execution.

No handler-level test for TOCTOU mid-batch error accumulation

apps/platform/src/api/handlers/cfe-jobs.handler.ts:817

pairBatchInsertResults is well unit-tested in isolation, but no integration or handler-level test exercises the full handler path: records enter insertRows, DB returns a subset, skippedErrors is pushed into errors, and the response body's errors array contains JobAlreadyExists entries. A future refactor omitting errors.push(...skippedErrors) would go undetected.

No test for analytics rpu_count after partial-conflict batch

apps/platform/src/api/handlers/cfe-jobs.handler.ts:880

rpu_count: insertedJobs.length now correctly counts DB-inserted jobs. No test asserts that after a partial-conflict batch (e.g. 3 in, 1 skipped), rpu_count equals 2, not 3. This is the analytics surface of the fix.

No test for empty insertRows early-return in insertBatchSkippingActive

domains/utility/src/cfe-job/__tests__/cfe-job.integration.test.ts

The query short-circuits on `if (rows.length === 0) return []`. No integration test covers this path. Minor, but makes the contract explicit and prevents accidental guard removal.

Integration test cleanup loops DELETE per publicId instead of bulk delete

domains/utility/src/cfe-job/__tests__/cfe-job.integration.test.ts

afterAll issues one DELETE per publicId. A single DELETE WHERE public_id = ANY(batchPublicIds) would be one round-trip. Low consequence in test code.

Analytics event skipped when SFN fails — jobs inserted but rpu_count never tracked

apps/platform/src/api/handlers/cfe-jobs.handler.ts

The 503 early-return inside the SFN try/catch exits before trackServer. At that point insertedJobs.length > 0 (jobs are in the DB) but no analytics event fires. Pre-existing, but the restructuring makes the gap more prominent. Consider moving the analytics call to immediately after the insert, before the SFN block.

Bulk insert bypasses transaction boundary — no atomic outbox write

domains/utility/src/cfe-job/cfe-job.queries.ts

insertBatchSkippingActive is called on the raw database (service-role), not inside a transaction. The single-job path uses createJobShell with createRLSDb and a transaction boundary (fetch → decide → write + outbox event atomically). The batch path has no outbox event. If future work adds per-job events, they will not be written atomically with the insert.

pairBatchInsertResults dispatch order follows DB order, not input order — undocumented

apps/platform/src/api/utils/cfe-batch-dispatch.ts

The dispatch array is in Postgres-returned order (insertion order minus skipped rows), not in insertRows order. The SFN consumer does not care about order, but a future caller assuming positional correspondence would silently mismatch. A comment on the return type noting this would prevent the misread.

onConflictDoNothing `where` is predicate on existing row — easy to misread as filter on new rows

domains/utility/src/cfe-job/cfe-job.queries.ts

Drizzle's .onConflictDoNothing({ where }) maps to ON CONFLICT (...) WHERE ... DO NOTHING — the WHERE matches the *conflicting existing* row, not the incoming one. A reader who interprets `where: inArray(cfeJobs.status, ['queued', 'running'])` as filtering new rows would believe queued inserts are dropped. An inline comment would close the misread risk.

pairBatchInsertResults builds Map then separate Set — single-loop simplification available

apps/platform/src/api/utils/cfe-batch-dispatch.ts

The function iterates insertedJobs twice (Map constructor + Set for skippedErrors filter). Both can be collapsed: build the Map, and collect matched publicIds into a Set in the same loop. Not a performance issue at batch sizes seen, but reduces intermediate allocations and makes data-flow clearer.

BatchInsertRow.config typed as Record<string,unknown> — intentionally loose

apps/platform/src/api/utils/cfe-batch-dispatch.ts

Wide enough to accept CfeJobConfig without importing the type. Correct tradeoff for a pure utility that should not depend on handler-layer types. Do not tighten to CfeJobConfig.