Repository navigation
Conversation
… Sentence (#2955) Adds a Disproved Claims entry for the claim that git accepts an attached `-c<name>=<value>` or `-C<dir>` global option, and rewords the closing sentence of the `scripts/README.md` exit `41` paragraph to state when the digest's `refusal=` field and the exit code differ. Prose only, no code change. Fixes #2941 A local strict review raised two findings on the first wording of the README sentence, both fixed. A second pass over the fix found none. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ows Decode Probe (#2956) ## Summary Four follow-ups to the CLI usage gate, for #2946, #2947, #2948, and #2949. - **#2946.** The Windows reader-thread `UnicodeDecodeError` comes from `locale_patch_bites()` in `tests/test_prose_lint.py`, the probe whose call deliberately names no encoding, so adding `encoding=` was not the fix. On Windows `communicate()` decodes in a reader thread, so the error never reached the probe's `except`. The probe returned False, and every `TestGitOutputDecodesAsUtf8` case skipped on the one platform the class exists for. The probe now records the reader thread's exception through `threading.excepthook`, which also stops the unhandled traceback. Every other text-mode call under `tests/` already names an encoding, which `tests/test_tooling_encoding.py` enforces. - **#2947.** `scripts/carry.py` gains `git_status_records(root, *, ignored=False)`, which returns `(status, path)` pairs and refuses malformed or incomplete records as before. `git_status_paths()` is now a projection of it, and the CLI gate's `tree_state()` calls it. `tests/test_carry.py` covers status pairing, the ignored option, rename order, and each refusal. - **#2948.** `tests/test_cli_usage.py` declares `NO_REQUIRED_ARGUMENT`. Each member was checked by reading its parser. The gate fails an entry point declared nowhere, and also a stale or duplicate declaration. `catalog/snippets/hub-fetch-run.py` takes a required hub path but refuses a leading flag before reading it, so it fits neither set. It is declared in a one-entry `REFUSES_A_LEADING_FLAG` with that reason. The module docstring now describes the declarations. - **#2949.** `CODESTYLE.md` "Command-Line Arguments" states the gate's requirements: the same help text for `-h` and `--help` on stdout, opening on a usage line that names the script, and a refusal that names the argument as an unknown or unrecognized option, argument, flag, or parameter. PowerShell's own binder refusal also counts. The gate's `REFUSED_AS_OPTION` now accepts exactly that set. Before this change it accepted only "unrecognized argument/option". The python-codestyle and shell-codestyle Skills are regenerated. ## Verification - Ran the full local gate set from `OPERATIONS.md` "Local Verification", including the unit suite (2720 tests OK), `spec/validate.py`, `build_dist.py --check`, `repo_gate.py --check eol` and `--check eol-coverage`, both prose invocations, a `--diff` run with and without `sentence-length`, and `docker_lint.py`. All passed. - Ran two `local-strict-review` passes. The first raised five findings, all fixed in 1a8640c. The second raised none. - The Windows probe path was exercised on Linux by simulating the reader-thread exception. The Windows CI leg is the real check. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rators in the Write Guard (#2963) ## Summary Closes the three write-guard ALLOW gaps #2951 names, in `host-setup/agent-safety/claude/gh-write-guard.py`, with each of the issue's commands added as a constructed self-test case. ## Changes - **Per-refspec push operations.** `_push_targets` no longer keeps one aggregate force and delete flag per push. Each refspec carries its own operation set: `+src` forces only its own branch, `:dst` deletes only its own, and `--force`/`--delete` apply to every refspec. `_check_push_bypass` now checks every operation, a force and a delete included, against `pull_request`, after the force and delete checks. A lone `:` and a branch pattern (a destination holding `*` that can name a branch) are read as writing every protected default and each branch named beside them with their own operations. A pattern is also still read as the branch its literal destination spells, so a ruleset pattern such as `release/*` is still consulted. A tag pattern writes no branch. - **Include overrides.** A git command under an `include.path` or `includeIf.<condition>.path` override, given by `-c` or `--config-env`, whose subcommand is not in `git --list-cmds=builtins,main,others`, is an unreadable push and denies, since the included file may define it as a push alias. The `classify` pre-filter gains `_GIT_INCLUDE` so a command holding no `push` word reaches the check. `_config_key` is shared with the hooks-path check. - **A separator fused with a redirection.** The tokenizer fuses `;<`, `&<`, `&&<`, `|<`, newline+`<` and similar into one token, which was read as a redirection, so the next command's words read as the push's own refspecs. A push holding such a token is now read twice in `_push_invocations`: once as before, and once ending at the separator (`_collect_arglist(..., split_fused=True)`). Both readings are checked, and in the split one a number before the fused token is a word rather than a file descriptor. Every other caller of `_collect_arglist` keeps develop's reading, because no quote mask the guard has can tell a fused separator from a quoted word or one inside an expansion. `_forks_out_of_reach` reads the separator part of such a token (`_leading_separator`) for a background `&`. - **Self-test.** New cases in `_GIT_CASES`, `_PUSH_DIR_CASES`, and the wait table, and the `_GIT_COMMAND_NAMES` pin now covers the `_GIT_CASES` loop so the include cases do not depend on the host's git. - **READMEs.** Requirement 4 in `host-setup/agent-safety/README.md` and the guard summary in `host-setup/agent-safety/claude/README.md` describe the per-refspec reading and the include deny. ## Differential Against Develop's Guard As for #2945 and #2952, develop's guard (the merge base) and this branch's guard were loaded side by side and run over the same calls. - **Real-repository mode:** 12,855 constructed commands, each in a develop checkout and a feature worktree of a constructed repository, under five rule sets, with no current-branch seam (128,550 calls). The corpus crosses push heads with separators, fused separator-redirection tokens, quoted, backslash-escaped, backtick, `$'...'`, `${...}`, `$(...)`, `$(<...)`, process-substitution and comment spellings, branch and tag patterns, include overrides, and gh and wait shapes. A pipeline continued onto the next line and a separator fused with a redirection are both in it. - **Seam mode:** every `classify` call the self-test makes (815) plus 10,245 extra shapes under 20 current-branch and rule seams (205,715 calls). - **Rule 6:** 3,042 primary-checkout calls. No verdict differs. Every allow flip (develop denies, this branch allows) is in the intended per-refspec class, and none occurs under a rule set holding `pull_request`. In each, another refspec's `+` or `:dst` no longer forces or deletes a branch it does not name: - `git push origin +feature/x develop` and `git push origin :feature/z develop`, with develop protected only by `non_fast_forward` or `deletion`. - `git push origin : +feature/y` and `git push origin '+refs/tags/*' develop` no longer force develop. - `git push origin +develop feature/y`, with rules on `feature/y` only, no longer forces `feature/y`. The separator fix flips nothing toward allow by construction, since develop's reading is still checked. Every other flip is toward deny. Those are the issue's fixes, a force or delete checked against `pull_request`, the include denies, and the over-denials below. ## Known New Over-Denials - A push whose words hold a fused separator is checked under both readings. A feature-checkout push such as `git push ;</dev/null origin develop`, where `origin develop` is really a separate command, denies. - A branch pattern with a fixed prefix, such as `refs/heads/feature/*:refs/heads/feature/*`, is read as writing the protected defaults too, so it denies where a default holds `pull_request`. - A fused token holding a background `&`, such as `&<`, now reads as a background job for the unbounded-wait rule. - Every `includeIf.` key reads as an include, one git ignores included, since a `--config-env` condition holding an `=` cuts the key the guard reads short of its `.path`. - A branch pattern's operations also reach a branch the push names explicitly, which git leaves to that branch's own refspec. That and the fixed-prefix case above are #2964. ## Local Review Seven frontier passes ran. The first six each found an allow flip that an earlier fix had introduced, and each is fixed: - A quoted operator word (`';<'`) ending the argument list. - Fused tokens inside a backtick, `$'...'`, `${...}` or `$(<...)` that the quote mask marked unquoted. The mask was then dropped for the two-reading design above. - The split reading displacing a push only the whole-command scan finds. - A forced branch pattern beside a lone `:`. - A pattern's literal destination no longer being looked up. This ran past the two-round edit budget `local-strict-review` sets, deliberately. Each remaining finding was a new allow in a safety hook, and stopping would have shipped it. The seventh pass, recorded on the pushed head, found no unintended allow flip. Its outcomes: - **Declined (style):** sentence-length findings on the new docstrings and README sentences. That check is not a gate here, and the claude README bullet is one physical line, so the older sentences on it are flagged too. - **Accepted over-deny:** the fixed-prefix branch pattern above, filed as #2964. - **Pre-existing, filed:** `refs/*`-style patterns and a pattern matching a non-default protected branch, added to #2943. Other pre-existing gaps the passes found, each equal on develop: - #2957: clustered and abbreviated push flags. - #2958: include overrides setting the hooks path or a command-running key. - #2959: quote-split, backslash-newline-split and run-time words, and uppercase `GIT`. - #2960: include overrides reaching `rebase --exec`, `submodule foreach` and `eval`. - #2961: comment and backtick words. - #2962: a local branch name losing its first segment, and `--exec-path` beside an include. ## Copilot Round Copilot's first round raised two threads and two body-only findings, each answered on the pull request. A number before a fused separator is now a word in the split reading, and the `pull_request` deny message says the push writes the branch. Narrowing the include check to `.path` keys, as one finding asked, opened the `--config-env` shape above, so a ninth local pass caught it and the narrowing is reverted. The pattern over-denial is #2964. Two local passes ran on the fix commits, and the last found no new allow. ## Verification - `gh-write-guard.py --selftest`: 858 cases, SELFTEST PASS. Each new deny case fails against develop's guard, and the split-bookkeeping case also fails against the commit before its fix. - `ruff check`, `ruff format --check`, `mypy`, `python3 -m unittest discover -s tests`, `test_install.py`, `build_dist.py --check`, `repo_gate.py` (default, `eol`, `eol-coverage`), `prose_lint.py` on the changed files and `--diff origin/develop`, `spec/validate.py`, and `docker_lint.py` all pass. Closes on promotion: #2951 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Push checks now evaluate each refspec independently, correctly distinguishing branch updates, force-pushes, and deletions. * Writes to branches that require pull requests are denied, including force-pushes and deletions. * Pushes with targets the safety check cannot inspect are denied when Git configuration may redirect the command through an alias. * Command checks more reliably recognize shell separators, redirections, wrapped commands, and explicit Git directory settings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit details
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 |
There was a problem hiding this comment.
🔵 Needs a closer look
The broad safety-guard and tooling changes require final human review.
0 open findings
What changed in this PR
Promotes write-guard hardening, CLI usage validation, shared Git status parsing, Windows decoding detection, and refusal-field documentation.
Changes:
- Adds per-refspec push enforcement and safer include/separator handling.
- Strengthens CLI declarations and status parsing.
- Updates documentation, generated skills, and source digests.
| File | Description |
|---|---|
tests/test_prose_lint.py |
Captures Windows reader-thread decode failures. |
tests/test_cli_usage.py |
Adds CLI declarations and shared status parsing. |
tests/test_carry.py |
Tests Git status record parsing. |
scripts/README.md |
Clarifies refusal-field and exit-code behavior. |
scripts/carry.py |
Adds reusable status parsing. |
host-setup/agent-safety/README.md |
Documents guard behavior. |
host-setup/agent-safety/claude/README.md |
Updates guard summary. |
host-setup/agent-safety/claude/gh-write-guard.py |
Hardens push and include analysis. |
CODESTYLE.md |
Defines stricter CLI requirements. |
.github/skills/shell-codestyle/SKILL.md |
Updates shell guidance. |
.github/skills/python-codestyle/SKILL.md |
Updates Python guidance. |
.github/copilot-instructions.md |
Records the Git option disproof. |
.claude-plugin/fleet-skills/skills/shell-codestyle/SKILL.md |
Updates plugin shell guidance. |
.claude-plugin/fleet-skills/skills/python-codestyle/SKILL.md |
Updates plugin Python guidance. |
.claude-plugin/fleet-skills/.source-digests/shell-codestyle |
Refreshes shell source digest. |
.claude-plugin/fleet-skills/.source-digests/python-codestyle |
Refreshes Python source digest. |
.agents/skills/shell-codestyle/SKILL.md |
Updates canonical shell guidance. |
.agents/skills/python-codestyle/SKILL.md |
Updates canonical Python guidance. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2965 +/- ##
==========================================
+ Coverage 59.96% 60.02% +0.06%
==========================================
Files 17 17
Lines 8778 8784 +6
==========================================
+ Hits 5264 5273 +9
+ Misses 3514 3511 -3
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:
|
|
Answer to the Copilot round on |
|
@coderabbitai review |
|
gh-write-guardread each push refspec's own operation, so a+srcforces and a:dstdeletes only its own branch while--forceand--deletereach every refspec, and every operation, force and delete included, is checked againstpull_request. A git command under aninclude.pathorincludeIf.<condition>.pathoverride whose subcommand is not a known git command now denies, since the included file may alias it to a push. A separator fused with a redirection (;<,&&<,|<and similar) is read both ways, so the next command's words no longer read as the push's refspecs. A differential against the previous guard found every allow flip in the intended per-refspec class and none under a rule set holdingpull_request. The known new over-denials are listed in that pull request, and the fixed-prefix branch pattern case is Scope a Branch Pattern's Push Operations to the Branches It Can Match in the Write Guard #2964tests/test_cli_usage.pydeclares the entry points that need no argument and fails one declared nowhere,CODESTYLE.md"Command-Line Arguments" states the gate's stricter reading of help and unknown-argument refusals,scripts/carry.pyshares one porcelain status parser with the gate, and the Windows UTF-8 decode probe intests/test_prose_lint.pynow sees the reader thread's exception-c<name>=<value>/-C<dir>git option disproof in the Disproved Claims ledger and states inscripts/README.mdwhen the digest'srefusal=field andwait's exit code differCloses #2941
Closes #2946
Closes #2947
Closes #2948
Closes #2949
Closes #2951
🤖 Generated with Claude Code