Conversation
`messages send` documents `--content -` as "read from stdin" and implements it via `read_or_stdin`. `messages edit` documents nothing and never calls the helper, so `--content -` is stored as a literal `-`, silently overwriting the message it was meant to correct. Route edit's content through the same helper, document the flag, and add an example to --help. Also add a `read_or_from` seam so the sentinel behaviour is unit testable without touching the process's stdin; `read_or_stdin` keeps its signature and delegates to it. Signed-off-by: MrSucik <15636982+MrSucik@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🔐 Codex Security Review
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
messages senddocuments--content -as "read from stdin" and implements it viaread_or_stdin.messages editdocuments nothing and never calls the helper, so--content -is stored as a literal-.That is worse than a missing feature: the flag is the natural way to pass a long or
shell-hostile body, it silently succeeds, and it overwrites the message being corrected.
We hit this on a production channel — an edit meant to fix a meeting summary replaced it
with a single bullet.
This routes edit's content through the same helper, documents the flag, and adds an
example to
--help. Also adds aread_or_fromseam so the sentinel behaviour is unittestable without touching the process's stdin.
read_or_stdinkeeps its existingsignature and delegates to the seam, so no caller changes.
Testing
Toolchain 1.95.0 from
rust-toolchain.toml.justis not installed on this machine, sothe Justfile's own commands were run directly;
cargo-nextestis absent, socargo testwas used where
test-unitprefers nextest.cargo fmt --all— clean, no changes outside the three edited files.cargo clippy -p buzz-cli --all-targets -- -D warnings— passes. (Scoped tobuzz-clirather than the Justfile's
--workspace; the change touches only this crate.)cargo test -p buzz-cli— 474 passed, 0 failed, including the newvalidate::tests::read_or_from_dash_is_a_sentinel_not_literal_content.Manual end-to-end proof against a local isolated relay
(
./scripts/start-isolated-test-relay.sh, no credentials, no shared or production relay),A/B on the same relay with the same command:
Reading the resulting kind-40003 edit event back out of the relay's database:
main(unpatched)-x\ny\nAs the author notes, the unit test alone would not have caught the original bug — it was a
missing call in
edit, not a defect in the helper. The manual A/B above is what actuallycovers the regression; the crate has no CLI-surface test harness to encode it in.