Repository navigation
Conversation
…wered (#1461) The measurement is done. Its result is recorded on #1321, and this removes the workflow that carried it along with the extra setup job it added to every Copilot review in this repository. ## What it established, in one line each - A setup step **can** place content where the Copilot reviewer reads it. The reviewer returned a value generated at run time from `/dev/urandom` and never printed, and the value hashes to the digest in the run log. - A Copilot code review setup environment receives **no repository secret**. Measured on a run whose step failed saying so. - Copilot invokes the setup job directly, so the workflow's `on:` triggers play no part in it. - Copilot's **incremental** re-review does not honour custom review-body instructions. ## Also here The runner plan keeps the `ubuntu-latest` correction and loses the note that its workflow path was occupied, which it is not any more. Its two mentions of that workflow now name the file rather than its full path. The path has been tracked once, and the prose gate reads a mention of a once-tracked path the tree no longer holds as a stale reference, which is exactly what it should do: the plan proposes a file it has not created, so the mention now says so rather than asserting a path that resolves to nothing. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lled (#1462) Addresses #1445. `spec/validate.py` compared a tree source against `"."` as a raw string, so `"./"` and `".///."` passed the validator and were then refused by `scripts/carry.py`, which compares `PurePosixPath` objects. The validator is the gate a manifest author runs first, so the failure surfaced late and in a different tool. The comparison is now normalized in a named helper beside `escapes_repo_root`. The helper is scoped to POSIX path rules and says so, since backslash and drive-letter spellings are the neighbouring function's subject. ## On the test It asserts the helper for five spellings of the root and seven ordinary sources, three of them carrying a redundant separator or a `./` prefix, so an over-eager normalizer that refused `./docs` fails rather than passes. It does not assert the validator's own refusal of a manifest. The tree check sits inside `main()`, which this module has no fixture for, so reverting the call site while leaving the helper intact would leave every test green. That gap is stated rather than papered over. ## Left for #1452 Two defects of the same class remain in the same check, each an escape rather than a root reduction: a backslash spelling passes here and is refused by the carrier on Windows, and a symlinked source passes `is_dir()` here and is refused by the carrier everywhere. Both exist because this check hand-rolls a weaker subset of this file's own `escapes_repo_root`, and the fix is to call that function rather than restate part of it, which is a wider change than this one. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 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 |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The validator fix is narrowly scoped, covered by a regression test, and the workflow/documentation cleanup is consistent with the stated retirement.
Pull request overview
Retires the temporary Copilot review-environment measurement workflow and fixes spec/validate.py so tree sources that reduce to the repository root (e.g., "./", ".///.") are rejected consistently with scripts/carry.py.
Changes:
- Remove the
.github/workflows/copilot-code-review.ymlmeasurement workflow introduced for issue #1321. - Add
reduces_to_repo_root()and use it in the tree source/target validation rule to normalize"."-equivalent spellings. - Add focused unit coverage for the new normalization helper and update the homelab runner plan to reflect the workflow’s absence.
File summaries
| File | Description |
|---|---|
spec/validate.py |
Normalizes the “tree path must be below repo root” check via reduces_to_repo_root(). |
scripts/tests/test_spec_validate.py |
Adds unit tests covering root-reducing vs non-root tree-source spellings. |
docs/homelab-runner-selection-plan.md |
Removes now-stale references to the retired measurement workflow and keeps the plan consistent. |
.github/workflows/copilot-code-review.yml |
Deletes the temporary measurement workflow for #1321. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes two commits and closes out the measurement series for #1321.
2f85123removes the Copilot review environment workflow. Its question is answered and the evidenceis recorded on that issue, so the extra setup job it added to every review in this repository goes
with it. This is the revert the promotion that landed it committed to.
bcc06e3fixes #1445:spec/validate.pycompared a tree source against"."as a raw string, so"./"and".///."passed the validator and were then refused byscripts/carry.py. The comparisonis normalized in a named helper, with a test that fails when the helper is implemented the old way.
Two defects of the same class in the same check are filed as #1452 rather than fixed here, since each
is an escape rather than a root reduction.
Closes #1445