Repository navigation
Description
Finding (from PR review)
fleet proxy ssh argument splitting does not parse operand-bearing SSH options inside flag clusters, and omits at least one operand-bearing option (-P), so valid invocations are broken.
Reviewer chatgpt-codex-connector[bot] (Codex, P2) on PR #6403, 2026-11-06T00:49:20Z:
#6403 (comment)
"Parse operand-bearing SSH options inside flag clusters — For valid invocations such as
fleet proxy ssh -- -vL 127.0.0.1:2222:localhost:22, the exact-string lookup does not recognize that-vLrequires an operand. It consequently insertsuser@100.99.113.103immediately after-vL, where OpenSSH consumes it as the forwarding specification and exits withBad local forwarding specification. The table also omits-P tag, documented in the OpenSSH manual, causing another operand-boundary error. Parse clustered option characters and account for all operand-bearing options before separating the remote command."
Source PR: #6403 feat(fleet): add proxy ssh recovery verb (open; parent design issue #5697 does not cover this defect — dedupe searched by PR number, split_ssh_options, "operand-bearing", "proxy ssh", "flag cluster": no existing issue covers it).
No-response evidence
- The comment is now over 24 hours old (grace period for open PRs passed; first flagged by pr-comment-review 2026-11-06 15:54 CST, rechecked this run).
- No replies in the thread; the review thread is unresolved (GraphQL
reviewThreadsverified this run). - PR head is unchanged since the review: single commit
2817c11837(2026-11-06T00:42:38Z, before the comment); no fix commit followed.
Still-live evidence (verified on PR head 2817c11837 this run)
pkgs/fleet/crates/fleet/src/main.rs on the PR head, fresh gh api raw read:
- Line 638:
fn split_ssh_options_and_command(...); line 639-642:OPTIONS_WITH_OPERANDStable contains-B -b -c -D -E -e -F -I -i -J -L -l -m -O -o -p -Q -R -S -W -w— no-P. - Line 655:
if OPTIONS_WITH_OPERANDS.contains(&arg.as_str())— exact-string match only; a clustered argument such as-vLnever matches, so its operand is treated as the start of the remote command and the fixed destination is inserted in the operand's place.
Acceptance criteria
- Clustered SSH options are parsed character-by-character: an operand-bearing character inside a cluster consumes the rest of the cluster or the next argument as its operand, and the destination is inserted only after the full option prefix.
-P(and any other documented operand-bearing ssh options missing from the table) are accounted for.- Tests cover at least:
-vL <spec>clustered forwarding,-P <tag>, a separated-operand option (existing behavior), and a plain remote command with no options. - Or: resolve this issue by a documented decision explaining why clustered forms are intentionally unsupported, with the rejection made explicit (a clear error instead of a malformed ssh invocation).
Activity
developing-today commentedon Nov 7, 2026
CODE-2070
dezren39 commentedon Nov 7, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 7, 2026
Picked up by repo-worker-code-t (Lane T, fallback mode — queue backend down, fleet queue health 503 PostgREST PGRST002). Triage of #6679: revalidating the Codex P2 clustered-options finding against PR #6403 head first.
🤖 Agent note · picked up
21 remaining items
dezren39 commentedon Nov 7, 2026
Picked up by repo-worker-code-r (lane R, fallback mode — queue backend down) for the small-fix: parse clustered SSH options character-by-character in split_ssh_options_and_command and add -P to the operand-bearing set, as a new commit on PR #6403's branch queue/10604-5697-proxy-ssh (current head bb71f3d, includes the #6680 fix), per lane T's verified triage route on this issue.
dezren39 commentedon Nov 7, 2026
Progress from repo-worker-code-r: implementation_complete in ~/git/fleet/fb-6680: split_ssh_options_and_command now walks each cluster char-by-char (operand char consumes rest-of-cluster or next argument), -P added to the operand-bearing char set; new unit test covers -vL , -P , attached -L, and missing-operand error. Targeted tests pass (unit 4/4, integration 1/1). Next: full fleet suite, commit + push to PR #6403's branch, disposition Codex thread r4487818000.
fix(fleet): parse clustered ssh options and -P in proxy ssh splitting
dezren39 commentedon Nov 7, 2026
Fixed — commit
201836ee9dpushed to PR #6403's branchqueue/10604-5697-proxy-ssh(on top of the #6680 fixbb71f3d97f).split_ssh_options_and_command(pkgs/fleet/crates/fleet/src/main.rs) no longer matches whole arguments against an operand table. It walks each leading-dash argument character-by-character over an operand-bearing char set perssh(1)that now includesP: an operand-bearing character consumes the rest of its cluster as the operand, or the next argument when it ends the cluster — so the fixed destination is inserted only after the full option prefix.-L<spec>,-o<opt>) and separated operands behave exactly as before;--and missing-operand errors keep their meanings (messages now name the operand character and cluster).proxy_ssh_splits_clustered_options_and_operandscovers-vL <spec>,-P <tag>, attached-L<spec>, and the clustered missing-operand error; existing separated-operand and plain-command cases remain covered by the pinned argv tests. Verified on laptop: full fleet crate suite green (17 unit + 1 integration).201836ee9dwas still pending (basic checks queued) at close-out; the earlier run forbb71f3d97fwas cancelled as superseded by this push, which is expected. PR feat(fleet): add proxy ssh recovery verb #6403 merges under its own lifecycle.Worked by repo-worker-code-r (lane R), fallback mode (queue backend down).
dezren39 commentedon Nov 7, 2026
Released by repo-worker-code-r (outcome: done). Fix landed as commit 201836e on PR #6403's branch queue/10604-5697-proxy-ssh: split_ssh_options_and_command parses clusters char-by-char, -P added to the operand-bearing set; tests cover -vL, -P, attached -L and missing-operand. Full fleet suite green on laptop; Codex P2 thread replied (r4499735056). PR #6403 CI pending at release. Issue #6679 closed as completed.