fix(hooks): tokenize git push flags in block-force-push

[Nitjsefnie]

Fixes the block-force-push policy so it inspects actual git push argv tokens instead of substring-matching the raw command string.

Defects fixed

1. False negative: git push -fu origin main was allowed. Git bundles short flags, so -fu is force + set-upstream. The old regex /-f\b/ required a word boundary after f, which fails when more letters follow.
2. False positive: git push --force-with-lease was denied. The old regex matched the --force prefix of the safer variant, which refuses to force-push if the remote has moved.

Approach

• Tokenize each git push … pipeline segment on whitespace.
• Stop inspecting flags after an explicit -- end-of-options marker.
• Block:
  • exact --force
  • any unknown --force-* long flag (fail-safe)
  • any short-flag bundle containing f (-f, -fu, -uf, -fq, …)
• Allow explicitly safe variants:
  • --force-with-lease and --force-with-lease=<refname>
  • --force-if-includes
• Allow unrelated flags such as --follow-tags.

Case table covered by tests

Command	Expected
git push --force origin main	deny
git push -f origin main	deny
git push -fu origin main	deny
git push -uf origin main	deny
git push -fq origin main	deny
git push --force-with-lease origin main	allow
git push --force-with-lease=main	allow
git push --force-if-includes origin main	allow
git push --follow-tags origin main	allow
git push origin -- --force	allow (positional)
git push origin feat/branch	allow
git push origin worktree/hn-fetch-job	allow
gh pr create --body "blocks --force and -f flags"	allow

Gates

bun run lint       # 0 errors, 5 pre-existing warnings
bunx tsc --noEmit  # pass
bun run test:run   # 2071 passed; 1 pre-existing failure in __tests__/hooks/integrations.test.ts unrelated to this change
bun run build      # pass

The single test failure (Pi integration > writeHookEntries adds a packages-array entry to a fresh settings.json) is pre-existing on main and unrelated to the force-push policy.

Closes #526

Prepared with AI assistance (Kimi K2.7 Code), human-reviewed before submission.

Summary by CodeRabbit

• Bug Fixes
  • Improved detection of force-push commands to more accurately block risky force flags.
  • Added support for bundled short force flags.
  • Prevented false positives for safer options such as force-with-lease, force-if-includes, follow-tags, and arguments after --.
• Tests
  • Expanded coverage for blocked and allowed force-push command variants.

[coderabbitai]

Caution

Review failed

Failed to post review comments.

GitHub was unavailable or timed out while CodeRabbit was posting the review. Please request a new review later if the pull request still needs one. This happened while posting 2 inline comments. Use @coderabbitai full review to retry the review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74faf0b7-203a-4980-ac11-3d434ce386d4

📥 Commits

Reviewing files that changed from the base of the PR and between 446484b and 4d78b9f.

📒 Files selected for processing (2)

• __tests__/hooks/builtin-policies.test.ts
• src/hooks/builtin-policies.ts

🧰 Additional context used

📓 Path-based instructions (4)

src/hooks/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/hooks/**/*.{ts,tsx,js,jsx}: Ensure ESM import (from 'failproofai') in custom policy resolves correctly.
Ensure CJS require (require('failproofai')) in custom policy resolves correctly.
Ensure transitive local imports inside custom policy file work correctly.

Files:

• src/hooks/builtin-policies.ts

src/hooks/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Builtin policies must run when no custom file is configured.

Files:

• src/hooks/builtin-policies.ts

**/src/hooks/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Hook policy evaluation must emit each CLI's required deny/retry response contract and preserve canonical tool/event input mappings; stop-gate behavior must force remediation where the CLI supports it.

Files:

• src/hooks/builtin-policies.ts

**/__tests__/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/__tests__/**/*.{ts,tsx,js,jsx}: Never edit a test to make it pass. Fix the code. Tests may only be changed when the test itself is wrong or when the feature under test was intentionally changed.
Always add unit tests for new behaviour. Place tests in tests/. Unit tests live in tests/hooks/, e2e tests in tests/e2e/hooks/.

Every new or changed behavior must have a corresponding unit test under __tests__/; do not alter existing tests merely to make them pass.

Files:

• __tests__/hooks/builtin-policies.test.ts

🧠 Learnings (2)

📚 Learning: 2026-05-05T23:13:01.551Z

Learnt from: NiveditJain
Repo: exospherehost/failproofai PR: 299
File: src/hooks/policy-evaluator.ts:0-0
Timestamp: 2026-05-05T23:13:01.551Z
Learning: In src/hooks/builtin-policies.ts, the five require-*-before-stop policies should register only match: { events: ["Stop"] } and must not subscribe to SubagentStop. They act as session-completion gates (global state checks when the human session ends), not subagent-return gates. Do not extend these policies to SubagentStop, as that would be semantically incorrect and noisy in cloud-agent flows that delegate to subagents. SubagentStop is reserved for custom policies that care about subagent boundaries (e.g., cleanup or logging). When reviewing, verify these five policies are limited to Stop and exclude SubagentStop across the relevant definitions, and ensure any changes to SubagentStop follow explicit custom-policy pathways.

Applied to files:

• src/hooks/builtin-policies.ts

📚 Learning: 2026-06-26T10:17:22.613Z

Learnt from: NiveditJain
Repo: FailproofAI/failproofai PR: 461
File: __tests__/auth/auth-cli-telemetry.test.ts:1-145
Timestamp: 2026-06-26T10:17:22.613Z
Learning: In the FailproofAI repo, unit tests should be placed by feature area under `__tests__/` to mirror the corresponding `src/` area. For example, code under `src/auth/` (e.g., `src/auth/cli.ts`) should have its unit tests in `__tests__/auth/` (e.g., `__tests__/auth/auth-cli-telemetry.test.ts`), matching the same area-based structure used for other features like `__tests__/audit/` for `src/audit/`.

Applied to files:

• __tests__/hooks/builtin-policies.test.ts

📝 Walkthrough

Walkthrough

The force-push policy now performs token-aware detection, recognizes bundled short force flags, allows safe force variants, honors --, and adds coverage for these argument cases.

Changes

Force-push policy

Layer / File(s)	Summary
Token-aware force flag detection src/hooks/builtin-policies.ts	blockForcePush scans tokenized push segments, stops option parsing after --, and uses explicit rules for dangerous and safe force flags.
Force flag edge-case coverage __tests__/hooks/builtin-policies.test.ts	Tests cover bundled short force flags, safe force options, and force-like arguments after --.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: niveditjain

Poem

A rabbit hops through flags with care,
Bundled forces caught mid-air.
Lease-safe pushes pass the gate,
-- makes later tokens wait.
Tests thump happily: “All is great!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change to block-force-push token handling.
Description check	✅ Passed	The description explains the bug, approach, test coverage, and validation results, even if it doesn't mirror every template heading.
Linked Issues check	✅ Passed	The changes address #526 by tokenizing push args, blocking bundled -f flags, allowing --force-with-lease[=...], and adding the requested tests.
Out of Scope Changes check	✅ Passed	The diff stays focused on block-force-push logic and its tests, with no obvious unrelated changes.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[hermes-exosphere]

Automated code review started - full review. Results will be posted here.

[hermes-exosphere]

⏳ Review in progress — Phase 1-2: diff analysis complete. Running test suite now...

[hermes-exosphere]

✅ Build & test complete.

• Build: pass (tsc --noEmit clean)
• Tests: 2064 passed, 0 failed (121 test files)
• Force-push tests: 17/17 passed (including new bundled short-flag tests)
• Type check: clean
• All tests pass with zero regressions.

[hermes-exosphere]

💡 Good design: fail-safe catch-all for unknown --force- variants.*

Line 698 if (token.startsWith("--force")) return true; blocks any future git flag that starts with --force but isn't on the explicit allowlist. This is strong defense-in-depth — if git ever adds --force-something-new, failproofai defaults to blocking it until you audit and add it to SAFE_FORCE_PREFIXES.

[hermes-exosphere]

Automated Code Review

Executive Summary

This PR fixes two defects in the block-force-push policy by replacing regex-based detection with proper argv tokenization. The old regex allowed bundled short flags like -fu and incorrectly blocked the safer --force-with-lease variant. The rewrite uses proper tokenization with a fail-safe catch-all and respects the -- end-of-options marker. The change is surgical, well-tested, and introduces zero regressions.

---

Change Architecture

Old: regex match on raw args
  |
  v
New: tokenize per pipeline segment -> isForcePushFlag(token)
  +-- "--force" exact ..................... BLOCK
  +-- "--force-with-lease" / "--force-if-includes" ... ALLOW
  +-- unknown "--force-*" ................. BLOCK (fail-safe)
  +-- short flag bundle with "f" .......... BLOCK
  +-- stop after "--" (end-of-options) .... positional, ALLOW

---

Breaking Changes

No breaking changes detected. Policy behavior is expanded (more cases correctly detected) but backward-compatible.

---

Issues Found

No issues found. The code is clean and correct.

---

Logical / Bug Analysis

What the old code did wrong:

1. False negative: regex /-f\b/ required word boundary after f, so -fu slipped through
2. False positive: --force-with-lease starts with --force, matching the regex

What the new code fixes:

1. Tokenizes pipeline segments on whitespace, then iterates tokens
2. Handles -- end-of-options correctly
3. isForcePushFlag order: safe prefixes first, then --force exact, then --force* catch-all, then short flag bundle
4. SHORT_FLAG_BUNDLE_RE requires token starts with -, so branch names like feat/my-feat are never matched

Edge cases verified:

• -fu, -uf, -fq -> blocked (bundled short flags)
• --force-with-lease, --force-with-lease=main, --force-if-includes -> allowed
• --follow-tags -> allowed (starts with --fo, not --force)
• git push origin -- --force -> allowed (positional after --)
• gh pr create with body mentioning the flags -> allowed (not in pipeline segment)

---

Evidence - Build & Test Results

Type check: bunx tsc --noEmit — clean (no errors)
Unit tests: 2064 passed, 0 failed (121 test files)
Force-push tests: 17/17 passed

blocks git push --force ............... PASS
blocks git push -f ..................... PASS
blocks git push -fu (bundled flags) .... PASS
blocks git push -uf (alt order) ........ PASS
blocks git push -fq (quiet+force) ...... PASS
allows git push --force-with-lease ..... PASS
allows git push --force-with-lease=main  PASS
allows git push --force-if-includes .... PASS
allows git push --follow-tags .......... PASS
allows --force after -- (positional) ... PASS
allows normal git push ................. PASS
allows branch with -f substring ........ PASS
allows gh pr create with body mention .. PASS
replay: block-force-push fires ......... PASS

---

Issue Linkage

This PR closes #526. All cases covered by tests.

---

Human Review Feedback

No human review comments on this PR.

---

Suggestions

Consider mentioning --force-with-lease as a safe alternative in the deny message. (Low priority, cosmetic.)

---

Verdict

VERDICT: APPROVED

---

Automated code review - 2026-07-16 12:05 UTC

[hermes-exosphere]

Automated review: Approved. Clean, well-tested fix for the block-force-push tokenization. No issues found. 2064 tests pass, type check clean.

[hermes-exosphere]

Automated review: Approved. ✅

[hermes-exosphere]

Your PR is awaiting review by a moderator. Till then you can join the Discord for conversation: https://discord.gg/qaFM2uYFb

[hermes-exosphere]

Your PR is awaiting review by a moderator. Till then you can join the Discord for conversation: https://discord.befailproof.ai

[hermes-exosphere]

Your PR is awaiting review by a reviewer. Till then you can join the Discord for conversation: https://discord.befailproof.ai

[hermes-exosphere]

Hermes Agent Code Review — PR #527

PR: fix(hooks): tokenize git push flags in block-force-push
Author: Nitjsefnie (Peter Z)
Files: 2 changed (+61, −4)

---

✅ Looks Good

The approach is sound. The old regex (?:--force|-f\b) had two real defects — false negative on bundled short flags (-fu) and false positive on safer variants (--force-with-lease). Token-based inspection with explicit allowlisting of the two safe --force-* variants fixes both.

Key design decisions I agree with:

1. Fail-safe for unknown --force-*: The startsWith("--force") catch-all after SAFE_FORCE_PREFIXES is the right approach. Any future Git force-push flag that doesn't match the safe allowlist is blocked — this is conservative and appropriate for a security policy.

2. -- end-of-options handling: Correctly stops inspecting tokens after --, so git push origin -- --force (positional ref named --force) is allowed. This matches Git's actual behavior.

3. Reuses existing extractGitPushArgs: The segment-splitting logic remains unchanged — it still splits on shell operators (&&, ||, |, ;) and filters for git push segments. The fix only changes what happens inside each segment.

4. Clean separation of concerns: isForcePushFlag() is a pure predicate, easily testable, and the three-step cascade (--force exact → safe prefixes → --force-* unknown → short bundle regex) is readable and maintainable.

---

💡 Suggestions (non-blocking)

1. --force-with-lease without a value: Both --force-with-lease (implied refspec) and --force-with-lease=main (explicit) are correctly allowed. Consider adding a comment noting why the bare form is safe — it refuses to push if the remote tracking branch has moved.

2. The f-in-branch-name test (worktree/hn-fetch-job) was already passing before this change (old regex -f\b wouldn't match in the middle of a word), but it's good defensive coverage to keep it. No action needed.

---

🔬 Test Coverage

All 12 cases from the PR's case table are covered in tests:

Command	Expected	Test exists
--force	deny	✅ (pre-existing)
-f	deny	✅ (pre-existing)
-fu	deny	✅ new
-uf	deny	✅ new
-fq	deny	✅ new
--force-with-lease	allow	✅ new
--force-with-lease=main	allow	✅ new
--force-if-includes	allow	✅ new
--follow-tags	allow	✅ new
origin -- --force	allow	✅ new
feat/branch	allow	✅ (pre-existing)
branch with -f in name	allow	✅ (pre-existing)
gh pr create with --force in body	allow	✅ (pre-existing)

Gates:

• ✅ bunx tsc --noEmit — pass
• ✅ bun run test:run — 2072 passed (0 failures; the 1 pre-existing failure noted in the PR body appears resolved)
• ⚠️ bun run lint — timed out after 60s (pre-existing ESLint perf issue, not related to this change)

---

Verdict: APPROVED ✅

The change is minimal, correct, and well-tested. The token-based approach closes two real defects without introducing new false positives. No blocking issues found.

---

Reviewed by Hermes Agent