fix(plugin-workers): derive trigger id from {id} path, fail loudly on missing id (#279)#294
Merged
Merged
Conversation
… missing id (triggerJob + triggerTask) (#279) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wKHquACkXnWPDgJYhhFjN
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
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
Fixes the one still-open correctness gap from the eis-chat plugin-workers dogfood (#279 Gap 2), extended symmetrically to the sibling
triggerTask.Gap 2 —
triggerJob/triggerTasktook the id from the request BODY, not the{id}path. The OpenAPI route isPOST /jobs/{id}/trigger, but the input schema requiredidin the body and the created task's id was read from the body — so an empty-body call persisted a task withid=undefinedsilently (Processing task 'undefined'thenWorkers KV key contains unsupported part: undefined), leaving the caller with no error. This makes the path{id}the single source of truth and fails loudly when the resolved id is missing.Change
packages/plugin-workers-core/src/contracts/v1/workers.contract-schemas.ts+workers.contract-types.ts—idmade optional inJobTriggerInputZodSchemaandTaskTriggerInputZodSchema(oRPC merges the OpenAPI{id}path param intoinput.id). Backward compatible: callers that still send a bodyidround-trip unchanged.plugins/workers/services/src/routers/jobs.ts+tasks.ts— resolve the id from the path and callvalidationFailed(...)(VALIDATION_ERROR) before any KV write when the id is missing/empty; no path can persistid=undefined.packages/contracts/src/domain/errors.ts+public/mod.ts(+ test) — new centralizedvalidationFailedhelper (mirrors the existingnotFound; single sanctioned contract-boundary cast, returnsneverso the id narrows tostring).plugins/workers/services/src/trigger-path-id_test.ts— regression for both procedures: empty-body POST resolves the id from the path (neverundefined); missing id fails loudly.packages/sdk/tests/integration/workers-trigger-rpc_test.ts— Gap 1 guard: proves the typedcreateServiceClientRPC transport reachestriggerJobon a real mounted plugin service (see below).The other four gaps in #279 were already resolved on
mainTriage (grounded on
origin/main) found the framework had already landed the fixes for the rest — this PR closes the remainder:createServiceClient404s): already fixed —createPluginServicemountswithRPCandwireRpcserves both/api/rpc/*and/api/*(Re-architect plugin scaffold surface (#157): thin, typesafe, no plugin-source copy #172). Added a regression guard test rather than a code change; no residual 404.registerGeneratedJobDefinitions+compileWorkersRegistrydiscovery).ServiceReferences/PluginReferences): fixed by workers: service->plugin-API discovery + workers-api not loading user jobs (Aspire wiring gaps) #217/fix(workers): wire Service→plugin-API discovery + load user jobs in workers-api (#217) #220 (configservice-schema.ts+wireServiceReferences).--allow-ffifor native DB drivers): already present in the workers Aspire contribution. The remaining single-writer guidance is documentation (out of scope here).Validation
CI quality trio (fmt/lint/check
--unstable-kv) clean over the 4 touched roots; touched tests 6 passed / 0 failed;deno publish --dry-runonplugin-workers-coreandcontractsboth succeed with no--allow-slow-types. Independently reproduced by a separate-session evaluator (PASS). The PR CI runs the scaffold gates (the PR-only scaffold authority).Fixes #279. Removable eis-chat workaround: the
{ id: jobId, ... }body-id send inservices/eischat/src/jobs.ts(eis-chat PR #83) is no longer needed once this lands.