From 0a9d0429ec0bed008c62af9078a57734c691fb4a Mon Sep 17 00:00:00 2001 From: rookiestar28 Date: Sat, 14 Mar 2026 02:42:37 +0800 Subject: [PATCH] fix: catch pre-commit drift before push --- scripts/pre_push_checks.sh | 35 +++++++++++++++++++++++++++ scripts/run_full_tests_linux.sh | 35 +++++++++++++++++++++++++++ scripts/run_full_tests_windows.ps1 | 25 +++++++++++++++++++ tests/e2e/specs/approvals.spec.js | 4 +-- tests/test_r141_inventory_indexing.py | 32 +++++++++++++++--------- 5 files changed, 118 insertions(+), 13 deletions(-) diff --git a/scripts/pre_push_checks.sh b/scripts/pre_push_checks.sh index 096bb43..f9aba2b 100644 --- a/scripts/pre_push_checks.sh +++ b/scripts/pre_push_checks.sh @@ -50,6 +50,36 @@ pip_install_or_fail() { exit 1 } +capture_precommit_snapshots() { + # IMPORTANT: compare both worktree and index; pre-commit can mutate staged files while exiting 0. + PRECOMMIT_WORKTREE_SNAPSHOT="$(mktemp)" + PRECOMMIT_INDEX_SNAPSHOT="$(mktemp)" + git diff --binary -- . >"$PRECOMMIT_WORKTREE_SNAPSHOT" + git diff --cached --binary -- . >"$PRECOMMIT_INDEX_SNAPSHOT" +} + +cleanup_precommit_snapshots() { + rm -f "${PRECOMMIT_WORKTREE_SNAPSHOT:-}" "${PRECOMMIT_INDEX_SNAPSHOT:-}" \ + "${PRECOMMIT_WORKTREE_SNAPSHOT_AFTER:-}" "${PRECOMMIT_INDEX_SNAPSHOT_AFTER:-}" +} + +precommit_changed_repo_state() { + PRECOMMIT_WORKTREE_SNAPSHOT_AFTER="$(mktemp)" + PRECOMMIT_INDEX_SNAPSHOT_AFTER="$(mktemp)" + git diff --binary -- . >"$PRECOMMIT_WORKTREE_SNAPSHOT_AFTER" + git diff --cached --binary -- . >"$PRECOMMIT_INDEX_SNAPSHOT_AFTER" + ! cmp -s "$PRECOMMIT_WORKTREE_SNAPSHOT" "$PRECOMMIT_WORKTREE_SNAPSHOT_AFTER" || \ + ! cmp -s "$PRECOMMIT_INDEX_SNAPSHOT" "$PRECOMMIT_INDEX_SNAPSHOT_AFTER" +} + +report_precommit_repo_drift_and_exit() { + echo "[pre-push] ERROR: pre-commit hooks modified tracked files (worktree or index)." >&2 + echo "[pre-push] Please review/stage the hook fixes, then push again." >&2 + git status --short + cleanup_precommit_snapshots + exit 1 +} + is_wsl() { grep -qiE "(microsoft|wsl)" /proc/version 2>/dev/null } @@ -322,6 +352,7 @@ echo "[pre-push] 1/7 detect-secrets" run_pre_commit_safe run detect-secrets --all-files echo "[pre-push] 2/7 pre-commit all hooks (pass 1)" +capture_precommit_snapshots if run_pre_commit_safe run --all-files --show-diff-on-failure; then : else @@ -338,6 +369,10 @@ else exit 1 fi fi +if precommit_changed_repo_state; then + report_precommit_repo_drift_and_exit +fi +cleanup_precommit_snapshots # IMPORTANT: generated spec drift must fail before backend tests so docs-only # edits cannot hide until deep in the pre-push unit suite. diff --git a/scripts/run_full_tests_linux.sh b/scripts/run_full_tests_linux.sh index 9f8ebd2..ac0cd06 100644 --- a/scripts/run_full_tests_linux.sh +++ b/scripts/run_full_tests_linux.sh @@ -53,6 +53,36 @@ pip_install_or_fail() { exit 1 } +capture_precommit_snapshots() { + # IMPORTANT: compare both worktree and index; pre-commit can mutate staged files while exiting 0. + PRECOMMIT_WORKTREE_SNAPSHOT="$(mktemp)" + PRECOMMIT_INDEX_SNAPSHOT="$(mktemp)" + git diff --binary -- . >"$PRECOMMIT_WORKTREE_SNAPSHOT" + git diff --cached --binary -- . >"$PRECOMMIT_INDEX_SNAPSHOT" +} + +cleanup_precommit_snapshots() { + rm -f "${PRECOMMIT_WORKTREE_SNAPSHOT:-}" "${PRECOMMIT_INDEX_SNAPSHOT:-}" \ + "${PRECOMMIT_WORKTREE_SNAPSHOT_AFTER:-}" "${PRECOMMIT_INDEX_SNAPSHOT_AFTER:-}" +} + +precommit_changed_repo_state() { + PRECOMMIT_WORKTREE_SNAPSHOT_AFTER="$(mktemp)" + PRECOMMIT_INDEX_SNAPSHOT_AFTER="$(mktemp)" + git diff --binary -- . >"$PRECOMMIT_WORKTREE_SNAPSHOT_AFTER" + git diff --cached --binary -- . >"$PRECOMMIT_INDEX_SNAPSHOT_AFTER" + ! cmp -s "$PRECOMMIT_WORKTREE_SNAPSHOT" "$PRECOMMIT_WORKTREE_SNAPSHOT_AFTER" || \ + ! cmp -s "$PRECOMMIT_INDEX_SNAPSHOT" "$PRECOMMIT_INDEX_SNAPSHOT_AFTER" +} + +report_precommit_repo_drift_and_exit() { + echo "[tests] ERROR: pre-commit hooks modified tracked files (worktree or index)." >&2 + echo "[tests] Review/stage the hook changes, then rerun the acceptance gate." >&2 + git status --short + cleanup_precommit_snapshots + exit 1 +} + require_cmd node require_cmd npm @@ -153,12 +183,17 @@ echo "[tests] 1/8 detect-secrets" "$VENV_PY" -m pre_commit run detect-secrets --all-files echo "[tests] 2/8 pre-commit all hooks (pass 1: autofix)" +capture_precommit_snapshots if "$VENV_PY" -m pre_commit run --all-files --show-diff-on-failure; then : else echo "[tests] INFO: pre-commit reported changes/issues; running pass 2 verification..." "$VENV_PY" -m pre_commit run --all-files --show-diff-on-failure fi +if precommit_changed_repo_state; then + report_precommit_repo_drift_and_exit +fi +cleanup_precommit_snapshots echo "[tests] 3/8 backend unit tests" MOLTBOT_STATE_DIR="$ROOT_DIR/moltbot_state/_local_unit" "$VENV_PY" scripts/run_unittests.py --start-dir tests --pattern "test_*.py" --enforce-skip-policy tests/skip_policy.json diff --git a/scripts/run_full_tests_windows.ps1 b/scripts/run_full_tests_windows.ps1 index c893519..484122a 100644 --- a/scripts/run_full_tests_windows.ps1 +++ b/scripts/run_full_tests_windows.ps1 @@ -29,6 +29,28 @@ function Invoke-Checked { } } +function Get-GitDiffSnapshot { + param([switch]$Cached) + # IMPORTANT: compare both worktree and index; pre-commit can mutate staged files while exiting 0. + if ($Cached) { + return (& git diff --cached --binary -- . | Out-String) + } + return (& git diff --binary -- . | Out-String) +} + +function Assert-PreCommitDidNotMutateRepo { + param( + [Parameter(Mandatory = $true)][string]$BeforeWorktree, + [Parameter(Mandatory = $true)][string]$BeforeIndex + ) + + $afterWorktree = Get-GitDiffSnapshot + $afterIndex = Get-GitDiffSnapshot -Cached + if ($BeforeWorktree -ne $afterWorktree -or $BeforeIndex -ne $afterIndex) { + throw "[tests] ERROR: pre-commit hooks modified tracked files (worktree or index). Review/stage the hook changes, then rerun the acceptance gate.`n$(& git status --short | Out-String)" + } +} + Require-Cmd node Require-Cmd npm @@ -210,11 +232,14 @@ Write-Host "[tests] 1/8 detect-secrets" Invoke-Checked "detect-secrets" { & $venvPython -m pre_commit run detect-secrets --all-files } Write-Host "[tests] 2/8 pre-commit all hooks (pass 1: autofix)" +$preCommitWorktreeBefore = Get-GitDiffSnapshot +$preCommitIndexBefore = Get-GitDiffSnapshot -Cached & $venvPython -m pre_commit run --all-files --show-diff-on-failure if ($LASTEXITCODE -ne 0) { Write-Host "[tests] INFO: pre-commit reported changes/issues; running pass 2 verification..." Invoke-Checked "pre-commit all hooks (pass 2 verify)" { & $venvPython -m pre_commit run --all-files --show-diff-on-failure } } +Assert-PreCommitDidNotMutateRepo -BeforeWorktree $preCommitWorktreeBefore -BeforeIndex $preCommitIndexBefore Write-Host "[tests] 3/8 backend unit tests" $env:MOLTBOT_STATE_DIR = "$root\moltbot_state\_local_unit" diff --git a/tests/e2e/specs/approvals.spec.js b/tests/e2e/specs/approvals.spec.js index 35ad45f..db0089c 100644 --- a/tests/e2e/specs/approvals.spec.js +++ b/tests/e2e/specs/approvals.spec.js @@ -168,14 +168,14 @@ test.describe('Approvals surfaces', () => { await expect(page.locator('.openclaw-error-box')).toContainText('approval_list_failed'); }); - test('keeps admin console pending approvals aligned with sidebar data', async ({ page }) => { + test('keeps admin console pending approvals aligned with sidebar data', async ({ page, baseURL }) => { await mockApprovalApis(page); await page.goto('test-harness.html'); await waitForOpenClawReady(page); await clickTab(page, 'Approvals'); await expect(page.locator('#apr-list')).toContainText('apr-001'); - await page.goto('http://127.0.0.1:3000/web/admin_console.html'); + await page.goto(new URL('/web/admin_console.html', baseURL).toString()); await page.locator('#refreshApprovals').click(); await expect(page.locator('#approvalsList')).toContainText('apr-001'); diff --git a/tests/test_r141_inventory_indexing.py b/tests/test_r141_inventory_indexing.py index a52251f..e072cee 100644 --- a/tests/test_r141_inventory_indexing.py +++ b/tests/test_r141_inventory_indexing.py @@ -46,7 +46,9 @@ class TestR141InventoryIndexing(unittest.TestCase): patch.object( services.preflight, "folder_paths", MagicMock(), create=True ) as mock_folder_paths, - patch.object(services.preflight, "_scan_model_inventory", side_effect=slow_scan), + patch.object( + services.preflight, "_scan_model_inventory", side_effect=slow_scan + ), ): mock_folder_paths.folder_names_and_paths = {"checkpoints": [("/tmp", None)]} @@ -67,10 +69,10 @@ class TestR141InventoryIndexing(unittest.TestCase): ) ) - final = services.preflight.get_model_inventory_snapshot(trigger_refresh=False) - self.assertEqual( - final["models"], {"checkpoints": ["sdxl.safetensors"]} + final = services.preflight.get_model_inventory_snapshot( + trigger_refresh=False ) + self.assertEqual(final["models"], {"checkpoints": ["sdxl.safetensors"]}) self.assertEqual(final["scan_state"], "idle") self.assertFalse(final["stale"]) self.assertIsNone(final["last_error"]) @@ -105,7 +107,9 @@ class TestR141InventoryIndexing(unittest.TestCase): ) ) - final = services.preflight.get_model_inventory_snapshot(trigger_refresh=False) + final = services.preflight.get_model_inventory_snapshot( + trigger_refresh=False + ) self.assertEqual(final["models"], {}) self.assertEqual(final["scan_state"], "error") self.assertTrue(final["stale"]) @@ -117,13 +121,15 @@ class TestR141InventoryIndexing(unittest.TestCase): services.preflight, "folder_paths", MagicMock(), create=True ) as mock_folder_paths: mock_folder_paths.folder_names_and_paths = {"checkpoints": [("/tmp", None)]} - services.preflight._CACHE[services.preflight._INVENTORY_SCAN_STATE_KEY] = "error" + services.preflight._CACHE[services.preflight._INVENTORY_SCAN_STATE_KEY] = ( + "error" + ) services.preflight._CACHE[services.preflight._INVENTORY_LAST_ERROR_KEY] = ( "scan boom" ) - services.preflight._CACHE[services.preflight._INVENTORY_LAST_ATTEMPT_TS_KEY] = ( - time.time() - ) + services.preflight._CACHE[ + services.preflight._INVENTORY_LAST_ATTEMPT_TS_KEY + ] = time.time() with patch.object( services.preflight, "_schedule_inventory_refresh_locked" @@ -174,7 +180,9 @@ class TestR141InventoryApi(AioHTTPTestCase): "last_error": None, }, ), - patch("api.preflight_handler.get_model_inventory_snapshot") as mock_snapshot, + patch( + "api.preflight_handler.get_model_inventory_snapshot" + ) as mock_snapshot, patch("api.preflight_handler._get_node_class_mappings") as mock_nodes, ): mock_snapshot.return_value = { @@ -218,7 +226,9 @@ class TestR141InventoryApi(AioHTTPTestCase): patch.object( services.preflight, "folder_paths", MagicMock(), create=True ) as mock_folder_paths, - patch.object(services.preflight, "_scan_model_inventory", side_effect=slow_scan), + patch.object( + services.preflight, "_scan_model_inventory", side_effect=slow_scan + ), patch("api.preflight_handler._get_node_class_mappings", return_value={}), ): mock_folder_paths.folder_names_and_paths = {"checkpoints": [("/tmp", None)]}