Add Jujutsu (jj) workspace support - #2393
Conversation
Detect jj workspaces and route VCS queries through a repository-backend abstraction so prek works in both colocated and secondary (non-colocated) workspaces, which have no .git directory. - Add a jj module: workspace detection, backing Git dir resolution from .jj/repo/store/git_target, and working-copy file queries via jj. - Add a repo module: RepoContext detects Git or Jujutsu once at startup and dispatches default/added/changed/conflicted file selection, stashing, and staged-config rules per backend. - Inject the backing Git store into git commands per-command rather than via global env; isolate_from_git_env strips it for commands targeting other repos.
Follow-up fixes on top of the initial jj support, from a code review pass: - Report git-root-relative paths for default/changed/conflicted file selection by running jj from the workspace root, so prek works when its config lives in a subdirectory of the jj root (paths were doubly-stripped before). - Exclude deletions from changed-file queries and restrict added-files to files absent in the parent, matching git's --diff-filter (fixes check-added-large-files false positives and deleted-path errors). - Detect conflicts via 'jj resolve --list' instead of 'jj diff --types', which does not report an unresolved auto-merged conflict. - Pass path arguments to jj as literal filesets so names with fileset metacharacters (e.g. '[') match, mirroring git's --literal-pathspecs; keep '.'. - Canonicalize the backing git dir with dunce to avoid Windows UNC paths. - Isolate try-repo's git commands (including staged-files and diff checks) from the workspace env so they operate on the target/shadow repo, not the jj backing store. - Install jj in the cargo test CI job so the jj integration tests run.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2393 +/- ##
==========================================
- Coverage 93.38% 92.83% -0.56%
==========================================
Files 129 131 +2
Lines 27637 28079 +442
==========================================
+ Hits 25810 26067 +257
- Misses 1827 2012 +185 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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: f1b143c382
ℹ️ 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.
Pull request overview
Adds first-class support for running prek inside Jujutsu (jj) workspaces (including secondary/non-colocated workspaces without a .git directory) by introducing a repository-backend abstraction and routing file selection + related logic through it, while keeping existing Git behavior intact.
Changes:
- Introduces
repo+jjmodules to detect the active repo backend once and provide backend-specific implementations for root detection, changed/added file queries, tracked-file listing, and conflict detection. - Updates
prek runand relevant built-in hooks to use the backend abstraction (and skips Git-only behaviors like index-stash / staged-config checks under jj). - Adds jj-focused integration tests and updates CI to install jj on Linux/macOS; updates documentation to mention jj behavior.
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 |
|---|---|
| docs/workspace.md | Updates workspace discovery boundary to treat .git or .jj as the repo boundary. |
| docs/quickstart.md | Documents that default prek run works with jj working-copy changes. |
| docs/faq.md | Adds FAQ entry describing how prek behaves in jj workspaces. |
| crates/prek/tests/run.rs | Adds integration tests for prek run behavior in jj colocated/non-colocated/nested/conflict/diff modes. |
| crates/prek/tests/builtin_hooks.rs | Adds jj-backed test coverage for built-in hooks in jj workspaces. |
| crates/prek/src/workspace.rs | Skips staged-config checks when the backend doesn’t have a staging area (jj). |
| crates/prek/src/repo.rs | New backend routing layer (Git vs jj) for root/files/conflicts/stash behavior. |
| crates/prek/src/main.rs | Registers new jj and repo modules. |
| crates/prek/src/jj.rs | New jj adapter for workspace discovery, backing Git dir resolution, and file queries. |
| crates/prek/src/hooks/pre_commit_hooks/check_case_conflict.rs | Uses backend-agnostic tracked/added file queries (git vs jj). |
| crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs | Uses backend-agnostic “added files” selection (git index vs jj working-copy adds). |
| crates/prek/src/git.rs | Integrates repo-context root detection + per-command git env injection; improves env isolation for try-repo. |
| crates/prek/src/cli/try_repo.rs | Ensures try-repo Git operations are isolated from ambient repo env (esp. jj backing-store env). |
| crates/prek/src/cli/run/run.rs | Adjusts run flow to avoid Git-only stash/merge-index behaviors in jj mode. |
| crates/prek/src/cli/run/filter.rs | Routes file selection (--all-files, diff modes, default selection) through the repo backend. |
| crates/prek/src/cli/install.rs | Updates messaging to include jj repositories as “inside a supported repo”. |
| crates/prek/src/cli/hook_impl.rs | Uses repo backend root instead of Git-only root. |
| .github/workflows/ci.yml | Installs a pinned/checksummed jj binary on Linux/macOS to run the new jj tests. |
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:414
- The jj installer step selects the macOS target solely by
RUNNER_OSand hard-codesaarch64-apple-darwin.macos-latestis currently ARM64, but selecting byRUNNER_ARCHavoids breakage if the runner architecture changes or the matrix adds x86_64 macOS.
case "$RUNNER_OS" in
Linux)
target="x86_64-unknown-linux-musl"
sha256="$JJ_SHA256_LINUX_X86_64"
;;
macOS)
target="aarch64-apple-darwin"
sha256="$JJ_SHA256_MACOS_AARCH64"
;;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Cargo Bloat ComparisonBinary size change: +0.68% (29.2 MiB → 29.4 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
⚡️ Hyperfine BenchmarksSummary: 0 regressions, 0 improvements above the 10% threshold. Environment
CLI CommandsBenchmarking basic commands in the main repo:
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base --version |
1.8 ± 0.1 | 1.7 | 2.2 | 1.00 |
prek-head --version |
1.8 ± 0.1 | 1.7 | 2.2 | 1.02 ± 0.07 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
8.3 ± 0.2 | 7.8 | 9.4 | 1.01 ± 0.04 |
prek-head list |
8.2 ± 0.2 | 7.7 | 8.8 | 1.00 |
prek validate-config .pre-commit-config.yaml
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base validate-config .pre-commit-config.yaml |
2.8 ± 0.1 | 2.6 | 3.1 | 1.04 ± 0.03 |
prek-head validate-config .pre-commit-config.yaml |
2.7 ± 0.1 | 2.6 | 2.9 | 1.00 |
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
2.1 ± 0.0 | 2.0 | 2.2 | 1.01 ± 0.04 |
prek-head sample-config |
2.0 ± 0.1 | 1.9 | 2.3 | 1.00 |
Cold vs Warm Runs
Comparing first run (cold) vs subsequent runs (warm cache):
prek run --all-files (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
47.1 ± 1.0 | 45.7 | 49.5 | 1.01 ± 0.03 |
prek-head run --all-files |
46.6 ± 0.8 | 45.0 | 48.1 | 1.00 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
46.5 ± 0.7 | 45.3 | 47.8 | 1.00 ± 0.02 |
prek-head run --all-files |
46.4 ± 0.7 | 45.1 | 48.1 | 1.00 |
Full Hook Suite
Running the builtin hook suite on the benchmark workspace:
prek run --all-files (full builtin hook suite)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
46.8 ± 1.1 | 44.9 | 48.9 | 1.00 |
prek-head run --all-files |
46.9 ± 0.9 | 45.1 | 49.8 | 1.00 ± 0.03 |
Individual Hook Performance
Benchmarking each hook individually on the test repo:
prek run trailing-whitespace --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run trailing-whitespace --all-files |
14.8 ± 0.4 | 14.0 | 15.6 | 1.03 ± 0.04 |
prek-head run trailing-whitespace --all-files |
14.3 ± 0.3 | 13.9 | 15.0 | 1.00 |
prek run end-of-file-fixer --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run end-of-file-fixer --all-files |
14.5 ± 0.6 | 13.7 | 15.9 | 1.02 ± 0.07 |
prek-head run end-of-file-fixer --all-files |
14.3 ± 0.8 | 13.3 | 17.0 | 1.00 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
6.3 ± 0.2 | 6.0 | 6.9 | 1.00 ± 0.07 |
prek-head run check-json --all-files |
6.3 ± 0.4 | 5.7 | 7.6 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
6.1 ± 0.2 | 5.7 | 6.5 | 1.00 |
prek-head run check-yaml --all-files |
6.1 ± 0.1 | 5.8 | 6.4 | 1.00 ± 0.04 |
prek run check-toml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-toml --all-files |
6.0 ± 0.2 | 5.7 | 6.8 | 1.00 |
prek-head run check-toml --all-files |
6.1 ± 0.2 | 5.7 | 6.6 | 1.01 ± 0.05 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
6.1 ± 0.2 | 5.9 | 6.6 | 1.05 ± 0.04 |
prek-head run check-xml --all-files |
5.8 ± 0.2 | 5.5 | 6.2 | 1.00 |
prek run detect-private-key --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run detect-private-key --all-files |
8.4 ± 0.5 | 7.7 | 9.7 | 1.01 ± 0.08 |
prek-head run detect-private-key --all-files |
8.3 ± 0.5 | 7.6 | 9.9 | 1.00 |
prek run fix-byte-order-marker --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run fix-byte-order-marker --all-files |
14.0 ± 0.5 | 13.3 | 15.1 | 1.01 ± 0.05 |
prek-head run fix-byte-order-marker --all-files |
13.8 ± 0.5 | 13.2 | 15.4 | 1.00 |
Installation Performance
Benchmarking hook installation (fast path hooks skip Python setup):
prek install-hooks (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
3.8 ± 0.1 | 3.7 | 4.0 | 1.04 ± 0.05 |
prek-head install-hooks |
3.7 ± 0.1 | 3.6 | 3.8 | 1.00 |
prek install-hooks (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
3.9 ± 0.1 | 3.8 | 4.1 | 1.05 ± 0.04 |
prek-head install-hooks |
3.7 ± 0.1 | 3.6 | 3.8 | 1.00 |
File Filtering/Scoping Performance
Testing different file selection modes:
prek run (staged files only)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run |
26.6 ± 0.4 | 25.6 | 27.4 | 1.01 ± 0.02 |
prek-head run |
26.3 ± 0.3 | 25.8 | 26.7 | 1.00 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
6.1 ± 0.1 | 6.0 | 6.3 | 1.00 |
prek-head run --files '*.json' |
6.2 ± 0.3 | 5.8 | 7.1 | 1.01 ± 0.05 |
Workspace Discovery & Initialization
Benchmarking hook discovery and initialization overhead:
prek run --dry-run --all-files (measures init overhead)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --dry-run --all-files |
5.5 ± 0.1 | 5.2 | 5.7 | 1.02 ± 0.03 |
prek-head run --dry-run --all-files |
5.4 ± 0.1 | 5.2 | 5.6 | 1.00 |
Meta Hooks Performance
Benchmarking meta hooks separately:
prek run check-hooks-apply --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-hooks-apply --all-files |
9.1 ± 0.2 | 8.8 | 9.6 | 1.00 |
prek-head run check-hooks-apply --all-files |
9.2 ± 0.1 | 9.0 | 9.4 | 1.01 ± 0.03 |
prek run check-useless-excludes --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-useless-excludes --all-files |
9.3 ± 0.1 | 9.1 | 9.6 | 1.06 ± 0.05 |
prek-head run check-useless-excludes --all-files |
8.8 ± 0.4 | 8.2 | 9.4 | 1.00 |
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
7.9 ± 0.1 | 7.7 | 8.1 | 1.03 ± 0.03 |
prek-head run identity --all-files |
7.7 ± 0.2 | 7.3 | 7.9 | 1.00 |
fbb16a2 to
2e9e61f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
crates/prek/src/jj.rs:278
get_conflicted_files()treats any non-zero exit status fromjj resolve --listas “no conflicts”. That’s correct for the “no conflicts” case, but it will also silently swallow real failures (e.g., running outside a jj repo, incompatible jj version, or other command errors), causing prek to proceed without surfacing the underlying problem.
pub(crate) async fn get_conflicted_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
let output = jj_cmd()?
.current_dir(root)
.arg("resolve")
.arg("--list")
.check(false)
.output()
.await?;
if !output.status.success() {
return Ok(Vec::new());
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e9e61f4cb
ℹ️ 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".
2e9e61f to
4b775dc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b775dc551
ℹ️ 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".
723deea to
ca7e4fa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
crates/prek/src/repo.rs:224
changed_files_between()takesworkspace_root, but the Jujutsu branch ignores it and diffs the full repository (repo.root()), relying on later filtering. In large monorepos with nested prek workspaces this can make--from-ref/--to-refnoticeably slower than the Git backend, which scopes the diff toworkspace_root. Consider adding a workspace-root scope to the jj diff call (fileset/path filter) so only relevant paths are computed.
pub(crate) async fn changed_files_between(
old: &str,
new: &str,
workspace_root: &Path,
) -> Result<Vec<PathBuf>> {
let repo = current()?;
match repo.kind() {
RepoKind::Git => git::get_changed_files(old, new, workspace_root)
.await
.map_err(Into::into),
RepoKind::Jujutsu => jj::get_changed_files_between(old, new, repo.root())
.await
.map_err(Into::into),
crates/prek/tests/builtin_hooks.rs:26
jj_cmd()is duplicated across multiple integration test files (this one andcrates/prek/tests/run.rs). Duplicated helpers tend to drift (e.g., env vars, defaults), which can make future jj-related test changes inconsistent. Consider moving this helper intocrates/prek/tests/common/mod.rs(or a dedicated test helper module) and reusing it from both test files.
// Give jj a deterministic identity so `jj commit` does not depend on host config.
cmd.env("JJ_USER", "prek test");
cmd.env("JJ_EMAIL", "prek-test@example.com");
Some(cmd)
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca7e4fabb5
ℹ️ 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: 21ee3de3ee
ℹ️ 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".
- Use merge-base (fork_point) semantics for jj --from-ref/--to-ref, matching git's old...new, so edits only on the from-side are not selected. - Detect the nearest repository boundary: a git repo nested inside a jj workspace is treated as Git, not the outer workspace. - Test jj on the Windows CI job (pinned x86_64-pc-windows-msvc release); add the macOS x86_64 checksum and select the jj archive by RUNNER_OS/RUNNER_ARCH. - Give the builtin_hooks jj test helper a deterministic identity. - Fix stale git-only wording in docs/workspace.md. - Handle jj resolve --list exit code correctly instead of silently swallowing errors. - Fix docstring for added_files to accurately state it returns newly introduced files in jj. - Map Git's default `HEAD` and `HEAD~1` to jj's `@` and `@-` respectively in `get_changed_files_between` for robust target defaulting. - Clarify in comments that jj operations run from the *repository* root, not the workspace root. - Scope jj diff to workspace_root in default_files and changed_files_between for performance. - Handle non-colocated jj stores by falling back to .jj/repo/store/git when git_target is missing. - Fix Windows CI test failures by using dunce::canonicalize consistently in tests. - Improve CI portability by using sha256sum with a fallback to shasum.
21ee3de to
44da0ad
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
crates/prek/src/jj.rs:104
JJ.as_ref().map_err(|&e| ...)attempts to movewhich::Errorout of a reference.which::ErrorisClonebut notCopy, so this pattern does not compile. Clone the error (or use?with a cloned error) instead.
/// Create a new `Cmd` for running Jujutsu.
pub(crate) fn jj_cmd() -> Result<Cmd, Error> {
let cmd = Cmd::new(JJ.as_ref().map_err(|&e| Error::JjNotFound(e))?);
Ok(cmd)
crates/prek/src/git.rs:146
- This
git_cmd()function still usesGIT.as_ref().map_err(|&e| ...)(a couple lines above) which attempts to movewhich::Errorout of a&which::Error.which::ErrorisClonebut notCopy, so this pattern does not compile. Clone the error (and apply the same fix to the other occurrences of this pattern in this file, e.g.get_root()and any tests/helpers).
if let Some(work_tree) = git_work_tree() {
cmd.env(EnvVars::GIT_WORK_TREE, work_tree);
}
// For a Jujutsu workspace this points Git at the backing Git store; for a plain
// Git repo it is a no-op.
crate::repo::apply_git_env(&mut cmd);
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44da0add01
ℹ️ 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".
872a1e3 to
156951c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
crates/prek/src/jj.rs:158
parse_diff_entries()also usesString::from_utf8_lossy(), which can corrupt non-UTF-8 filenames on Unix and lead to incorrect file selection (e.g., skipping a changed file because the produced path doesn’t match the filesystem). Parsing the<status><space><path>format as raw bytes on Unix avoids silent corruption.
fn parse_diff_entries(output: &[u8]) -> Vec<DiffEntry> {
String::from_utf8_lossy(output)
.lines()
.filter_map(|line| {
let (status, path) = line.split_once(' ')?;
| fn parse_path_lines(output: &[u8]) -> Vec<PathBuf> { | ||
| String::from_utf8_lossy(output) | ||
| .lines() | ||
| .filter(|line| !line.is_empty()) | ||
| .map(PathBuf::from) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 156951c1fd
ℹ️ 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".
156951c to
40cc0a5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40cc0a50d1
ℹ️ 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".
40cc0a5 to
327696c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 327696c759
ℹ️ 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".
| /// Template for `jj diff` that emits one `<status> <path>` line per changed file. | ||
| /// This avoids `--types`, whose git-style `{a => b}` rename compaction yields a | ||
| /// "path" that names no real file; `path` here is always the real target path. | ||
| const DIFF_TEMPLATE: &str = r#"status_char ++ " " ++ path ++ "\n""#; |
There was a problem hiding this comment.
In jj workspaces, a tracked file whose name contains a newline is a valid repository path, but this template uses newline as the record separator and parse_diff_entries() consumes the output with .lines(). For default and --from-ref/--to-ref runs that splits one real path into multiple records, so hooks can be run on a truncated non-existent path and skip the actual file; the analogous FILE_LIST_TEMPLATE above has the same problem for --all-files. Git paths are already parsed with NUL separators, and jj templates support the \0 string escape (docs), so emit and parse NUL-delimited records instead.
Useful? React with 👍 / 👎.
From audit and completeness passes:
- Select renamed files by their real path. jj diff --types used git-style
'{a => b}' compaction that named no file, so renames silently bypassed hooks;
parse a 'status_char ++ path' template instead.
- Report added files relative to the project dir. That template's path is
repo-root-relative, so added-file detection keeps using cwd-relative --types,
scoped to the project, matching git's --relative (fixes nested-project hooks).
- Skip no-commit-to-branch under jj: no current-branch equivalent, and a
non-colocated backing store's unborn HEAD blocked every run.
- Document the index/merge-state hooks with limited jj support and the colocated
git-hook file-selection behavior.
- Add end-to-end secondary-workspace and nested-project tests, and make CI fail
rather than silently skip the jj tests when jj is missing (PREK_TEST_REQUIRE_JJ).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
327696c to
f50b3b3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f50b3b340f
ℹ️ 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".
| .filter(|path| !path.as_os_str().is_empty()) | ||
| .unwrap_or(path) |
There was a problem hiding this comment.
. when scoping the jj root
When the prek workspace root is exactly the jj workspace root, this drops the empty relative path and falls back to the absolute root, so default runs and --all-files build filesets like cwd:"/tmp/repo". Jujutsu's fileset docs define cwd: as a cwd-relative pattern, so that scope does not match repository paths and root-level jj workspaces collect no files; return ./all() for the equal-root case instead.
Useful? React with 👍 / 👎.
Adds Jujutsu (jj) workspace support so
prek runworks inside jj workspaces, including secondary workspaces created withjj workspace addthat have no.gitdirectory.Builds on #1677 by @martijnberger, rebased onto current master. The first commit preserves the original work (and its authorship); a second commit holds the follow-up review fixes.
.jj/repo/store/git_targetand point git commands at it per-command, so no.gitis required.prek runuses the working-copy changeset;--all-files,--from-ref/--to-ref, and conflict detection (jj resolve --list) use jj equivalents. Deleted paths are excluded, matching git's--diff-filter.try-repo's git commands so they target the external/shadow repo, not the jj backing store.Supersedes #1677.