Repository navigation
Conversation
## Summary The GitHub Pre-Release shield regressed to rendering the stable version. `include_prereleases` only adds prereleases as candidates, so whenever a stable release is newer than the latest prerelease, which is the state after every promotion, the badge renders the stable version. #84 added `filter=*-g*` and the repurpose in #226 dropped it. - Put `&filter=*-g*` back on the hub `README.md` pre-release shield. - A shield's `requireQuery` in `spec/readme-sections.json` is now a list of substrings that must all appear in the URL, and the GitHub pre-release shield requires both `include_prereleases` and `filter=*-g*`, so the audit flags a carrier missing the filter. The schema and `spec/audit.py` take the list form only. - The missing-shield finding now names the required and forbidden query substrings, so a shield present at the endpoint but excluded by its query reads as what it is. - `spec/readme-structure.md` states the requirement, its reason (NBGV prerelease tags carry a `-g<sha>` suffix and stable tags do not), and the shields.io limit the comment on #3320 raises: the filter applies to the newest 100 releases only, so a repository whose newest 100 releases are all stable renders "no matching releases found", even where older prereleases exist. - The audit self-test fixtures carry the filter. One new case asserts a pre-release shield without it is a finding naming `filter=*-g*`, and another asserts a missing release shield names the query it forbids. Reverting the model to the bare `include_prereleases` requirement fails the first, and changing the forbidden-query wording fails the second. ## Verification - `python3 spec/audit.py --selftest`: `SELFTEST PASS`. - `python3 spec/validate.py`: OK. - `python3 -m unittest discover -s tests`: 3033 tests OK. - ruff format, ruff check, mypy, `prose_lint.py --diff`, and `repo_gate.py --check eol`: clean. Closes on promotion: #3320 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * The GitHub pre-release badge now filters for prerelease tags, avoiding misleading results from unrelated releases. * Badge checks now report missing required or forbidden URL parameters more reliably. * **Documentation** * Clarified how prerelease badge filtering works and what the badge displays when no matching releases are found. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Narrows the prose gate's `comment-added` promotion stand-down by workflow model, per the maintainer's decision recorded on #1663. - `validate-task.yml` takes a new optional `workflow-model` input. It defaults to `release`, matching the registry's own default. - A new `Resolve comment stand-down step` in the `lint` job computes the promotion stand-down in bash from the event's facts and that input. The `allow-comments` expression now reads the label and that step's output. - A release caller's `develop -> default branch` promotion still stands the rule down. Any other value, `operational` included, runs the rule there, since an operational repository may commit to `develop` directly and its promotion is then the first review those lines get. A misspelled value therefore reports comments rather than hiding them. - The decision allowed an input or a registry-reading step. This uses the input, since a workflow expression cannot read `registry/repos.json` and the reusable workflow already takes its other per-repository facts as inputs. - Tests run the step's own script. They cover a release promotion, an operational promotion, a feature branch, a fork branch named `develop`, another base, a push, and several non-`release` values. They also pin the step's `env:` wiring and the input's default. Three mutations were confirmed to fail the suite: dropping the model term, flipping the default, and hard-coding the env value. - `OPERATIONS.md`, `scripts/README.md`, `docs/reusable-workflows.md`, the composite action's input description, and the gate's stand-down note now say the stand-down applies to a release-model promotion only. The new step moves the promotion rationale's comment lines and adds a few, so this pull request carries the `comments` label. Local review: three recorded `local-strict-review` passes. Round one raised eight findings, round two four, and round three none. One round-one finding is deferred to #3332: nothing yet reports an operational caller that omits the input, and the no-build caller snippet can only pass it once a release carries it. The four operational repositories adopt the input in their own resyncs. Closes on promotion: #1663 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * The prose check now applies consistently during promotions: release-model promotions from `develop` to the default branch are exempt, while operational-model promotions remain subject to the check. Comments are allowed when the comments label is present or the release-model exemption applies. * **Documentation** * Clarified the `workflow-model` setting, its `release` default, and how promotion rules differ between release and operational workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## Summary A self-test or test that builds a temporary repository with `git -C` or `cwd=` inherits the `GIT_DIR`, `GIT_INDEX_FILE`, and similar variables a git hook sets, and writes into the hook's own repository. `spec/audit.py --selftest` cleared nothing and ran init, config, add, and commit that way. - `spec/git_env.py` holds the list (what `git rev-parse --local-env-vars` prints), `without_git_location()`, and `git_location_cleared()`. - `spec/audit.py` runs its self-test inside `git_location_cleared()`. - `scripts/local_review.py` (`INHERITED_REDIRECTS`) and `tests/test_pr_review.py` (`_clean_env`) use the shared list. - `tests/test_git_env.py` runs the audit self-test under a `GIT_DIR` naming a scratch repository, asserts it exits 0 with `SELFTEST PASS`, and asserts the scratch repository's config, history, and status are unchanged. It also checks the list covers every name the installed git prints. Refs #3324. The `host-setup/agent-safety/` item (test_install.py missing `GIT_INDEX_FILE`) lands in the guard lane, so this does not close the issue. ## Not moved - `tests/test_pre_commit_hook.py` already strips every `GIT_`-prefixed variable, a superset of the list, so moving it would weaken it. Left unchanged. - `scripts/pr_review.py` `origin_owner()` keeps its own four-name strip, and the `.github/actions/prose-gate/prose_lint.py` copy cannot import from `spec/` because it is carried downstream. ## Verification - Revert proof: with the wrapper replaced by a bare call, the new test fails on the scratch repository's changed config. With it, it passes. - `test_git_env`, `test_local_review`, `test_pr_review`, `test_canonical_review`, `test_pre_commit_hook`, `test_audit`, `audit.py --selftest`, `validate.py`, ruff, mypy, prose gate, and the EOL gate pass. ## Local review Two passes. The first raised seven findings: the pre-commit test fixture got weaker (fixed by leaving that file unchanged), the discovery names in the list (fixed by dropping them), a docstring inaccuracy (fixed), a stale docstring claim in `_clean_env` (fixed), two test-quality issues (fixed). Declined: `local_review` now also clears `GIT_DIR` and `GIT_CONFIG_PARAMETERS`, which are on git's own list of per-repository variables, and every call there names its working directory. The second pass found nothing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
) ## Summary Closes the write guard's launcher and exported-directory allow gaps. Every new reading only adds denials, and none narrows an existing deny. This is the first of two sequential lane PRs. The second takes the tokenizer and directory-value issues (#2991, #2995, #3090, #3074, #3075) once this one merges. - **#3050, every launcher in front of a push.** `_repo_may_move` walks every launcher in front of git with `_prefix_walk`, not only the first. So `timeout 60 uv run --directory <dir> git push` and `sudo uv run ...` are judged as moved. The first-launcher reading is kept beside the walk. A `watch` anywhere in front of git hands it to a shell, so it moves the push too. - **#3083, a foreign shell behind any launcher.** `_foreign_shell_at` reads `pwsh`, `fish`, `cmd`, and the rest behind a launcher it does not know, the safe way round, as `_launcher_prefix` reads one, where the shell is given its command string option. So `winpty pwsh -Command '...'` and `uv run fish --command='...'` bodies are read, while `go build ./cmd` reads no shell. - **#3085, environment through a launcher.** `_prefix_walk` passes `flock <file>`, `watch`, `winpty`, `coproc`, `while`, and `until` with their options and positional words. `_git_word_assignments` falls back to `_launcher_prefix` behind a launcher it does not know, and that reads the assignments after an `env` among the launcher's words. Any other assignment there is the launcher's own argument. The GH_REPO readers keep develop's launcher reading, since they take a prefix in place of the shell's state. - **#3304, a direct git command's `GIT_DIR`.** A git command is read again under each export state before it, not only a leading `export ... &&`. That includes a bare-name `export NAME` of an earlier assignment, a `declare -x`, and an export a `builtin` runs. It is also read under its whole prefix past a precommand or launcher, and a command-string body under the outer command's exports. Each is a reading beside the one without it. - **#3307, the config-change scan, in part.** It reads a run-time command word behind `flock`, `watch`, `coproc`, `while`, or `until`. It reads an `eval` or `source` inside a wrapper body or a substitution, and a shell there running a script file. It reads a `-c` shell with no string, one whose next word is an option, and one whose string holds the string an `xargs -I`, `-i`, or `--replace` replaces. - **#2943, the nesting-cap item.** Rule 6 fails closed on a mutating subcommand named in a wrapper body past the reading cap. The gh-write rules fail closed on a gh write or `gh api` call there, read by regex and at the argv level through `_names_gh_write`. - **#3324, one item.** `TestAliasReadThroughLinkedWorktreeGitfile` also drops `GIT_INDEX_FILE`. ## Issue Status Fixes #3050 Fixes #3083 Fixes #3085 Fixes #3304 Refs #3307 Refs #2943 Part of #3324 #3307 stays open for two items. One is a run-time command word behind a launcher the guard does not know, as in `uv run "$CMD"`. It cannot be told apart from an argument such as `mkdir -p "$D"`, which the issue requires to stay allowed. The other is the config-change deny reason, a message rather than an allow gap. #2943 stays open for the items other lanes own. ## Verification - **Differential.** Develop's guard and this branch's guard each classified every command the branch's self-test classifies, with the same seams and pinned globals (the MSYS flag and the swapped `_git_stdout`, `_live_branch_rules`, `_origin_owner_repo`, and `_current_push_branch` included). That was 1654 classify calls over 1540 distinct commands: - **deny to allow:** 0 - **allow to deny:** 43, each one a new or changed self-test deny case, one per fix above. One changed case is the old "only a leading export is read" accepted gap, which #3304 closes. - **unchanged:** 493 allow on both, 1118 deny on both - The local review passes also probed constructed shapes outside that corpus against both guards. They found three deny-to-allow flips, each fixed with a regression case: `watch` and `coproc` in front of a push, and an assignment-shaped `find` argument replacing a gh write's GH_REPO. One more flip is correct and kept: `flock <file> env GH_REPO=<repo> gh ...` now reads the GH_REPO that `env` really passes, where develop read the shell's. - **Revert proof.** The 43 cases above are the new deny tests, and each gives allow on develop's guard, so each fails with the fix reverted. Four more new deny cases are regression guards develop already denies: `watch -n 60 git push origin develop`, the subshell-export case, and the two `find ... -name GH_REPO=...` cases. The 14 new allow controls allow on both guards. - **Self-test.** `gh-write-guard.py --selftest` passes. - **Install tests.** `test_install.py` passes, 163 tests run, 1 skipped. - **Gates.** ruff format, ruff check, mypy, `prose_lint.py`, `repo_gate.py --check eol`, and `docker_lint.py` pass. - **Local review.** Seven frontier-tier passes, each recorded with `local_review.py`. The last found no deny-to-allow flip. Every finding got an outcome below. ## Remaining Local Review Findings - **Filed as #3340 (introduced, near variants develop also allows).** A name reassigned after it is exported, `set -a` and `local -x`, an export after a git word a function or loop runs first, a clustered `xargs -0I{}`, a wrapper body's config scan skipping `_string_read_elsewhere`, an assignment `sudo` passes behind an unknown launcher, `uv run env GH_REPO=... gh ...`, and a run-time word in a named coproc's group. - **Filed as #3341 (introduced, over-denials).** The rule 6 nesting-cap check matches subcommand words anywhere in the overflowed text. A shell name `command -v` looks up reads as a shell run with no string inside a substitution or body. A `watch`-named lock file in front of a push reads as `watch`. Its first item, every assignment after an unknown launcher read as git's environment, is fixed here. - **Fixed in this pull request.** README requirement 6 states how a leading export and a subshell export are read, and `_leading_export_dirs` no longer calls a later export an accepted gap. `go build ./cmd`, `docker exec "$C" ls`, and an `xargs -I{} sh -c '... "$1"' _ {}` are no longer over-denied. - **Declined, design.** Folding `_exported_states` into one export-state primitive for every reader is #3339's direction rather than this change's. Folding `_LAUNCHER_POSITIONALS` into `_EVAL_RUNNERS` or `_COND_PRECEDERS` would change rule 7's and the eval reader's readings. Dropping the `_STAYING_LAUNCHERS` tail of `_repo_may_move` flips `timeout.exe`, `Sudo`, and `coproc` pushes from deny to allow against develop. `_xargs_runs` walks back itself because `_run_start` returns None after a redirection or a `)`. `watch`'s `-q` takes a required value, per `watch --help`. - **Filed, pre-existing.** #3343 (a run-time `GIT_DIR` value read literally), two `xargs` config-change shapes noted on #3307, #3336 (words `xargs` appends to a gh write), #3337 (an outer variable adding commands to a double-quoted wrapper body, with a decision), #3339 (the other export readers read only `export NAME=value`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…e Guard (#3345) ## Summary Closes the write guard's tokenizer, directory-value, launched-string, and expanded-body allow gaps. Every new reading only adds denials, and none narrows an existing deny. This is the second of two sequential lane PRs, after #3342. - **#2991, a quote pair spanning two comments.** shlex ignores comments, so two comments each holding an apostrophe pair up as one quoted span and hide the lines between them. Where a `#` opening a word has a quote after it on its line, `_shell_tokens` adds `_context_lex`'s comment-aware tokens wherever shlex's do not hold every one of them. - **#2995, directory values and the tokenizer.** - A command naming git or gh that holds a carriage return fails closed, since bash reads one as part of a word. - A word a backslash-newline splits fails closed on any git or gh command, not only a push. So does an escaped backslash before a line break, which bash reads differently from the guard's fold. git or gh is named through quoting too, as `_names_push` reads a push. - The text an `eval` runs and the string `watch` hands a shell are read, so `eval git -C "$W" status` is denied. - An unquoted pathname pattern in a `-C`, `--git-dir`, or `--work-tree` value is denied, literal or after a run-time word. - **#3090, `env -S`.** `_launched_command_string` reads the string an `env -S`, `-S<string>`, clustered `-iS`, or `--split-string` splits into a command, and the string a `watch` or a `flock -c` hands a shell. An option taking a value before it is read as one. - **#3074, words xargs appends.** A push, a commit, or a `gh pr merge` that `xargs` runs or may run fails closed, since `xargs` appends words the guard does not read. The merge is read by its command path, so `xargs gh pr comment 5 --body merge` runs no merge. - **#3075, a double-quoted wrapper body.** A body the outer shell has already expanded is an `_ExpandedText`. That covers a double-quoted body with an unescaped `$` or backtick, a `watch` command, and an `eval` payload written outside single quotes. `_quoting` then reads every `$` word in it as one that word splitting may split, a literal quote left in a word included. One change covers the merge and commit sites. ## Issue Status Fixes #2991 Fixes #2995 Fixes #3090 Fixes #3074 Fixes #3075 ## Verification - **Differential.** Develop's guard and this branch's guard each classified every command the branch's self-test classifies, with the same seams and pinned globals, as #3342 did. That was 1701 classify calls over 1587 distinct commands: - **deny to allow:** 0 - **allow to deny:** 31, each one a new self-test deny case, one per fix above - **unchanged:** 508 allow on both, 1162 deny on both - The local review passes also ran transform differentials outside that corpus, such as wrapping every develop case in comment pairs, `eval`, `watch`, `flock -c`, `env -S`, and a double-quoted `sh -c`. They found three deny-to-allow flips, each fixed with a regression case: a double-quoted body's escaped backslash before a line break, a quoted git word hiding a split word, and a dashed git binary such as `git-push` beside a split word. - **Revert proof.** The 31 cases above are the new deny tests, and each gives allow on develop's guard, so each fails with the fix reverted. The new controls allow on both guards. - **Self-test.** `gh-write-guard.py --selftest` passes. - **Install tests.** `test_install.py` passes, 163 tests run. - **Gates.** ruff format, ruff check, mypy, `prose_lint.py` (with and without `--check sentence-length`), `repo_gate.py --check eol`, and `docker_lint.py` pass. - **Local review.** Six frontier-tier passes, each recorded with `local_review.py`. The last found no deny-to-allow flip. Every finding got an outcome below. ## Remaining Local Review Findings - **Filed as #3344 (introduced).** The backslash-newline and escaped-line-break denies read no quoting or heredoc data, so a literal one inside single quotes or a quoted heredoc is denied. Two comments with apostrophes leave the union's quoting unknown, so a quoted `"$X"` beside them is denied. An operator glued to a line continuation, as in `git add x &&\` before a `git commit` line, reads as a split word. A `watch` reads its own option values and `-x` as expanding its command. An eval payload is marked expanded per text rather than per eval. Near variants still allowed, each shared with develop: `xargs git` and `xargs gh pr` with the subcommand on input, GNU env long-option prefixes such as `--split=`, `\$` inside a double-quoted body, and GNU `parallel`. - **Filed on #3344 as well.** The README's wording of the `xargs` merge deny, which should say "runs or may run". `xargs -I{} sh -c 'gh pr merge {}'`, where the inner shell splits the replaced line, and macOS `gxargs`. - **Declined on this pull request with evidence.** flock's attached `-c<string>` and `--command=<string>`, which util-linux flock runs as a program name, and git's attached `-C<dir>`, which git rejects as an unknown option. - **Declined, design.** Folding `_GIT_OR_GH_WORD` into `_NAMES_GH`, whose `$` branch only gates the nesting-cap walk. `_outer_expands` and `_unquoted_pattern` sit beside `_expands_unquoted` because each asks a different question, an expansion inside double quotes and a pattern character. The env cluster walk sits beside `_option_takes_next`, which says only whether an option takes the next word, not where an `S` ends the cluster. - **Filed, pre-existing, from #3342.** #3336, #3337, #3339, #3340, #3341, #3343. 🤖 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 |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate write-guard findings must be addressed before approval.
5 open issues
What changed in this PR
This pull request promotes write-guard hardening, shared Git environment isolation, prerelease shield validation, and workflow-model-aware prose gating from develop to main.
Changes:
- Expands write-guard parsing for launchers, wrappers, exports, tokenizer edge cases,
env -S,xargs, and nested commands. - Shares Git repository-location environment handling across audit, review, and test utilities.
- Restores prerelease shield filtering and narrows prose-gate promotion stand-down behavior to release-model workflows.
| File | Summary |
|---|---|
tests/test_prose_lint.py |
Tests release-model-specific comment stand-down behavior. |
tests/test_pr_review.py |
Uses shared Git environment cleanup in review tests. |
tests/test_git_env.py |
Adds shared Git environment and audit self-test coverage. |
spec/readme-structure.md |
Documents the required prerelease shield filter. |
spec/readme-sections.schema.json |
Changes required shield queries to a list of substrings. |
spec/readme-sections.json |
Requires both prerelease query fragments. |
spec/git_env.py |
Provides shared Git repository-location environment helpers. |
spec/audit.py |
Applies shield query validation and clears Git location variables during self-tests. |
scripts/README.md |
Documents workflow-model-specific prose gating. |
scripts/local_review.py |
Clears shared Git repository-location variables for review commands. |
README.md |
Restores the prerelease filter on the GitHub shield. |
OPERATIONS.md |
Updates local prose-gate guidance for workflow models. |
host-setup/agent-safety/README.md |
Documents expanded write-guard coverage and fail-closed behavior. |
host-setup/agent-safety/claude/test_install.py |
Adds GIT_INDEX_FILE to the isolated test environment. |
host-setup/agent-safety/claude/gh-write-guard.py |
Adds launcher, wrapper, export, tokenizer, directory, xargs, and nesting analysis. Findings remain unresolved: critical assignment preservation (2 votes), critical clustered xargs replacement detection (1), moderate watch launcher classification (1), moderate command -v handling (1), moderate overflowed git-subcommand matching (2), critical per-eval expansion state (1), critical shell-wide export modes (1), and moderate quoted line-break handling (1). |
docs/reusable-workflows.md |
Documents the reusable workflow-model input. |
.github/workflows/validate-task.yml |
Computes release-model promotion stand-down behavior. |
.github/actions/prose-gate/prose_lint.py |
Updates the stand-down diagnostic text. |
.github/actions/prose-gate/action.yml |
Clarifies release-model promotion behavior. |
Review effort: Lite
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3346 +/- ##
==========================================
+ Coverage 65.49% 65.53% +0.03%
==========================================
Files 17 18 +1
Lines 10011 10036 +25
==========================================
+ Hits 6557 6577 +20
- Misses 3454 3459 +5
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:
|
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.
Summary
Promotes these pull requests from develop.
Refs #3307
Refs #2943
Closes #3320
Closes #1663
Closes #3324
Closes #3050
Closes #3083
Closes #3085
Closes #3304
Closes #2991
Closes #2995
Closes #3090
Closes #3074
Closes #3075
🤖 Generated with Claude Code