Repository navigation
Conversation
This branch has not been deployed
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.
Middleware can replace
TaskContextmethods with wrappers by passing them tonext, for examplenext({ invoke: wrapped }). This covers an operation policy hook without adding new API: wrappers can transform outgoing metadata, reject reserved step keys and destinations, and let internal helpers bypass the policy by calling originals captured withbind(ctx). This PR makes that pattern a documented and tested contract.Each wrapper runs once per call
I audited
task-context.tsfor public methods that call another public method throughthis, which would run a user's wrapper twice or route internal calls through it. I found four, and they now use private implementations:step()calledthis.checkpoint()→ privatehangUpIfStopped()signalWithStart()calledthis.checkpoint()→ privatehangUpIfStopped()waitForResult()calledthis.waitForAny()→ privaterace()subscribe()'swait()calledthis.waitForEvent()→ privateeventWait()Types
nextnow takesTNext & Partial<TContext>. A wrapper passed tonextgets its parameter types from the original method, generics included, soinvoke: async (key, task, payload, options = {}) => …needs no annotations, and a wrapper with a different signature is a type error.AddedContextomits the base keys it overrides, so handlers see the method's normal type rather than an intersection. One limitation: TypeScript cannot infer parameter types for an overloaded method likewaitForResultfrom context, and it narrows the type parameters of a generic method passed into a generic helper. The docs show akeyed<typeof method>(method)helper with an explicit type argument for those cases.ctx.schedule()takesmetadatactx.schedule()was the only metadata-carrying operation without ametadataoption, so a wrapper had no way to transform it. It now takesmetadatain its options. The value is validated like the other operations and defaults to the execution's metadata, as before.Derived step keys
The step keys that
TaskContextderives are now documented intask-context.ts(branchStepKey) and in the docs:<stepKey>:<branch>(so<stepKey>:result) andpgconductor.next-occurrence. An application can reserve a key prefix if the prefix contains no:and does not start withpgconductor..Docs
There is a new "Wrapping Context Methods" section in the custom context / middleware guide, with an example covering metadata hygiene, a reserved key prefix, reserved destinations and the internal bypass. It states that batch tasks and internal tasks skip middleware. The API docs link to it, and
AGENTS.mddescribes the contract.Verification
tests/integration/middleware-wrap.test.tscovers:task-context.ts)tests/unit/middleware-types.test.tsgains type tests for overrides that keep their signature and for overrides that are rejected.just lint,just formatandpnpm typecheckpass. In the fullpnpm testrun, 622 of 623 tests passed. The one failure,orchestrator-signal-subprocess"real process handles SIGTERM gracefully" (exit not within 5s), passed twice when re-run alone, so it looks like a timing flake under load.Follow-ups / out of scope
next({ signal }),next({ metadata })and other getter-only keys typecheck because ofPartial<TContext>, butObject.assignthrows at runtime when assigning to a getter. This was already the case before this PR.