fix(guard): scope the freshness gate to git commit, and fix its hint#164
Conversation
Two related changes to the repository freshness control. 1. Narrow the freshness demand to `git commit` only (CJ, 2026-07-20). Previously every repo-sensitive action — edits, tests, pushes, and review-reads — required a same-turn fetch. A commit is the moment a stale local base is actually recorded, so only it is gated now. Edits, tests, pushes, and reads still require the git-rules consult; they no longer require a fetch. `repo-work-fresh-base` fires solely on a `git commit` (new helper `_is_commit_action`). 2. Fix the self-contradictory block message. Its worked example (`git fetch --all --prune && git commit …`) could never satisfy the check: a command that also contains the commit is not a pure freshness command, so `_is_freshness_command` returns False, nothing is recorded, and the commit stays blocked. The message now tells the agent to run a standalone literal-path fetch as its own command first, then commit as a separate command, and spells out that any non-git-read step (even `&& echo`) disqualifies the fetch and that freshness resets each turn. Tests: rewrite the freshness tests to probe with commits (non-commit repo work is now allowed after the rules-note consult), add test_freshness_gate_applies_only_to_commits, and assert the message no longer offers a chained fetch->commit remedy. ruff clean; 782 passed. Bump to 4.2.2 and record both changes in CHANGELOG. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughVersion 4.2.2 narrows repository freshness enforcement to ChangesGit freshness enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 12-13: Add blank lines immediately after the `### Changed` and
`### Fixed` headings in `CHANGELOG.md`, before their respective list content, to
satisfy markdownlint MD022.
In `@tests/test_guard.py`:
- Around line 164-181: Make the freshness assertions independent of the checkout
remote by creating an isolated temporary Git repository with a fake remote in
tests/test_guard.py lines 164-181 and 993-1015, then run every fetch and commit
command against it using git -C <repo>. Preserve the existing chained-fetch and
standalone-fetch assertions while ensuring both scenarios exercise a
remote-backed repository.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef2ec6b6-223f-4979-8f6f-b8f36c81a1a6
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
CHANGELOG.mdpyproject.tomlsrc/omind/__init__.pysrc/omind/guard.pytests/test_guard.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: test (windows-latest, 3.10)
- GitHub Check: test (windows-latest, 3.14)
- GitHub Check: test (macos-latest, 3.10)
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/omind/guard.py
[warning] 885-885: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(rf"(?:^|[;&|\n(]\s*)git[ \t]+{_GIT_GLOBAL_OPTS}commit\b")
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
tests/test_guard.py
[error] 277-280: Command coming from incoming request
Context: subprocess.run(
["git", "-C", str(repo), "remote", "add", "origin", "https://x.invalid/y.git"],
check=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 markdownlint-cli2 (0.23.0)
CHANGELOG.md
[warning] 12-12: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (4)
pyproject.toml (1)
3-3: LGTM!src/omind/__init__.py (1)
5-5: LGTM!src/omind/guard.py (1)
67-84: LGTM!Also applies to: 884-901, 1096-1106
tests/test_guard.py (1)
10-10: LGTM!Also applies to: 248-300, 1067-1076
| ### Changed | ||
| - **Narrowed the repository freshness gate to `git commit` only.** Previously any |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines after the release subsection headings.
Insert a blank line after ### Changed and ### Fixed so the changelog passes markdownlint MD022.
Also applies to: 20-21
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 12-12: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 12 - 13, Add blank lines immediately after the
`### Changed` and `### Fixed` headings in `CHANGELOG.md`, before their
respective list content, to satisfy markdownlint MD022.
Source: Linters/SAST tools
| # A commit, however, still demands freshness. | ||
| blocked = guard.decide({"tool": "Bash", "command": "git commit -am x", "session": "repo"}) | ||
| assert not blocked.allow | ||
| assert blocked.rule_id == "repo-work-fresh-base" | ||
|
|
||
| compound = guard.decide( | ||
| {"tool": "Bash", "command": "git fetch --all --prune && pytest", "session": "repo"} | ||
| ) | ||
| # A fetch chained with the commit is NOT a pure freshness command, so it | ||
| # records nothing and the commit stays blocked. | ||
| compound_cmd = "git fetch --all --prune && git commit -am x" | ||
| compound = guard.decide({"tool": "Bash", "command": compound_cmd, "session": "repo"}) | ||
| assert not compound.allow | ||
| assert compound.rule_id == "repo-work-fresh-base" | ||
|
|
||
| # A standalone fetch establishes freshness for the separate next commit. | ||
| fresh = guard.decide( | ||
| {"tool": "Bash", "command": "git fetch --all --prune", "session": "repo"} | ||
| ) | ||
| assert fresh.allow | ||
| assert guard.decide({"tool": "Bash", "command": "pytest", "session": "repo"}).allow | ||
| assert guard.decide({"tool": "Bash", "command": "git commit -am x", "session": "repo"}).allow |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make freshness assertions independent of the checkout remote.
decide() only blocks stale commits for repositories with configured remotes. These commands target the runner’s CWD, so source-archive or no-remote runs will allow the commits and invalidate the assertions.
tests/test_guard.py#L164-L181: create a temporary Git repository with a fake remote and usegit -C <repo>for each command.tests/test_guard.py#L993-L1015: likewise use an isolated remote-backed repository for the chained and standalone-fetch scenarios.
📍 Affects 1 file
tests/test_guard.py#L164-L181(this comment)tests/test_guard.py#L993-L1015
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_guard.py` around lines 164 - 181, Make the freshness assertions
independent of the checkout remote by creating an isolated temporary Git
repository with a fake remote in tests/test_guard.py lines 164-181 and 993-1015,
then run every fetch and commit command against it using git -C <repo>. Preserve
the existing chained-fetch and standalone-fetch assertions while ensuring both
scenarios exercise a remote-backed repository.
1 participant
What
Two related changes to the repository freshness control in
guard.py.1. Scope the freshness gate to
git commitonly (per CJ, 2026-07-20)Previously every repo-sensitive action — edits, tests, pushes, review-reads —
demanded a same-turn
git fetch. A commit is the moment a stale local baseactually gets recorded, so only it is gated now. Edits, tests, pushes, and reads
still require the git-rules-note consult; they no longer require a fetch.
repo-work-fresh-basenow fires solely on agit commit(new_is_commit_action).2. Fix the self-contradictory block message
The old hint told agents to chain the fetch onto the commit:
That can never satisfy the check.
_is_freshness_commandrequires every partof the command to be a git read/fetch, so a command that also contains the commit
records nothing and the commit stays blocked. The message now instructs a
standalone fetch first, then the commit as a separate command, and calls
out that any non-git-read step (even
&& echo) disqualifies the fetch and thatfreshness resets each turn.
Tests
allowed after the rules-note consult).
test_freshness_gate_applies_only_to_commits.behaviour holds).
ruff check .clean;782 passed, 1 skipped.Version
Bumped to
4.2.2; both changes recorded inCHANGELOG.md.Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/