test: throwaway PR for the Mistral review pipeline #1250

2026-10-11T13:34:30Z

claude-bot commented

2026-10-11 13:34:30 +00:00

Throwaway PR to check the new pr-review pipeline end to end. Will be closed unmerged right after.

🤖 Generated with Claude Code

Throwaway PR to check the new pr-review pipeline end to end. Will be closed unmerged right after. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-10-11 13:34:30 +00:00

test: throwaway probe for the Mistral review pipeline

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

061c8ddbc1

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

review-bot requested changes

2026-10-11 13:41:43 +00:00

review-bot left a comment

🤖 Automated review — model mistral-large-4-0, commit 061c8ddbc1312c8f76e8b3d9f5a4194e2d2cc07f.

Automated review by the review-bot pipeline: it approves only when it found nothing red or yellow, and a human still merges. Push new commits to get a fresh review.

Summary

PR #1250 (branch test/review-e2e, head 061c8dd) adds exactly one file, scripts/ci/e2e-review-probe.sh, a self-declared throwaway probe for the review pipeline that the author says will be closed unmerged. The script is not referenced by any workflow or agent script I could find, but its entire body is a destructive rm -rf built from an unvalidated, unquoted $1, plus a cd that breaks the deletion path for relative arguments. No secrets, no injection attempts in the PR text.

🔴 Blocking

scripts/ci/e2e-review-probe.sh:3-5 — DIR=$1 feeds an unvalidated, unquoted argument straight into rm -rf $DIR/old-*. Run with no argument, cd (no operand) changes to $HOME per POSIX and the script then executes rm -rf /old-* at the filesystem root; a $1 containing spaces is word-split by both cd and rm, so rm receives unintended operands; glob characters in $1 also expand. There is no set -e, no argument check, and no quoting — unlike the repo's own shell conventions (ci/agents/pr-review/run.sh uses set -eu and quotes "$D"; scripts/ci/test-pr-review-mistral.sh uses set -eu). Nothing references this script (checked .woodpecker/ci-scripts-test.yml, e2e.yml, e2e-test.yml, pr-review.yml, ci/agents/pr-review/run.sh), which limits exposure, but a repo that runs multiple shell-executing agents should not gain a script whose failure mode is an unscoped rm -rf.
scripts/ci/e2e-review-probe.sh:4-5 — cd $DIR followed by rm -rf $DIR/old-* double-applies a relative argument: with $1 = some/dir, the glob resolves under the new cwd to some/dir/some/dir/old-*, so the script silently deletes nothing (rm -f suppresses the error) or, if that nested path exists, deletes the wrong tree. The script only behaves as intended for absolute paths, which nothing documents or enforces.

⚪ Unverified

Whether any of the other agent pipelines (issue-fix, security-fix, update-audit, wiki-maintenance) might discover and execute scripts under scripts/ci/. I verified the Woodpecker workflows and ci/agents/pr-review/run.sh invoke only named scripts, but I did not read every ci/agents/*/prompt.md to confirm no agent is instructed to run arbitrary scripts from that directory. This only affects how much the blocking finding above matters in practice, not the defects themselves.

🟡 Suggestions

A pipeline probe does not need a destructive script at all: any added file (e.g. a plain text file) exercises the review path end to end. If a script must be committed, make it harmless or follow repo conventions — set -eu, [ $# -eq 1 ] guard, quoted "$DIR", and rm -rf -- "$DIR"/old-* only after a successful cd.
The cd is redundant given the glob already uses $DIR; dropping it (or using an absolute path) removes the relative-path defect entirely.

🟢 Nits

The file is committed as mode 100644 (non-executable, per the diff header) while sibling shell scripts under scripts/ are 100755. Cosmetic, since CI invokes scripts via sh script.sh.
echo "cleaned $DIR" (line 6) reports success even when nothing was deleted.

Tests

No tests added. The repo's convention is scripts/ci/test-*.sh wired by name into .woodpecker/ci-scripts-test.yml, but those cover pipeline helper scripts the review depends on; a throwaway probe with no intended behavior has nothing meaningful to test. If this script were kept rather than deleted, it would need a test and a CI wiring decision like its siblings.

Recommendation

Request changes — 2 blocking issues (both in scripts/ci/e2e-review-probe.sh:3-5); practically, closing this throwaway PR unmerged, as the author intends, also resolves them.

🤖 **Automated review** — model `mistral-large-4-0`, commit `061c8ddbc1312c8f76e8b3d9f5a4194e2d2cc07f`. > Automated review by the `review-bot` pipeline: it approves only when it found nothing red or yellow, and a human still merges. Push new commits to get a fresh review. --- ## Summary PR #1250 (branch `test/review-e2e`, head `061c8dd`) adds exactly one file, `scripts/ci/e2e-review-probe.sh`, a self-declared throwaway probe for the review pipeline that the author says will be closed unmerged. The script is not referenced by any workflow or agent script I could find, but its entire body is a destructive `rm -rf` built from an unvalidated, unquoted `$1`, plus a `cd` that breaks the deletion path for relative arguments. No secrets, no injection attempts in the PR text. ## 🔴 Blocking - `scripts/ci/e2e-review-probe.sh:3-5` — `DIR=$1` feeds an unvalidated, unquoted argument straight into `rm -rf $DIR/old-*`. Run with no argument, `cd` (no operand) changes to `$HOME` per POSIX and the script then executes `rm -rf /old-*` at the filesystem root; a `$1` containing spaces is word-split by both `cd` and `rm`, so `rm` receives unintended operands; glob characters in `$1` also expand. There is no `set -e`, no argument check, and no quoting — unlike the repo's own shell conventions (`ci/agents/pr-review/run.sh` uses `set -eu` and quotes `"$D"`; `scripts/ci/test-pr-review-mistral.sh` uses `set -eu`). Nothing references this script (checked `.woodpecker/ci-scripts-test.yml`, `e2e.yml`, `e2e-test.yml`, `pr-review.yml`, `ci/agents/pr-review/run.sh`), which limits exposure, but a repo that runs multiple shell-executing agents should not gain a script whose failure mode is an unscoped `rm -rf`. - `scripts/ci/e2e-review-probe.sh:4-5` — `cd $DIR` followed by `rm -rf $DIR/old-*` double-applies a relative argument: with `$1 = some/dir`, the glob resolves under the new cwd to `some/dir/some/dir/old-*`, so the script silently deletes nothing (`rm -f` suppresses the error) or, if that nested path exists, deletes the wrong tree. The script only behaves as intended for absolute paths, which nothing documents or enforces. ## ⚪ Unverified - Whether any of the other agent pipelines (`issue-fix`, `security-fix`, `update-audit`, `wiki-maintenance`) might discover and execute scripts under `scripts/ci/`. I verified the Woodpecker workflows and `ci/agents/pr-review/run.sh` invoke only named scripts, but I did not read every `ci/agents/*/prompt.md` to confirm no agent is instructed to run arbitrary scripts from that directory. This only affects how much the blocking finding above matters in practice, not the defects themselves. ## 🟡 Suggestions - A pipeline probe does not need a destructive script at all: any added file (e.g. a plain text file) exercises the review path end to end. If a script must be committed, make it harmless or follow repo conventions — `set -eu`, `[ $# -eq 1 ]` guard, quoted `"$DIR"`, and `rm -rf -- "$DIR"/old-*` only after a successful `cd`. - The `cd` is redundant given the glob already uses `$DIR`; dropping it (or using an absolute path) removes the relative-path defect entirely. ## 🟢 Nits - The file is committed as mode `100644` (non-executable, per the diff header) while sibling shell scripts under `scripts/` are `100755`. Cosmetic, since CI invokes scripts via `sh script.sh`. - `echo "cleaned $DIR"` (line 6) reports success even when nothing was deleted. ## Tests No tests added. The repo's convention is `scripts/ci/test-*.sh` wired by name into `.woodpecker/ci-scripts-test.yml`, but those cover pipeline helper scripts the review depends on; a throwaway probe with no intended behavior has nothing meaningful to test. If this script were kept rather than deleted, it would need a test and a CI wiring decision like its siblings. ## Recommendation Request changes — 2 blocking issues (both in `scripts/ci/e2e-review-probe.sh:3-5`); practically, closing this throwaway PR unmerged, as the author intends, also resolves them.

claude-bot referenced this pull request

2026-10-11 13:53:53 +00:00

feat(pr-review): run the review through a listed Mistral agent #1251

claude-bot closed this pull request

2026-10-11 13:53:59 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#1250