fix(triage_daemon): anchor the force-push deny glob to its own token - #4792
Merged
Conversation
_PR_FORCE_PUSH_DENIALS used "Bash(git push*-f*)", an unanchored substring match that reads "-f" anywhere in the command as the force-push flag. A branch name containing "-flow", "-fix", "-format" etc. trips it: pushing fix/power-flow-car-outside-ct-clamp-4788 was denied outright because "power-flow" contains "-flow" (issue #4788 - the AI PR bot reported the push blocked and never opened a PR, even though the fix itself was complete and verified). Anchor "-f" to its own token (spaces on both sides, or end of command) so real force-push spellings (-f, --force, --force-with-lease) still deny, without matching substrings inside ordinary branch names.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new deny globs still allow combined short-flag spellings (e.g., -fu/-uf), and the new tests don’t currently cover (or reliably select) those cases.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes a false-positive in the triage daemon’s permission-policy denylist where git push branch names containing substrings like -flow could be misread as a -f force-push flag, blocking the /issue-pr flow from pushing branches and opening PRs.
Changes:
- Tightens the
_PR_FORCE_PUSH_DENIALSglob patterns to anchor-fto a flag token rather than matching it as an arbitrary substring. - Adds regression tests to ensure ordinary branch names containing
-flow/-format/-fieldare not denied, while explicit force-push spellings still are.
File summaries
| File | Description |
|---|---|
| tools/triage_daemon.py | Replaces the overly-broad git push*-f* denial glob with token-anchored variants to prevent branch-name substring false positives. |
| tools/test_triage_daemon.py | Adds unit tests simulating prefix-glob matching to prevent regressions for both false positives and real force-push denials. |
Review details
Suppressed comments (1)
tools/test_triage_daemon.py:423
- To ensure the force-push deny rules really cover the combined short-option spellings, add
git push -fu ...andgit push -uf ...toreal_force_pushes(these won’t match the current" ... -f "/ end-of-command patterns). Also make sure theforce_push_denialsfilter includes"-uf", otherwise the test can’t match against the new-ufdenial rules even if they exist.
"git push -f origin main",
"git push origin main -f",
"git push --force origin main",
"git push --force-with-lease origin main",
]
force_push_denials = [rule for rule in triage_daemon.DISALLOWED_TOOLS_PR.split(",") if "force" in rule or "-f" in rule]
for command in real_force_pushes:
matched = any(bash_rule_matches(rule, command) for rule in force_push_denials)
self.assertTrue(matched, f"{command!r} should still be denied, but no rule in {force_push_denials} matched")
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Read through the daemon's per-issue logs at ~/predbat-triage-bot/logs (both the pending and reviewed/ directories) and added confirmed, working-tree-verified findings the file didn't already have: - create_debug_yaml() masks only the "args" key; args_from_apps_yaml (added v8.48.1) is a second, unmasked deepcopy with real secrets.yaml values, and is still present on main - a live credential-leak trap for anyone quoting a reporter's debug yaml in a comment. - Solis: adjust_force_export()/adjust_inverter_mode() press the update button every cycle on H-M-format inverters regardless of whether anything changed, distinct from the existing CID 636 TOU-bit notes. - AlphaESS: the one-write-behind schedule pacer bug (now fixed in PR #4776) and the legacy batUseCap floor (errno 10001 signature). - Ohme's vendored ohmepy client was several versions behind upstream with withdrawn v1 control routes - also since fixed on main. - New rows: Load ML CPU-spike mechanism, the savings_total vs savings_yesterday metric mismatch, and the components.py auto-config gate's false-positive "skipping interface" warning. - Two new traps: SoC-quantisation false positives on "stuck charge" reports, and the Companion app ignoring Content-Disposition. Each entry either cites the issue it came from or is marked as already fixed on main, per the file's own "confirm before you write it down" rule. Added the small handful of new dictionary words (BLAS, threadpoolctl, unredacted, unreproduced) cspell flagged.
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
The AI triage bot's PR flow (
/issue-pr) got blocked pushing a completelyordinary branch:
git push -u origin fix/power-flow-car-outside-ct-clamp-4788was denied by the permission policy, so no PR was ever opened for #4788
even though the fix was complete and both quality gates had passed.
Root cause:
_PR_FORCE_PUSH_DENIALSused"Bash(git push*-f*)"- anunanchored glob that matches
-fas a substring anywhere in thecommand, not just as an actual flag.
fix/power-flow-car-outside-ct-clamp-4788contains
power-flow, whose-flowincludes-f, so the permissionengine read the branch name itself as a force-push attempt. Any branch
with
-fix,-format,-field, etc. would trip the same false positive.Fix
Anchor
-fto its own token (spaces on both sides, or end of thecommand):
Bash(git push* -f),Bash(git push* -f *), plus the existingBash(git push* --force*). Still denies every real force-push spelling(
-f,--force,--force-with-lease); no longer matches substringsinside ordinary branch names.
Testing
tools/test_triage_daemon.pythatsimulate Claude Code's prefix-glob matching: one pins that branch names
containing
-flow/-format/-fieldare no longer falsely denied,the other pins that real force-push spellings still are.
python3 -m unittest test_triage_daemon- 110/110 pass (was 108 beforethe two new tests).
pre-commit run --files tools/triage_daemon.py tools/test_triage_daemon.pyNotes
Issue #4788's own branch (
fix/power-flow-car-outside-ct-clamp-4788,commit
fd7f4f1a) already reachedorigindespite the bot's failurereport - likely a later retry with a different push spelling that
happened to dodge this bug, which the bot itself never noticed. That
issue is labelled
BOT_PR_FAILEDand is left for manual follow-upseparately from this fix.
The daemon (currently running as a long-lived process from the dedicated
clone) won't pick this fix up until that clone is synced to
mainandthe process restarted.