Skip to content

feat(middleware): make wrapping context methods a supported contract - #104

Open
psteinroe wants to merge 1 commit into
mainfrom
feat/middleware-wrap-contract
Open

psteinroe wants to merge 1 commit into
mainfrom
feat/middleware-wrap-contract

Conversation

@psteinroe

Copy link
Copy Markdown
Owner

Middleware can replace TaskContext methods with wrappers by passing them to next, for example next({ 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 with bind(ctx). This PR makes that pattern a documented and tested contract.

Each wrapper runs once per call

I audited task-context.ts for public methods that call another public method through this, which would run a user's wrapper twice or route internal calls through it. I found four, and they now use private implementations:

  • step() called this.checkpoint() → private hangUpIfStopped()
  • signalWithStart() called this.checkpoint() → private hangUpIfStopped()
  • waitForResult() called this.waitForAny() → private race()
  • subscribe()'s wait() called this.waitForEvent() → private eventWait()

Types

next now takes TNext & Partial<TContext>. A wrapper passed to next gets its parameter types from the original method, generics included, so invoke: async (key, task, payload, options = {}) => … needs no annotations, and a wrapper with a different signature is a type error. AddedContext omits 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 like waitForResult from context, and it narrows the type parameters of a generic method passed into a generic helper. The docs show a keyed<typeof method>(method) helper with an explicit type argument for those cases.

ctx.schedule() takes metadata

ctx.schedule() was the only metadata-carrying operation without a metadata option, so a wrapper had no way to transform it. It now takes metadata in 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 TaskContext derives are now documented in task-context.ts (branchStepKey) and in the docs: <stepKey>:<branch> (so <stepKey>:result) and pgconductor.next-occurrence. An application can reserve a key prefix if the prefix contains no : and does not start with pgconductor..

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.md describes the contract.

Verification

  • tests/integration/middleware-wrap.test.ts covers:
    • metadata is merged for invoke, invokeAndWait, signalWithStart, emit and schedule
    • reserved keys and destinations (all keyed methods, sendSignal, emit, schedule, invoke) are rejected before any execution, step, signal or subscription row is stored
    • the internal bypass works
    • wrappers run again on resume while memoized results stay unchanged
    • each wrapper runs once per call (this test fails on the old task-context.ts)
    • wrappers apply to calls made by later middleware
  • tests/unit/middleware-types.test.ts gains type tests for overrides that keep their signature and for overrides that are rejected.
  • just lint, just format and pnpm typecheck pass. In the full pnpm test run, 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 of Partial<TContext>, but Object.assign throws at runtime when assigning to a getter. This was already the case before this PR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant