Repository navigation
Conversation
… Caller Omits It (#2588) `install-tools.sh` now puts its own `$BIN_DIR` (`/usr/local/bin`) first on `PATH` at the start of `main` when the caller's `PATH` leaves it out, so a cron-style `PATH=/usr/bin:/bin` reads the managed jq, uv and git-restore-mtime instead of calling them missing or adding a false shadow note. A `PATH` that already names `$BIN_DIR` keeps its order, so a copy genuinely ahead of it is still reported as a shadow. Prepending (not appending) makes the managed copy win, which matches the script's own stated intent. Tests: four-plus unit tests in `tests/test_install_tools.py` cover the prepend, an empty `PATH`, an unchanged `PATH` with a real shadow still reported, an equivalent spelling of the directory, no false shadow on a minimal `PATH`, and `main` calling the function. Host evidence (WSL2 Debian 13): ``` $ env -i PATH=/usr/bin:/bin host-setup/linux/install-tools.sh --report jq uv git-restore-mtime jq 1.8.2 1.8.2 jqlang/jq current git-restore-mtime 2025.08 2025.08 MestreLion/git-tools current uv 0.12.21 0.12.24 astral-sh/uv outdated ``` `--report --json` returns the same installed versions with empty `notes`. Shellcheck clean, ruff clean, `python3 -m unittest tests.test_install_tools` OK. Closes on promotion: #2257 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ools (#2589) Adds `TestKeptTreeEndToEnd` to `tests/test_bootstrap.py`: option 1 of #1767 for the bash loader. It runs the real `host-setup/bootstrap.sh --host --yes --dir <scratch>` with a scratch HOME, a stub `curl` on PATH serving a local tarball, and stub tools inside that tarball that log what the kept tree held when they ran and fail at a chosen stand-up step. A stub `mv` records whether the previous tree is still held aside at the moment the new one moves into place. No change to `bootstrap.sh` was needed. Asserted from disk: - a failure at any step before the skills step leaves the previous tree loading, with no `.new`/`.old`/archive beside it, and the run stops at that step; - a first run failing early leaves no tree; - a success leaves only the new tree; - every tool before the skills step runs from `skills-tree.new` with the old tree intact, and the skills installer runs from the swapped-in `skills-tree`; - the old tree is still held at `.old` when the new one moves into place; - a failing installer leaves the new tree whole with no `.old`. Verification on this host (WSL2 Debian): `Ran 7 tests ... OK` for the class, and the whole module `OK (skipped=46)`. Local mutations, reverted and never committed: swapping in unconditionally during download failed 8 cases, deleting the old tree before staging failed 5, and replacing `mv "$tree" "$retired"` with `rm -rf "$tree"` failed the new held-aside case. The PowerShell half is a `windows` lane item: #2587. Closes on promotion: #1767 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
#2586) ## Summary #2282 reported `pr_review.py status` reading `coverage=unstated` on a pull request whose first Copilot round named every changed file in its table, while a later round on a moved head, at Lite effort with no findings and no table, stated nothing. The changed-file set was the same at both commits. The carry that case asks for landed in #2276, which reached `main` in #2284 after #2282 was filed. With it, an earlier round's full table carries to a head that states nothing, under the same changed-file-set bound a coverage statement carries under, and reads as `coverage=carried:table`. Re-reading the pull request #2282 names with the current script gives `coverage=carried:table`. The existing carry tests use first-format bodies on both rounds. This adds one test pinning #2282's exact shape: both rounds written in the second overview format, the later at Lite effort with a findings total of none and no table. It asserts exit 0, `coverage=carried:table`, and no `NO FILE TABLE STANDS IN` line. Patching `carried_table` to return `None` fails it with exit 45, so the test proves the carry rather than passing for another reason. ## Verification - `python3 -m unittest tests.test_pr_review` passes. - The mutation above fails the new test. - Ruff format, Ruff check, and mypy pass through the pre-commit hook. Closes on promotion: #2282 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2591 +/- ##
=======================================
Coverage 58.85% 58.85%
=======================================
Files 16 16
Lines 8181 8181
=======================================
Hits 4815 4815
Misses 3366 3366
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🔵 Needs a closer look
Strengthen the Lite table-carry test assertions before approval.
0 open findings
What changed in this PR
Promotes develop to main with PATH handling and regression coverage for bootstrap flow and Lite table carry.
Changes:
- Ensures
/usr/local/binis available in minimal PATH environments. - Adds end-to-end Linux bootstrap tests.
- Adds second-format Lite table-carry coverage.
| File | Summary |
|---|---|
tests/test_pr_review.py |
Tests Lite-round table carry; needs stronger assertions for format, effort, and zero findings. |
tests/test_install_tools.py |
Tests managed PATH handling and shadow behavior. |
tests/test_bootstrap.py |
Exercises the Linux bootstrap flow with stubs. |
host-setup/linux/install-tools.sh |
Prepends the managed bin directory when absent. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Answering the round on 05c92d3, verdict Fixed in #2593. The test now asserts both bodies read as second format, the first table names both changed files while the Lite body carries none, and the digest reads |
## Summary Answers the headline Copilot raised on the promotion pull request #2591, "Strengthen the Lite table-carry test assertions before approval", which named format, effort, and zero findings. The test added in #2586 claimed both rounds were in the second overview format, the later one at Lite effort with no findings and no table, but asserted only the carry, so it still passed with the format reader disabled. It now asserts that: - both bodies read as second format - the first body's table names both changed files and the Lite body carries none - the digest reads `effort=lite` and `overview=0/0` Patching `second_format` to return `False` now fails the test. ## Verification - `python3 -m unittest tests.test_pr_review` passes. - Ruff format, Ruff check, and mypy pass through the pre-commit hook. Closes on promotion: #2282 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Answering the round on a7ddc97, verdict No change needed: the capability check. The class skips unless No change needed: the comment. The lines this change adds hold no If the round meant a specific line, a review thread naming it gets its own answer. |
Summary
Promotes develop to main, carrying:
Fixes #2257
Fixes #1767
Fixes #2282
🤖 Generated with Claude Code