feat(relay): route relay handlers through work admission; producer publication drain

[OrcaWin]

	Files	Added	Deleted	Net
Test	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​478	0	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​478
Prod	10	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​158	$\color{#cf222e}{\Huge{\mathbf{−}}}$​38	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​120

ELI5

This is the next piece of the #16741 port. Relay request handlers now run through the work-admission gate from #24156, so a later shutdown or reset can drain in-flight work safely. Producer notifications also settle through the publication drain. Nothing in production starts a drain yet, so behavior is unchanged.

What Changed

• Commit 1: the dispatcher routes every relay handler through RelayWorkAdmission.
  • It adds beginWorkDrain(exclude?) and assertActiveWorkContext.
  • RequestContext gains an optional transportGeneration.
  • run returns the handler's own promise, so no extra microtask turn is added to each response.
• Commit 2: producer notifications settle through the publication drain, and RelayProducerPublicationDrain is added.
• Porting and wire notes: each commit body carries both. There are no new wire fields or opcodes, so mixed-version clients and relays are unaffected.

Why

Design D9: these are the first consumers of the T1 seam. T3 (shutdown and reset) and T4 (tunnels) will call the drains.

Linked Issue

Part of the Node runtime plan (Phase 3). Stacked on #24156.

Visual Proof

N/A. There is no behavior change.

Testing

• The ported tests pass, including dispatcher drain and admission.

• Existing relay handler tests that depend on response timing still pass.

• Next: the remaining T1 slices are the fs and git stream shutdown and the WebSocket transport lifecycle.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

[greptile-apps]

Confidence Score: 5/5

[High risk] Adds work admission gating and producer publication draining to relay handlers.

No new issue was identified that would prevent merging.

Summary

The PR routes relay request and notification handlers through work admission and adds a producer-publication drain that tracks write settlement. There have been no changes since the previous review, and no new findings were identified.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Relay handler] --> B[Work admission]
  B --> C[Admitted work]
  C --> D[Work drain waits for completion]
  E[Producer notification] --> F[Publication drain]
  F --> G[Dispatcher writer]
  G --> H[Write settlement]
  H --> F

Loading

_{Reviews (4) · Last reviewed commit: "feat(relay): settle producer notificatio..."}

[pullfrog]

ℹ️ No critical issues — two non-blocking observations below.

Reviewed changes

Both commits reviewed (70772a9587, 0f2363d76a; 13 files).

• Handler work admission — onRequest/onNotification now route every dispatcher handler through RelayWorkAdmission.run/runNotification; beginWorkDrain(exclude?) and assertActiveWorkContext(context) are exposed.
• run stops being async — returns the handler's own promise and tracks completion on the side, dropping an extra microtask from every relay response while keeping rejection-on-refusal.
• transportGeneration — stamped from client.generation on request and notification RequestContexts.
• pty.cancelDelivery — added to the drain request allowlist so cancellation stays executable mid-drain.
• Producer settlement — publishProducerNotification gains onSettled/settledTransportGeneration/isStillAdmitted and settles exactly once on every refusal path; assertSettledProducerTransport rejects replaced/closed generations and callback-less sinks.
• RelayProducerPublicationDrain — counts each publish until local write completion, re-checks admission at dequeue, and fails sticky on authority/transport change.
• Tests — dispatcher-level drain/admission and producer settlement/publication drain.

ℹ️ Drain admission never reopens — confirm the reset path

beginDrain latches draining = true and has no reset, so once this PR makes beginWorkDrain consumable, any caller that does not tear the dispatcher down afterward leaves every later request rejected (relay_work_admission_closed) and every later notification silently dropped. That is clearly correct for the terminal shutdown path, but the PR body advertises "shutdown or reset", and a reset that reuses the live dispatcher would break it. Worth confirming which path T3 takes.

Technical details

# One-way drain admission

## Affected sites
- `src/relay/relay-work-admission.ts:59` — `this.draining = true` is set but never cleared.
- `src/relay/dispatcher-client-state.ts:68` — `beginWorkDrain` exposes it on the dispatcher.

## Required outcome
- Confirm the reset caller either rebuilds the dispatcher/admission, or add a reopen path; otherwise post-reset work is permanently refused.

## Open questions for the human
- Is `beginWorkDrain` intended only for the shutdown path (`RelayGraceLifecycle.shutdown` eventually calls `process.exit(0)`), with reset constructing a fresh admission?

ℹ️ Nitpicks

• The abstract enqueueLeasedFrame in dispatcher-client-state.ts still declares five parameters while the concrete dispatcher-producer-transport.ts implementation adds the new optional isStillAdmitted sixth. Since the base is the only contract for the mixin chain, syncing the declaration keeps the abstract honest.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f1588871-91c3-416f-b421-fd3a1211d56f

📥 Commits

Reviewing files that changed from the base of the PR and between 3aa2d3a and 808078a.

📒 Files selected for processing (13)

• src/relay/dispatcher-client-state.ts
• src/relay/dispatcher-client-writer.ts
• src/relay/dispatcher-contract.ts
• src/relay/dispatcher-frame-codec.ts
• src/relay/dispatcher-notification-publication.ts
• src/relay/dispatcher-producer-settlement.test.ts
• src/relay/dispatcher-producer-transport.ts
• src/relay/dispatcher-rpc-routing.ts
• src/relay/dispatcher-work-drain.test.ts
• src/relay/relay-producer-publication-drain.test.ts
• src/relay/relay-producer-publication-drain.ts
• src/relay/relay-work-admission.ts
• src/shared/relay-work-drain-contract.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Request handlers and notifications now use work admission, and request contexts include a transport generation. The dispatcher adds drain checks and permits pty.cancelDelivery during a drain. Producer notification publication now validates transport state, checks live admission, and reports write settlement. RelayProducerPublicationDrain tracks publication outcomes and includes tests for settlement, failures, transport replacement, and ownership changes.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 80807

The relay drain and publication-settlement changes have no identified merge-blocking issue and are ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 80807

The reviewed paths preserve ownership and connection-identity checks without demonstrating a new authorization bypass. Production shutdown/reset activation is deferred. Full rollout and recovery behavior remains unverified, so the assessment is low rather than minimal risk.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated scope is registered handler work and outgoing publications within a relay dispatcher and its connected client transports. Remote method and parameter inputs still reach registered handlers; transport generation and publication-authority callbacks are supplied internally. Wider tenant, service, credential, or deployment exposure was not established.

Trust Boundaries and Controls

• observed — The publication drain checks injected producer authority together with captured client identity and transport generation. Queued authority or transport assertion failures become a false admission result, causing retirement rather than a sink write. Existing PTY publication admission is combined with, not replaced by, the new predicate.

Resilience and Maintainability Implications

• inferred — The existing writer invokes live-admission predicates outside its write exception handler. A directly throwing internal predicate can skip normal settlement and lease release after selection. The supplied publication drain catches its assertion failures, so this is a latent failure-containment limitation, not an established new external attack path or introduced security finding.

Hardening Proposals

• proposed — Before adding further production callers, consider converting live-admission callback exceptions into failed settlement with exact-once lease release, so future authority-check implementations cannot strand publication capacity.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 13 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the two primary changes: routing relay handlers through work admission and adding producer publication drain support.
Description check	✅ Passed	The description covers the main required sections, including ELI5, changes, rationale, issue context, visual proof, testing, and the agent-skill boundary. It also records manual testing and automated …

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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