feat: add native RPC client and wire types by bandzalkin · Pull Request #897 · band-ai/band-sdk-python

bandzalkin · 2026-10-21T15:28:33Z

Why

A native OMP adapter needs direct access to sessions, model selection, host tool calls and structured UI requests without changing the ACP transport used by existing omp_acp callers.

What

Add the persistent JSONL OmpRpcClient, typed wire models and native launch/MCP plugin support, plus per-room native session history conversion and the omp extra.
Support RPC v2 negotiation, chunk reassembly, correlated commands, host tools and structured ask questions/answers, including empty answer selections. Keep protocol tags and plugin command names derived from their shared definitions.
Bound the event backlog at 256 entries. Overflow fails the turn and retires the transport; slow event callbacks do not block correlated RPC control traffic.
Preserve process custody through cancellation and failed close. Capture identity-checked, discoverable descendants before and across EOF, and retain ownership until cleanup completes.

Scope

Transport and process ownership only; the native adapter is the next PR. POSIX descendant cleanup is best-effort ownership tracking, not a sandbox or a guarantee against unobservable process escape. Existing omp_acp behavior and its public adapter remain supported. Base: main.

linear-code · 2026-10-21T15:41:10Z

INT-1890

Source-Base: d099198 Source-Head: 68580bc Upstream-PR: band-ai#897

…s tree close() killed the child's session with SIGKILL before closing stdin, so Codex never saw the EOF it exits on and could do nothing at shutdown. Stdin now closes first and the session gets one second to end on its own; the tree kill remains the fallback. The session id stays reserved while any member lives, so a launcher shim's real CLI child that keeps the room directory is still reaped.

The ACP bridge hides OMP's native lifecycle: sessions, model and thinking selection, host tool calls, permission dialogs and ask questions. This adds the transport a native adapter needs: OmpRpcClient (JSONL RPC over the omp process stdio, with process-tree reaping), typed wire frames, the native launch settings and SDK-owned MCP plugin projection in band.integrations.ompprocess, and the per-room session converter. The shared normalizer gains a namespace keyword so plugin-projected tool names map back to Band tools; omp_acp's spellings are unchanged.

Source-Base: a3c187e Source-Head: 0b6c1e4 Upstream-PR: band-ai#897

AlexanderZ-Band

Blocking findings cover process-tree cleanup, event backpressure, UI ask payloads, and test synchronization; see inline comments.

AlexanderZ-Band · 2026-10-22T05:41:23Z

Could we make the two OMP transports an intentional supported surface? I want to keep the actual OMP-over-ACP wire path alongside the native RPC path. The Python import/API name is not part of that requirement and may change; please do not keep a compatibility wrapper solely to preserve it.

Upstream OMP exposes distinct modes: omp acp speaks ACP, while omp --mode rpc uses its custom JSONL protocol (not JSON-RPC 2.0) (RPC protocol docs at the pinned source revision). I checked the source revision resolved by this PR's @oh-my-pi/pi-coding-agent@18.8.9 pin: it contains separate ACP mode and RPC mode implementations. Keeping both wire paths therefore appears feasible at the pinned version; the native adapter complements the ACP path rather than replacing its protocol.

Definition of done for maintaining both OMP transports

Document which Band-facing capabilities and limitations belong to each transport, including supported OMP versions and configuration.
Keep behavioral coverage at each real wire boundary. Cover the shared promises on both paths: ordinary replies, Band/custom tools, allow/deny, workspace and room-scoped MCP, multi-turn behavior, cancellation, background-work settlement, and cleanup. Cover asks/activity on the native path where supported; explicitly record protocol-specific gaps rather than assuming parity.
Keep live OMP baseline coverage for both transports and require the native backend lane to pass on supported CI platforms. The current feat: add native RPC client and wire types #897 Windows run still leaves descendants alive after native client.close(); resolve that before treating native lifecycle coverage as complete.
Keep the generic ACP integration and OMP-specific ACP wire support, while allowing the Python adapter/config API to be renamed or changed. Revisit removing ACP support only if its wire-level use case is deliberately ended and migration is planned.

#898, #899, #900, #902, and #907 cover native RPC work. Please track the dual-transport support matrix and the remaining parity/lifecycle work as explicit follow-ups with owners.

Source-Base: a3c187e Source-Head: 0f8e226 Upstream-PR: band-ai#897

AlexanderZ-Band · 2026-10-22T09:13:44Z

Root cause found on this exact PR head: WindowsProcess.close() calls TerminateJobObject() and then Job.wait_until_idle() treats ActiveProcesses == 0 as proof that every member has exited; OmpRpcClient.close() returns after that path. The Windows 3.13 logs show seven failures (five process-tree cases and two generated lifecycle cases), all observing owned descendants still alive after cleanup returned.

That counter is not a documented completion barrier after forced termination. TerminateJobObject behaves like TerminateProcess, whose completion must be established by waiting on process handles; Microsoft staff has also reproduced ActiveProcesses == 0 while a descendant process handle remains unsignaled. This explains the observed early return. TerminateJobObject documentation, Microsoft staff confirmation and reproduction.

AlexanderZ-Band

Requesting changes on the four blocking items above; suggestions and no-fix records are non-blocking context.

AlexanderZ-Band

One non-blocking maintainability suggestion; the frame-size boundary hypothesis was checked against pinned OMP 18.8.9 and dropped.

Fail closed on unmatched host frames and MCP startup UI, log transport failures, and track Windows descendants with the same env-marker scope POSIX already used. Shared process helpers now live with the Windows job helpers instead of the Codex client. Co-authored-by: Cursor <cursoragent@cursor.com>

AlexanderZ-Band · 2026-10-22T09:44:25Z

On the Windows descendant cleanup in CI run 39746971790: CONFIRMED. 13ee3f3 keeps OmpProcessScope on Windows, stamps the same env marker the POSIX path already used, and reaps marked descendants after the job closes. That is ownership tracking for discoverable children, not a redesign of Windows job assignment. I did not re-run the Windows job locally.

On dual-transport: out of scope for this transport-only ticket (INT-1890). Existing omp_acp is unchanged; the native adapter and the later stack PRs are the place for the dual-transport matrix.

Co-authored-by: Cursor <cursoragent@cursor.com>

The previous guidance named coverage in the abstract, so reviews treated plumbing assertions, private patches, and wall-clock grace as style nits instead of blocking test-quality issues. Co-authored-by: Cursor <cursoragent@cursor.com>

The reader must honor negotiated frame limits immediately, stdin close must wait for in-flight writes, and fail-closed parse errors must log through the transport. Tests now inject public close/join seams and cover the complementary settlement and cancel branches. Co-authored-by: Cursor <cursoragent@cursor.com>

Group D kept finding leftover OS skips, in-test ifs, and private seams as a family. Drive those scenarios from public hooks, OS tables, and shared peer constants so each branch is a named claim. Co-authored-by: Cursor <cursoragent@cursor.com>

Nested jobs and breakaway look like missing features on this path. They are out of scope while close() drains the existing kill-on-close job. Co-authored-by: Cursor <cursoragent@cursor.com>

Overflow now records the transport ERROR, protocol failures omit frame payloads from logs, and close no longer waits on a drain that holds the write lock. Co-authored-by: Cursor <cursoragent@cursor.com>

…logy Co-authored-by: Cursor <cursoragent@cursor.com>

Record root-exit as a transport failure, join callbacks that start during the wait, and stop treating an empty UI reply as cancelled. Tighten plugin and converter tests onto product names. Co-authored-by: Cursor <cursoragent@cursor.com>

create_process wrappers that used create_subprocess_exec skipped the Windows job, so generated restart sequences could not reap descendants. Co-authored-by: Cursor <cursoragent@cursor.com>

Keep the wire vocabulary in one enum and send commands through the same frame encoder as host-tool replies. Co-authored-by: Cursor <cursoragent@cursor.com>

A five-second connection timeout was proving Windows children had arrived. Identify them as descendants of the current root instead. Co-authored-by: Cursor <cursoragent@cursor.com>

Source-Base: 0d5d8de Source-Head: 0b0620e Upstream-PR: band-ai#897

bandzalkin force-pushed the feat/omp-native-rpc-client-INT-1890 branch from 78aa0de to 0c68d15 Compare October 21, 2026 15:35

bandzalkin mentioned this pull request Oct 21, 2026

fix(integrations): preserve native OMP lifecycle and prompt authority #763

Closed

bandzalkin added this pull request to stack #903 October 21, 2026 15:41

bandzalkin force-pushed the feat/omp-native-rpc-client-INT-1890 branch from 0c68d15 to 68580bc Compare October 21, 2026 16:13

bandzalkin added a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 21, 2026

feat(omp): add native RPC client and wire types

2adc16d

Source-Base: d099198 Source-Head: 68580bc Upstream-PR: band-ai#897

bandzalkin force-pushed the feat/omp-native-rpc-client-INT-1890 branch from 68580bc to 4b01955 Compare October 21, 2026 17:52

bandzalkin force-pushed the feat/omp-native-rpc-client-INT-1890 branch from 4b01955 to 0b6c1e4 Compare October 21, 2026 17:56

bandzalkin added a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 21, 2026

feat(omp): add native RPC client and wire types

730a176

Source-Base: a3c187e Source-Head: 0b6c1e4 Upstream-PR: band-ai#897

github-actions Bot pushed a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 21, 2026

feat(omp): add native RPC client and wire types

018e51a

Source-Base: a3c187e Source-Head: 0b6c1e4 Upstream-PR: band-ai#897

github-actions Bot pushed a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 21, 2026

feat(omp): add native RPC client and wire types

303c785

Source-Base: a3c187e Source-Head: 0b6c1e4 Upstream-PR: band-ai#897

github-actions Bot pushed a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 21, 2026

feat(omp): add native RPC client and wire types

a0c1f22

Source-Base: a3c187e Source-Head: 0b6c1e4 Upstream-PR: band-ai#897

bandzalkin added 2 commits October 22, 2026 07:13

fix: retain native process containment and prompt admission ownership

81c3a14

fix: reap discoverable native detached descendants

259d1eb

AlexanderZ-Band requested changes Oct 22, 2026

View reviewed changes

bandzalkin added 2 commits October 22, 2026 09:27

fix: bound native event delivery and complete wire contracts

f696f09

fix: serialize structured native answers in the transport

0f8e226

bandzalkin changed the title ~~feat(omp): add native RPC client and wire types~~ feat: add native RPC client and wire types Oct 22, 2026

bandzalkin marked this pull request as ready for review October 22, 2026 08:20

bandzalkin requested a review from amit-gazal-band as a code owner October 22, 2026 08:20

github-actions Bot pushed a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 22, 2026

feat: add native RPC client and wire types

7da5cec

Source-Base: a3c187e Source-Head: 0f8e226 Upstream-PR: band-ai#897

AlexanderZ-Band requested changes Oct 22, 2026

View reviewed changes

AlexanderZ-Band reviewed Oct 22, 2026

View reviewed changes

Comment thread src/band/integrations/ompprocess.py Outdated

Comment thread src/band/integrations/omprpc/client.py

refactor: compare MCP startup events to OmpEventType

28ba70a

Co-authored-by: Cursor <cursoragent@cursor.com>

AlexanderZ-Band and others added 10 commits October 22, 2026 19:51

docs: record Windows job containment as closed unless close leaks

e1adbd6

Nested jobs and breakaway look like missing features on this path. They are out of scope while close() drains the existing kill-on-close job. Co-authored-by: Cursor <cursoragent@cursor.com>

fix: fail-close native RPC overflow, logs, and blocked-drain close

1abe091

Overflow now records the transport ERROR, protocol failures omit frame payloads from logs, and close no longer waits on a drain that holds the write lock. Co-authored-by: Cursor <cursoragent@cursor.com>

refactor: assert the backlog ERROR itself instead of a filtered tauto…

d621cb4

…logy Co-authored-by: Cursor <cursoragent@cursor.com>

fix: preserve Windows ownership for injected native process creation

7243890

test: allow backlog overflow to retire before acknowledgement

e893e4c

fix: recheck native process group identity before claiming custody

cbba792

test: cover immediate ready limits and platform-neutral EOF custody

f73c2be

AlexanderZ-Band marked this pull request as ready for review October 22, 2026 16:54

Base automatically changed from feat/fix-preserve-accepted-turn-finalizers-during-shutd-INT-1904 to main October 22, 2026 17:45

bandzalkin and others added 8 commits October 22, 2026 21:21

fix: bound native callback retirement and preserve response identity

8756ce8

chore: integrate merged shutdown ownership scenarios

959e410

test: initialize callback retirement clock before native startup

c4ae7d8

fix: keep Windows native RPC tests on the owned job spawn

a65aea2

create_process wrappers that used create_subprocess_exec skipped the Windows job, so generated restart sequences could not reap descendants. Co-authored-by: Cursor <cursoragent@cursor.com>

refactor: type native RPC command names and host-tool results

7b4857a

Keep the wire vocabulary in one enum and send commands through the same frame encoder as host-tool replies. Co-authored-by: Cursor <cursoragent@cursor.com>

fix: wait for owned descendants on the gate event, not a clock

b39b41a

A five-second connection timeout was proving Windows children had arrived. Identify them as descendants of the current root instead. Co-authored-by: Cursor <cursoragent@cursor.com>

test: keep owned launch doubles in the shared RPC kit

0b0620e

bandzalkin force-pushed the feat/omp-native-rpc-client-INT-1890 branch from 614a7b5 to 0b0620e Compare October 22, 2026 21:11

github-actions Bot pushed a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 22, 2026

feat: add native RPC client and wire types

68bb474

Source-Base: 0d5d8de Source-Head: 0b0620e Upstream-PR: band-ai#897

github-actions Bot pushed a commit to bandzalkin/band-sdk-python that referenced this pull request Oct 22, 2026

feat: add native RPC client and wire types

344520d

Source-Base: 0d5d8de Source-Head: 0b0620e Upstream-PR: band-ai#897

AlexanderZ-Band approved these changes Oct 23, 2026

View reviewed changes

AlexanderZ-Band enabled stacked auto-merge October 23, 2026 02:10

AlexanderZ-Band added this pull request to the merge queue Oct 23, 2026

Merged via the queue into main with commit fd241f0 Oct 23, 2026
35 checks passed

AlexanderZ-Band deleted the feat/omp-native-rpc-client-INT-1890 branch October 23, 2026 02:35

band-release-please-public Bot mentioned this pull request Oct 23, 2026

chore: release main #876

Open

Conversation

bandzalkin commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Scope

Uh oh!

linear-code Bot commented Oct 21, 2026

Uh oh!

AlexanderZ-Band left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexanderZ-Band commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexanderZ-Band commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexanderZ-Band left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexanderZ-Band left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexanderZ-Band commented Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bandzalkin commented Oct 21, 2026 •

edited

Loading

AlexanderZ-Band commented Oct 22, 2026 •

edited

Loading

AlexanderZ-Band commented Oct 22, 2026 •

edited

Loading