mirror of
https://github.com/codestable/CodeStable.git
synced 2026-09-19 09:03:09 +08:00
fix: stop installing protected main push hooks
This commit is contained in:
@@ -18,8 +18,7 @@ meant to run before AI tool calls, and it can also install Git hook fallbacks.
|
||||
with hosts whose project-dir env var does not follow `EnterWorktree`.
|
||||
|
||||
Git cannot stop branch switches before they happen, so command-hook enforcement
|
||||
is the primary guard. Git hooks only catch commit, merge, rebase, and push
|
||||
fallbacks.
|
||||
is the primary guard. Git hooks are only a commit-time fallback.
|
||||
|
||||
## Agent Hook
|
||||
|
||||
@@ -34,11 +33,11 @@ The hook reads JSON from stdin. It recognizes common `tool_name` /
|
||||
edit tools. A blocked action exits with status `2` and prints the reason to
|
||||
stderr.
|
||||
|
||||
For shell tools, the guard blocks known Git write commands (`git add`,
|
||||
`commit`, `merge`, `push`, etc.) and branch switches. It does not parse arbitrary
|
||||
shell programs such as `python -c 'open("app.py", "a")...'`; direct file writes
|
||||
must be caught by Edit/Write tool payload paths or by the implementation review
|
||||
and worktree gates after the command.
|
||||
For shell tools, the agent hook blocks known Git write commands (`git add`,
|
||||
`commit`, `merge`, `push`, etc.) and branch switches before they run. It does
|
||||
not parse arbitrary shell programs such as `python -c 'open("app.py", "a")...'`;
|
||||
direct file writes must be caught by Edit/Write tool payload paths or by the
|
||||
implementation review and worktree gates after the command.
|
||||
|
||||
## Git Hook Fallback
|
||||
|
||||
@@ -51,39 +50,11 @@ python3 .codestable/tools/codestable-ai-branch-guard.py --root . --install-git-h
|
||||
Installed fallbacks:
|
||||
|
||||
- `pre-commit`: blocks staged implementation files on `main` / `master`.
|
||||
- `pre-merge-commit`: blocks protected-branch merge commits.
|
||||
- `pre-rebase`: blocks protected-branch rebases.
|
||||
- `pre-push`: blocks protected-branch pushes.
|
||||
|
||||
This repository no longer installs Git hooks for protected-branch merge, rebase
|
||||
or push. Publishing `main` remains an owner workflow, not a local hook gate.
|
||||
Use `--force` only when replacing an existing local hook is intentional.
|
||||
|
||||
## Owner-Intent Main Publish
|
||||
|
||||
Protected-branch merge and push are allowed only during a short owner-approved
|
||||
publish window. Start the window from a clean `main` checkout that matches the
|
||||
publish remote's `main`.
|
||||
|
||||
The publish remote defaults to the **current branch's upstream remote** (so fork
|
||||
workflows work out of the box), falling back to `origin`. If your `origin` is an
|
||||
upstream mirror and you publish to a fork, pass `--remote` explicitly:
|
||||
|
||||
```bash
|
||||
python3 .codestable/tools/codestable-main-publish.py --root . --json begin \
|
||||
--owner-intent "owner approved publishing branch X to main" \
|
||||
--remote <your-fork-remote> \
|
||||
--branch feat/example
|
||||
```
|
||||
|
||||
Then run the merge / validation / push. The guard allows `git merge`,
|
||||
merge-conflict resolution commits, and `git push` while the intent is active.
|
||||
It still blocks `git switch` / `git checkout`.
|
||||
|
||||
Finish by removing the intent:
|
||||
|
||||
```bash
|
||||
python3 .codestable/tools/codestable-main-publish.py --root . --json end
|
||||
```
|
||||
|
||||
## Recovery
|
||||
|
||||
If work has already started in the coordinator checkout, stop and create a
|
||||
|
||||
@@ -413,16 +413,7 @@ def guard_git_hook(root: Path, hook_name: str, protected: set[str]) -> GuardResu
|
||||
)
|
||||
return GuardResult(True, "allowed", "allowed", branch, linked)
|
||||
|
||||
if hook_name in {"pre-merge-commit", "pre-push"} and intent:
|
||||
return GuardResult(True, "allowed by owner-intent main publish", "owner_intent_main_publish", branch, linked)
|
||||
|
||||
return GuardResult(
|
||||
False,
|
||||
f"Protected branch {hook_name} is blocked for AI-managed checkouts. Use an execution worktree and merge only with owner intent.",
|
||||
f"{hook_name}_on_protected_branch",
|
||||
branch,
|
||||
linked,
|
||||
)
|
||||
return GuardResult(True, "allowed", "allowed", branch, linked)
|
||||
|
||||
|
||||
def hook_path(root: Path, hook_name: str) -> Path:
|
||||
@@ -436,7 +427,7 @@ def install_git_hooks(root: Path, force: bool) -> list[Path]:
|
||||
installed: list[Path] = []
|
||||
script = root / ".codestable" / "tools" / "codestable-ai-branch-guard.py"
|
||||
fallback = Path(__file__).resolve()
|
||||
for hook_name in ("pre-commit", "pre-merge-commit", "pre-rebase", "pre-push"):
|
||||
for hook_name in ("pre-commit",):
|
||||
path = hook_path(root, hook_name)
|
||||
if path.exists() and HOOK_MARKER not in path.read_text(encoding="utf-8", errors="ignore") and not force:
|
||||
raise RuntimeError(f"refusing to overwrite existing hook without --force: {path}")
|
||||
|
||||
@@ -23,7 +23,6 @@ def load_tool(module_name: str, filename: str):
|
||||
guard = load_tool("codestable_ai_branch_guard", "codestable-ai-branch-guard.py")
|
||||
main_publish = load_tool("codestable_main_publish", "codestable-main-publish.py")
|
||||
|
||||
|
||||
def run(repo: Path, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(
|
||||
["git", *args],
|
||||
@@ -124,59 +123,48 @@ def test_pre_commit_blocks_staged_implementation_on_main(tmp_path: Path) -> None
|
||||
assert result.paths == ("src/app.py",)
|
||||
|
||||
|
||||
def test_owner_intent_allows_protected_pre_push(tmp_path: Path) -> None:
|
||||
def test_git_hook_does_not_block_protected_pre_push(tmp_path: Path) -> None:
|
||||
repo = init_repo_with_remote(tmp_path)
|
||||
|
||||
blocked = guard.guard_git_hook(repo, "pre-push", {"main", "master"})
|
||||
created = main_publish.begin(repo, "main", "origin", [], "owner approved release", 5)
|
||||
allowed = guard.guard_git_hook(repo, "pre-push", {"main", "master"})
|
||||
result = guard.guard_git_hook(repo, "pre-push", {"main", "master"})
|
||||
|
||||
assert created["ok"]
|
||||
assert not blocked.ok
|
||||
assert allowed.ok
|
||||
assert allowed.reason == "owner_intent_main_publish"
|
||||
assert result.ok
|
||||
assert result.reason == "allowed"
|
||||
|
||||
|
||||
def test_owner_intent_allows_merge_command_but_not_switch(tmp_path: Path) -> None:
|
||||
def test_git_hooks_do_not_block_protected_merge_or_rebase(tmp_path: Path) -> None:
|
||||
repo = init_repo_with_remote(tmp_path)
|
||||
main_publish.begin(repo, "main", "origin", [], "owner approved release", 5)
|
||||
|
||||
merge_result = guard.guard_git_hook(repo, "pre-merge-commit", {"main", "master"})
|
||||
rebase_result = guard.guard_git_hook(repo, "pre-rebase", {"main", "master"})
|
||||
|
||||
assert merge_result.ok
|
||||
assert rebase_result.ok
|
||||
|
||||
|
||||
def test_agent_hook_still_blocks_protected_merge_and_force_push(tmp_path: Path) -> None:
|
||||
repo = init_repo_with_remote(tmp_path)
|
||||
merge_payload = {"tool_name": "Bash", "tool_input": {"command": "git merge origin/feat/demo"}}
|
||||
switch_payload = {"tool_name": "Bash", "tool_input": {"command": "git switch feat/demo"}}
|
||||
force_push_payload = {"tool_name": "Bash", "tool_input": {"command": "git push --force-with-lease origin main"}}
|
||||
|
||||
assert guard.guard_payload(merge_payload, repo, {"main", "master"}).ok
|
||||
assert guard.guard_payload(switch_payload, repo, {"main", "master"}).reason == "branch_switch_command"
|
||||
merge_result = guard.guard_payload(merge_payload, repo, {"main", "master"})
|
||||
force_push_result = guard.guard_payload(force_push_payload, repo, {"main", "master"})
|
||||
|
||||
assert not merge_result.ok
|
||||
assert merge_result.reason == "git_merge_on_protected_branch"
|
||||
assert not force_push_result.ok
|
||||
assert force_push_result.reason == "git_push_on_protected_branch"
|
||||
|
||||
|
||||
def test_owner_intent_does_not_allow_force_push(tmp_path: Path) -> None:
|
||||
def test_installed_git_hooks_only_include_pre_commit(tmp_path: Path) -> None:
|
||||
repo = init_repo_with_remote(tmp_path)
|
||||
main_publish.begin(repo, "main", "origin", [], "owner approved release", 5)
|
||||
payload = {"tool_name": "Bash", "tool_input": {"command": "git push --force-with-lease origin main"}}
|
||||
installed = guard.install_git_hooks(repo, force=False)
|
||||
|
||||
result = guard.guard_payload(payload, repo, {"main", "master"})
|
||||
|
||||
assert not result.ok
|
||||
assert result.reason == "git_push_on_protected_branch"
|
||||
|
||||
|
||||
def test_owner_intent_allows_real_hooked_merge_and_push(tmp_path: Path) -> None:
|
||||
repo = init_repo_with_remote(tmp_path)
|
||||
run(repo, "switch", "-c", "feat/demo")
|
||||
(repo / "README.md").write_text("published\n", encoding="utf-8")
|
||||
run(repo, "add", "README.md")
|
||||
run(repo, "commit", "-m", "demo change")
|
||||
run(repo, "push", "-u", "origin", "feat/demo")
|
||||
run(repo, "switch", "main")
|
||||
guard.install_git_hooks(repo, force=False)
|
||||
|
||||
main_publish.begin(repo, "main", "origin", ["feat/demo"], "owner approved release", 5)
|
||||
merge = run(repo, "merge", "--no-ff", "--no-edit", "origin/feat/demo", check=False)
|
||||
push = run(repo, "push", "origin", "main", check=False)
|
||||
ended = main_publish.end(repo)
|
||||
|
||||
assert merge.returncode == 0, merge.stderr
|
||||
assert push.returncode == 0, push.stderr
|
||||
assert ended["removed"]
|
||||
assert [path.name for path in installed] == ["pre-commit"]
|
||||
assert (repo / ".git/hooks/pre-commit").exists()
|
||||
assert not (repo / ".git/hooks/pre-push").exists()
|
||||
assert not (repo / ".git/hooks/pre-merge-commit").exists()
|
||||
assert not (repo / ".git/hooks/pre-rebase").exists()
|
||||
|
||||
|
||||
def test_allows_implementation_edit_in_linked_worktree_branch(tmp_path: Path) -> None:
|
||||
|
||||
@@ -260,7 +260,7 @@ def test_runtime_tool_paths_are_documented() -> None:
|
||||
encoding="utf-8"
|
||||
)
|
||||
assert "codestable-ai-branch-guard.py" in hook_doc
|
||||
assert "codestable-main-publish.py" in hook_doc
|
||||
assert "Publishing `main` remains an owner workflow" in hook_doc
|
||||
|
||||
|
||||
def test_missing_unit_cli_returns_json_finding(tmp_path: Path) -> None:
|
||||
|
||||
Reference in New Issue
Block a user