Skip to content

feat(relay): port the #16741 T1 seam (work drain, publication drain, release gate) - #24156

Merged
OrcaWin merged 1 commit into
mainfrom
OrcaWin/node-rt-p3-t1
Oct 1, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
OrcaWin/node-rt-p3-t1

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​387 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​387
Prod 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​251 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​250

ELI5

This is the first slice of porting #16741 (SSH and remote convergence) onto today's code. It's a small, inactive base that later slices build on: a work-drain contract, a publication drain, a call-queue retirement option, and the opt-in switch for live terminal transfer. Nothing uses it yet except tests.

What Changed

  • Added:
    • pty-ownership-transfer-release-gate.ts, taken as-is.
    • runtime-rpc-call-queue with holdIdleSelectors and RuntimeRpcCallQueueBusyError.
    • relay-work-drain-contract.ts, adapted: T3's reset methods and T4's tunnel methods are left out until those contracts exist.
    • The relay publication and work drain.
  • Promise.withResolvers is replaced, because the relay bundle targets node18.
  • No wire changes and no behavior changes. The porting note is in the commit body.

Why

Design D9: port #16741 by subsystem, starting with the T1 seam that T2 and later tracks import. Live transfer (T7) stays out, per D9.2.

Linked Issue

Part of the Node runtime plan (Phase 3). Files were chosen from the #24137 manifest. Stacked on OrcaWin/node-rt-p2-integration.

Visual Proof

N/A. Nothing is wired to this code yet.

Testing

  • 377 test lines are ported with the code, and they pass. pnpm tc and the quality gates pass.

  • Next: the dispatcher-wiring and relay producer-drain slices are the first real consumers. They also need to decide whether pty.cancelDelivery belongs in the drain allowlist.

  • 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

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Adds work admission and drain gates to the relay runtime.

The PR appears safe to merge; no actionable new issue or outstanding previous finding was established.

Summary

The PR adds an inactive relay work-drain seam, publication-write tracking, a PTY transfer canary gate, and runtime RPC selector retirement.

  • Follow-up changes give worktree.rm a separate long-wait queue lane and ensure selector retirement treats that lane as busy.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Request[Runtime RPC call] --> Queue{Method}
  Queue -->|worktree.rm| LongWait[Long-wait selector lane]
  Queue -->|other methods| Ordinary[Ordinary selector lane]
  LongWait --> Hold[Idle-selector retirement check]
  Ordinary --> Hold
  Hold -->|both idle| Held[Hold selector until released]
Loading

Reviews (4) · Last reviewed commit: "feat(relay): land the #16741 T1 seam (wo..."

@pullfrog pullfrog Bot 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.

✅ No new issues found.

Reviewed changes

This is the T1 seam port of #16741 (64c6db20b0) onto the P2 integration branch: new, currently-unwired relay drain primitives plus their ported tests. I diffed every added file against the upstream source.

  • relay-work-admission.ts — behavior identical to upstream; the only edit factors trackEntry out so run/runNotification avoid Promise.withResolvers for the Node 18 relay bundle.
  • relay-work-drain-contract.ts — upstream allowlists minus the T3 owner-reset and T4 network-tunnel methods, as the port note says. Every kept literal matches a real registered method on this branch.
  • runtime-rpc-call-queue.ts — adds RuntimeRpcCallQueueBusyError, heldSelectors, holdIdleSelectors, and the top-of-enqueue held check; identical to upstream. Inert while nothing holds a selector, so the live queue is unchanged.
  • transport-publication-drain.ts — logic identical to upstream, same withResolvers removal.
  • pty-ownership-transfer-release-gate.ts — taken as-is.
  • Tests — 34 new tests pass; the existing queue, transport-error-agreement, and call-routing tests still pass; pnpm run check:code-quality:changed is clean.

ℹ️ The new busy code has no downstream classification yet

runtime_rpc_queue_busy is a new wire-visible structured error code, but it is not a member of RECOVERABLE_CODES and runtimeEnvironmentCallFailure only passes through RemoteRuntimeClientError / RuntimeRpcCallQueueOverloadError. Nothing is wired in this PR, so it cannot fire yet — flagging it as a gap for the dispatcher-wiring slice rather than a defect here.

Technical details
# Busy-code classification is a follow-up for the wiring slice

## Affected sites
- `src/shared/remote-runtime-client-error-classification.ts:8` — `RECOVERABLE_CODES` does not include `runtime_rpc_queue_busy`.
- `src/main/ipc/runtime-environment-connectivity-handlers.ts:205` — `runtimeEnvironmentCallFailure` returns `null` for anything that is neither `RemoteRuntimeClientError` nor `RuntimeRpcCallQueueOverloadError`, so a `RuntimeRpcCallQueueBusyError` from `enqueue`/`holdIdleSelectors` is not formatted as a structured `RuntimeRpcFailure`.

## Required outcome
- When `holdIdleSelectors` is wired into the runtime call path, decide whether busy rejections are retryable on the client (add to `RECOVERABLE_CODES` or a distinct handler) or should surface as a hard failure, and cover it in `remote-runtime-transport-error-agreement.test.ts`.

## Open questions for the human
- Should `runtime_rpc_queue_busy` be treated like `runtime_rpc_queue_overloaded` (retry) or as a permanent "routing is changing" signal?

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…release gate)

Dormant seam for the Phase 3 port of #16741 (design D9.3): the shared
contracts later T1/T2 slices build on. No production module imports any of
it yet; only tests do. Existing behavior is unchanged: the call-queue hold
check is a no-op until something calls holdIdleSelectors.

Adds:
- RelayWorkAdmission + relay-work-drain-contract: closes relay handler
  admission and waits for in-flight work, still admitting the cleanup
  requests/notifications (cancel, acks, unwatch) that let that work finish.
- TransportPublicationDrain: counts local write completions against one
  transport and fails sticky once that transport is replaced.
- pty-ownership-transfer-release-gate: exact-match opt-in
  (ORCA_ENABLE_PTY_OWNERSHIP_TRANSFER_MUTATION=1) that SSH core uses to keep
  live transfer off (D9.2).
- RuntimeRpcCallQueuePool.holdIdleSelectors + RuntimeRpcCallQueueBusyError:
  atomically fence idle selectors while routing changes.

Porting note (source: #16741 head a68b6f3, merge-base 277c289; T1 per
the #24137 track manifest):
- Taken verbatim: pty-ownership-transfer-release-gate(.test),
  runtime-rpc-call-queue delta (+ retirement test), relay-work-admission
  API and behavior, transport-publication-drain API and behavior.
- Adapted: relay-work-drain-contract omits relay.reset,
  relay.recoverPreparedReset (T3) and relay.networkTunnel.close/.frame
  (T4); those contracts do not exist on this base and join the sets when
  their tracks land. Promise.withResolvers replaced with plain promises in
  relay-reachable code, because the legacy relay bundle still targets
  node18 hosts. RequestContext.transportGeneration is not added here (it
  belongs with the dispatcher slice), so the admission test drops it.
  Added transport-publication-drain.test.ts, since #16741 covered it only
  through relay-producer-publication-drain.
- Dropped: the pty-source-credit-contract/-validation/-record/ledger-test
  deltas. They only carry the ownershipTransfer output envelope, which is
  live-transfer engine (T7, deferred per D9.2), so the credit contract on
  this base is already the seam T2 needs. The post-slice
  assertPtySourceSpan re-validation came with that envelope and is
  dropped with it.
- Not in this slice (next T1 slices, which are also the first consumers):
  dispatcher wiring (dispatcher-client-state / rpc-routing /
  producer-transport / notification-publication +
  RequestContext.transportGeneration) that constructs RelayWorkAdmission;
  relay-producer-publication-drain on TransportPublicationDrain; fs/git
  stream shutdown; ws-transport + node-websocket-lifecycle +
  websocket-transport-limits; renderer flow-controller deltas. Later
  consumers: T2 ssh-pty-preparation-admission (drain contract),
  ssh-pty-source-preparation-drain (release gate); T4 tunnels
  (publication drain); T5 runtime-environment-call-queue (selector hold).

No wire change: no new RPC fields, methods, or opcodes.
@OrcaWin
OrcaWin force-pushed the OrcaWin/node-rt-p3-t1 branch from db0ab2c to c0395a8 Compare October 1, 2026 12:33
@OrcaWin
OrcaWin changed the base branch from OrcaWin/node-rt-p2-integration to main October 1, 2026 12:38
@OrcaWin OrcaWin closed this Oct 1, 2026
@OrcaWin OrcaWin reopened this Oct 1, 2026

@pullfrog pullfrog Bot 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.

✅ No new issues found.

Reviewed changes

Since the prior pullfrog review (215021d, ✅ no issues), one commit landed — a rebuilt c0395a8b — and its delta is confined to runtime-rpc-call-queue.ts and its retirement test.

  • Made holdIdleSelectors count a selector's long-wait lane as busy, so a hold cannot be acquired while a worktree.rm call is still in flight.
  • Extracted the ${selector}\0long-wait key into a shared longWaitQueueKey helper, now used by both enqueue and the busy check.
  • Added a retirement test proving a selector whose only in-flight call is in the long-wait lane is refused, and becomes holdable once it settles.

This is a correct branch-specific adaptation: upstream 64c6db20b0 has no long-wait lane and checks only the plain selector key, but this base routes worktree.rm through a ${selector}\0long-wait queue, so the earlier check could acquire a hold while a long-wait call was active. The busy check remains synchronous and atomic, the new test fails without the fix, and the queue/retirement suites pass (14 tests).

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@OrcaWin
OrcaWin merged commit 3aa2d3a into main Oct 1, 2026
55 checks passed
OrcaWin added a commit that referenced this pull request Oct 2, 2026
* Revert "feat(orcad): source-side dormant export of a relay-hosted SSH target (#16741 T6-8) (#24519)"

This reverts commit 783101b.

* Revert "feat(ssh): update, roll back, recover and stop a managed orcad server (#16741 T6-5 follow-up) (#24463)"

This reverts commit 38c2d1d.

* Revert "feat(ssh): deploy and pair an empty managed orcad server over SSH (#16741 T6-5) (#24453)"

This reverts commit 8b76683.

* Revert "fix(ssh): orcad GC honors the activation journal; readiness requires proven daemon coverage (#16741 T6 follow-up) (#24451)"

This reverts commit d3f8c50.

* Revert "feat(ssh): remote orcad stop by request file and journaled decommission (#16741 T6-4) (#24449)"

This reverts commit 43d9b43.

* Revert "feat(orcad): supervisable server: stop requests, managed stop receipts and a lifetime that keeps its lock on failed teardown (#16741 T6-3) (#24433)"

This reverts commit b093d3a.

* Revert "feat(ssh): crash-safe orcad activation, rollback and recovery (#16741 T6-2) (#24423)"

This reverts commit 1a9ac0e.

* Revert "feat(runtime): SSH access links for paired servers in a downgrade-safe sidecar (#16741 T5-1+T5-2) (#24420)"

This reverts commit 99db2bf.

* Revert "feat(relay): capability-gated owner reset with a durable preparation journal (#16741 T3 R1) (#24418)"

This reverts commit 34a582b.

* Revert "feat(ssh): track connection-manager drains, test probes and provider continuations (#16741 T2 P3+P8a) (#24407)"

This reverts commit d53063d.

* Revert "feat(daemon): idle retirement, session census and recovery-only provider (#16741 T2 P4b) (#24409)"

This reverts commit ff212db.

* Revert "feat(ssh): add pty.resumeClient and split SSH PTY process listing (#16741 T2 P5+P6) (#24414)"

This reverts commit 92cb717.

* Revert "feat(relay): await owned watcher and agent children on shutdown (#16741 T2 P1) (#24400)"

This reverts commit 6b36e4f.

* Revert "feat(session): retry failed renderer session writes and verify local folder PTYs (#16741 T2 P9) (#24406)"

This reverts commit d23ecef.

* Revert "feat(ssh): remote orcad primitives on the pinned Node runtime (#16741 T6-1) (#24419)"

This reverts commit dd87ae5.

* Revert "fix(runtime): fence runtime-environment subscriptions and status probes by identity (#16741 T5-3) (#24421)"

This reverts commit ece9e4d.

* Revert "feat(orcad): migration manifest and dormant-state contracts (#16741 T6-7) (#24422)"

This reverts commit 3fbdaba.

* Revert "feat(ssh): wire SshConnection through the work and transport close ledgers (#16741 T2 P2) (#24401)"

This reverts commit 4e8edc8.

* Revert "feat(profiles): carry markdown frontmatter visibility in project transfers (#16741 T2 P7) (#24405)"

This reverts commit 60c9326.

* Revert "fix(runtime): project the PTY incarnation onto mobile session tabs (#24413)"

This reverts commit 99e0303.

* Revert "feat(daemon): tag daemon stream data with the PTY incarnation id (#16741 T2 P4a) (#24402)"

This reverts commit 817af76.

* Revert "feat(ssh): port the SSH connection work ledger and transport close ledger (#16741 T2) (#24210)"

This reverts commit c991893.

* Revert "feat(relay): fence and drain file and git response streams on shutdown (#24185)"

This reverts commit dc08ffe.

* Revert "refactor(runtime-rpc): extract the Node WebSocket lifecycle; opt-in pinned port (#24186)"

This reverts commit a789233.

* Revert "feat(relay): route relay handlers through work admission; producer publication drain (#24181)"

This reverts commit 0b812bd.

* Revert "feat(relay): land the #16741 T1 seam (work drain, publication drain, release gate) (#24156)"

This reverts commit 3aa2d3a.

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
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