fix: address #1939 P2/P3 review items by miyaontherelay · Pull Request #1955 · AgentWorkforce/relay

miyaontherelay · 2026-10-09T16:35:18Z

Addresses the P2/P3 review threads on #1939 (trunk → main, carrying #1891). The must-fix items M1–M6, and the OpenCode any_output thread (detection.rs:48), are in #1954.

Changes

b8fb668: docs and the test harness

CHANGELOG: the PTY delivery entry moves from the released [13.1.1] section to [Unreleased - Minor] / Fixed. It is rewritten impact-first, and the release-only "13.2.0 rollout" sentence is dropped. git merge-base --is-ancestor 6bdec7b2 v13.2.0 → 1 and … origin/main → 1, so fix(broker): verify PTY turn submission #1891 has not shipped in any release.
AGENTS.md: "When the PR is ready" goes back on its own line, so items 3–4 render as one ordered list with nested bullets.
fleet.rs: the enqueue_delivery_ack comment now lists process echo as acceptance evidence, alongside activity and a cleared composer.
real-codex.mjs: --broker with no value (or followed by another flag) falls back to the friendly "Pass --broker" error instead of path.resolve(undefined). A directory is now rejected by the executable-file guard (stat().isFile()).

a80f920: delivery_verification.rs:312, prompt-like rows inside a body

current_composer anchored on the last prompt-looking row, so a body containing > b cut the composer short. A parked draft then read as Inconclusive (no submit-key recovery), and its visible-composer echo was never recorded.
The new composer_holds_tail keeps the latest-anchor containment check. It also accepts an earlier anchor, but only when that composer ends with the tail at the cursor, so a submitted body above a fresh prompt is not treated as parked.
Tests: prompt_like_body_rows_do_not_hide_a_parked_draft (red on trunk, failing on the echo assertion; green now) and the control submitted_prompt_like_body_above_a_fresh_prompt_is_not_parked (green both before and after).

5447d54: review follow-ups on this PR

real-codex.mjs: a supplied option with no value now fails (--broker requires a value.) instead of falling back to RELAY_REAL_CODEX_BROKER_BINARY.
CHANGELOG: split into three impact-first bullets; the limit is stated as three total attempts.
fleet.rs: process echo is named as cat-only transport receipt.

Verification (finn-mini, macOS arm64, rustc 1.99.0 stable, node v25.8.1)

command	result
`cargo +stable fmt --all -- --check`	rc 0 (re-run at `5447d54`)
`cargo +stable clippy -- -D warnings` (the CI form)	rc 0
`cargo +stable test -p agent-relay-broker --lib`	1425 passed, 5 failed, 7 ignored
`cargo +stable test -p agent-relay-broker --lib delivery_verification` (after fmt)	37 passed, 0 failed
`cargo +stable test -p relay-pty`	262 + 3 passed, 0 failed

The 5 broker failures are spawner::tests::broker_hook_* (git commit-hook trailer tests). The same 5 fail with this branch's code change stashed (git stash → cargo test … spawner::tests::broker_hook → 5 passed, 5 failed), so they pre-exist and are environmental on this node, not caused by this PR. clippy --all-targets -D warnings also reports 2 pre-existing errors in node_control.rs test code, which CI's clippy form does not lint.

real-codex.mjs, red on trunk then green. Each run uses env -i PATH=<dir containing only node> node tests/relayflows/cases/1891-codex-parked-composer-recovery/real-codex.mjs <args>, so the real codex is never launched:

args	trunk	this branch
`--broker`	`TypeError [ERR_INVALID_ARG_TYPE]`	`Error: --broker requires a value.`
`--broker --timeout-seconds 20` with `RELAY_REAL_CODEX_BROKER_BINARY=/bin/ls`	(at `a80f920`) silently uses the env broker	`Error: --broker requires a value.`
no flag, no env (control)	n/a	`Error: Pass --broker <built-agent-relay-broker> or RELAY_REAL_CODEX_BROKER_BINARY.`
`--broker /tmp`	passes the guard, then fails with `codex must be on PATH`	`Error: broker must be an executable file: /tmp`
`--broker /bin/ls` (control)	n/a	passes the guard, then `codex must be on PATH`

Interaction with #1954

Both PRs edit crates/broker/src/broker/delivery_verification.rs in nearby but non-overlapping hunks; whichever merges second may need a small rebase. #1954 adds failed_draft_released, which still uses current_composer(..).contains(tail). Once both land, it should call composer_holds_tail so a failed draft containing a > row is held as well.

🤖 Generated with Claude Code

Agent Relay sessions

claude session 2a0590dc · opened via gh pr create · last active 2026-10-10

…ards) - CHANGELOG: move the PTY delivery entry from the released 13.1.1 section to [Unreleased - Minor] / Fixed, impact-first, without the release-only sentence. 6bdec7b is not an ancestor of v13.2.0 or main. - AGENTS.md: restore "When the PR is ready" as its own line so items 3 and 4 render as one ordered list with nested bullets. - fleet.rs: the enqueue_delivery_ack comment names process echo as acceptance evidence alongside activity and a cleared composer. - real-codex.mjs: `--broker` without a value falls back to the friendly error instead of path.resolve(undefined); a directory is rejected by the executable-file guard. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Session-Id: 2a0590dc-a82a-4174-9eff-c23d59571061

coderabbitai · 2026-10-09T16:35:26Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 7350ebf4-6746-45d2-8c89-1e854e792ec9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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

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

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

current_composer anchored on the last prompt-looking row, so a relay body containing a line such as `> b` cut the live composer short at that row. The tail check then missed a parked draft: observe_visible_composer never saw the echo, and assess_harness_acceptance returned Inconclusive instead of Parked, so the delivery failed without submit-key recovery. composer_holds_tail keeps the latest-anchor containment check and also accepts an earlier prompt anchor, but only when its composer ends with the body's tail at the cursor. A submitted body in the transcript above a fresh prompt does not end at the cursor and is not treated as parked. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Session-Id: 2a0590dc-a82a-4174-9eff-c23d59571061

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

cubic-dev-ai

All reported issues were addressed across 5 files

_{You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

_{Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic}

- real-codex.mjs: a supplied option with no value now fails with "<flag> requires a value" instead of falling back, so `--broker --timeout-seconds 20` can no longer silently run the broker from RELAY_REAL_CODEX_BROKER_BINARY. - CHANGELOG: split the PTY delivery entry into three impact-first bullets and state the limit as three total attempts. - fleet.rs: process echo is acceptance evidence only for a plain `cat` recipient, where it proves transport receipt, not harness acceptance. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Session-Id: 2a0590dc-a82a-4174-9eff-c23d59571061

kjgbot · 2026-10-10T02:28:20Z

@codex review

chatgpt-codex-connector · 2026-10-10T02:28:24Z

To use Codex here, create a Codex account and connect to github.

miyaontherelay · 2026-10-10T05:38:49Z

Superseded by #1959: a single PR into main that carries the #1939 promotion (#1891, #1946, #1947, the prettier fix) plus the #1954 and #1955 review fixes. Relay no longer has a merge train, so per-PR CI runs there. Its head contains this PR's head unchanged.

…-submission #1945 carried the original #1891 PTY delivery change into main without the #1954/#1955 fixes. Resolved against the #1891 pre-image (origin/trunk, same content as d153837 for these files) so only #1945's own edits conflicted. Kept from #1945: file attachments (wrap-mode attachment references, attachment echo test), CRLF-normalized echo matching, word-boundary "working" busy detection, the delivery_unconfirmed SDK event for rejected delivery_verified frames, and the completed_replay non-confirmation guard. Superseded (orchestrator decision): #1945's M4 variant (event-loop delayed CR follow-up plus refusing human write_pty with pty_write_queue_full during recovery) in favour of #1954's drainer-level cancellation, which never refuses operator keystrokes. #1945's generic echo_left_composer acceptance is replaced by #1954's guarded rule (post-echo output, body not at/after the cursor); attachment deliveries do not depend on the shortcut (confirmed by #1945's author) and are covered by a new test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Session-Id: a4d98f2e-cd4b-47a9-87f8-1deb0f44eb41

…bmission Promote #1939: verify PTY turn submission (#1891) + review fixes (#1954, #1955)

miyaontherelay marked this pull request as ready for review October 9, 2026 19:49

miyaontherelay requested review from khaliqgant and willwashburn as code owners October 9, 2026 19:49

devin-ai-integration Bot reviewed Oct 9, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Oct 9, 2026

View reviewed changes

Comment thread tests/relayflows/cases/1891-codex-parked-composer-recovery/real-codex.mjs Outdated

Comment thread CHANGELOG.md Outdated

Comment thread crates/broker/src/runtime/fleet.rs Outdated

This was referenced Oct 10, 2026

ci: back to per-PR CI (revert trunk gating) #1956

Merged

ci: restore per-PR CI on main (drop the trunk gate) #1957

Closed

miyaontherelay added the mergeable Ready for the merge agent to batch into trunk label Oct 10, 2026

This was referenced Oct 10, 2026

Promote #1939: verify PTY turn submission (#1891) + review fixes (#1954, #1955) #1959

Merged

trunk → main: fix(broker): verify PTY turn submission (#1891) #1939

Closed

fix(broker): address #1939 must-fix PTY delivery review findings #1954

Closed

miyaontherelay closed this Oct 10, 2026

khaliqgant added a commit that referenced this pull request Oct 11, 2026

Merge pull request #1959 from AgentWorkforce/promote/1939-pty-turn-su…

1a01457

…bmission Promote #1939: verify PTY turn submission (#1891) + review fixes (#1954, #1955)

Navigation Menu

Repository navigation

fix: address #1939 P2/P3 review items - #1955

fix: address #1939 P2/P3 review items#1955
miyaontherelay wants to merge 3 commits into
trunkfrom
fix/1939-p2p3-review-items

miyaontherelay commented Oct 9, 2026 •

edited by agent-relay-code Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

Review skipped

Uh oh!

devin-ai-integration Bot left a comment

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kjgbot commented Oct 10, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026

Uh oh!

miyaontherelay commented Oct 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

miyaontherelay commented Oct 9, 2026 • edited by agent-relay-code Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Verification (finn-mini, macOS arm64, rustc 1.99.0 stable, node v25.8.1)

Interaction with #1954

Uh oh!

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

Uh oh!

Review skipped

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kjgbot commented Oct 10, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026

Uh oh!

miyaontherelay commented Oct 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

miyaontherelay commented Oct 9, 2026 •

edited by agent-relay-code Bot

Loading

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading