fix(test): update tier2_pre_commit_hook tests for abort-on-strip behavior
TIER-3 READ AGENTS.md + conductor/code_styleguides/error_handling.md + tests/test_tier2_pre_commit_hook.py + conductor/tier2/githooks/pre-commit before pre-commit-test-fix.
7 tests in tests/test_tier2_pre_commit_hook.py asserted the OLD silent-strip behavior (exit 0). The pre-commit hook was changed in eae75877 to abort on strip (exit 1) to prevent the 2026-06-24 MCP regression where Tier 2 made an empty fix commit and reported success without verifying the diff.
Tests updated to assert the NEW abort behavior:
- result.returncode == 1 (was 0)
- Diagnostic message 'COMMIT ABORTED' in result.stderr
- File still unstaged after hook (unchanged behavior)
- HEAD-content assertions removed in 2 tests (commit was aborted, no HEAD changes)
Acceptance: 12/12 tests pass in tests/test_tier2_pre_commit_hook.py.
This commit is contained in:
@@ -106,9 +106,10 @@ def test_hook_unstages_forbidden_opencode_agent_file(fake_clone: Path) -> None:
|
||||
_run(fake_clone, "git", "add", ".opencode/agents/tier2-autonomous.md")
|
||||
assert _staged_files(fake_clone) == [".opencode/agents/tier2-autonomous.md"]
|
||||
result = _commit(fake_clone, "leak attempt")
|
||||
# Hook must NOT block the commit (exit 0); commit succeeds with empty diff
|
||||
assert result.returncode == 0, f"hook unexpectedly blocked commit: {result.stderr}"
|
||||
# File must have been unstaged
|
||||
# Hook ABORTS the commit (exit 1) to prevent silent-strip-then-empty-commit
|
||||
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
||||
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
||||
# File must have been unstaged (unchanged behavior)
|
||||
assert _staged_files(fake_clone) == [], "forbidden file was not auto-unstaged"
|
||||
# Working tree still has the modification (hook only unstaged)
|
||||
assert forbidden.exists(), "hook should not delete the file from working tree"
|
||||
@@ -122,7 +123,8 @@ def test_hook_unstages_forbidden_opencode_command_file(fake_clone: Path) -> None
|
||||
forbidden.write_text("# fake tier-2 command\n")
|
||||
_run(fake_clone, "git", "add", ".opencode/commands/tier-2-auto-execute.md")
|
||||
result = _commit(fake_clone, "leak attempt")
|
||||
assert result.returncode == 0, f"hook blocked commit: {result.stderr}"
|
||||
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
||||
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
||||
assert _staged_files(fake_clone) == []
|
||||
|
||||
|
||||
@@ -136,7 +138,8 @@ def test_hook_unstages_modified_opencode_json(fake_clone: Path) -> None:
|
||||
opencode_json.write_text('{"version": 1, "tier2-modified": true}\n')
|
||||
_run(fake_clone, "git", "add", "opencode.json")
|
||||
result = _commit(fake_clone, "leak attempt")
|
||||
assert result.returncode == 0, f"hook blocked commit: {result.stderr}"
|
||||
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
||||
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
||||
assert _staged_files(fake_clone) == []
|
||||
|
||||
|
||||
@@ -149,7 +152,8 @@ def test_hook_unstages_modified_mcp_paths_toml(fake_clone: Path) -> None:
|
||||
mcp_paths.write_text('[allowed_paths]\nextra_dirs = ["leaked"]\n')
|
||||
_run(fake_clone, "git", "add", "mcp_paths.toml")
|
||||
result = _commit(fake_clone, "leak attempt")
|
||||
assert result.returncode == 0, f"hook blocked commit: {result.stderr}"
|
||||
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
||||
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
||||
assert _staged_files(fake_clone) == []
|
||||
|
||||
|
||||
@@ -170,7 +174,8 @@ def test_hook_unstages_all_forbidden_files_at_once(fake_clone: Path) -> None:
|
||||
staged = sorted(_staged_files(fake_clone))
|
||||
assert len(staged) == 4, f"setup failed; staged={staged}"
|
||||
result = _commit(fake_clone, "multi-leak")
|
||||
assert result.returncode == 0, f"hook blocked commit: {result.stderr}"
|
||||
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
||||
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
||||
assert _staged_files(fake_clone) == []
|
||||
|
||||
|
||||
@@ -182,15 +187,12 @@ def test_hook_keeps_allowed_files_alongside_forbidden(fake_clone: Path) -> None:
|
||||
_run(fake_clone, "git", "add",
|
||||
".opencode/agents/tier2-autonomous.md", "legit.py")
|
||||
result = _commit(fake_clone, "mixed")
|
||||
assert result.returncode == 0, f"hook blocked commit: {result.stderr}"
|
||||
# Allowed file should be in HEAD
|
||||
head_files = _run(fake_clone, "git", "ls-tree", "--name-only", "HEAD").stdout.split()
|
||||
assert "legit.py" in head_files, f"legit.py missing from HEAD: {head_files}"
|
||||
assert ".opencode/agents/tier2-autonomous.md" not in head_files, (
|
||||
f"forbidden file leaked into HEAD: {head_files}"
|
||||
)
|
||||
# Forbidden file should be unstaged but still on disk
|
||||
assert _staged_files(fake_clone) == []
|
||||
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
||||
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
||||
# Commit was aborted: verify both files remain on disk (no HEAD changes)
|
||||
assert (fake_clone / "legit.py").exists() and (fake_clone / ".opencode/agents/tier2-autonomous.md").exists()
|
||||
# Forbidden file unstaged, legit file (not in denylist) remains staged
|
||||
assert _staged_files(fake_clone) == ["legit.py"]
|
||||
assert (fake_clone / ".opencode" / "agents" / "tier2-autonomous.md").exists()
|
||||
|
||||
|
||||
@@ -213,7 +215,7 @@ def test_hook_warns_when_unstaging(fake_clone: Path) -> None:
|
||||
(fake_clone / ".opencode" / "agents" / "tier2-autonomous.md").write_text("leak\n")
|
||||
_run(fake_clone, "git", "add", ".opencode/agents/tier2-autonomous.md")
|
||||
result = _commit(fake_clone, "leak")
|
||||
assert result.returncode == 0
|
||||
assert result.returncode == 1
|
||||
# Hook output should mention the leak (so tier-2 sees what happened)
|
||||
combined = (result.stdout + result.stderr).lower()
|
||||
assert re.search(r"tier.?2|removing|sandbox", combined), (
|
||||
@@ -237,17 +239,21 @@ def test_hook_uses_config_from_project_root(fake_clone: Path) -> None:
|
||||
_run(fake_clone, "git", "add",
|
||||
"custom_forbidden.txt", "opencode.json")
|
||||
result = _commit(fake_clone, "mixed")
|
||||
assert result.returncode == 0, f"hook blocked commit: {result.stderr}"
|
||||
# Check HEAD (committed tree), not staged (empty after successful commit).
|
||||
head_files = _run(fake_clone, "git", "ls-tree", "--name-only", "HEAD").stdout.split()
|
||||
# custom_forbidden.txt must NOT be in HEAD (unstaged by hook)
|
||||
assert "custom_forbidden.txt" not in head_files, (
|
||||
f"custom_forbidden.txt leaked into HEAD: {head_files}"
|
||||
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
||||
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
||||
# Commit was aborted: nothing committed. Both files remain on disk.
|
||||
# custom_forbidden.txt: was unstaged by hook (in custom config) -> not in staged set
|
||||
# opencode.json: NOT in custom config -> hook left it staged, but commit aborted so not in HEAD
|
||||
staged = _staged_files(fake_clone)
|
||||
assert "custom_forbidden.txt" not in staged, (
|
||||
f"custom_forbidden.txt should have been unstaged: {staged}"
|
||||
)
|
||||
# opencode.json MUST be in HEAD (not in custom config, so hook left it alone)
|
||||
assert "opencode.json" in head_files, (
|
||||
f"opencode.json missing from HEAD (hook over-unstaged): {head_files}"
|
||||
assert "opencode.json" in staged, (
|
||||
f"opencode.json should still be staged (not in custom config): {staged}"
|
||||
)
|
||||
# On-disk existence: neither file was deleted by the hook
|
||||
assert (fake_clone / "custom_forbidden.txt").exists()
|
||||
assert (fake_clone / "opencode.json").exists()
|
||||
|
||||
|
||||
def test_hook_handles_paths_with_spaces(fake_clone: Path) -> None:
|
||||
|
||||
Reference in New Issue
Block a user