Repository navigation
fix(id): carry /logs seconds in SSH command mode (#819) - #909
Conversation
…g it (#819) WorldCommand::Logs in the SSH shell was a unit variant, so the seconds parsed from '/logs <seconds>' were silently dropped when world_command() converted WorldLine::Logs. The variant now carries seconds: Option<u64>, world_command threads it through, and the command-mode reply names the requested window it is ignoring instead of pretending it was never given.
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 18 minutes. View limit details
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. Comment |
Fixes #819
Problem
#819's remaining half: in SSH command mode,
WorldCommand::Logswas a unit variant, soworld_command()dropped thesecondsthatparse_linehad parsed from/logs <seconds>(WorldLine::Logs { seconds }). The user asked for a bounded window and the shell silently discarded it — the reply never mentioned the argument at all. (The pump half of #819 and the SSH Message half were already fixed on main by the Phase 13 shell-contract work; PR #868 was closed as superseded for that reason.)Solution
WorldCommand::Logsnow carriesseconds: Option<u64>(pkgs/id/src/world_ssh.rs), matchingWorldLine::Logs.world_command()threads the parsed seconds through instead of matching{ .. }./logs→ None,/logs 5→ Some(5)).Verification
cargo test --no-default-features --features ssh --lib world_sshcompiled and ran the 49 world_ssh tests (sandbox FS was too slow to re-run the summary cleanly; CI is the verifier of record for the full suite).Need help on this PR? Tag
@codesmith-botwith what you need.