RFC-0002: orchestrate legacy batch processing - #17
EstandarMustaq merged 2 commits into
Conversation
* feat: secure worker runtime with OIDC * RFC-0002: reject cross-tenant domain jobs (#15) * fix: reject cross-tenant domain jobs * RFC-0002: publish workbench public API contract (#16) * feat: publish versioned public api contract * RFC-0002: orchestrate legacy batch processing (#17) * feat: orchestrate legacy batch processing * docs: expand public workbench API contract (#18)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ab034447e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| @Get('metrics') | ||
| @Public() |
There was a problem hiding this comment.
Remove public access from protected metrics endpoint
In deployments where /api/metrics is reachable without a separate auth layer, this new @Public() decorator makes the route bypass both AccessTokenGuard and PermissionsGuard, because those guards return early for PUBLIC_ROUTE before checking the bearer token or observability.read. That leaves the permission annotation on the next line ineffective and exposes worker/queue/batch operational metrics to unauthenticated callers; drop @Public() unless metrics scraping is intentionally unauthenticated at this service boundary.
Useful? React with 👍 / 👎.
|
|
||
| private async enqueue(batchId: string, tenantId: string, type: 'LEGACY_EXPORT' | 'LEGACY_IMPORT'): Promise<void> { | ||
| await this.jobs.enqueue({ | ||
| job_id: `legacy-${batchId}`, queue: 'legacy', type, tenant_id: tenantId, |
There was a problem hiding this comment.
Ensure the legacy queue is actually consumed
When an installation already sets WORKBENCH_QUEUES/FWK_QUEUES to the old payments,platform value, this still writes batch work to the hard-coded legacy queue, but WorkerService only creates workers for config.queues and never consumes queues outside that list. In that context regulatory exports/imports return QUEUED and remain stuck indefinitely; validate/force-add legacy for these endpoints or enqueue to a configured queue.
Useful? React with 👍 / 👎.
| 'LEGACY_EXPORT', | ||
| 'LEGACY_IMPORT', |
There was a problem hiding this comment.
Keep internal legacy jobs out of the public job API
Adding these internal batch job types to the shared JOB_TYPES list also makes /api/jobs accept them, because JobsController.validateCreateJob authorizes any value in this list with only workbench.jobs.write. In tenants where a job writer knows a batch id, they can enqueue LEGACY_EXPORT/LEGACY_IMPORT on the legacy queue and drive compliance batch processing without the compliance.manage permission required by the new legacy endpoints; use a separate allowlist for public job submissions or keep these types internal.
Useful? React with 👍 / 👎.
Summary
LEGACY_EXPORTandLEGACY_IMPORTjobs on thelegacyqueuelegacy-connectorsruntime and Ledger Core sourceValidation
pnpm test:allpnpm buildpnpm openapi:checkpnpm guardian:checkgit diff --checkDepends on
mavulahq/legacy-connectorsbranchrfc-0002-executable-batch-runtime.