Repository navigation
Batch jobs from a worktree run on the main checkout's environment - #5
Conversation
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 <noreply@anthropic.com>
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. |
![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: 903d28116e
ℹ️ 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".
|
|
||
| #: 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 |
There was a problem hiding this comment.
When a .sh or .sbatch file formats an unsafe invocation as pixi \ followed by run python ... on the next line, this line-oriented regex sees neither line as a match even though the shell executes pixi run. Because the resulting batch command runs outside the Bash hook, it can build the root environment inside the disposable worktree; normalize shell continuations or parse complete commands before applying this check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2fcb989: backslash continuations are joined before the job-script check, with a test for pixi \ / run ….
|
|
||
| #: 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 |
There was a problem hiding this comment.
For a job command such as pixi run --manifest-path ../../../pixi.toml ..., the mere presence of --manifest-path suppresses the match even when that path resolves to the worktree's root manifest. The batch job then has exactly the unsafe environment behavior this guard is intended to prevent, so the explicit path needs to be resolved and checked rather than accepted unconditionally.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2fcb989: only a manifest named through a shell variable (--manifest-path "$MAIN/pixi.toml") is exempt now. A relative path resolves against the job's directory, which the guard can't know when the script is written, so it is refused; an absolute path into the repo was already refused by the path rule. Test added.
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fcb98956b
ℹ️ 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".
| 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 |
There was a problem hiding this comment.
Fresh evidence in this revision is that continuation normalization is applied only to the current tool payload, not the resulting file: an Edit that adds pixi \ and a later Edit that adds run python a.py allows both fragments because neither new_string contains the complete invocation. The completed job script then runs against the worktree workspace, so this check needs to inspect the edited command in surrounding file context.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this one: the guard inspects only the text being written, by design (see its docstring), so it catches accidents rather than a command split across two edits. Reconstructing the file after each edit would cost more than this case is worth.
| #: 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 |
There was a problem hiding this comment.
When a line contains an unsafe call followed by a safe one, such as pixi run python bad.py && pixi run --manifest-path "$MAIN/pixi.toml" python good.py, the negative lookahead sees the safe manifest option anywhere on the line and exempts both calls. The first invocation will still build or use the worktree's root environment, so commands must be separated and each pixi invocation checked independently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4bdd4ad: each command on a line (split at &&, ||, ;, |, &) is checked on its own, so a safe call no longer exempts an unsafe one. Test added.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a4ee1372a
ℹ️ 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".
| if ( | ||
| file_path.suffix in JOB_SCRIPT_SUFFIXES | ||
| and PIXI_ON_FOUND_WORKSPACE.search(written.replace("\\\n", " ")) # one line per command | ||
| and is_root_of_worktree(nearest_manifest(file_path.parent)) |
There was a problem hiding this comment.
When a task directory contains a task-local pixi.toml but its script is submitted from the repository root, or the scheduler selects another working directory, this checks the manifest nearest to file_path.parent and permits a bare pixi run. Executing a shell script does not change into the script's directory, so Pixi discovers from the runtime working directory and can select the worktree-root manifest instead; require the job to identify the task-local manifest explicitly or otherwise validate its runtime directory before granting this exemption.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this one: the skill and the task template submit batch jobs from the task directory, where a task-local manifest is the one pixi finds. Requiring every task-local job to name its manifest through a variable would cost more than a submission from elsewhere, which the conventions already rule out.
| #: 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 |
There was a problem hiding this comment.
When a generated job script uses pixi update, upgrade, reinstall, or remove, this alternation does not match it. The sibling Bash guard explicitly classifies all four as workspace-mutating in _MUTATING, but a submitted job bypasses that hook, allowing it to modify or build the disposable worktree-root environment; share the blocked command set or add the omitted subcommands here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4bdd4ad: both guards now share one set, PIXI_ENV_COMMANDS in _repo.py, so the job-script check covers update, upgrade, reinstall and remove too. Test added for pixi update.
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bdd4add0c
ℹ️ 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".
| 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[= ]["']?\$""") |
There was a problem hiding this comment.
$MAIN
When a job derives the current worktree with ROOT=$(git rev-parse --show-toplevel) and runs pixi run --manifest-path "$ROOT/pixi.toml" ..., this pattern exempts it merely because the argument starts with $. The variable then resolves to the worktree-root manifest, and pixi run may install that workspace's environment, recreating the condition this guard is intended to prevent; only the prescribed $MAIN form should qualify for the exemption. Pixi documents that --manifest-path selects the workspace and that run installs it when required.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d89ed25: only --manifest-path "$MAIN/..." (or ${MAIN}) exempts a call now, the form the skill and template prescribe; a path derived in the job, as with git rev-parse --show-toplevel, is refused. Test added.
| if line.lstrip().startswith("#"): | ||
| continue | ||
| for command in re.split(r"&&|\|\||[;|&]", line): | ||
| if PIXI_CALL.search(command) and not NAMED_MANIFEST.search(command): |
There was a problem hiding this comment.
For a line such as pixi run python a.py # TODO: use --manifest-path "$MAIN/pixi.toml", both regexes match, so the hook permits the script even though the shell discards the comment and invokes Pixi without a manifest argument. Pixi will then discover the worktree-root workspace and may install it; tokenize or remove shell comments so only an option actually passed to the Pixi invocation can exempt it. Pixi's CLI reference confirms that --manifest-path is the option selecting the workspace manifest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d89ed25: shell comments are stripped before the check, so a comment that mentions the flag no longer exempts the call before it. Test added.
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d89ed25d4d
ℹ️ 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".
| 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\}?/""") |
There was a problem hiding this comment.
When a job uses pixi run python script.py --manifest-path "$MAIN/pixi.toml", this regex grants the exemption even though the option occurs in the task arguments. The official pixi run reference defines the syntax as pixi run [OPTIONS] [TASK]... and states that extra arguments are passed to the task, so Pixi still discovers and installs the worktree-root workspace in this case; validate that the manifest option belongs to Pixi's option segment rather than merely appearing anywhere in the command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a79d649: for pixi run, a --manifest-path counts only before the task; after it, it is the task's argument. Both guards share this rule (manifest_arg in _repo.py). Test added.
| 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 |
There was a problem hiding this comment.
Fresh evidence in the current comment-stripping fix is that a quoted hash preceded by whitespace is treated as a comment: for example, echo "starting # job"; pixi run python scripts/a.py is truncated before the real Pixi invocation and is allowed. Because # starts a shell comment only when it is unquoted, comment removal needs to be quote-aware or this common status-message pattern bypasses the new job guard.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a79d649: job scripts are now tokenised like the Bash guard does (shlex, quote-aware), one line at a time, so # starts a comment only outside quotes. echo "starting # job"; pixi run … is refused. Test added.
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 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a79d649723
ℹ️ 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".
| 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 |
There was a problem hiding this comment.
Fresh evidence in the current comment-stripping fix is that shlex.commenters treats every unquoted # as the start of a comment, whereas Bash starts a comment only when # begins a word. Consequently, a valid script such as echo job#1; pixi run python a.py is truncated before the Pixi invocation and passes this guard, even though the unsafe invocation executes. Apply Bash's word-boundary rule when removing comments so the remainder of the command is still inspected. The Bash manual documents the comment boundary semantics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this one: it needs an unquoted # inside a word followed by a pixi call on the same line, which job scripts rarely have. The guard is against accidents, and a Bash-exact comment scanner costs more than the case is worth.
| if command == "run" and not a.startswith("-"): | ||
| return None |
There was a problem hiding this comment.
Fresh evidence in this revision is that the new “first non-option is the task” rule also mistakes values belonging to Pixi options for the task. For example, pixi run --environment cuda --manifest-path "$MAIN/pixi.toml" python a.py returns at cuda and is refused despite explicitly selecting the main manifest. The official run reference defines both --environment <ENVIRONMENT> and --manifest-path <MANIFEST_PATH> as options, so this scan must consume option arguments before deciding that the task has begun. Pixi run CLI reference.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Leaving this one: it errs towards blocking, as the Bash guard does, and the fix is to put the flag first, pixi run --manifest-path "$MAIN/pixi.toml" -e cuda python a.py, the form the refusal message, skill and template show. Recognising pixi's value-taking options would mean tracking its CLI.
1 participant
guard_task_pathsalso refuses a job script (.sh/.sbatch) in a worktree that runspixion the workspace it finds. On the compute node that would build the root environment into the worktree, where the Bashpixiguard never sees it.nearest_manifestandis_root_of_worktreemove to_repo.py, shared by both guards.scripts/, take the main checkout as$MAINat submit (the path guard refuses it hardcoded), runpixi run --manifest-path "$MAIN/pixi.toml", and log toPATHS.logs; the template shows thesbatchline.No version bump; release separately.
🤖 Generated with Claude Code