Repository navigation
fix(psmux): refuse -X=value, -t... and -- option values psmux 3.4.0 cannot carry - #908
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 7 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe psmux backend now rejects option values that psmux 3.4.0 rewrites or rejects. Unit and live tests cover the rejected shapes and accepted dash-prefixed controls. Changespsmux option-value filtering
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: rejecting option-value shapes that psmux 3.4.0 cannot carry. Linked Issues check Issue #905is directly linked and remains open. The reviewed change adds_transportablerefusals for-[A-Za-z]=.with a non-empty suffix, values starting with-t, and--. Existing transportabi…Out of Scope Changes check The changed backend code implements the workaround requested by #905. The unit tests verify refused and admitted argv shapes. The live tests verify the psmux premises and retirement signals. The chang…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- 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.
A rabbit checks each dash-shaped string,
And guards the values psmux may bring.
Some keys are freed when flags interfere,
While safe controls still pass through clear.
The test burrow checks what was stored,
Then hops away, its work restored.
Comment @coderabbitai help to get the list of available commands.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…annot carry psmux 3.4.0's client argv normalizers rewrite a few dash-leading values before set-option parses them (psmux/psmux#795): -X=value becomes -X value at rc 0, and -t... and -- are refused with rc 1, which left a stale value in place. _transportable now refuses those shapes, so they take the existing visible path: a warning, and the key freed. Every other dash-leading value stays admitted. A live premise row turns red when a psmux release stores -x=foo verbatim, so the refusal can retire behind a floor bump. Closes bmad-code-org#905
The refused-values paragraph named only empty, quoted, multiline and edge-whitespace values. It now also names the three dash-leading shapes that psmux's client rewrites before parsing (-X=value, -t..., --), which take the same warn-and-unset path.
7b73024 to
6422962
Compare
2 participants
psmux 3.4.0's client argv normalizers rewrite a few dash-leading option values before
set-optionparses them (psmux/psmux#795). #900 retired_transportable's blanket refusal of dash-leading values, so these shapes reached psmux:-X=value(one ASCII letter, then=, then at least one character) is stored as-X value, with rc 0 and no warning. This is the silent case the gate exists to prevent.-t…and--are refused with rc 1. On the session path that raised partway through the write and left the previous value in place.Change
_transportablerefuses exactly those shapes:-[A-Za-z]=.;-t;--.A refused value takes the gate's existing visible path: a warning on stderr, and the key freed. Every other dash-leading value stays admitted. The measured safe controls are
-u,-1=x,--x=yand-x=, so the retirement in #900 keeps its effect for them.Tests
tests/test_psmux_backend.pycovers the refused shapes and the admitted controls through the gate.tests/test_psmux_live.pyadds atest_premise_*tripwire. It pins the (rc, readback) pair of each shape on the installed psmux and turns red when a psmux release stores-x=fooverbatim, so the refusal can retire behind a floor bump. It was ablated to fail against a fake that stores verbatim.test_adopted_a_dash_leading_option_value_is_stored), from PowerShell 7 withSHELLunset on psmux 3.4.0: 4 passed.Closes #905
Summary by CodeRabbit
-X=value,-t…, and--, are now recognized as values psmux may rewrite or reject. They are warned about rather than treated as reliably transportable, and the associated option keys are freed.