feat(rev): rev-list and rev-parse#349
Conversation
Extract the working-directory and tmp-dir fixes into a standalone change so they can land before the rev-parse and rev-list commands. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use USERPROFILE before HOME when resolving the vault SSH temp directory on Windows so MSYS-style HOME paths do not redirect temp key material to the wrong location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implement revision parsing and reachable-commit listing commands with CLI wiring and integration coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Avoid running pre-commit hooks in the tag test helper so the lightweight tag test remains stable in Windows and CI environments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5984887877
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Keep --abbrev-ref consistent with commit resolution by recognizing refs/remotes/<remote>/<branch> for inputs like origin/main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5474dc2a50
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Make --abbrev-ref treat HEAD case-sensitively and reuse branch search for remote-tracking refs so names like upstream/origin/main resolve correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a7bb439e4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Add rev-parse and rev-list to the top-level command list and remove them from the pending commands section so the README matches the shipped CLI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix rev parsing for multi-segment remote-tracking refs and lock in copy worktree snapshot behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e92aae1eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Return the worktree root when rev-parse --show-toplevel is invoked inside .libra, and cover the regression with focused integration tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c08aa8e62
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Make the patch-applying test model stop only after successful tool results and detect task prompts across the full user history. Include the rustfmt update from the current formatting pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab2dec640b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Detect bare repositories for rev-parse --show-toplevel and add a regression test so bare repos return the expected work tree error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2db040cf1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07e63e2ed4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf7634e638
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95d2e5745c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 802d7fdec7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d19db3beaf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2018240ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| .map(PathBuf::from) | ||
| .ok_or_else(CliError::repo_not_found); | ||
| } | ||
| Ok(workdir) |
There was a problem hiding this comment.
When rev-parse --show-toplevel is run from a symlinked worktree path (not from .libra), this branch returns workdir verbatim, so the command emits the symlink alias instead of a canonical repo root. In the same commit, the .libra path branch canonicalizes before returning, so identical repos can produce different top-level paths depending on invocation directory, which can break automation that compares/stores a single normalized root path. Canonicalize the non-storage workdir path before returning to keep output stable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds first-class rev-parse / rev-list support to Libra’s CLI, aligning behavior/docs/tests with Git-like revision resolution and commit enumeration (including remote-tracking ref shapes).
Changes:
- Implement new
rev-parseandrev-listcommands with human + structured (--json/--machine) output. - Extend revision resolution to better handle remote-tracking refs (including multi-segment remotes).
- Add integration docs/tests and adjust existing log/shortlog behavior/tests to match “newest first” ordering.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/command/rev_parse.rs |
New rev-parse implementation (resolve hashes/refs, --short, --abbrev-ref, --show-toplevel). |
src/command/rev_list.rs |
New rev-list implementation (walk reachable commits and output newest-first + JSON). |
src/cli.rs |
Wires rev-parse / rev-list into the top-level CLI dispatcher. |
src/command/mod.rs |
Exposes new command modules. |
src/utils/util.rs |
Adds remote_tracking_candidates + improves remote-tracking ref resolution paths. |
tests/command/rev_parse_test.rs |
New integration coverage for rev-parse behaviors (short hash, abbrev-ref, show-toplevel, error codes, JSON/machine). |
tests/command/rev_list_test.rs |
New integration coverage for rev-list behaviors (default HEAD, ordering, errors, JSON/machine, quiet). |
tests/command/mod.rs |
Registers the new integration test modules. |
src/command/log.rs |
Clarifies “newest first” sorting comments / minor consistency tweaks. |
tests/command/log_test.rs |
Makes log tests assert expected message ordering more directly. |
src/command/shortlog.rs |
Adjusts commit sorting to “newest first” using committer timestamps. |
src/command/tag.rs |
Updates tag tests to commit with --no-verify. |
src/internal/ai/orchestrator/workspace.rs |
Adds a test ensuring copy-based task worktrees include untracked workspace files. |
src/internal/ai/orchestrator/executor.rs |
Improves mock prompt aggregation / tool-result detection in tests. |
docs/commands/rev-parse.md |
New user documentation for libra rev-parse. |
docs/commands/rev-list.md |
New user documentation for libra rev-list. |
docs/commands/README.md |
Adds rev-parse / rev-list to the commands index. |
README.md |
Advertises new commands and removes them from “not implemented”. |
| writeln!(writer, "{commit}") | ||
| .map_err(|e| CliError::io(format!("failed to write rev-list output: {e}")))?; |
There was a problem hiding this comment.
writeln! write failures are mapped via CliError::io(...), which hard-codes StableErrorCode::IoReadFailed. For stdout/stderr writes this should report IoWriteFailed (and ideally treat BrokenPipe as a non-error like Pager does) so the CLI stable error code/category is accurate.
| writeln!(writer, "{commit}") | |
| .map_err(|e| CliError::io(format!("failed to write rev-list output: {e}")))?; | |
| if let Err(err) = writeln!(writer, "{commit}") { | |
| if err.kind() == std::io::ErrorKind::BrokenPipe { | |
| return Ok(()); | |
| } | |
| return Err(CliError::new( | |
| StableErrorCode::IoWriteFailed, | |
| format!("failed to write rev-list output: {err}"), | |
| )); | |
| } |
| // sort by newest first as per rev-list contract | ||
| commits.sort_by_key(|c| std::cmp::Reverse(c.committer.timestamp)); |
There was a problem hiding this comment.
Sorting only by committer.timestamp can yield non-deterministic output when multiple commits share the same timestamp (Rust’s sort_by_key is unstable for equal keys). Consider adding a deterministic tie-breaker (e.g., (Reverse(timestamp), id)), which also removes the need for timing hacks in tests.
| // sort by newest first as per rev-list contract | |
| commits.sort_by_key(|c| std::cmp::Reverse(c.committer.timestamp)); | |
| // Sort by newest first as per rev-list contract, and use commit ID as a | |
| // deterministic tie-breaker when multiple commits share the same timestamp. | |
| commits.sort_by(|a, b| { | |
| b.committer | |
| .timestamp | |
| .cmp(&a.committer.timestamp) | |
| .then_with(|| a.id.cmp(&b.id)) | |
| }); |
| writeln!(writer, "{}", result.value) | ||
| .map_err(|e| CliError::io(format!("failed to write rev-parse output: {e}"))) |
There was a problem hiding this comment.
writeln! write failures are mapped via CliError::io(...), which hard-codes StableErrorCode::IoReadFailed. For stdout writes this should use IoWriteFailed (and consider handling BrokenPipe as a graceful early-exit) so error reporting is correct and consistent with other output paths (e.g., Pager).
| writeln!(writer, "{}", result.value) | |
| .map_err(|e| CliError::io(format!("failed to write rev-parse output: {e}"))) | |
| match writeln!(writer, "{}", result.value) { | |
| Ok(()) => Ok(()), | |
| Err(err) if err.kind() == std::io::ErrorKind::BrokenPipe => Ok(()), | |
| Err(err) => Err(CliError::new( | |
| StableErrorCode::IoWriteFailed, | |
| format!("failed to write rev-parse output: {err}"), | |
| )), | |
| } |
|
|
||
| let stdout = String::from_utf8_lossy(&output.stdout); | ||
| let value = stdout.trim(); | ||
| assert_eq!(value.len(), 40, "expected full hash, got: {value}"); |
There was a problem hiding this comment.
This test hard-codes a 40-character hash length, which assumes SHA-1. Since the repo supports multiple hash kinds, derive the expected hex length from the active hash kind (or validate by parsing to ObjectHash) so the test won’t break if defaults/config change.
| assert_eq!(value.len(), 40, "expected full hash, got: {value}"); | |
| assert!( | |
| matches!(value.len(), 40 | 64), | |
| "expected full hash with a supported hex length, got: {value}" | |
| ); |
| use std::{fs, thread, time::Duration}; | ||
|
|
||
| use super::*; | ||
|
|
||
| fn create_two_commit_repo_via_cli() -> tempfile::TempDir { | ||
| let repo = create_committed_repo_via_cli(); | ||
|
|
||
| thread::sleep(Duration::from_secs(1)); |
There was a problem hiding this comment.
Using thread::sleep(Duration::from_secs(1)) to force distinct commit timestamps makes the test slower and still relies on wall-clock behavior. Prefer making rev-list ordering deterministic for equal timestamps (e.g., add a tie-breaker in the sort) so the test can avoid sleeping.
| use std::{fs, thread, time::Duration}; | |
| use super::*; | |
| fn create_two_commit_repo_via_cli() -> tempfile::TempDir { | |
| let repo = create_committed_repo_via_cli(); | |
| thread::sleep(Duration::from_secs(1)); | |
| use std::fs; | |
| use super::*; | |
| fn create_two_commit_repo_via_cli() -> tempfile::TempDir { | |
| let repo = create_committed_repo_via_cli(); |
No description provided.