[bandzalkin]

SDK replay repair onto main a96fb54d0c9e (2026-10-18)

Rebased onto upstream main a96fb54. New head: 8794cd2. Reconciled #811 teardown with the resolver: preserve Windows helper/job custody, executable resolution and bounded pipe handling; preserve POSIX stdin-EOF grace, TERM/KILL group settlement, PID-reuse protection, retryable unknown observations and redactor lifetime. Updated two subprocess test doubles to model returncode/is_closing; replaced signal-count assertions with a real unrelated-process liveness check and removed an incidental kill-before-EOF assertion while retaining descendant/workspace cleanup proof. The startup-cancellation follow-up is unchanged. The new unknown-PID-observation regression failed before the correction and passes after it. Existing credential-URL finding F1 remains open and is not waived by this replay repair: #797 (comment)

Verification on the combined six-PR candidate replay, not a standalone full-suite claim for this PR: macOS, Python 3.12.15; full pytest 9,191 passed, 746 skipped, 108 warnings; Ruff check/format and Pyrefly passed; root and echo-agent lock checks passed. Focused Codex teardown/privacy scope: 36 passed. Actual Codex CLI 0.163.0 initialized, answered model/list, exited with status 0 and left no owned process group. The complete pre-commit gate is delegated to the hosted replay, not claimed as locally executed.

Hosted replay #28 succeeded: all five pre-commit hooks passed; Linux Python 3.12 pytest reported 9,192 passed, 745 skipped, 107 warnings. The workflow published band-agent-patches-latest at 14ff3f96c13d08e57b00b547ec2e6ca168603cd0 and tag staged/2026-10-18-14ff3f96c13d. This does not clear the separate existing #797 credential-URL finding or approve PR merges.

Published with explicit old-head leases; hosted replay: https://github.com/bandzalkin/band-sdk-python/actions/runs/39123911370 . Its result and each PR’s fresh CI matrix are separate evidence. No PR merge or band-agent dependency-pin update. Earlier head and verification notes below are historical.

---

Rebased onto main d7ab3429cbdb (2026-10-15)

Rebased onto current main (after #709, #804, #789/#775, #796 and #802) as one commit; head e38e7ffb63dfd2daeeb415c9da3a6550d8ef9ed0. Integrated into #709's retained acquisition/release instead of restoring the removed lifecycle bookkeeping; shared core/retention and turn_lifecycle gain the cancellation-settlement hooks the resolver needs (default stop budget unchanged). After the first CI run, Codex close no longer fails with "process leader remained unreaped" when a forced kill's exit is reported late (seen on Windows 3.12): it waits for the reap, as callers are already bounded by the room owner's budget; regression test_forced_close_waits_for_an_exit_reported_late.

CHANGELOG.md edits are dropped: Release Please generates it.

Verified at the new head (macOS, Python 3.13): uv sync --all-packages --locked --extra dev, all pre-commit hooks, and the full pytest -q: 8708 passed, 738 skipped. Verification notes further down refer to the previous head.

---

Summary

Resolve room-scoped external MCP servers inside owned Codex connection startup rather than capturing ambient configuration before room startup/cleanup ownership exists. Related: #709.

Changes

• Resolve one fresh room snapshot and preserve Band's endpoint and launch precedence.
• Retain startup/readiness ownership through cancellation/failure; settle the owned process group before releasing resources and preserve failed cleanup for retry.
• Extend the shared TurnTask lifecycle at its existing owner, with cancellation/readiness and sibling-adapter regressions rather than an adapter-local scheduler.
• Update examples, operator docs and user-facing Unreleased entries.

Diagnostic correction

RPC method and payload contents are never logged. Non-JSON/non-object/unknown payload diagnostics report transport, type and length; stderr diagnostics report byte length; cleanup diagnostics report exception type. Queue-full warnings report only the dropped event kind. A synthetic credential matching the old protocol-identifier grammar was reproduced in logs, so that grammar and its helper were removed in 39c2d473. No redactor runs in transport log paths. Forwarded error freezing/redaction is preserved. No CodeQL alert is waived.

Test portability and observer corrections

The Windows held-resolver fixture matches the host-native workspace basename instead of a POSIX slash suffix. B retains its one-second startup bound while A remains held; A's admission/release are bounded and asserted. No product timeout increased.
The shared OpenCode wait_for test helper observes readiness before checking an expired deadline, fixing deterministically reproduced lost observations after a delayed poll. The one-second budget and 0.01-second interval are unchanged; false readiness at/after expiry fails without another wait. Four edge regressions remain. This is a test-helper change, not a shared product-runtime change; its exact failing coverage wake delay is unmeasured. Test-only changelog entries were removed.

Windows support boundary

ad176506 refuses stdio servers with nonempty env on Windows during snapshot validation, before constructing or connecting a client. The safe category is unsupported-platform: “secret-bearing stdio MCP servers require POSIX exec”. Windows env-free stdio, all HTTP servers (including secret headers/bearer tokens), and POSIX behavior are unchanged. Windows os.execvpe spawns and exits rather than replacing the owned process; quoting alone cannot preserve ownership. No Windows supervisor is included; an owned supervisor is a separate follow-up.
The refusal regression forces the platform predicate on every OS and checks the actual adapter construction boundary. The POSIX alias-launch/ownership case is skipped on Windows with that explicit reason; configuration-only POSIX tests force their platform, not the real launch case.

Stack and merge order

No prerequisite; based on upstream d7ab3429cbdb5381b9ca572cf7eac210a8196052.
Exact feature-only range: d7ab3429cbdb5381b9ca572cf7eac210a8196052..e38e7ffb63dfd2daeeb415c9da3a6550d8ef9ed0.
Merge #797 before #800, which includes this prerequisite. OMP and Copilot lanes are independent.

Testing

Final local correction scope: snapshot/launch, startup/recovery, 13 diagnostic cases and all 125 OpenCode consumers. Python 3.11.16: 265 passed / 1 eager-host skip; Python 3.12.14 and 3.13.14: 266 passed each. All five static hooks passed.
The method-privacy regression failed four cases before 39c2d473; all 13 diagnostics pass after it. Eighteen grammar/format cases were replaced with eight request/notification privacy cases. Actual parser output changed from disclosing a synthetic credential to event-kind-only warnings.
The Windows refusal regression failed before the guard and failed again when the guard was removed; the restored interpreter matrix passed. Actual rendered error: unsupported-platform: Codex MCP startup failed: secret-bearing stdio MCP servers require POSIX exec. The forced-Windows supported-transport case launches an env-free stdio target alongside secret-bearing HTTP without passing its aliases to that target.
Prior published 200293f3 had three Windows alias-launch failures, one Windows 3.13 PowerShell timeout and two high CodeQL alerts. The complete matrix and code-scanning clearance at this corrected head are awaited; the unchanged PowerShell test retains its timeout. No failure rerun, timeout relaxation or alert dismissal. Local proofs are not native-provider/live-room receipts or staged-package validation.

[bandzalkin]

CodeQL diagnostic findings

Fixing commits: bc0df019 removes wire-payload contents, 39c2d473 removes RPC method contents, and ad176506 adds the prelaunch Windows secret-bearing stdio refusal.

RPC frame/queue diagnostics retain only event-kind and length metadata: queue drops identify server requests or notifications; parse/unknown-frame diagnostics retain their transport/type labels and length. No payload or method content is logged by these diagnostics. Forwarded error freezing is unchanged.

Recorded CodeQL instances are fixed without dismissal. All seven bot threads below are resolved; the two existing replies remain. This summary replaces further per-thread replies.

• Alert 36: thread.
• Alert 37: thread.
• Alert 38: thread.
• Alert 39: thread.
• Alert 40: thread.
• Alert 41: thread.
• Alert 42: thread.

[bandzalkin]

Rebased onto current main (d7ab3429) at 81ed02c178b3; the description's new first section lists the conflict resolutions and the verification at this head.

[bandzalkin]

Pushed e38e7ffb63df: Codex close now waits for the reap after a forced kill instead of failing at 1s (Windows 3.12 CI failure); regression test added. Description updated.

[bandzalkin]

Scope

Re-review of 1b61d7ca1a440e9be0e86d51fb5cb3c6b2e59ba8 against d7ab3429cbdb5381b9ca572cf7eac210a8196052: rebased owned MCP acquisition, cancellation-settlement follow-up, launch credentials, readiness and shared lifecycle consumers. All earlier reviews and seven CodeQL-thread dispositions reconciled. GitHub requires a comment review on one's own PR.

Constraints

Room retention owns acquisition/release; startup cancellation must retain custody until settlement. Credential-bearing launch configuration must not bypass the secret-safe boundary through another descriptor field. Earlier transport-log repairs and the Windows secret-stdio refusal remain accepted.

Decisions

Keep the retained-acquisition integration and provider-owned cancellation callback rather than restoring adapter-local lifecycle bookkeeping. Actual readiness and teardown work on the installed native CLI. Correct the unguarded URL credential path before sign-off; protecting only header/bearer/env fields leaves another authentication carrier exposed.

Findings and proof

One new blocking High/P1 finding, F1, inline. A1, URL authentication bypassing secrecy: confirmed by snapshot/launch/serialization probes. A2, cancellation, settlement custody and sibling lifecycle regressions: 973 Codex/OpenCode/core/integration tests passed in locked Python 3.12 dev. A3, native launch/readiness/teardown: actual Codex 0.163.0 initialized a local stdio FastMCP server under an isolated CODEX_HOME, returned server info/capabilities/tools, kept its env grant out of argv, and closed its process group with return code 0. Earlier CodeQL findings are not reopened.

Current PR checks: 41 successful, one skipped. No live Band/provider requests, native Windows run or full-repository local gate was performed. Local smoke is not a staged-package or live-room receipt.

Architectural and overall verdict

HELD — not merge-ready. Fix F1's credential boundary and reverify; no additional architectural redesign is requested.

[bandzalkin]

F1 · [P1/High] Keep credential-bearing MCP URLs out of native argv and public descriptors

CodexMcpHttpServer accepts credential-bearing URLs unchanged, and this assignment copies the entire URL into the -c mcp_servers... process argument. A resolver adopting an endpoint such as https://user:password@host/mcp therefore exposes the password in the native command line. The same problem applies to common API-key query URLs. Unlike bearer/header/env values, URL credentials are also included in the descriptor's repr and supported Pydantic serialization, and are absent from _server_secret_values, so forwarded diagnostics do not redact them.

Confirmed at this head with synthetic userinfo and ?api_key= URLs: freeze_mcp_servers(...).launch(...) retains the credential in argv; repr(server) and TypeAdapter(CodexMcpHttpServer).dump_json(server) retain it; snapshot.redactor.text(url) leaves it unchanged. Existing secrecy tests use a noncredential URL and secrets only in the explicitly excluded fields, so they do not reach this authentication carrier. No real credentials or external requests were used.

Reject URL credentials/unsupported secret-bearing endpoints before launch and direct authentication to the protected bearer/header contract, or provide an equally secret-safe mechanism for credential-bearing endpoints. Cover generated argv and descriptor/diagnostic exposure at that boundary. High confidence; blocks sign-off.

[bandzalkin]

Superseded by #855, maintained from bandzalkin:feat/codex-owned-mcp-resolver because this upstream source branch is no longer writable by the author. The replacement includes the credential-URL correction from F1 and preserves the earlier diagnostic/ownership repairs. Review history remains here; closed without merging.