Skip to content

docs: stop advertising client interceptors — they were cut - #417

Merged
btravers merged 1 commit into
mainfrom
feat/per-contract-interceptors-374
Sep 2, 2026
Merged

btravers merged 1 commit into
mainfrom
feat/per-contract-interceptors-374

Conversation

@btravers

@btravers btravers commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #374, by finding that the surface it is about no longer exists — and fixing the two places that still promise it.

What #374 asked for, and why it cannot be done

The issue proposes scoping client interceptors per contract (for(contract, { interceptors })), and/or adding contract identity to ClientInterceptorArgs. Both halves address a surface 533c796 deleted after the issue was filed:

Client interceptors: CreateClientOptions.interceptors, ClientInterceptor and friends. Shipped as public API with no consumer in the repo beyond a single test; every client method already returns an AsyncResult that composes.

CreateClientOptions is { client } today, for() takes only a contract, and ClientInterceptorArgs is gone. There is nothing left to rescope.

What was still lying

  • README.md:129 — "Schedules, cancellation scopes, continue-as-new, activity middleware, client interceptors — all contract-aware". A reader would go looking for an option that does not exist.
  • .agents/rules/handlers.md — told an agent the client has "the mirror-image seam: TypedClient.create({ interceptors })".

The rule now says why there is no mirror-image seam, since that is the part worth knowing: a client call hands you an AsyncResult, so wrapping it is composing one — .tap, .flatMap, .mapErrCases at the call site, no registration and no hook to learn. Activity middleware exists because the platform invokes the activity and the application never holds its AsyncResult.

The asymmetry #374 noticed is real, and now points the other way

@amqp-contract kept per-contract publishInterceptors / callInterceptors, with a live consumer in its example. So the family is asymmetric — but the choice is "re-add or stay cut", not "rescope", and staying cut is the decision recorded here. Re-adding would want a consumer first; there is none in this repo or in btravstack/start's temporal example.

Gate

format --check, lint (0 warnings), typecheck 12/12, unit 9/9.

https://claude.ai/code/session_01GGixjxi5AQ2cNK62bBymfF

Summary by CodeRabbit

  • Documentation
    • Clarified that client-call composition is handled at call sites using AsyncResult methods.
    • Documented that middleware remains available for activities invoked directly by the platform.
    • Updated the features list to remove references to client interceptors while retaining other contract-aware capabilities.

`533c796` removed `CreateClientOptions.interceptors`, `ClientInterceptor`
and friends ("no consumer in the repo beyond a single test; every client
method already returns an `AsyncResult` that composes"). Two places kept
promising them: the README's feature list, where a reader would go
looking for an option that does not exist, and the handlers rule, which
told an agent the client has a mirror-image seam to the activity
middleware.

The rule now says why there is none, since that is the part worth
knowing: a client call hands you an `AsyncResult`, so wrapping it is
composing one — no registration, no hook. Activity middleware exists
because the platform invokes the activity and the application never
holds its `AsyncResult`.

Refs #374

Claude-Session: https://claude.ai/code/session_01GGixjxi5AQ2cNK62bBymfF
Copilot AI lite review requested due to automatic review settings September 2, 2026 12:42
@coderabbitai

coderabbitai Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 6256e910-2e4c-4afd-8c41-e010aeafa768

📥 Commits

Reviewing files that changed from the base of the PR and between 3724375 and fd09879.

📒 Files selected for processing (2)
  • .agents/rules/handlers.md
  • README.md

Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The documentation removes client interceptor references. It documents AsyncResult composition for client calls and middleware for activities.

Changes

Client interceptor documentation

Layer / File(s) Summary
Update client middleware documentation
.agents/rules/handlers.md, README.md
The handlers guide now describes AsyncResult composition and activity middleware. The Features list no longer includes client interceptors.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to fd098

This localized documentation change removes promises for a client-interceptor API that no longer exists; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: removing documentation that advertises client interceptors.
Linked Issues check ✅ Passed The PR addresses [#374] by documenting that client interceptors were removed and will not be re-added. It removes stale references and explains the supported AsyncResult composition model instead of i…
Out of Scope Changes check ✅ Passed The changes are limited to documentation updates related to client interceptors. The README and handler guidance directly support the stated objective for [#374].
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Full details: Linked Issues check

Explanation

The PR addresses [#374] by documenting that client interceptors were removed and will not be re-added. It removes stale references and explains the supported AsyncResult composition model instead of implementing per-contract interceptors.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/per-contract-interceptors-374

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are limited to documentation updates that correctly reflect the current client API surface and do not affect runtime behavior.

Pull request overview

Removes outdated documentation that still implied @temporal-contract/client supports client interceptors, aligning the docs and agent guidance with the current client surface (where calls already return AsyncResult and interception is done via composition at the call site).

Changes:

  • Updated README feature list to stop advertising “client interceptors” as a contract-aware capability.
  • Updated agent handler guidance to explicitly document that there is intentionally no client-interceptor seam, and why.
File summaries
File Description
README.md Removes “client interceptors” from the advertised feature set to match the current public API.
.agents/rules/handlers.md Replaces the outdated claim about TypedClient.create({ interceptors }) with an explanation of the deliberate absence of client interceptors and the recommended composition approach.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@btravers
btravers merged commit 05b50b2 into main Sep 2, 2026
14 checks passed
@btravers
btravers deleted the feat/per-contract-interceptors-374 branch September 2, 2026 12:48
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.

feat(client): allow per-contract interceptors — they are connection-scoped and cannot discriminate by contract

2 participants