Skip to content

cmux-rd: one typed app line for cmux rd serve|connect, read back through a C line reader (ABI 5; Mac Screen step A) - #35828

Merged
lawrencecchen merged 2 commits into
feat-cmux-nextfrom
swift-logic/macscreen-rd-lines
Nov 4, 2026
Merged

lawrencecchen merged 2 commits into
feat-cmux-nextfrom
swift-logic/macscreen-rd-lines

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

First Rust move for the tip's swift-logic red (lead order after 18:00Z; the task was offered to Abdulaziz Albahar in STATUS 12204 first). The Mac Screen's cmux rd serve|connect stdout lines had two definitions: json! literals in cmux-rd-host and a hand decoder in Swift (MacScreenRdMessage, MacScreenRdLines). This PR makes Rust the only definition and gives the app a C reader for it. It is step A of the two-step app FFI path: no Swift caller of the new C ABI until the app FFI release that includes it is pinned.

  • cmux-rd-proto app_line (feature serde): AppLine and LinePath, the line shape, and LineSplitter, the stdout byte splitter. Golden vectors: tests/vectors/app_lines.json (every line kind of the old Swift decoder: key, candidate, candidates_done, connected, listening, ended, path with kind and rtt_ms, displays with current, the displays error, plus an unknown line, a non-JSON line and a field of the wrong type).
  • cmux-rd-host serve.rs and connect.rs print AppLine values. Same fields; the key order of a line now follows the struct (no reader depends on key order).
  • cmux-rd-ffi: CmuxRdLineReader (cmux_rd_line_reader_new, _free, _push, _next) and CmuxRdLine, ABI 4 -> 5. No I/O, no threads, panics caught, a panic poisons only that reader.
  • CmuxNextMacScreenTests (new test target, MacScreenRdLineTests): the Mac Screen's current Swift decoder reads every golden vector as its want. That is the behavior-neutral proof: for every line the host prints, the Rust reader gives the Swift code the same values it reads today.

Step B (written and on branch swift-logic/macscreen-rd-lines-b, head 306b582, not in this PR): RemoteRdLineReader (CmuxNextRemoteView, a thin wrapper over cmux_rd_line_reader_*), MacScreenRdProcess reads lines through it, MacScreenRdMessage and MacScreenRdLines are deleted, and MacScreenRdLineTests feeds the vectors through the Rust reader. It opens as its own PR after this PR lands, app-ffi-release.yml publishes the release and the re-pin PR lands. Not in scope: MacScreenRdDocument and MacScreenRdServers (the cmux.rd/1 offer and answer text format). No Rust code owns that format today, so they stay Swift.

Numbers

swift-logic-lines.py, swift_logic_lines (mirror_lines 71,927 everywhere; missing 15, invalid 0):

Revision swift_logic_lines
base 2d102f8 415,764
this PR, c6450d9 415,764 (step A adds no Swift logic)
step B branch, 306b582 (4 PLATFORM rows included) 415,621 (-143)

Step B deletes MacScreenRdMessage (23) and MacScreenRdLines (15). MacScreenRdProcess (105) becomes process plumbing only, and its new row says so with the FFI call (lead rule c). The new wrapper RemoteRdLineReader (80) gets PLATFORM rows like the other RemoteRd* cmux-rd-ffi wrappers. This move alone does not turn swift-logic green: the tip is +546 over 415,218, and step B gives -143.

Changelog

none

Observed / Not run

Observed, fleet steps on c6d9ab7 or c6450d9 (this head):

  • cmux-ci run --class light --script scripts/ci/feat-parity-check.sh --arg rust-workspace --arg cmux-tui/crates/cmux-rd-ffi at c6d9ab7: step 402081659a6ef2ef5854918f, green. 57 tests, including the 5 line_reader_tests and header_declares_exactly_the_exported_functions_and_codes (the header and the exports agree, with ABI 5).
  • the same with --arg cmux-tui/crates/cmux-app-ffi at c6d9ab7: step 06415966171e0394243cf985, green.
  • cmux-ci run --class light --script scripts/ci/run-swift-testing-suites.sh --arg Packages/macOS/CmuxNext --env CMUX_SWIFT_TEST_SUITES=MacScreenRdLineTests at c6d9ab7: step 7921ed72ed76c99430274f2a, PASS ^CmuxNextMacScreenTests\.MacScreenRdLineTests/ (the old Swift decoder matches every vector).
  • cmux-tui-rust-check.sh --os macos --arg clippy at c6450d9: step a7e9929aa05ac79a0927f3f1, green. The same at c6d9ab7 was red, step 8c236eb2cc562486441cfe49 (E0282 in my splitter test), fixed in c6450d9.
  • cmux-tui-rust-check.sh --os linux --arg fmt at c6d9ab7: step 92f81d99e2d7676f0d6168ed, red on base files only (cmux-app-manifest tests/customize_example.rs, cmux-tui-core apps/grants.rs and lib.rs, cmux-tui link/sync_cli.rs and store.rs). None of them is in this PR; rustfmt --check of the changed files passes with each crate's edition.
  • cmux-tui-rust-check.sh --os macos --arg rd-host at c6450d9: step 8d10f3568178258dfea4953d, clippy red on mac/capture.rs:247 displays is never used (this lane builds without the link feature, so serve.rs, the only caller, is out). The same lane on base 2d102f8 is red the same way: step c9df8dd88354824c7ba7df84. It is a base finding; rd-host tests pass in both.
  • cmux-tui-rust-check.sh --os linux --arg test --arg app_line at c6450d9: step 1901370296fb198e25d3ee9f, "no tests to run". On Linux nothing in the cmux-tui workspace turns on cmux-rd-proto's serde feature (rd-host is a macOS-only dependency of cmux-tui), so app_line compiles out there, as control.rs does. The same on macOS: step 231d065bdaf96e7553d39927, green, 5 of 5 app_line tests.
  • cmux-ci run --class light --script scripts/cmux-next/build-app-ffi.sh at c6450d9: step 9a8bb411b755274ed000e890, green (CCmuxAppFFI.xcframework builds with ABI 5).
  • python3 scripts/cmux-next/prepush-gate.py: no new violations.

Not run: no build of the app with the new ABI (step B needs the published release). No GUI run of a Mac Screen session (no line shape changes: the vectors prove the values).


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Makes Rust the single source of the JSON lines cmux rd serve|connect print for the app, and adds a C line reader (CmuxRdLineReader, ABI 4→5) so the app reads them back as typed lines. The line shape was previously defined twice — json! literals in cmux-rd-host and a hand decoder in Swift — and now cmux-rd-proto's app_line owns it; the host prints AppLine and the Swift Mac Screen reads the same values as today. A new Swift test target proves the existing decoder reads every golden vector as its want, so the behavior is unchanged.

  • New app_line module (feature serde): AppLine, LinePath, LineSplitter, and golden vectors for every line kind the Swift decoder knew, plus unknown, non-JSON, and wrong-type lines.
  • New cmux-rd-ffi reader: cmux_rd_line_reader_new/_free/_push/_next, no I/O or threads, panics caught per reader.
  • Host serve.rs/connect.rs print AppLine values; same fields, key order now follows the struct.
  • Step A of two: no Swift caller of the new ABI until the app FFI release is pinned; step B (Swift reads through the reader, MacScreenRdMessage/MacScreenRdLines deleted) lands after the re-pin.

Written for commit c6450d9. Summary will update on new commits.

View guided diff Turn on auto-fix

…ugh a C line reader (ABI 5)

The JSON lines `cmux rd serve` and `cmux rd connect` print for the app had
two definitions: json! literals in cmux-rd-host and the Mac Screen's hand
decoder (MacScreenRdMessage, MacScreenRdLines). Now there is one:

- cmux-rd-proto app_line (feature serde): AppLine and LinePath, the line
  shape, and LineSplitter, the stdout byte splitter. Golden vectors in
  tests/vectors/app_lines.json.
- cmux-rd-host serve.rs and connect.rs print AppLine values (same fields; key
  order now follows the struct).
- cmux-rd-ffi: CmuxRdLineReader (cmux_rd_line_reader_new, _free, _push,
  _next) reads stdout bytes into typed CmuxRdLine values, ABI 5. Tests replay
  the golden vectors through the C ABI.
- CmuxNextMacScreenTests (new): the Mac Screen's current Swift decoder reads
  every golden vector as its `want`, so the Rust reader can replace it with
  no change for any line the host prints.

Step A of the two-step app FFI path: no Swift caller of the new C ABI until
the app FFI release that includes it is pinned; step B (the Mac Screen reads
lines through CmuxRdLineReader, MacScreenRdMessage and MacScreenRdLines
deleted) follows the re-pin.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review c6450d9: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr35828 -f ref=swift-logic/macscreen-rd-lines -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 00add02 to compare; do not merge until green; see lane-rules

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent): SHIP at c6450d9

Correctness first, then repo rules. Read-only review of git diff 2d102f877445 c6450d9fda6c, REVIEW.md, skills/cmux-review, CLAUDE.md. No local build or test; I read the author's fleet steps and their logs.

What I checked

  1. Wire compatibility.
    • Every say(...) in serve.rs and connect.rs prints the same fields and values as the old json! literals:
      • key, candidate and ended print as strings.
      • candidates_done, connected and listening print only as true (skip_serializing_if = is_false; the old code never printed false).
      • path always prints both kind and rtt_ms, with null when unknown (LinePath has no skip). rtt_ms is still us as f64 / 1000.0.
      • displays is Vec<(u32, u32)> and serializes as [[w,h],...], as before. current is a usize, as before.
      • The displays error line is unchanged.
    • Only the key order changed: the struct order replaced serde_json's sorted Value order. Every reader is order-free: Swift JSONDecoder (MacScreenRdMessage) and the Python dict readers (hub-pane-local.py, hub-session-local.py). No script, test, iOS or docs file matches the exact line text.
    • bench.rs still uses json!, but it prints to stderr and is not an app line.
    • Exact [w,h] pairs: mac::capture::displays() returns Res<(Vec<(u32, u32)>, usize)>, and serve.rs is its only producer, so no producer can print another shape. The app runs its bundled cmux, so the host and the reader come from one build.
    • Strict typing gives the same result as Swift: a wrong type drops the whole line in both. An unknown field reads as an empty line in both. CRLF works in both (a trailing \r is JSON whitespace). An empty line is dropped in both.
  2. C ABI.
    • The CmuxRdLine field order and types match the header. The size is 120, and the test asserts it.
    • Ownership: the line pointers point into handle.current, which the boxed reader keeps alive until the next next call or free.
    • NULL handling: NULL bytes with len > 0 returns ERR_NULL (bytes_in). NULL out, NULL reader and NULL free are all handled.
    • Panics: new, free and with_reader use catch_unwind with per-reader poisoning, the same as rb_client_ffi.rs.
    • The header test passes. The green build-app-ffi step proves the exported symbol list.
    • The only Swift ABI check is RemoteRdCore.abiVersion >= 2 (RbClient.swift:31), so the bump to 5 breaks nothing. No existing function changed.
  3. Splitter: it splits on \n and gives the same lines as MacScreenRdLines. The tests cover partial lines across chunks and byte-by-byte pushes. An empty line is a line, and its decode drops it, as Swift does.
  4. Cargo.lock: only the serde and serde_json edges of cmux-rd-proto change, in the cmux-app-ffi and cmux-rd-ffi locks. Both packages were already present.
    • App FFI pin: the base 2d102f8 matches its pin df4352d. This PR changes FFI inputs, so check-app-ffi-pin.sh --base HEAD^1 exits 1 (own change).
    • The step is continue-on-error, but cmux-next checks lists it among the failed steps, so the job turns red. It is not skipped.
    • That is the documented path: the script's own "Fix" text, the app-ffi-release.yml header (it publishes only from a feat-cmux-next push), and publish-app-ffi-repin.sh (it opens the repin PR). The 2026-10-05 entry in plans/cmux-next/coordination/remote-desktop.md shows the same red, and the later repin.
  5. Swift test target: it is wired in Package.swift (CmuxNextMacScreenTests -> CmuxNextMacScreen), and suites are found without a name list.
    • The test finds the repo root 6 levels up from #filePath (correct) and reads all 13 vectors. #expect(vectors.count >= 10) stops it from passing on an empty list.
    • It compares each vector, through NSDictionary equality, against the old Swift decoder.
    • The null wants expect a failed decode.
  6. Risk tier: risk:normal is right. No auth, relay or socket policy, migration, release or signing, risk gate, pre-push trust or new delete path is touched.
  7. Evidence: c6d9ab7 is the head's parent. The only difference is vec![Vec::<u8>::new(), Vec::new()] in one test. I checked these steps (state done, exit 0, on the stated refs):
    • 7921ed72: 1 test in suite MacScreenRdLineTests passed.
    • 402081659 (hq run 41887697145): 57 passed, with all 5 line_reader_tests and header_declares_exactly_the_exported_functions_and_codes.
    • a7e9929a: clippy, macOS, at the head.
    • 231d065b: 5 of 5 app_line unit tests.
    • 9a8bb411: build-app-ffi at the head.

Findings (none blocking)

  • N1 (landing): The app FFI pin check makes cmux-next checks red on this PR. After the merge, the tip is red until the automatic repin PR lands. That is by design (item 4); the lander should know to expect it.
  • N2 (coverage): cmux-tui/crates/cmux-rd-proto/tests/app_lines.rs (3 integration tests) ran in none of the steps I saw. The app_line filter matched only the unit tests. The rd-ffi line_reader_tests cover the same vectors through the C reader, so the gap is small. Run it once, for example as a cmux-rd-proto test with --features serde on macOS.
  • N3 (bounds and cost, same as the current Swift code): LineSplitter.pending and CmuxRdLineReader.ready have no limit. push searches again from the start of pending on every call, so a long unterminated line costs quadratic time across chunks. MacScreenRdLines behaves the same, so this is not a regression. For step B, think about keeping a scan offset and a maximum line length.
  • N4 (doc): In cmux_rd_ffi.h, the cmux_rd_line_reader_next comment does not say it can also return CMUX_RD_ERR_NULL or CMUX_RD_ERR_PANIC. The push comment does say this.
  • N5 (vectors): No vector covers the one real difference: Swift accepts displays arrays of any length, but Rust accepts only exact pairs. In step B, a vector such as [[1,2,3]] -> null would record the new behavior. Also, the displays vector lines use the old sorted key order (current first), not what the host prints now. That is harmless, because the round-trip test covers the new order.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Expected red for the lander: the app FFI pin step of cmux-next checks is red on this PR by design (step A of the two-step FFI path: it changes CCmuxAppFFI sources; app-ffi-release.yml publishes from the feat push and its repin PR follows). The same red landed with #35307 and #34507. The tip shows that red until the repin PR lands; step B (branch swift-logic/macscreen-rd-lines-b) opens after the repin. Any other red on this head is classified before landing.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 4, 2026
@blacksmith-sh

blacksmith-sh Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Found 7 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp6j28igdu/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmpfdybvewr/summary-missing.json (live):/summary-missing.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@lawrencecchen
lawrencecchen merged commit b24aac2 into feat-cmux-next Nov 4, 2026
264 of 285 checks passed
@lawrencecchen
lawrencecchen deleted the swift-logic/macscreen-rd-lines branch November 4, 2026 19:39
lawrencecchen added a commit that referenced this pull request Nov 4, 2026
…ader; the Swift decoder goes (step B of #35828) (#35876)

* Mac Screen: read cmux rd serve|connect lines through the Rust line reader; delete the Swift decoder

Step B of the two-step app FFI path (step A: the typed AppLine, the C line
reader and the golden vectors). MacScreenRdProcess hands stdout bytes to
RemoteRdLineReader (CmuxNextRemoteView, over cmux_rd_line_reader_* of
cmux-rd-ffi) and yields its typed lines; MacScreenRdMessage (the hand
decoder) and MacScreenRdLines (the byte splitter) are deleted. The process
keeps only the spawn, the pipes, the stderr tail and the stdin writes.

MacScreenRdLineTests now feeds every golden vector through the Rust reader;
the Swift decoder passed the same vectors before it was deleted.

Needs the CCmuxAppFFI release that includes ABI 5 pinned in Package.swift.

* swift-logic table: the Mac Screen's rd line reader wrapper and process plumbing are PLATFORM (cmux_rd_line_reader_*)

This branch was previously deployed

1 inactive deployment
ci-routing-agent — c6450d9f Deployed Nov 4, 2026 by lawrencecchen via namespace autofix #14534
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant