fix: consolidate reliability and safety fixes - #199
Open
flemzord wants to merge 23 commits into
Open
Conversation
The name filter used Where("Name ILIKE '%?%';", name): bun substitutes
the ? placeholder even inside the string literal, producing invalid SQL
(the value is quoted inside the literal and the stray ; lands mid-WHERE).
Any GET /triggers?name=... request therefore returned a 500.
Use a proper parameterized predicate with the wildcards on the argument,
and add an integration test covering substring/case-insensitive matching.
The link() expression function performed an HTTP GET using the
fx-provided *http.Client, which in production is the OAuth2
client-credentials client carrying the stack bearer token (broad
ledger/wallets/payments scopes). Because link() targets a URI taken
from a user-controlled trigger expression (reachable via
POST /v2/triggers/{id}/test, which also returns the response body),
an authenticated caller could point it at an arbitrary host and
exfiltrate the stack token, or reach internal-only services (SSRF).
Restrict link() to an allow-listed host (the configured stack URL),
reject non-http(s) schemes, and close the response body. With no
allow-listed host configured, link() network calls are denied.
The allowlist is threaded through triggers.NewModule(stack, stackURL,
taskQueue).
…andling
Initiate starts the real Run workflow as a detached child with id
"<instanceID>-main" (ParentClosePolicy ABANDON) and returns as soon as
that child has started. The Initiate workflow (id == instanceID) is thus
already completed by the time the API calls Wait/AbortRun:
- AbortRun cancelled the completed Initiate execution, so cancellation
never reached the running stages (wait_event/delay were unabortable).
- Wait returned immediately on the completed Initiate execution, so
?wait=true returned a non-terminated instance.
Both now target "<instanceID>-main" (matching ReadInstanceHistory).
Wait also mishandled errors: errors.Is(err, &serviceerror.NotFound{})
can never match (no Is/Unwrap on that type) and errors.Unwrap(err)
returned nil for non-wrapped errors, turning a failure into a success.
Use errors.As for NotFound and return the original error otherwise.
Adds TestWait covering both the terminate-wait and not-found paths.
…retries The event bus is at-least-once. Previously only SAVED_PAYMENT/SAVED_ACCOUNT got a deterministic, dedup-ing Temporal workflow id; every other event type ran with a server-generated id, so a redelivery (or a partial-failure NACK after some triggers had already started) re-executed triggers and replayed side-effecting stages such as money movements. - listener: derive a deterministic workflow id for ALL events (taskIDPrefix-triggerID-<objectID|msg.UUID>) with REJECT_DUPLICATE, so a redelivery is rejected as a duplicate instead of starting a second run. (H1) - occurrence id: ExecuteTrigger built the occurrence with uuid.NewString() in workflow code, yielding a different id on every Temporal replay. Use the (deterministic) workflow execution id instead. (M4) - insert activities: InsertNewInstance, InsertNewStage and InsertTriggerOccurrence now use ON CONFLICT DO NOTHING. With deterministic primary keys, a retry after a lost ack (row committed, result lost) would otherwise fail forever on the duplicate key and wedge the workflow. (M2) Adds a redelivery regression test for a non-payment event.
…events handleMessage had an unnamed error return, so its recover() returned nil and watermill ACKed the message. getWorkflowIDFromEvent deliberately panicked on (un)marshal failures (e.g. a SAVED_PAYMENT payload whose id is not a string), so a malformed event was permanently dropped with only a stdout print. - getWorkflowIDFromEvent now returns an error instead of panicking. - handleMessage uses a named return so a recovered panic is converted into a returned error (NACK + redelivery/DLQ) and logged with its stack via the structured logger. Adds a test asserting a malformed payment payload returns an error. Note: overlaps internal/triggers/listener.go with #184 (idempotence); the two are independent and may need a trivial merge.
RunWaitEvent called channel.ReceiveAsync inside a workflow.Await predicate. An Await predicate is only evaluated once per workflow-task wakeup, so if a non-matching signal and the matching signal were delivered in the same workflow task, the predicate consumed the non-matching one, returned false, and the matching signal stayed buffered with nothing left to re-wake the coroutine -- blocking the stage forever. Non-matching events were also silently destroyed. Replace it with the canonical blocking Receive loop that drains signals one at a time until the expected event arrives. Adds a regression test signaling a non-matching then matching event in the same task.
…text When a Run workflow is cancelled, config.run returns a CanceledError and the run context is already cancelled. The subsequent UpdateStage / SendWorkflowStageTerminationEvent and UpdateInstance / SendWorkflowTerminationEvent activities were executed on that cancelled context, so they failed immediately: the instance/stage rows were left stuck "running" and no FAILED_WORKFLOW(_STAGE) event was published. Add terminationContext(), which returns the original context normally but a workflow.NewDisconnectedContext when the workflow has been cancelled, and use it for the terminal bookkeeping activities in both run() and Run. The normal (non-cancelled) path is unchanged (still uses the live context); covered by the existing end-to-end TestConfig.
RunWorkflow and ReadWorkflow selected by id only, so a soft-deleted
workflow could still be executed (POST /workflows/{id}/instances) and
read. DeleteWorkflow had no deleted_at guard, so re-deleting rewrote the
timestamp instead of returning not-found. And the trigger-matching
queries filtered only trigger.deleted_at, so triggers attached to a
deleted workflow kept firing instances forever.
- RunWorkflow / ReadWorkflow: add deleted_at IS NULL.
- DeleteWorkflow: add deleted_at IS NULL guard (re-delete -> ErrWorkflowNotFound).
- listMatchingTriggers / Activities.ListTriggers: exclude triggers whose
workflow is soft-deleted.
Tests: TestListMatchingTriggers gains a soft-deleted-workflow case, and
TestSoftDeletedWorkflowIsNotUsable covers read/run/re-delete.
ReadInstanceHistory and ReadStageHistory ran inside HTTP handlers but panicked on json.Unmarshal failures and on any DescribeWorkflowExecution error other than NotFound, and indexed Input.Payloads[0] / Result.Payloads[0] without a length check (index out of range when a workflow/activity was started with no payload). chi's Recoverer turned each into an opaque 500. - Add unmarshalFirstPayload(), which tolerates a nil/empty payload set and returns the decode error instead of panicking. - Replace all four panic sites with wrapped returned errors; the DescribeWorkflowExecution NotFound case still maps to ErrInstanceNotFound. Adds TestUnmarshalFirstPayload (nil/empty/malformed/well-formed). Note: overlaps internal/workflow/manager.go with other PRs in this series (different functions); independent.
Most read/write handlers mapped every backend error to 500, and the trigger/workflow create+test handlers returned 500 for malformed request bodies. Unknown ids therefore looked like server faults and bad client payloads were misclassified. - Add api.WriteError(), mapping sql.ErrNoRows, the workflow not-found sentinels and Temporal NotFound to 404, ErrInvalidConfig to 400, and everything else to 500. Use it in readInstance, readWorkflow, runWorkflow, postEvent, abortWorkflowInstance, readInstanceHistory and testTrigger (v1 + v2). - createTrigger / testTrigger: malformed body -> 400 (was 500). - createWorkflow: wrap validation failures as workflow.ErrInvalidConfig so they surface as 400; drop two panics (post-wait GetInstance, json.Marshal) in favour of returned error responses. Tests: TestGetInstanceNotFound (404) and TestCreateWorkflowValidationError (400). Note: edits internal/workflow/manager.go (Create + new sentinel) and many v1/v2 handlers shared with other PRs in this series; independent, different regions.
The flag is registered as a float64 in go-libs, but it was read with GetInt, which fails on a float64 flag and returns 0. The worker option was therefore always 0 (Temporal default = unlimited), so the operator-set limit never took effect. Read it with GetFloat64, and wire it to MaxConcurrentActivityExecutionSize (activity concurrency) rather than TaskQueueActivitiesPerSecond (a queue-wide rate limit), which is what "max parallel activities" means. Behavioural note: the default (10) now actually caps activity concurrency where it previously had no effect.
…iles pkg/client is a separate (nested) Go module, so neither the root build nor CI ever compiled it. It was left inconsistent between two Speakeasy generations: formance.go declared 'package client' with github.com/formancehq/flows/pkg/client/... import paths, while sdk.go and the rest declare 'package openapi' (matching .speakeasy/gen.yaml's packageName: openapi and the README's openapi.New entrypoint). formance.go was a leftover duplicate of sdk.go (same ServerList, sdkConfiguration, helpers; SDK renamed to Formance) and nothing referenced it. Remove it and run 'go mod tidy' to restore the missing go.sum entries (cenkalti/backoff/v4, ericlagergren/decimal). The module now builds and vets cleanly. Note: this module is not covered by CI (nested module); verified locally with 'go build ./...' and 'go vet ./...' inside pkg/client.
The v1 listInstances and listWorkflows handlers passed a zero-value query, and bunpaginate applies no LIMIT when PageSize == 0 -- so each request loaded the entire workflow_instances / workflows table into memory. Read the page size via bunpaginate.GetPageSize (default 15, max 100, overridable with ?pageSize=), as the v2 handlers already do, while keeping the v1 flat-array response shape. Behavioural note: v1 list responses are now bounded to one page; clients needing more pass ?pageSize=. Adds TestListInstancesIsBounded.
Migration 8 dropped the (trigger_id, event_id) primary key in favour of (id), leaving trigger_id unindexed. ListTriggersOccurrences filters WHERE trigger_id = ? ordered by date, so every page was a sequential scan over an ever-growing table. Add a migration creating the composite index, built CONCURRENTLY (the migrator runs each migration on a dedicated non-transactional connection) and IF NOT EXISTS for idempotency.
WalkthroughThe PR migrates the service from go-libs v3 to v5. It also adds API error mapping and cursor pagination, safer workflow cleanup and payload handling, deterministic trigger processing, link host validation, and a trigger-occurrence index. ChangesApplication and library migration
API behavior
Workflow and trigger execution
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🛑 Changes requested — automated reviewAll previously raised blockers and major issues (duplicate *http.Client Fx provider, CREATE INDEX CONCURRENTLY in transaction, v1 pagination truncation, empty event object ID, HTTP scheme allowance, test DB connection leak, float→int overflow) have been confirmed resolved by the author. One remaining issue stands: in the wait-event stage, the return value of |
NumaryBot
reviewed
Aug 5, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #199 (comment)
NumaryBot
reviewed
Aug 5, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #199 (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
🤖 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 `@cmd/worker.go`:
- Around line 38-49: Update workerOptions in cmd/worker.go to validate the
temporalMaxParallelActivities flag value before building
temporalworker.NewWorkerModule and worker.Options. Reject any non-positive or
fractional value read via
cmd.Flags().GetFloat64(temporal.TemporalMaxParallelActivitiesFlag) instead of
casting it directly to int, and only pass an integer
MaxConcurrentActivityExecutionSize when the flag is a valid positive whole
number.
In `@internal/triggers/expression.go`:
- Around line 38-42: Update checkLinkURL to accept only the https scheme for the
initial link target, and enforce the same HTTPS-only requirement in its
CheckRedirect handling before following redirects. Revise the allowed-by-host
test to use an HTTPS/TLS test server while preserving the existing
host-allowlist assertions.
In `@internal/triggers/listener.go`:
- Around line 39-47: Update the event payload parsing flow around the local
object type and json.Unmarshal to reject payloads whose o.ID is empty, including
missing or null id values, by returning an appropriate error before
pointer.For(o.ID). Add coverage for a saved-event payload without an id.
In `@internal/workflow/manager_test.go`:
- Around line 160-165: Update TestSoftDeletedWorkflowIsNotUsable to close the
database returned by bunconnect.OpenSQLDB after the test completes, using
deferred cleanup immediately after successful creation while preserving the
existing migration and assertions.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fca805b2-9a74-4702-816a-6241858b8328
⛔ Files ignored due to path filters (103)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumpkg/client/README.mdis excluded by!pkg/client/**pkg/client/USAGE.mdis excluded by!pkg/client/**pkg/client/docs/sdks/v1/README.mdis excluded by!pkg/client/**pkg/client/docs/sdks/v2/README.mdis excluded by!pkg/client/**pkg/client/formance.gois excluded by!pkg/client/**pkg/client/go.modis excluded by!**/*.mod,!pkg/client/**pkg/client/go.sumis excluded by!**/*.sum,!**/*.sum,!pkg/client/**pkg/client/internal/hooks/clientcredentials.gois excluded by!pkg/client/**pkg/client/internal/utils/form.gois excluded by!pkg/client/**pkg/client/internal/utils/json.gois excluded by!pkg/client/**pkg/client/internal/utils/pathparams.gois excluded by!pkg/client/**pkg/client/internal/utils/queryparams.gois excluded by!pkg/client/**pkg/client/internal/utils/retries.gois excluded by!pkg/client/**pkg/client/models/components/activitystripetransfer.gois excluded by!pkg/client/**pkg/client/models/components/assetholder.gois excluded by!pkg/client/**pkg/client/models/components/creditwalletrequest.gois excluded by!pkg/client/**pkg/client/models/components/debitwalletrequest.gois excluded by!pkg/client/**pkg/client/models/components/monetary.gois excluded by!pkg/client/**pkg/client/models/components/payment.gois excluded by!pkg/client/**pkg/client/models/components/paymentadjustment.gois excluded by!pkg/client/**pkg/client/models/components/posting.gois excluded by!pkg/client/**pkg/client/models/components/posttransaction.gois excluded by!pkg/client/**pkg/client/models/components/security.gois excluded by!pkg/client/**pkg/client/models/components/stage.gois excluded by!pkg/client/**pkg/client/models/components/stagedelay.gois excluded by!pkg/client/**pkg/client/models/components/stagesend.gois excluded by!pkg/client/**pkg/client/models/components/stagestatus.gois excluded by!pkg/client/**pkg/client/models/components/subject.gois excluded by!pkg/client/**pkg/client/models/components/transaction.gois excluded by!pkg/client/**pkg/client/models/components/trigger.gois excluded by!pkg/client/**pkg/client/models/components/triggeroccurrence.gois excluded by!pkg/client/**pkg/client/models/components/v2activitystripetransfer.gois excluded by!pkg/client/**pkg/client/models/components/v2assetholder.gois excluded by!pkg/client/**pkg/client/models/components/v2creditwalletrequest.gois excluded by!pkg/client/**pkg/client/models/components/v2debitwalletrequest.gois excluded by!pkg/client/**pkg/client/models/components/v2monetary.gois excluded by!pkg/client/**pkg/client/models/components/v2payment.gois excluded by!pkg/client/**pkg/client/models/components/v2paymentadjustment.gois excluded by!pkg/client/**pkg/client/models/components/v2posting.gois excluded by!pkg/client/**pkg/client/models/components/v2posttransaction.gois excluded by!pkg/client/**pkg/client/models/components/v2stage.gois excluded by!pkg/client/**pkg/client/models/components/v2stagedelay.gois excluded by!pkg/client/**pkg/client/models/components/v2stagesend.gois excluded by!pkg/client/**pkg/client/models/components/v2stagestatus.gois excluded by!pkg/client/**pkg/client/models/components/v2subject.gois excluded by!pkg/client/**pkg/client/models/components/v2transaction.gois excluded by!pkg/client/**pkg/client/models/components/v2trigger.gois excluded by!pkg/client/**pkg/client/models/components/v2triggeroccurrence.gois excluded by!pkg/client/**pkg/client/models/components/v2volume.gois excluded by!pkg/client/**pkg/client/models/components/v2wallet.gois excluded by!pkg/client/**pkg/client/models/components/v2walletwithbalances.gois excluded by!pkg/client/**pkg/client/models/components/v2workflow.gois excluded by!pkg/client/**pkg/client/models/components/v2workflowinstance.gois excluded by!pkg/client/**pkg/client/models/components/v2workflowinstancehistory.gois excluded by!pkg/client/**pkg/client/models/components/v2workflowinstancehistorystage.gois excluded by!pkg/client/**pkg/client/models/components/volume.gois excluded by!pkg/client/**pkg/client/models/components/wallet.gois excluded by!pkg/client/**pkg/client/models/components/walletwithbalances.gois excluded by!pkg/client/**pkg/client/models/components/workflow.gois excluded by!pkg/client/**pkg/client/models/components/workflowinstance.gois excluded by!pkg/client/**pkg/client/models/components/workflowinstancehistory.gois excluded by!pkg/client/**pkg/client/models/components/workflowinstancehistorystage.gois excluded by!pkg/client/**pkg/client/models/operations/cancelevent.gois excluded by!pkg/client/**pkg/client/models/operations/createtrigger.gois excluded by!pkg/client/**pkg/client/models/operations/createworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/deletetrigger.gois excluded by!pkg/client/**pkg/client/models/operations/deleteworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/getinstance.gois excluded by!pkg/client/**pkg/client/models/operations/getinstancehistory.gois excluded by!pkg/client/**pkg/client/models/operations/getinstancestagehistory.gois excluded by!pkg/client/**pkg/client/models/operations/getserverinfo.gois excluded by!pkg/client/**pkg/client/models/operations/getworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/listinstances.gois excluded by!pkg/client/**pkg/client/models/operations/listtriggers.gois excluded by!pkg/client/**pkg/client/models/operations/listtriggersoccurrences.gois excluded by!pkg/client/**pkg/client/models/operations/listworkflows.gois excluded by!pkg/client/**pkg/client/models/operations/options.gois excluded by!pkg/client/**pkg/client/models/operations/readtrigger.gois excluded by!pkg/client/**pkg/client/models/operations/runworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/sendevent.gois excluded by!pkg/client/**pkg/client/models/operations/testtrigger.gois excluded by!pkg/client/**pkg/client/models/operations/v2cancelevent.gois excluded by!pkg/client/**pkg/client/models/operations/v2createtrigger.gois excluded by!pkg/client/**pkg/client/models/operations/v2createworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/v2deletetrigger.gois excluded by!pkg/client/**pkg/client/models/operations/v2deleteworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/v2getinstance.gois excluded by!pkg/client/**pkg/client/models/operations/v2getinstancehistory.gois excluded by!pkg/client/**pkg/client/models/operations/v2getinstancestagehistory.gois excluded by!pkg/client/**pkg/client/models/operations/v2getserverinfo.gois excluded by!pkg/client/**pkg/client/models/operations/v2getworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/v2listinstances.gois excluded by!pkg/client/**pkg/client/models/operations/v2listtriggers.gois excluded by!pkg/client/**pkg/client/models/operations/v2listtriggersoccurrences.gois excluded by!pkg/client/**pkg/client/models/operations/v2listworkflows.gois excluded by!pkg/client/**pkg/client/models/operations/v2readtrigger.gois excluded by!pkg/client/**pkg/client/models/operations/v2runworkflow.gois excluded by!pkg/client/**pkg/client/models/operations/v2sendevent.gois excluded by!pkg/client/**pkg/client/sdk.gois excluded by!pkg/client/**pkg/client/v1.gois excluded by!pkg/client/**pkg/client/v2.gois excluded by!pkg/client/**
📒 Files selected for processing (97)
cmd/root.gocmd/serve.gocmd/worker.gointernal/api/backend.gointernal/api/backend_generated.gointernal/api/errors.gointernal/api/handler_info.gointernal/api/module.gointernal/api/module_test.gointernal/api/router.gointernal/api/v1/handler_abort_workflow_instance.gointernal/api/v1/handler_create_trigger.gointernal/api/v1/handler_create_workflow.gointernal/api/v1/handler_delete_trigger.gointernal/api/v1/handler_delete_workflow.gointernal/api/v1/handler_delete_workflow_test.gointernal/api/v1/handler_get_trigger.gointernal/api/v1/handler_list_instances.gointernal/api/v1/handler_list_instances_test.gointernal/api/v1/handler_list_triggers.gointernal/api/v1/handler_list_triggers_occurrences.gointernal/api/v1/handler_list_triggers_occurrences_test.gointernal/api/v1/handler_list_workflows.gointernal/api/v1/handler_post_event.gointernal/api/v1/handler_read_instance.gointernal/api/v1/handler_read_instance_history.gointernal/api/v1/handler_read_instance_test.gointernal/api/v1/handler_read_stage_history.gointernal/api/v1/handler_read_workflow.gointernal/api/v1/handler_run_workflow.gointernal/api/v1/handler_run_workflow_test.gointernal/api/v1/main_test.gointernal/api/v1/router.gointernal/api/v2/handler_abort_workflow_instance.gointernal/api/v2/handler_create_trigger.gointernal/api/v2/handler_create_workflow.gointernal/api/v2/handler_create_workflow_test.gointernal/api/v2/handler_delete_trigger.gointernal/api/v2/handler_delete_workflow.gointernal/api/v2/handler_delete_workflow_test.gointernal/api/v2/handler_get_trigger.gointernal/api/v2/handler_list_instances.gointernal/api/v2/handler_list_instances_test.gointernal/api/v2/handler_list_triggers.gointernal/api/v2/handler_list_triggers_occurrences.gointernal/api/v2/handler_list_workflows.gointernal/api/v2/handler_post_event.gointernal/api/v2/handler_read_instance.gointernal/api/v2/handler_read_instance_history.gointernal/api/v2/handler_read_instance_test.gointernal/api/v2/handler_read_stage_history.gointernal/api/v2/handler_read_workflow.gointernal/api/v2/handler_run_workflow.gointernal/api/v2/handler_run_workflow_test.gointernal/api/v2/handler_test_trigger.gointernal/api/v2/handler_test_trigger_test.gointernal/api/v2/main_test.gointernal/api/v2/router.gointernal/schema/map.gointernal/storage/main_test.gointernal/storage/migrations.gointernal/storage/migrations_test.gointernal/temporalworker/module.gointernal/triggers/activities.gointernal/triggers/expression.gointernal/triggers/listener.gointernal/triggers/listener_test.gointernal/triggers/main_test.gointernal/triggers/manager.gointernal/triggers/manager_test.gointernal/triggers/module.gointernal/triggers/trigger.gointernal/triggers/trigger_test.gointernal/triggers/workflow_trigger.gointernal/triggers/workflow_trigger_test.gointernal/workflow/activities.gointernal/workflow/activities/activity.gointernal/workflow/activities/activity_ledger_create_transaction.gointernal/workflow/activities/activity_wallet_credit.gointernal/workflow/activities/activity_wallet_debit.gointernal/workflow/activities/activity_wallet_list.gointernal/workflow/activities_test.gointernal/workflow/config.gointernal/workflow/main_test.gointernal/workflow/manager.gointernal/workflow/manager_test.gointernal/workflow/run.gointernal/workflow/stage.gointernal/workflow/stages/delay/delay.gointernal/workflow/stages/delay/run.gointernal/workflow/stages/delay/run_test.gointernal/workflow/stages/send/run.gointernal/workflow/stages/send/run_test.gointernal/workflow/stages/send/send.gointernal/workflow/stages/wait_event/run.gointernal/workflow/stages/wait_event/wait_event_test.gopkg/events/events.go
NumaryBot
reviewed
Aug 5, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 6 new inline findings.
Summary: #199 (comment)
NumaryBot
reviewed
Aug 5, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 5 new inline findings.
Summary: #199 (comment)
This was referenced Aug 5, 2026
This was referenced Aug 5, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cmd/worker.go`:
- Around line 46-49: Update the validation around temporalMaxParallelActivities
in the worker options path to reject values at or above the exclusive signed-int
limit, using strconv.IntSize to derive the bound before any int conversion;
retain the positive whole-number checks. Extend
TestWorkerOptionsValidatesMaxParallelActivities with the 64-bit overflow
boundary case.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5878317a-b3b5-4039-8016-8599e89caab8
⛔ Files ignored due to path filters (67)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumopenapi.yamlis excluded by!**/*.yamlopenapi/v1.yamlis excluded by!**/*.yamlpkg/client/.speakeasy/gen.lockis excluded by!**/*.lock,!**/*.lock,!pkg/client/**pkg/client/.speakeasy/gen.yamlis excluded by!**/*.yaml,!pkg/client/**pkg/client/README.mdis excluded by!pkg/client/**pkg/client/USAGE.mdis excluded by!pkg/client/**pkg/client/docs/models/components/activitycreatetransferinitiation.mdis excluded by!pkg/client/**pkg/client/docs/models/components/activitycreatetransferinitiationmetadata.mdis excluded by!pkg/client/**pkg/client/docs/models/components/activitycreatetransferinitiationtype.mdis excluded by!pkg/client/**pkg/client/docs/models/components/listrunsresponse.mdis excluded by!pkg/client/**pkg/client/docs/models/components/listtriggersoccurrencesresponse.mdis excluded by!pkg/client/**pkg/client/docs/models/components/listtriggersresponse.mdis excluded by!pkg/client/**pkg/client/docs/models/components/listworkflowsresponse.mdis excluded by!pkg/client/**pkg/client/docs/models/components/payment.mdis excluded by!pkg/client/**pkg/client/docs/models/components/paymenttype.mdis excluded by!pkg/client/**pkg/client/docs/models/components/stagesenddestinationaccount.mdis excluded by!pkg/client/**pkg/client/docs/models/components/stagesenddestinationpayment.mdis excluded by!pkg/client/**pkg/client/docs/models/components/stagesendsourceaccount.mdis excluded by!pkg/client/**pkg/client/docs/models/components/stagesendsourcepayment.mdis excluded by!pkg/client/**pkg/client/docs/models/components/type.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2activitycreatetransferinitiation.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2activitycreatetransferinitiationmetadata.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2activitycreatetransferinitiationtype.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2stagesenddestinationaccount.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2stagesenddestinationpayment.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2stagesenddestinationpaymenttype.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2stagesendsourceaccount.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2stagesendsourcepayment.mdis excluded by!pkg/client/**pkg/client/docs/models/components/v2workflowinstancehistorystageinput.mdis excluded by!pkg/client/**pkg/client/docs/models/components/workflowinstancehistorystageinput.mdis excluded by!pkg/client/**pkg/client/docs/models/operations/listinstancesrequest.mdis excluded by!pkg/client/**pkg/client/docs/models/operations/listtriggersoccurrencesrequest.mdis excluded by!pkg/client/**pkg/client/docs/models/operations/listtriggersrequest.mdis excluded by!pkg/client/**pkg/client/docs/models/operations/listworkflowsrequest.mdis excluded by!pkg/client/**pkg/client/docs/sdks/v1/README.mdis excluded by!pkg/client/**pkg/client/docs/sdks/v2/README.mdis excluded by!pkg/client/**pkg/client/models/components/activitycreatetransferinitiation.gois excluded by!pkg/client/**pkg/client/models/components/listrunsresponse.gois excluded by!pkg/client/**pkg/client/models/components/listtriggersoccurrencesresponse.gois excluded by!pkg/client/**pkg/client/models/components/listtriggersresponse.gois excluded by!pkg/client/**pkg/client/models/components/listworkflowsresponse.gois excluded by!pkg/client/**pkg/client/models/components/payment.gois excluded by!pkg/client/**pkg/client/models/components/stagesenddestinationaccount.gois excluded by!pkg/client/**pkg/client/models/components/stagesenddestinationpayment.gois excluded by!pkg/client/**pkg/client/models/components/stagesendsourceaccount.gois excluded by!pkg/client/**pkg/client/models/components/stagesendsourcepayment.gois excluded by!pkg/client/**pkg/client/models/components/v2activitycreatetransferinitiation.gois excluded by!pkg/client/**pkg/client/models/components/v2stagesenddestinationaccount.gois excluded by!pkg/client/**pkg/client/models/components/v2stagesenddestinationpayment.gois excluded by!pkg/client/**pkg/client/models/components/v2stagesendsourceaccount.gois excluded by!pkg/client/**pkg/client/models/components/v2stagesendsourcepayment.gois excluded by!pkg/client/**pkg/client/models/components/v2workflowinstancehistorystageinput.gois excluded by!pkg/client/**pkg/client/models/components/workflowinstancehistorystageinput.gois excluded by!pkg/client/**pkg/client/models/operations/listinstances.gois excluded by!pkg/client/**pkg/client/models/operations/listtriggers.gois excluded by!pkg/client/**pkg/client/models/operations/listtriggersoccurrences.gois excluded by!pkg/client/**pkg/client/models/operations/listworkflows.gois excluded by!pkg/client/**pkg/client/models/operations/v2listinstances.gois excluded by!pkg/client/**pkg/client/models/operations/v2listtriggers.gois excluded by!pkg/client/**pkg/client/models/operations/v2listtriggersoccurrences.gois excluded by!pkg/client/**pkg/client/models/operations/v2listworkflows.gois excluded by!pkg/client/**pkg/client/orchestration.gois excluded by!pkg/client/**pkg/client/sdk.gois excluded by!pkg/client/**pkg/client/v1.gois excluded by!pkg/client/**pkg/client/v2.gois excluded by!pkg/client/**
📒 Files selected for processing (19)
cmd/root.gocmd/root_test.gocmd/serve.gocmd/worker.gocmd/worker_test.gointernal/api/v1/handler_list_instances.gointernal/api/v1/handler_list_instances_test.gointernal/api/v1/handler_list_triggers.gointernal/api/v1/handler_list_triggers_occurrences.gointernal/api/v1/handler_list_triggers_occurrences_test.gointernal/api/v1/handler_list_workflows.gointernal/api/v1/pagination.gointernal/triggers/expression.gointernal/triggers/listener.gointernal/triggers/listener_test.gointernal/triggers/module.gointernal/triggers/trigger_test.gointernal/workflow/activities_test.gointernal/workflow/manager_test.go
🚧 Files skipped from review as they are similar to previous changes (13)
- internal/api/v1/handler_list_workflows.go
- internal/triggers/listener_test.go
- internal/triggers/expression.go
- internal/triggers/listener.go
- cmd/root.go
- internal/api/v1/handler_list_instances.go
- internal/api/v1/handler_list_instances_test.go
- internal/workflow/manager_test.go
- internal/api/v1/handler_list_triggers_occurrences_test.go
- internal/workflow/activities_test.go
- internal/api/v1/handler_list_triggers_occurrences.go
- internal/triggers/trigger_test.go
- cmd/serve.go
NumaryBot
reviewed
Aug 6, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #199 (comment)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/api/v1/handler_list_instances_test.go (1)
129-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the continuation cursor advances.
The current assertions only check the record count and
hasMore. An implementation that returns five records from the first page can pass this test. Record the first-page IDs and assert that the second page has no overlap. Assert that both pages contain all 20 inserted IDs.🤖 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 `@internal/api/v1/handler_list_instances_test.go` around lines 129 - 140, The continuation-cursor test must verify pagination advances rather than only checking counts. In the test around firstPage and secondPage, collect IDs from both responses, assert the two page ID sets are disjoint, and assert their combined IDs equal all 20 inserted instance IDs.
🤖 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.
Nitpick comments:
In `@internal/api/v1/handler_list_instances_test.go`:
- Around line 129-140: The continuation-cursor test must verify pagination
advances rather than only checking counts. In the test around firstPage and
secondPage, collect IDs from both responses, assert the two page ID sets are
disjoint, and assert their combined IDs equal all 20 inserted instance IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 09b8ac6a-138c-4e66-8e86-413d309d0eba
⛔ Files ignored due to path filters (5)
openapi.yamlis excluded by!**/*.yamlopenapi/v2.yamlis excluded by!**/*.yamlpkg/client/.speakeasy/gen.lockis excluded by!**/*.lock,!**/*.lock,!pkg/client/**pkg/client/.speakeasy/gen.yamlis excluded by!**/*.yaml,!pkg/client/**pkg/client/sdk.gois excluded by!pkg/client/**
📒 Files selected for processing (8)
cmd/worker.gocmd/worker_test.gointernal/api/v1/handler_list_instances.gointernal/api/v1/handler_list_instances_test.gointernal/api/v1/handler_list_triggers.gointernal/api/v1/handler_list_triggers_occurrences.gointernal/api/v1/handler_list_workflows.gointernal/api/v1/pagination.go
🚧 Files skipped from review as they are similar to previous changes (6)
- internal/api/v1/handler_list_workflows.go
- internal/api/v1/handler_list_instances.go
- internal/api/v1/handler_list_triggers_occurrences.go
- cmd/worker_test.go
- cmd/worker.go
- internal/api/v1/handler_list_triggers.go
NumaryBot
reviewed
Aug 6, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #199 (comment)
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
2 participants
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
Integration fixes
Included pull requests
Supersedes the now-closed #180, #181, #182, #183, #184, #185, #186, #187, #188, #189, #190, #191, #192, #193, and #194.
Validation