From 903d28116e22fd21bd5bc2321bcbc1af7b6367f1 Mon Sep 17 00:00:00 2001 From: Marius1311 Date: Mon, 12 Oct 2026 14:33:41 +0200 Subject: [PATCH 1/6] Batch jobs from a worktree run on the main checkout's environment guard_task_paths also refuses a job script in a worktree that runs pixi on the workspace it finds: the job would build the root environment into the worktree, out of reach of the Bash pixi guard. The skill and task template say where job scripts go, how they get the main checkout ($MAIN at submit) and where they log; a worktree is removed only once its jobs are done. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- hooks/_repo.py | 22 ++++++++++++++++++ hooks/guard_task_paths.py | 23 ++++++++++++++++++- hooks/guard_worktree_pixi.py | 16 ++++--------- hooks/session_context.md | 4 ++-- skills/analysis-workflow/SKILL.md | 4 ++++ .../assets/task_template/README.md | 1 + tests/test_hooks.py | 19 +++++++++++++++ 8 files changed, 75 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 7ab4f81..8c54160 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ claude --worktree answer to the same question gets the next version, in a new session. 2. It asks you which working object is current, then works in scripts; outputs go to the task directory. 3. Results reach a shared working object only after you sign off the dry run. -4. It opens a PR. Once it is merged, leave with `/exit` and remove the worktree. +4. It opens a PR. Once it is merged and its batch jobs have finished, leave with `/exit` and remove the worktree. Notebooks and environment changes (`pixi install`, `pixi add`) stay with you, in the main checkout. With no analysis repo yet, ask Claude to start an analysis project: the skill sets one up. diff --git a/hooks/_repo.py b/hooks/_repo.py index 9bc75f8..bec7a1c 100644 --- a/hooks/_repo.py +++ b/hooks/_repo.py @@ -37,3 +37,25 @@ def adopted(checkout: Path) -> bool: return any(key.split("@")[0] == PLUGIN and on is True for key, on in enabled.items()) except (OSError, ValueError, AttributeError): return False + + +def nearest_manifest(directory: Path) -> Path: + """The `pixi.toml` pixi itself finds from ``directory``: the nearest at or above it.""" + return next( + (d / "pixi.toml" for d in (directory, *directory.parents) if (d / "pixi.toml").exists()), + directory / "pixi.toml", + ) + + +def is_root_of_worktree(manifest: Path) -> bool: + """Whether ``manifest`` is the root workspace of a worktree of an adopted repo. + + Its editable self-dependency (`path = "."`) would rebase the repo's package, and every path it + resolves, into the worktree. A task-local manifest is its own workspace. + """ + if ".claude/worktrees" not in str(manifest): + return False + if manifest.exists() and 'path = "."' not in manifest.read_text(errors="ignore"): + return False + found = checkouts(manifest.parent) + return found is not None and adopted(found[0]) diff --git a/hooks/guard_task_paths.py b/hooks/guard_task_paths.py index 49648ce..04d4458 100644 --- a/hooks/guard_task_paths.py +++ b/hooks/guard_task_paths.py @@ -6,6 +6,9 @@ usually gitignored, so git never warns. Absolute paths into a worktree, a temp dir or the checkout itself outlive what they point at, or break on the next machine. +A job script written in a worktree must not run `pixi` on the workspace it finds itself: the job +would build the root environment into the worktree, out of reach of the Bash pixi guard. + It acts only in a repo that has adopted the plugin. Only the text being written is inspected, so pre-existing violations elsewhere in a file do not trip the hook; they surface when that line is next touched, which is when to fix them. @@ -19,10 +22,16 @@ import sys from pathlib import Path -from _repo import adopted, checkouts +from _repo import adopted, checkouts, is_root_of_worktree, nearest_manifest # Code that runs. Prose may legitimately quote any of these paths. CODE_SUFFIXES = {".py", ".sh", ".sbatch", ".ipynb", ".R"} +JOB_SCRIPT_SUFFIXES = {".sh", ".sbatch"} + +#: A non-comment line running `pixi` on whatever workspace it finds, rather than one named by `--manifest-path`. +PIXI_ON_FOUND_WORKSPACE = re.compile( + r"^(?!\s*#)(?!.*--manifest-path).*\bpixi\b.*\b(run|shell|install|add|lock)\b", re.MULTILINE +) def _rules(repo: str) -> list[tuple[re.Pattern, str, str]]: @@ -65,6 +74,18 @@ def main() -> int: # Whatever this call would add: Write.content, Edit.new_string, NotebookEdit.new_source written = "\n".join(str(tool_input.get(key, "")) for key in ("content", "new_string", "new_source")) hits = [(what, fix) for pattern, what, fix in _rules(found[1].name) if pattern.search(written)] + if ( + file_path.suffix in JOB_SCRIPT_SUFFIXES + and PIXI_ON_FOUND_WORKSPACE.search(written) + and is_root_of_worktree(nearest_manifest(file_path.parent)) + ): + hits.append( + ( + "a `pixi` call on the worktree's own workspace", + 'jobs run on the main checkout\'s environment: pixi run --manifest-path "$MAIN/pixi.toml", ' + "with MAIN passed at submit (sbatch --export=ALL,MAIN=
)", + ) + ) if not hits: return 0 diff --git a/hooks/guard_worktree_pixi.py b/hooks/guard_worktree_pixi.py index 60dd9cb..bb3767b 100644 --- a/hooks/guard_worktree_pixi.py +++ b/hooks/guard_worktree_pixi.py @@ -24,7 +24,7 @@ import sys from pathlib import Path -from _repo import adopted, checkouts +from _repo import adopted, checkouts, is_root_of_worktree, nearest_manifest _MUTATING = {"install", "add", "lock", "upgrade", "update", "reinstall", "remove"} # These use the workspace environment, installing it first if it is missing. @@ -75,22 +75,14 @@ def _manifest(args: list[str], cwd: Path) -> Path | None: if a.startswith("--manifest-path="): path = Path(a.split("=", 1)[1]) break - else: # pixi's own discovery: the nearest pixi.toml at or above cwd - return next((d / "pixi.toml" for d in (cwd, *cwd.parents) if (d / "pixi.toml").exists()), cwd / "pixi.toml") + else: + return nearest_manifest(cwd) if any(c in str(path) for c in "$`"): return None # shell-expanded at run time, so not provably outside a worktree path = (cwd / path).resolve() if not path.is_absolute() else path.resolve() return path / "pixi.toml" if path.is_dir() else path -def _is_root_of_worktree(manifest: Path) -> bool: - if ".claude/worktrees" not in str(manifest): - return False - if manifest.exists() and 'path = "."' not in manifest.read_text(errors="ignore"): - return False # task-local workspace - return _in_adopted_repo(manifest.parent) - - def _in_adopted_repo(directory: Path) -> bool: found = checkouts(directory) return found is not None and adopted(found[0]) @@ -110,7 +102,7 @@ def main() -> int: blocked = ".claude/worktrees" in str(cwd) and re.search(pattern, command) and _in_adopted_repo(cwd) else: manifests = [_manifest(args, cwd) for args in calls] - blocked = any(_in_adopted_repo(cwd) if m is None else _is_root_of_worktree(m) for m in manifests) + blocked = any(_in_adopted_repo(cwd) if m is None else is_root_of_worktree(m) for m in manifests) if not blocked: return 0 diff --git a/hooks/session_context.md b/hooks/session_context.md index a40253e..8ccbd60 100644 --- a/hooks/session_context.md +++ b/hooks/session_context.md @@ -8,5 +8,5 @@ analysis code, outputs or data. Always: newest file name. 4. Shared objects grow by addition only (`commit_adata`), and only after sign-off on the dry run. 5. Agents never write to `data//results/` or the central `figures/`; those are the human lane. -6. A worktree runs on the main checkout's environment (`pixi run --manifest-path
/pixi.toml ...`); - environment changes happen in the main checkout. +6. A worktree, and every batch job submitted from it, runs on the main checkout's environment + (`pixi run --manifest-path
/pixi.toml ...`); environment changes happen in the main checkout. diff --git a/skills/analysis-workflow/SKILL.md b/skills/analysis-workflow/SKILL.md index 7449ef8..1066809 100644 --- a/skills/analysis-workflow/SKILL.md +++ b/skills/analysis-workflow/SKILL.md @@ -62,6 +62,9 @@ into the human lane. - Another task's untracked outputs: `task_paths().outputs`. Read the main checkout by absolute path; never `cd` into it. +- Batch-job scripts sit in `scripts/` beside the code they run. They take the main checkout as `$MAIN` at submit, + never hardcoded, run `pixi run --manifest-path "$MAIN/pixi.toml"`, and log to `PATHS.logs`; the template's + Running section shows the `sbatch` line. - A figure that prints a number may carry `scripts/verify.py` re-deriving it. A code package's invariants belong in that package's tests. @@ -71,6 +74,7 @@ into the human lane. 3. An object other tasks will load is promoted to `data//processed/` at sign-off. Ask which subdirectory; record the path. 4. `analysis-workflow check layout`, then push. +5. The worktree is removed only once no batch job submitted from it is still queued or running. ## Data diff --git a/skills/analysis-workflow/assets/task_template/README.md b/skills/analysis-workflow/assets/task_template/README.md index 52fa8dd..60d2fa5 100644 --- a/skills/analysis-workflow/assets/task_template/README.md +++ b/skills/analysis-workflow/assets/task_template/README.md @@ -38,6 +38,7 @@ From the task directory, on the main checkout's environment. Batch jobs log to ` PY="pixi run --manifest-path
/pixi.toml python" $PY scripts/.py LOGS=$($PY -c "from _common import PATHS; print(PATHS.ensure().logs)") +sbatch --output="$LOGS/%x-%j.out" --export=ALL,MAIN=
scripts/.sh ``` ## Notes diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 502993d..4c0449b 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -92,6 +92,25 @@ def test_pixi_allows_running_on_the_main_checkouts_environment(repo, pixi_worktr assert run("guard_worktree_pixi.py", bash(command, pixi_worktree)) == 0 +@pytest.mark.parametrize("line", ["pixi run python scripts/a.py", "srun -n1 pixi run python a.py", "pixi install"]) +def test_task_paths_blocks_job_scripts_running_pixi_on_the_worktree(pixi_worktree, line): + job = pixi_worktree / "analysis" / "t_v1" / "scripts" / "run.sh" + assert run("guard_task_paths.py", write(job, f"#!/bin/bash\n{line}\n")) == 2 + + +@pytest.mark.parametrize( + ("rel", "line"), + [ + ("analysis/t_v1/scripts/run.sh", 'pixi run --manifest-path "$MAIN/pixi.toml" python scripts/a.py'), + ("analysis/t_v1/scripts/run.sh", "# not pixi run python a.py, see the README"), + ("analysis/bench_v1/run.sh", "pixi run python a.py"), # task-local workspace + ("analysis/t_v1/scripts/run.py", 'print("pixi run python a.py")'), # not a job script + ], +) +def test_task_paths_allows_job_scripts(pixi_worktree, rel, line): + assert run("guard_task_paths.py", write(pixi_worktree / rel, f"#!/bin/bash\n{line}\n")) == 0 + + def test_pixi_allows_the_main_checkout(repo): main, _ = repo (main / "pixi.toml").write_text('[pypi-dependencies]\nmyproject = { path = ".", editable = true }\n') From 2fcb98956b2a3b102f2a398580ea297c3cef9d4b Mon Sep 17 00:00:00 2001 From: Marius1311 Date: Mon, 12 Oct 2026 14:46:11 +0200 Subject: [PATCH 2/6] Address review on #5: continued lines, relative manifest paths - Join backslash continuations before matching a job script's pixi calls. - Exempt only a manifest named through a shell variable ($MAIN); a relative path resolves against the job's directory, unknown when it is written. Co-Authored-By: Claude Opus 5.5 --- hooks/guard_task_paths.py | 7 ++++--- tests/test_hooks.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hooks/guard_task_paths.py b/hooks/guard_task_paths.py index 04d4458..9c8af05 100644 --- a/hooks/guard_task_paths.py +++ b/hooks/guard_task_paths.py @@ -28,9 +28,10 @@ CODE_SUFFIXES = {".py", ".sh", ".sbatch", ".ipynb", ".R"} JOB_SCRIPT_SUFFIXES = {".sh", ".sbatch"} -#: A non-comment line running `pixi` on whatever workspace it finds, rather than one named by `--manifest-path`. +#: A non-comment line running `pixi` on any workspace but one named through a shell variable (`$MAIN`): a relative +#: `--manifest-path` resolves against the job's directory, which is unknown when the script is written. PIXI_ON_FOUND_WORKSPACE = re.compile( - r"^(?!\s*#)(?!.*--manifest-path).*\bpixi\b.*\b(run|shell|install|add|lock)\b", re.MULTILINE + r"""^(?!\s*#)(?!.*--manifest-path[= ]["']?\$).*\bpixi\b.*\b(run|shell|install|add|lock)\b""", re.MULTILINE ) @@ -76,7 +77,7 @@ def main() -> int: hits = [(what, fix) for pattern, what, fix in _rules(found[1].name) if pattern.search(written)] if ( file_path.suffix in JOB_SCRIPT_SUFFIXES - and PIXI_ON_FOUND_WORKSPACE.search(written) + and PIXI_ON_FOUND_WORKSPACE.search(written.replace("\\\n", " ")) # one line per command and is_root_of_worktree(nearest_manifest(file_path.parent)) ): hits.append( diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 4c0449b..0b2b15e 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -92,7 +92,16 @@ def test_pixi_allows_running_on_the_main_checkouts_environment(repo, pixi_worktr assert run("guard_worktree_pixi.py", bash(command, pixi_worktree)) == 0 -@pytest.mark.parametrize("line", ["pixi run python scripts/a.py", "srun -n1 pixi run python a.py", "pixi install"]) +@pytest.mark.parametrize( + "line", + [ + "pixi run python scripts/a.py", + "srun -n1 pixi run python a.py", + "pixi install", + "pixi \\\n run python a.py", # continued line + "pixi run --manifest-path ../../../pixi.toml python a.py", # resolves against the job's directory + ], +) def test_task_paths_blocks_job_scripts_running_pixi_on_the_worktree(pixi_worktree, line): job = pixi_worktree / "analysis" / "t_v1" / "scripts" / "run.sh" assert run("guard_task_paths.py", write(job, f"#!/bin/bash\n{line}\n")) == 2 From 8a4ee1372a17379c749f055389cfe6d3b6c26228 Mon Sep 17 00:00:00 2001 From: Marius1311 Date: Mon, 12 Oct 2026 15:05:25 +0200 Subject: [PATCH 3/6] Leave how to submit batch jobs to the cluster's skill Co-Authored-By: Claude Opus 5.5 --- hooks/guard_task_paths.py | 2 +- skills/analysis-workflow/SKILL.md | 6 +++--- skills/analysis-workflow/assets/task_template/README.md | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hooks/guard_task_paths.py b/hooks/guard_task_paths.py index 9c8af05..359e01e 100644 --- a/hooks/guard_task_paths.py +++ b/hooks/guard_task_paths.py @@ -84,7 +84,7 @@ def main() -> int: ( "a `pixi` call on the worktree's own workspace", 'jobs run on the main checkout\'s environment: pixi run --manifest-path "$MAIN/pixi.toml", ' - "with MAIN passed at submit (sbatch --export=ALL,MAIN=
)", + "with MAIN passed in at submit", ) ) if not hits: diff --git a/skills/analysis-workflow/SKILL.md b/skills/analysis-workflow/SKILL.md index 1066809..85a22f7 100644 --- a/skills/analysis-workflow/SKILL.md +++ b/skills/analysis-workflow/SKILL.md @@ -62,9 +62,9 @@ into the human lane. - Another task's untracked outputs: `task_paths().outputs`. Read the main checkout by absolute path; never `cd` into it. -- Batch-job scripts sit in `scripts/` beside the code they run. They take the main checkout as `$MAIN` at submit, - never hardcoded, run `pixi run --manifest-path "$MAIN/pixi.toml"`, and log to `PATHS.logs`; the template's - Running section shows the `sbatch` line. +- Batch-job scripts sit in `scripts/` beside the code they run. They get the main checkout as `$MAIN` from the + submit command, never hardcoded, run `pixi run --manifest-path "$MAIN/pixi.toml"`, and log to `PATHS.logs`. How + to submit them is the cluster's skill. - A figure that prints a number may carry `scripts/verify.py` re-deriving it. A code package's invariants belong in that package's tests. diff --git a/skills/analysis-workflow/assets/task_template/README.md b/skills/analysis-workflow/assets/task_template/README.md index 60d2fa5..855abf4 100644 --- a/skills/analysis-workflow/assets/task_template/README.md +++ b/skills/analysis-workflow/assets/task_template/README.md @@ -32,13 +32,13 @@ Anything promoted to `data//processed/` at sign-off is recorded here wi ## Running -From the task directory, on the main checkout's environment. Batch jobs log to `PATHS.logs`: +From the task directory, on the main checkout's environment. Batch jobs get `MAIN=
` at submit and +log to `PATHS.logs`: ```bash PY="pixi run --manifest-path
/pixi.toml python" $PY scripts/.py LOGS=$($PY -c "from _common import PATHS; print(PATHS.ensure().logs)") -sbatch --output="$LOGS/%x-%j.out" --export=ALL,MAIN=
scripts/.sh ``` ## Notes From 4bdd4add0c2b39d1e3bd361413396f4024c65508 Mon Sep 17 00:00:00 2001 From: Marius1311 Date: Mon, 12 Oct 2026 15:17:52 +0200 Subject: [PATCH 4/6] Address review on #5: each pixi call checked, one subcommand set - A job script's pixi calls are checked one command at a time, so a safe call no longer exempts an unsafe one on the same line. - Both guards share PIXI_ENV_COMMANDS, which adds update, upgrade, reinstall and remove to the job-script check. Co-Authored-By: Claude Opus 5.5 --- hooks/_repo.py | 5 +++++ hooks/guard_task_paths.py | 24 +++++++++++++++++------- hooks/guard_worktree_pixi.py | 7 ++----- tests/test_hooks.py | 2 ++ 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/hooks/_repo.py b/hooks/_repo.py index bec7a1c..c971c83 100644 --- a/hooks/_repo.py +++ b/hooks/_repo.py @@ -11,6 +11,11 @@ PLUGIN = "analysis-workflow" +#: `pixi` subcommands that change the workspace environment, or use it and so install it first if it is missing. +PIXI_ENV_COMMANDS = frozenset( + {"install", "add", "lock", "upgrade", "update", "reinstall", "remove", "run", "shell", "shell-hook"} +) + def checkouts(directory: Path) -> tuple[Path, Path] | None: """The checkout containing ``directory`` and the repository's main checkout; None outside git.""" diff --git a/hooks/guard_task_paths.py b/hooks/guard_task_paths.py index 359e01e..236dc70 100644 --- a/hooks/guard_task_paths.py +++ b/hooks/guard_task_paths.py @@ -22,17 +22,27 @@ import sys from pathlib import Path -from _repo import adopted, checkouts, is_root_of_worktree, nearest_manifest +from _repo import PIXI_ENV_COMMANDS, adopted, checkouts, is_root_of_worktree, nearest_manifest # Code that runs. Prose may legitimately quote any of these paths. CODE_SUFFIXES = {".py", ".sh", ".sbatch", ".ipynb", ".R"} JOB_SCRIPT_SUFFIXES = {".sh", ".sbatch"} -#: A non-comment line running `pixi` on any workspace but one named through a shell variable (`$MAIN`): a relative -#: `--manifest-path` resolves against the job's directory, which is unknown when the script is written. -PIXI_ON_FOUND_WORKSPACE = re.compile( - r"""^(?!\s*#)(?!.*--manifest-path[= ]["']?\$).*\bpixi\b.*\b(run|shell|install|add|lock)\b""", re.MULTILINE -) +PIXI_CALL = re.compile(rf"\bpixi\b.*\b({'|'.join(sorted(PIXI_ENV_COMMANDS))})\b") +#: The one workspace a job may name: through a shell variable (`$MAIN`). A relative `--manifest-path` resolves +#: against the job's directory, which is unknown when the script is written. +NAMED_MANIFEST = re.compile(r"""--manifest-path[= ]["']?\$""") + + +def _pixi_on_found_workspace(script: str) -> bool: + """Whether any command in ``script`` runs `pixi` on a workspace it finds rather than one named via a variable.""" + for line in script.replace("\\\n", " ").splitlines(): # one line per command + if line.lstrip().startswith("#"): + continue + for command in re.split(r"&&|\|\||[;|&]", line): + if PIXI_CALL.search(command) and not NAMED_MANIFEST.search(command): + return True + return False def _rules(repo: str) -> list[tuple[re.Pattern, str, str]]: @@ -77,7 +87,7 @@ def main() -> int: hits = [(what, fix) for pattern, what, fix in _rules(found[1].name) if pattern.search(written)] if ( file_path.suffix in JOB_SCRIPT_SUFFIXES - and PIXI_ON_FOUND_WORKSPACE.search(written.replace("\\\n", " ")) # one line per command + and _pixi_on_found_workspace(written) and is_root_of_worktree(nearest_manifest(file_path.parent)) ): hits.append( diff --git a/hooks/guard_worktree_pixi.py b/hooks/guard_worktree_pixi.py index bb3767b..ab5a1e5 100644 --- a/hooks/guard_worktree_pixi.py +++ b/hooks/guard_worktree_pixi.py @@ -24,12 +24,9 @@ import sys from pathlib import Path -from _repo import adopted, checkouts, is_root_of_worktree, nearest_manifest +from _repo import PIXI_ENV_COMMANDS, adopted, checkouts, is_root_of_worktree, nearest_manifest -_MUTATING = {"install", "add", "lock", "upgrade", "update", "reinstall", "remove"} -# These use the workspace environment, installing it first if it is missing. -_ENV_USING = {"run", "shell", "shell-hook"} -_BLOCKED = _MUTATING | _ENV_USING +_BLOCKED = PIXI_ENV_COMMANDS # The first of these after `pixi` is the command, whatever global options precede it. _SUBCOMMANDS = _BLOCKED | { "auth", "build", "clean", "completion", "config", "exec", "global", "import", "info", "init", "list", diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 0b2b15e..910fa9f 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -100,6 +100,8 @@ def test_pixi_allows_running_on_the_main_checkouts_environment(repo, pixi_worktr "pixi install", "pixi \\\n run python a.py", # continued line "pixi run --manifest-path ../../../pixi.toml python a.py", # resolves against the job's directory + 'pixi run python a.py && pixi run --manifest-path "$MAIN/pixi.toml" python b.py', # each call checked + "pixi update", ], ) def test_task_paths_blocks_job_scripts_running_pixi_on_the_worktree(pixi_worktree, line): From d89ed25d4d72322304f9711d890d66d15099605f Mon Sep 17 00:00:00 2001 From: Marius1311 Date: Mon, 12 Oct 2026 15:35:30 +0200 Subject: [PATCH 5/6] Address review on #5: only $MAIN exempts, comments ignored - A job's pixi call is exempt only with --manifest-path "$MAIN/...": a path derived in the job can resolve to the worktree itself. - Shell comments are stripped before the check, so a comment mentioning the flag doesn't exempt the call before it. Co-Authored-By: Claude Opus 5.5 --- hooks/guard_task_paths.py | 11 +++++------ tests/test_hooks.py | 3 +++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/hooks/guard_task_paths.py b/hooks/guard_task_paths.py index 236dc70..74b93a3 100644 --- a/hooks/guard_task_paths.py +++ b/hooks/guard_task_paths.py @@ -29,16 +29,15 @@ JOB_SCRIPT_SUFFIXES = {".sh", ".sbatch"} PIXI_CALL = re.compile(rf"\bpixi\b.*\b({'|'.join(sorted(PIXI_ENV_COMMANDS))})\b") -#: The one workspace a job may name: through a shell variable (`$MAIN`). A relative `--manifest-path` resolves -#: against the job's directory, which is unknown when the script is written. -NAMED_MANIFEST = re.compile(r"""--manifest-path[= ]["']?\$""") +#: The one workspace a job may name: the main checkout's, as `$MAIN` passed in at submit. Any other path, relative +#: or derived in the job (`git rev-parse --show-toplevel`), can resolve to the worktree itself. +NAMED_MANIFEST = re.compile(r"""--manifest-path[= ]["']?\$\{?MAIN\}?/""") def _pixi_on_found_workspace(script: str) -> bool: - """Whether any command in ``script`` runs `pixi` on a workspace it finds rather than one named via a variable.""" + """Whether any command in ``script`` runs `pixi` on a workspace other than the main checkout's.""" for line in script.replace("\\\n", " ").splitlines(): # one line per command - if line.lstrip().startswith("#"): - continue + line = re.sub(r"(^|\s)#.*", "", line) # what the shell runs, not its comments for command in re.split(r"&&|\|\||[;|&]", line): if PIXI_CALL.search(command) and not NAMED_MANIFEST.search(command): return True diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 910fa9f..6f8ee29 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -102,6 +102,8 @@ def test_pixi_allows_running_on_the_main_checkouts_environment(repo, pixi_worktr "pixi run --manifest-path ../../../pixi.toml python a.py", # resolves against the job's directory 'pixi run python a.py && pixi run --manifest-path "$MAIN/pixi.toml" python b.py', # each call checked "pixi update", + 'ROOT=$(git rev-parse --show-toplevel); pixi run --manifest-path "$ROOT/pixi.toml" python a.py', + 'pixi run python a.py # TODO: --manifest-path "$MAIN/pixi.toml"', ], ) def test_task_paths_blocks_job_scripts_running_pixi_on_the_worktree(pixi_worktree, line): @@ -113,6 +115,7 @@ def test_task_paths_blocks_job_scripts_running_pixi_on_the_worktree(pixi_worktre ("rel", "line"), [ ("analysis/t_v1/scripts/run.sh", 'pixi run --manifest-path "$MAIN/pixi.toml" python scripts/a.py'), + ("analysis/t_v1/scripts/run.sh", "pixi run --manifest-path=${MAIN}/pixi.toml python scripts/a.py"), ("analysis/t_v1/scripts/run.sh", "# not pixi run python a.py, see the README"), ("analysis/bench_v1/run.sh", "pixi run python a.py"), # task-local workspace ("analysis/t_v1/scripts/run.py", 'print("pixi run python a.py")'), # not a job script From a79d649723f7d4318cec9b7b7fed191716e2f9a5 Mon Sep 17 00:00:00 2001 From: Marius1311 Date: Mon, 12 Oct 2026 15:42:21 +0200 Subject: [PATCH 6/6] Address review on #5: parse job scripts like the Bash guard Both guards share one quote-aware parser (pixi_calls) and one rule for the manifest a call names (manifest_arg). In job scripts, # starts a comment only outside quotes, and in `pixi run` a --manifest-path after the task is the task's argument, not pixi's. Co-Authored-By: Claude Opus 5.5 --- hooks/_repo.py | 55 +++++++++++++++++++++++++++++++++++ hooks/guard_task_paths.py | 15 ++++------ hooks/guard_worktree_pixi.py | 56 +++++------------------------------- tests/test_hooks.py | 2 ++ 4 files changed, 70 insertions(+), 58 deletions(-) diff --git a/hooks/_repo.py b/hooks/_repo.py index c971c83..145ffbf 100644 --- a/hooks/_repo.py +++ b/hooks/_repo.py @@ -6,6 +6,7 @@ """ import json +import shlex import subprocess from pathlib import Path @@ -15,6 +16,11 @@ PIXI_ENV_COMMANDS = frozenset( {"install", "add", "lock", "upgrade", "update", "reinstall", "remove", "run", "shell", "shell-hook"} ) +#: The first of these after `pixi` is the command, whatever global options precede it. +PIXI_SUBCOMMANDS = PIXI_ENV_COMMANDS | { + "auth", "build", "clean", "completion", "config", "exec", "global", "import", "info", "init", "list", + "search", "self-update", "task", "tree", "upload", "workspace", +} # fmt: skip def checkouts(directory: Path) -> tuple[Path, Path] | None: @@ -64,3 +70,52 @@ def is_root_of_worktree(manifest: Path) -> bool: return False found = checkouts(manifest.parent) return found is not None and adopted(found[0]) + + +def _is_separator(tok: str) -> bool: + """A token made only of command separators (`;`, `&&`, `|`, newlines, parentheses).""" + return bool(tok) and set(tok) <= set(";&|()\n") + + +def pixi_calls(command: str, comments: bool = False) -> list[list[str]] | None: + """Argument lists of the `pixi` calls in ``command`` that change or use an environment; None if unparsable. + + With ``comments``, an unquoted `#` starts a comment to the end of its line, as in a script. Quotes are + honoured either way, so quoted text (a PR body, an echoed message) is never read as a call. + """ + tokens = [] + for text in command.splitlines(keepends=True) if comments else [command]: + lexer = shlex.shlex(text, posix=True, punctuation_chars="();<>|&\n") + lexer.whitespace = " \t\r" # newlines separate commands, so keep them as tokens + lexer.commenters = "#" if comments else "" # one line at a time: a comment swallows its newline + lexer.whitespace_split = True + try: + tokens += [*lexer, "\n"] + except ValueError: + return None + calls = [] + for i, tok in enumerate(tokens): + if Path(tok).name != "pixi": + continue + args = [] + for t in tokens[i + 1 :]: + if _is_separator(t): + break + args.append(t) + if next((a for a in args if a in PIXI_SUBCOMMANDS), None) in PIXI_ENV_COMMANDS: + calls.append(args) + return calls + + +def manifest_arg(args: list[str]) -> str | None: + """The `--manifest-path` a pixi call passes, if any. In `pixi run`, arguments after the task are the task's.""" + command = next((a for a in args if a in PIXI_SUBCOMMANDS), None) + rest = args[args.index(command) + 1 :] if command else args + for i, a in enumerate(rest): + if a in ("--manifest-path", "-m"): + return rest[i + 1] if i + 1 < len(rest) else None + if a.startswith("--manifest-path="): + return a.split("=", 1)[1] + if command == "run" and not a.startswith("-"): + return None + return None diff --git a/hooks/guard_task_paths.py b/hooks/guard_task_paths.py index 74b93a3..bcc2f3d 100644 --- a/hooks/guard_task_paths.py +++ b/hooks/guard_task_paths.py @@ -22,26 +22,23 @@ import sys from pathlib import Path -from _repo import PIXI_ENV_COMMANDS, adopted, checkouts, is_root_of_worktree, nearest_manifest +from _repo import PIXI_ENV_COMMANDS, adopted, checkouts, is_root_of_worktree, manifest_arg, nearest_manifest, pixi_calls # Code that runs. Prose may legitimately quote any of these paths. CODE_SUFFIXES = {".py", ".sh", ".sbatch", ".ipynb", ".R"} JOB_SCRIPT_SUFFIXES = {".sh", ".sbatch"} -PIXI_CALL = re.compile(rf"\bpixi\b.*\b({'|'.join(sorted(PIXI_ENV_COMMANDS))})\b") #: The one workspace a job may name: the main checkout's, as `$MAIN` passed in at submit. Any other path, relative #: or derived in the job (`git rev-parse --show-toplevel`), can resolve to the worktree itself. -NAMED_MANIFEST = re.compile(r"""--manifest-path[= ]["']?\$\{?MAIN\}?/""") +MAIN_MANIFEST = re.compile(r"\$\{?MAIN\}?/") def _pixi_on_found_workspace(script: str) -> bool: """Whether any command in ``script`` runs `pixi` on a workspace other than the main checkout's.""" - for line in script.replace("\\\n", " ").splitlines(): # one line per command - line = re.sub(r"(^|\s)#.*", "", line) # what the shell runs, not its comments - for command in re.split(r"&&|\|\||[;|&]", line): - if PIXI_CALL.search(command) and not NAMED_MANIFEST.search(command): - return True - return False + calls = pixi_calls(script.replace("\\\n", " "), comments=True) + if calls is None: # unparsable: err towards blocking + return bool(re.search(rf"\bpixi\b.*\b({'|'.join(PIXI_ENV_COMMANDS)})\b", script)) + return any(not MAIN_MANIFEST.match(manifest_arg(args) or "") for args in calls) def _rules(repo: str) -> list[tuple[re.Pattern, str, str]]: diff --git a/hooks/guard_worktree_pixi.py b/hooks/guard_worktree_pixi.py index ab5a1e5..2b00758 100644 --- a/hooks/guard_worktree_pixi.py +++ b/hooks/guard_worktree_pixi.py @@ -20,62 +20,20 @@ import json import re -import shlex import sys from pathlib import Path -from _repo import PIXI_ENV_COMMANDS, adopted, checkouts, is_root_of_worktree, nearest_manifest - -_BLOCKED = PIXI_ENV_COMMANDS -# The first of these after `pixi` is the command, whatever global options precede it. -_SUBCOMMANDS = _BLOCKED | { - "auth", "build", "clean", "completion", "config", "exec", "global", "import", "info", "init", "list", - "search", "self-update", "task", "tree", "upload", "workspace", -} # fmt: skip - - -def _is_separator(tok: str) -> bool: - """A token made only of command separators (`;`, `&&`, `|`, newlines, parentheses).""" - return bool(tok) and set(tok) <= set(";&|()\n") - - -def _pixi_calls(command: str) -> list[list[str]] | None: - """Argument lists of blocked `pixi` calls; None if unparsable.""" - lexer = shlex.shlex(command, posix=True, punctuation_chars="();<>|&\n") - lexer.whitespace = " \t\r" # newlines separate commands, so keep them as tokens - lexer.commenters = "" # a `#` comment must not swallow the newline after it - lexer.whitespace_split = True - try: - tokens = list(lexer) - except ValueError: - return None - calls = [] - for i, tok in enumerate(tokens): - if Path(tok).name != "pixi": - continue - args = [] - for t in tokens[i + 1 :]: - if _is_separator(t): - break - args.append(t) - if next((a for a in args if a in _SUBCOMMANDS), None) in _BLOCKED: - calls.append(args) - return calls +from _repo import PIXI_ENV_COMMANDS, adopted, checkouts, is_root_of_worktree, manifest_arg, nearest_manifest, pixi_calls def _manifest(args: list[str], cwd: Path) -> Path | None: """The manifest a call acts on; None if it is only known at run time.""" - for i, a in enumerate(args): - if a in ("--manifest-path", "-m") and i + 1 < len(args): - path = Path(args[i + 1]) - break - if a.startswith("--manifest-path="): - path = Path(a.split("=", 1)[1]) - break - else: + named = manifest_arg(args) + if named is None: return nearest_manifest(cwd) - if any(c in str(path) for c in "$`"): + if any(c in named for c in "$`"): return None # shell-expanded at run time, so not provably outside a worktree + path = Path(named) path = (cwd / path).resolve() if not path.is_absolute() else path.resolve() return path / "pixi.toml" if path.is_dir() else path @@ -93,9 +51,9 @@ def main() -> int: if ".claude/worktrees" not in str(cwd) and "--manifest-path" not in command and " -m " not in command: return 0 - calls = _pixi_calls(command) + calls = pixi_calls(command) if calls is None: # unparsable: fall back to the conservative text match - pattern = rf"\bpixi\b.*\b({'|'.join(_BLOCKED)})\b" + pattern = rf"\bpixi\b.*\b({'|'.join(PIXI_ENV_COMMANDS)})\b" blocked = ".claude/worktrees" in str(cwd) and re.search(pattern, command) and _in_adopted_repo(cwd) else: manifests = [_manifest(args, cwd) for args in calls] diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 6f8ee29..9215f5d 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -104,6 +104,8 @@ def test_pixi_allows_running_on_the_main_checkouts_environment(repo, pixi_worktr "pixi update", 'ROOT=$(git rev-parse --show-toplevel); pixi run --manifest-path "$ROOT/pixi.toml" python a.py', 'pixi run python a.py # TODO: --manifest-path "$MAIN/pixi.toml"', + 'echo "starting # job"; pixi run python a.py', # a quoted hash is no comment + 'pixi run python a.py --manifest-path "$MAIN/pixi.toml"', # after the task, the flag is the task's ], ) def test_task_paths_blocks_job_scripts_running_pixi_on_the_worktree(pixi_worktree, line):