chore(ci): enforce pre-push quality gate with detect-secrets, pre-commit, and Node18 Playwright tests

.githooks/pre-push

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+ROOT_DIR="$(git rev-parse --show-toplevel)"
+cd "$ROOT_DIR"
+
+bash scripts/pre_push_checks.sh

scripts/pre_push_checks.sh

@@ -0,0 +1,61 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+cd "$ROOT_DIR"
+
+echo "[pre-push] repo: $ROOT_DIR"
+
+# Force a repo-local pre-commit cache to avoid readonly $HOME cache issues
+# (common on WSL / mixed shell / sandbox setups).
+export PRE_COMMIT_HOME="${PRE_COMMIT_HOME:-$ROOT_DIR/.tmp/pre-commit}"
+mkdir -p "$PRE_COMMIT_HOME"
+
+require_cmd() {
+  local cmd="$1"
+  if ! command -v "$cmd" >/dev/null 2>&1; then
+    echo "[pre-push] ERROR: missing command: $cmd" >&2
+    exit 1
+  fi
+}
+
+require_cmd pre-commit
+require_cmd npm
+
+# Ensure Node 18+ for Playwright/E2E.
+# CI uses Node 20; local baseline is Node 18.
+if [ -n "${NVM_DIR:-}" ] && [ -s "${NVM_DIR}/nvm.sh" ]; then
+  # shellcheck disable=SC1090
+  . "${NVM_DIR}/nvm.sh"
+elif [ -s "${HOME}/.nvm/nvm.sh" ]; then
+  # shellcheck disable=SC1091
+  . "${HOME}/.nvm/nvm.sh"
+fi
+
+if command -v nvm >/dev/null 2>&1; then
+  if [ -f ".nvmrc" ]; then
+    nvm use >/dev/null
+  else
+    nvm use 18 >/dev/null
+  fi
+fi
+
+require_cmd node
+NODE_MAJOR="$(node -p "process.versions.node.split('.')[0]")"
+if [ "$NODE_MAJOR" -lt 18 ]; then
+  echo "[pre-push] ERROR: Node >=18 required, current=$(node -v)" >&2
+  echo "[pre-push] Hint: install nvm and run 'nvm use 18'." >&2
+  exit 1
+fi
+
+echo "[pre-push] Node version: $(node -v)"
+echo "[pre-push] 1/3 detect-secrets"
+pre-commit run detect-secrets --all-files
+
+echo "[pre-push] 2/3 pre-commit all hooks"
+pre-commit run --all-files
+
+echo "[pre-push] 3/3 npm test (Playwright)"
+npm test
+
+echo "[pre-push] PASS"

services/llm_client.py

@@ -144,6 +144,31 @@ class LLMClient:
 
             self.model = DEFAULT_MODEL_BY_PROVIDER.get(self.provider, "default")
 
+        # R23 (plugin wiring) + R57 (precedence compatibility):
+        # CRITICAL: keep model alias resolution in __init__.
+        # Some callers instantiate LLMClient and execute immediately without calling Settings save flow,
+        # and tests assert that "model.resolve" runs during initialization.
+        # Removing this block regresses alias behavior (e.g., gpt4 -> gpt-4) and breaks unit tests.
+        # CI guard: tests/test_llm_client_plugins.py::test_model_alias_resolution_on_init.
+        if PLUGINS_AVAILABLE and self.model:
+            try:
+                from .plugins.async_bridge import run_async_in_sync_context
+
+                resolve_ctx = RequestContext(
+                    provider=self.provider,
+                    model=str(self.model),
+                    trace_id="init",
+                )
+                resolved_model = run_async_in_sync_context(
+                    plugin_manager.execute_first(
+                        "model.resolve", resolve_ctx, str(self.model)
+                    )
+                )
+                if isinstance(resolved_model, str) and resolved_model.strip():
+                    self.model = resolved_model.strip()
+            except Exception as e:
+                logger.warning(f"Model alias resolution failed (non-fatal): {e}")
+
         self.timeout = (
             timeout if timeout is not None else eff_config.get("timeout_sec", 120)
         )

tests/TEST_SOP.md

@@ -11,6 +11,20 @@ This document defines the **mandatory test workflow** for this repo. Run it **be
 
 ## Required Pre-Push Workflow (Must Run)
 
+### Optional automation (recommended)
+
+Enable the repository-managed Git pre-push hook once:
+
+```bash
+git config core.hooksPath .githooks
+```
+
+Then every `git push` will run:
+
+```bash
+bash scripts/pre_push_checks.sh
+```
+
 1) Detect Secrets (baseline-based)
 
 ```bash

tests/repro_r53_apply.py

@@ -1,9 +1,11 @@
-
-import unittest
 import asyncio
-from unittest.mock import patch, MagicMock
+import json
+import unittest
+from unittest.mock import MagicMock, patch
+
 from api.config import config_put_handler
 
+
 class TestR53ApplyFeedbackRepro(unittest.TestCase):
     def setUp(self):
         self.loop = asyncio.new_event_loop()
@@ -17,15 +19,17 @@ class TestR53ApplyFeedbackRepro(unittest.TestCase):
     @patch("api.config.require_admin_token")
     @patch("api.config.check_rate_limit")
     @patch("api.config.require_same_origin_if_no_token")
-    def test_put_returns_apply_metadata(self, mock_csrf, mock_rate, mock_auth, mock_get_config, mock_update):
+    def test_put_returns_apply_metadata(
+        self, mock_csrf, mock_rate, mock_auth, mock_get_config, mock_update
+    ):
         # Setup mocks
         mock_csrf.return_value = None
         mock_rate.return_value = True
         mock_auth.return_value = (True, None)
-        
+
         # update_config succeeds
         mock_update.return_value = (True, [])
-        
+
         # get_effective_config returns the new state
         mock_get_config.return_value = ({"provider": "openai"}, {"provider": "file"})
 
@@ -42,6 +46,6 @@ class TestR53ApplyFeedbackRepro(unittest.TestCase):
         # Assertion: R53 requires an 'apply' field in the PUT /config response.
         self.assertIn("apply", body, "R53 Failure: Response missing 'apply' metadata")
 
-import json
+
 if __name__ == "__main__":
     unittest.main()

tests/repro_r57_precedence.py

@@ -1,31 +1,36 @@
-
 import unittest
-from unittest.mock import patch, MagicMock
+from unittest.mock import MagicMock, patch
+
 from services.llm_client import LLMClient
 
+
 class TestR57PrecedenceRepro(unittest.TestCase):
     @patch("services.runtime_config.get_effective_config")
     def test_provider_model_contamination(self, mock_get_config):
         # Scenario: Config has Provider A and Model A
         mock_get_config.return_value = (
             {"provider": "anthropic", "model": "claude-3-opus", "base_url": ""},
-            {"provider": "file", "model": "file"}
+            {"provider": "file", "model": "file"},
         )
 
         # User requests Provider B explicitly (e.g. via "Test Connection" with overrides)
         # They do NOT specify a model (failed to select one, or just testing provider default)
         client = LLMClient(provider="openai")
 
-        # CURRENT BEHAVIOR (Expected Failure): 
+        # CURRENT BEHAVIOR (Expected Failure):
         # The client picks up "claude-3-opus" from config because it's not None.
         # But "claude-3-opus" is invalid for "openai".
-        
+
         print(f"DEBUG: Client Provider={client.provider}, Model={client.model}")
 
         # Assertion: We want the model to BE CLEAN (None or provider default), not the config's model.
         # If this fails, it means we reproduced the issue.
-        self.assertNotEqual(client.model, "claude-3-opus", 
-            "R57 Failure: Client inherited incompatible model from config for a different provider")
+        self.assertNotEqual(
+            client.model,
+            "claude-3-opus",
+            "R57 Failure: Client inherited incompatible model from config for a different provider",
+        )
+
 
 if __name__ == "__main__":
     unittest.main()