Skip to content

Commit 65f4669

Browse files
btraversclaude
andcommitted
docs: record v8 review remediation decisions and work breakdown
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2065ff0 commit 65f4669

1 file changed

Lines changed: 182 additions & 0 deletions

File tree

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
# v8 review remediation — decisions and work breakdown
2+
3+
- **Date:** 2026-07-30
4+
- **Status:** in progress on `feat/v8-full-review-fixes`
5+
- **Origin:** full six-track review (client, worker, contract+testing, unthrown audit,
6+
amqp-contract consistency, DX/docs) performed 2026-07-29/30. This spec records the
7+
decisions and the fix plan. All breaking changes land inside the unshipped 8.0 beta.
8+
- **Companion:** [2026-07-29-typed-client-contract-binding-design.md](2026-07-29-typed-client-contract-binding-design.md)
9+
(the `TypedClient`/`ContractClient` split), implemented as part of this work.
10+
11+
## Design decisions
12+
13+
### D1. Wire format: validate on send, parse on receive (fixes double-transform)
14+
15+
Today both sides of every boundary run the same Standard Schema and the sender
16+
transmits the **parsed** value, so a transforming schema (`z.coerce.*`,
17+
`.transform(...)`) is applied twice — silent data corruption.
18+
19+
Decision: **each boundary parses exactly once, on the receiving side.** The sender
20+
still validates (to surface a typed `Err`/`ValidationError` early and to guarantee it
21+
never emits garbage) but transmits the **original** value, discarding the parsed
22+
result. Concretely:
23+
24+
- Client `startWorkflow`/`executeWorkflow`/`signalWithStart` args: validate, send the
25+
caller's original args. The worker parses and the handler receives the parsed value.
26+
- Workflow/activity/query/update **results**: the producing side validates and returns
27+
the original value; the consuming side (client `result()`/`executeWorkflow`,
28+
workflow's activity proxy, update/query result paths) parses.
29+
- Same rule for signals, updates, queries, child workflows, and both directions of
30+
activities.
31+
32+
Type surface is unchanged (`ClientInfer*`/`WorkerInfer*` duality already models
33+
input-on-send / output-on-receive). Add tests with a transforming schema proving each
34+
transform applies exactly once end-to-end. Update the worker's "double validation"
35+
rationale comments to describe the new contract.
36+
37+
### D2. Signals: invalid payloads are dropped and logged, never thrown
38+
39+
Throwing `SignalInputValidationError` (non-retryable `ApplicationFailure`) from the
40+
signal handler terminally kills the workflow execution — wrong for a fire-and-forget
41+
message any stale client can send. Decision: on validation failure, **drop the signal
42+
and `log.warn`** (via `@temporalio/workflow`'s `log`) with signal name + issues.
43+
Delete `SignalInputValidationError`. Queries/updates keep their current (correct)
44+
semantics. No configurable policy yet — YAGNI, addable without a break.
45+
46+
### D3. Contract-misuse errors in workflow code become non-retryable ApplicationFailures
47+
48+
`bindSignalHandler`/`bindQueryHandler`/`bindUpdateHandler` ("not found in contract",
49+
"schema must be synchronous") and `buildRawActivitiesProxy` (activity-options coverage)
50+
currently throw plain `Error` from inside the workflow sandbox, hanging executions in
51+
infinite Workflow Task retries. Decision: introduce a `ContractMisuseError` extends
52+
`ValidationError` (non-retryable `ApplicationFailure`) and use it at all such sites.
53+
54+
### D4. Scope cuts (deferred, not forgotten)
55+
56+
- Typed **local activities** path — new feature, separate spec.
57+
- Client `list`/`count` — covered by the new raw escape hatch for now.
58+
- amqp-contract / async-contract convergence (`declare*` verbs, `for()` in amqp,
59+
`sideEffects`) — separate repos, separate track.
60+
- Configurable invalid-signal policy (D2) and per-binding interceptor overrides.
61+
62+
## Work breakdown
63+
64+
### Wave 1 — contract package + meta/docs (parallel)
65+
66+
Contract (`packages/contract`):
67+
68+
1. Activity-collision check: allow same-named activities across workflows when they are
69+
the **same object** (reference equality); error message for real collisions should
70+
recommend hoisting shared activities to the global `activities` block.
71+
2. Reject workflow-name vs **global-activity-name** collisions in `defineContract`
72+
(they share the root of the implementations map).
73+
3. Replace the zod meta-validation of contract shape with a hand-rolled structural
74+
validator (amqp-contract style); **drop the zod runtime dependency**. Root shape
75+
check becomes strict (unknown keys rejected), matching `defaultOptions`.
76+
4. Make `input` optional on signal/query/update definitions (absent ⇒ handler input is
77+
`undefined`, no `z.void()` ceremony). Mirror in worker's `extractHandlerInput`
78+
(zero args ⇒ `undefined`) — worker side lands in Wave 3.
79+
5. Delete `InferContractWorkflows` (trivial alias). Fix stale "unthrown 4" comment in
80+
`errors.ts`.
81+
6. Drop the CJS build (ESM-only, rule 5): remove `main`/`module`/`require` conditions,
82+
align `types`.
83+
84+
Meta/docs (no package source):
85+
86+
7. `@beta` dist-tag warning + install commands on root README, `docs/how-to/install.md`,
87+
tutorial step 1 (npm `latest` is v7; docs teach v8).
88+
8. CLAUDE.md rule 2 correction: unthrown 5 **does** export `OkAsync`/`ErrAsync`; the
89+
rule should say "no lowercase `okAsync`/`errAsync`; use `OkAsync`/`ErrAsync` or
90+
`.toAsync()`". Same fix in `.agents/rules/handlers.md` (also its free-function
91+
`isErr(result)` example → method style).
92+
9. `dependencies.md`: add `@temporalio/testing ^1` to the testing row; align unthrown
93+
ranges.
94+
10. `examples/README.md`: fix stale "Promise-based worker" / "Result/Future" wording,
95+
list all three example packages.
96+
97+
### Wave 2 — wire format (client + worker together)
98+
99+
Implement D1 across `packages/client` and `packages/worker` with transform-schema
100+
tests (unit level in each package; one end-to-end case in the client integration
101+
suite). No other refactors in this wave.
102+
103+
### Wave 3 — client and worker overhauls (parallel; contract package is frozen)
104+
105+
Client (`packages/client`):
106+
107+
11. Implement the `TypedClient`/`ContractClient` split per the companion spec
108+
(including its Testing/Documentation tables).
109+
12. Fix `handle.result()` passing `workflowId` as `workflowName` to
110+
`WorkflowValidationError`; add a `workflowId` field to that error.
111+
13. Schedule surface parity: typed `ScheduleAlreadyExistsError` /
112+
`ScheduleNotFoundError` (classified like workflow errors, replacing
113+
defect-channel-everything), add `update`, `backfill` on the handle and `list` on
114+
the schedule client; `TypedScheduleClient` constructor becomes non-public.
115+
14. Escape hatch + identifiers: `readonly raw` (underlying `Client`) on the root;
116+
`firstExecutionRunId`/`runId` carried on typed handles; `getHandle` accepts
117+
`runId`/options and becomes **synchronous** returning
118+
`Result<TypedWorkflowHandle, WorkflowNotInContractError>`; add typed `startUpdate`
119+
alongside `executeUpdate`.
120+
15. Rename `WorkflowNotFoundError``WorkflowNotInContractError` (SDK-name squat).
121+
16. Delete the six unused `ClientInfer*` type exports; fix the inverted direction
122+
comment; `: {}` fallbacks → `Record<never, never>`.
123+
17. Dedupe `executeWorkflow`'s inline copies of `classifyStartError`/
124+
`classifyResultError`; fold rehydrate-then-classify into one helper.
125+
18. TSDoc/doc fixes: `createOrThrow` note (gone anyway with the split), orphaned
126+
`TypedSearchAttributeMap` doc block, "Thrown when…" → "Surfaced…", module docs
127+
above imports, `readonly` on handle fields and error arrays, search-attribute
128+
value `typeof`-per-kind check, `interceptors.ts` combinator names
129+
(`tapErrCases`/`recoverDefect`). Client README quick start `.getOrThrow()`
130+
`.get()` and rewritten for the split. Type-level tests per the companion spec plus
131+
method-level inference pins.
132+
133+
Worker (`packages/worker`):
134+
135+
19. D2 (signal drop-and-log) and D3 (`ContractMisuseError`).
136+
20. `declareActivitiesHandler`: error on workflow-name/global-activity collision
137+
(defense-in-depth with contract check); iterate **definitions** to fail fast on
138+
declared-but-missing implementations; consistent stray-key handling when
139+
`contract.activities` is undefined.
140+
21. `TypedChildWorkflowHandle`: add typed `signals` map (validated per D1) and
141+
`firstExecutionRunId`.
142+
22. `declareWorkflow`: runtime guard for unknown `workflowName` with available-names
143+
message.
144+
23. Workflow-only workers: `activities` optional on `createWorker`; activity-less
145+
workflows no longer need `{}` entries (key remapping); `extractHandlerInput` zero
146+
args ⇒ `undefined` (pairs with contract change 4).
147+
24. Rename exported `qualify``qualifyFailure` (no alias; beta window).
148+
25. `ValidationError` `name` property `enumerable: false`; extract the repeated
149+
triple-nested conditional type helper; dedupe the duplicated sync-schema message.
150+
26. Docs: fix the phantom same-contract child-workflow overload TSDoc; fix
151+
`declareWorkflow`'s `.getOrThrow()` example; rewrite worker README
152+
(`createWorker` + `workflowsPathFromURL`, `.js` imports, named contract export,
153+
correct `cause` idiom); `declareActivitiesHandler` TSDoc example uses
154+
`createWorker`.
155+
156+
### Wave 4 — testing package, examples, docs sweep, release (after Wave 3)
157+
158+
Testing (`packages/testing`) — needs the final client API:
159+
160+
27. Contract-aware fixtures: `createContractTest(contract, options)` yielding
161+
`{ client, worker, testEnv }`; `runActivity(definition, implementation, input)`
162+
over `MockActivityEnvironment`.
163+
28. Forward `TimeSkippingTestWorkflowEnvironmentOptions` through
164+
`createTimeSkippingEnvironment(opts?)` and a fixture factory; descriptive error
165+
when `inject` values are missing (global-setup not registered); close
166+
`workerConnection` in try/catch; add `"./package.json"` export;
167+
`createGlobalSetup(options?)` factory (image tags, env, quiet).
168+
29. Fix `time-skipping.ts` TSDoc `.getOrThrow()` examples.
169+
170+
Repo-wide:
171+
172+
30. Examples: extend order-processing with a signal + query + one typed contract
173+
error + a schedule, using the new client API.
174+
31. Docs sweep: `upgrade-to-v8.md` migration sections for every breaking change here;
175+
`client-surface.md`; all construction snippets to
176+
`TypedClient.create({ client }).for(contract)`; regenerate API docs.
177+
32. Adopt `publint --strict` + `attw --pack` (`check:package` script per package,
178+
catalog entries, CI wiring) — from amqp-contract.
179+
33. Changesets: one `major` changeset per affected package (folds into next
180+
`8.0.0-beta.N`).
181+
34. Full verification: build, typecheck, lint, unit tests everywhere; integration
182+
tests if Docker is available.

0 commit comments

Comments
 (0)