refactor(wfutils): one body behind Execute and ExecuteWithLowPrioQueue - #451
Open
KillerX wants to merge 2 commits into
Open
refactor(wfutils): one body behind Execute and ExecuteWithLowPrioQueue#451KillerX wants to merge 2 commits into
KillerX wants to merge 2 commits into
Conversation
KillerX
force-pushed
the
refactor/remove-dynamic-trigger-rpc
branch
from
August 13, 2026 08:48
c6bfbfe to
10c2de2
Compare
KillerX
force-pushed
the
refactor/execute-shared-body
branch
from
August 13, 2026 08:48
971252a to
e72258d
Compare
…stry GET /schemas published the full JSON Schema of every triggerable workflow's param struct, and POST /trigger-dynamic executed any of them by name through reflection — including scheduled.CleanupTemp and scheduled.MediabankenPurgeTrash, on a service with no authn or authz of its own. Nothing in this repository calls either endpoint. They were also the only reader of TriggerableWorkflows, the second of the two hand-maintained registration lists. Rather than derive one list from the other and add a test to keep them in step, both endpoints and the list go, which closes the drift by construction: a workflow is either in WorkerWorkflows or it cannot run. That also settles the six workflows the audit found missing from the triggerable list — BulkExportShorts, ExportShort, GenerateShort, IsilonExport, AssetJSON and ImportSubtitles. They were not reachable through /trigger-dynamic and now there is nothing for them to be missing from. /trigger/:job is untouched. That is the named-job endpoint the FileCatalyst and watcher integrations post to, and it has an explicit case per job rather than reflecting over a registry. Also removed: httpin's own copy of getFunctionName, which only the schema handler used, and the invopop/jsonschema dependency. The jsonschema struct tag on VXExportParams.Destinations went with it — the valid values are the AssetExportDestination enum, which VXExport already validates against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two differed only in the queue they pick, but each had its own copy of the scheduling code, and the copies had drifted. Two of the three divergences the audit listed are already gone — the ActivityWG bookkeeping no longer exists, and both call activityOptionsWithDefaults — leaving the duplicated retry-policy switch. Both now call executeOnQueue with a queue. Extracting it exposed a gap the duplication hid. The switch only names the worker, transcode and audio queues, so an activity on any other queue got no retry policy at all unless the workflow had set one — and low-priority is by definition a queue the switch does not name, so ExecuteWithLowPrioQueue was routing activities to exactly the case that falls through. It is latent today because both callers set GetDefaultActivityOptions, which carries a policy. retryPolicyForQueue now has a default branch: low priority and live ingest carry the same kind of work as the worker queue, so they get the same loose policy. Two details worth keeping when reading this: The low-priority remap compares against environment.QueueWorker, the constant, not GetWorkerQueue(). In debug mode the accessor returns the debug queue and the debug worker polls only that queue, so remapping there would leave the activity unscheduled. The comparison is deliberate, not an oversight. Debug mode also collapses the worker, transcode and audio accessors onto one value, which made the original a switch with three identical cases where the first won. The extracted version keeps worker first for that reason, and retryPolicyForQueueNames takes the names as arguments so a test can exercise the collapsed arrangement without the process-wide QUEUE variable that decides it — a variable read at package-var time, so t.Setenv cannot reach it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
KillerX
force-pushed
the
refactor/remove-dynamic-trigger-rpc
branch
from
August 13, 2026 13:26
10c2de2 to
ccc6e9a
Compare
KillerX
force-pushed
the
refactor/execute-shared-body
branch
from
August 13, 2026 13:26
e72258d to
726653a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
12/n of a stack. Base:
refactor/remove-dynamic-trigger-rpc(#450). Closes ExecuteWithLowPrioQueue has drifted from Execute.The two differed only in the queue they pick, but each had its own copy of the scheduling code and the copies had drifted. Two of the three divergences the audit listed are already gone earlier in this stack — the
ActivityWGbookkeeping no longer exists (#440) and both callactivityOptionsWithDefaults(#443) — leaving the duplicated retry-policy switch. Both now callexecuteOnQueuewith a queue.Extracting it exposed a gap the duplication hid. The switch only names the worker, transcode and audio queues, so an activity on any other queue got no retry policy at all unless the workflow set one — and low-priority is by definition a queue the switch does not name, so
ExecuteWithLowPrioQueuerouted activities to exactly the case that falls through. Latent today because both callers passGetDefaultActivityOptions, which carries a policy.retryPolicyForQueuenow has a default branch: low priority and live ingest carry the same kind of work as the worker queue, so they get the same loose policy.Two details worth keeping in mind when reading the diff, both about debug mode:
environment.QueueWorker, the constant, notGetWorkerQueue(). WithQUEUE=debugthe accessor returns the debug queue and the debug worker polls only that queue, so remapping there would leave the activity unscheduled. The asymmetry is deliberate.retryPolicyForQueueNamestakes the names as arguments so a test can exercise the collapsed arrangement — theQUEUEvariable that decides them is read at package-var time, sot.Setenvcannot reach it.Worth knowing while reviewing: the test environment here runs with
QUEUE=debug, so anything asserting on the accessors is asserting the collapsed arrangement.🤖 Generated with Claude Code