fix(hooks): pytest-guard the two static-path JSONL writers — S304 F6/F43 closed. diagnostics + telegram_response resolved log paths at import, bypassing prax test routing; now per-write resolvers route to tmp under pytest. Devpulse marker-bounded verify: 1154 tests green, engine.jsonl 1291 lines before AND after, zero suite-attributable entries. CHANGELOG wave entry + prep skill feedback-check habit (F46). DPLAN-0250 Track A, dispatched to @hooks
This commit is contained in:
@@ -54,6 +54,7 @@ Quick checks beat assumptions: `ls`/`find` for files, `git ls-files`/`grep` for
|
||||
|
||||
- Run `drone @ai_mail inbox 2>/dev/null` — report any unread emails
|
||||
- Close any that were already processed but not formally closed
|
||||
- Run `drone @devpulse feedback 2>/dev/null` — report unread cross-project feedback; view/reply/clear anything NEW (S304 F46: this box sat unread for 3 months because nothing invoked it)
|
||||
|
||||
## 5. Compass Review (Devpulse only)
|
||||
|
||||
|
||||
@@ -9,6 +9,30 @@ PyPI version — not the changelog header.
|
||||
|
||||
---
|
||||
|
||||
## [2026-07-19]
|
||||
|
||||
**fix(spawn, commons, prax, hooks)** — S304 audit fix campaign, Track A
|
||||
(DPLAN-0250, four owner dispatches verified + committed by devpulse):
|
||||
|
||||
- **spawn** — shared `is_protected()` (infrastructure floor / registry owner /
|
||||
active passport) now guards both pollution repair and branch delete; repair no
|
||||
longer flags the live `src/aipass/aipass` branch, and deleting a protected or
|
||||
actively-passported branch is refused with the reason.
|
||||
- **commons** — branch-name resolution lowercased across all five ops sites
|
||||
(trade/artifact/profile/welcome/search) to match identity normalization;
|
||||
gifting and trading work again (guards had never matched since mid-June).
|
||||
- **prax** — pytest detection now also checks `_pytest` in `sys.modules`, so
|
||||
`patch.dict(os.environ, clear=True)` in test suites can no longer strip the
|
||||
guard and freeze prod-path log handlers into the setup cache.
|
||||
- **hooks** — the two static-path JSONL writers (engine diagnostics, telegram
|
||||
delivery log) resolve their path per-write and route to the tmp test dir
|
||||
under pytest; a full 1154-test suite run now adds zero lines to prod logs
|
||||
(marker-bounded proof).
|
||||
|
||||
Also: `/prep` now checks the cross-project feedback box every run — the S304
|
||||
"unread since April" backlog (F46) is processed to zero and can't silently
|
||||
rot again.
|
||||
|
||||
## [2026-07-18]
|
||||
|
||||
**fix(tests)** — the immortal `MagicMock/LOG_FILE/` directory is dead: a hooks
|
||||
|
||||
@@ -10,19 +10,38 @@
|
||||
|
||||
"""JSONL diagnostic logging — appends structured entries for hook activity."""
|
||||
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from aipass.prax import append_jsonl
|
||||
from aipass.prax.apps.modules.logger import system_logger as logger
|
||||
|
||||
BRANCH_ROOT = Path(__file__).resolve().parent.parent.parent.parent
|
||||
LOG_FILE = BRANCH_ROOT / "logs" / "engine.jsonl"
|
||||
_PROD_LOG_FILE = BRANCH_ROOT / "logs" / "engine.jsonl"
|
||||
|
||||
|
||||
def _is_pytest_session() -> bool:
|
||||
"""Detect pytest via sys.modules (immune to patch.dict(os.environ, clear=True))."""
|
||||
return "_pytest" in sys.modules
|
||||
|
||||
|
||||
def _get_log_file() -> Path:
|
||||
"""Resolve log path — temp dir during pytest, prod path otherwise."""
|
||||
if _is_pytest_session():
|
||||
p = Path(tempfile.gettempdir()) / "aipass_test_logs" / "hooks"
|
||||
p.mkdir(parents=True, exist_ok=True)
|
||||
return p / "engine.jsonl"
|
||||
return _PROD_LOG_FILE
|
||||
|
||||
|
||||
LOG_FILE = _PROD_LOG_FILE
|
||||
|
||||
|
||||
def log_entry(entry: dict) -> None:
|
||||
"""Append a JSONL log entry for detailed diagnostics."""
|
||||
try:
|
||||
append_jsonl(LOG_FILE, entry)
|
||||
append_jsonl(_get_log_file(), entry)
|
||||
except OSError as exc:
|
||||
logger.error("[HOOKS] log write failed: %s", exc)
|
||||
|
||||
|
||||
@@ -23,6 +23,8 @@ import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import time
|
||||
from pathlib import Path
|
||||
from urllib.error import HTTPError, URLError
|
||||
@@ -35,7 +37,19 @@ PENDING_DIR = Path.home() / ".aipass" / "telegram_pending"
|
||||
MIRROR_DIR = Path.home() / ".aipass" / "telegram_bots"
|
||||
PENDING_TTL = 3600
|
||||
TELEGRAM_CHAR_LIMIT = 4096
|
||||
_DELIVERY_LOG = Path(__file__).resolve().parent.parent.parent.parent / "logs" / "telegram_delivery.jsonl"
|
||||
_PROD_DELIVERY_LOG = Path(__file__).resolve().parent.parent.parent.parent / "logs" / "telegram_delivery.jsonl"
|
||||
|
||||
|
||||
def _get_delivery_log() -> Path:
|
||||
"""Resolve delivery log path — temp dir during pytest, prod path otherwise."""
|
||||
if "_pytest" in sys.modules:
|
||||
p = Path(tempfile.gettempdir()) / "aipass_test_logs" / "hooks"
|
||||
p.mkdir(parents=True, exist_ok=True)
|
||||
return p / "telegram_delivery.jsonl"
|
||||
return _PROD_DELIVERY_LOG
|
||||
|
||||
|
||||
_DELIVERY_LOG = _PROD_DELIVERY_LOG
|
||||
|
||||
|
||||
def _is_expired(data: dict) -> bool:
|
||||
@@ -771,6 +785,6 @@ def _write_delivery_log(intended_text: str, chunks: list[str], chunk_results: li
|
||||
record["culprit"] = culprit
|
||||
|
||||
try:
|
||||
append_jsonl(_DELIVERY_LOG, record)
|
||||
append_jsonl(_get_delivery_log(), record)
|
||||
except OSError as e:
|
||||
logger.warning("[HOOKS] telegram: delivery log write failed: %s", e)
|
||||
|
||||
@@ -320,7 +320,7 @@ class TestLog:
|
||||
|
||||
def test_writes_jsonl_entry(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "test.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "Test", "action": "test_write"})
|
||||
lines = log_file.read_text().strip().split("\n")
|
||||
assert len(lines) == 1
|
||||
@@ -330,13 +330,13 @@ class TestLog:
|
||||
|
||||
def test_creates_parent_directory(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "subdir" / "test.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "Test"})
|
||||
assert log_file.exists()
|
||||
|
||||
def test_appends_multiple_entries(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "test.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "A"})
|
||||
_log({"event": "B"})
|
||||
lines = log_file.read_text().strip().split("\n")
|
||||
@@ -440,7 +440,7 @@ class TestErrorResilience:
|
||||
# via append_jsonl's Path(...).parent.mkdir — use a real tmp path.
|
||||
log_file = tmp_path / "engine.jsonl"
|
||||
with patch("builtins.open", side_effect=OSError("disk full")):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "Test"})
|
||||
|
||||
|
||||
@@ -458,7 +458,7 @@ class TestDataStructureContracts:
|
||||
|
||||
def test_log_entry_has_required_fields(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "test.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"ts": 123.0, "event": "Test", "action": "check"})
|
||||
entry = json.loads(log_file.read_text().strip())
|
||||
assert "ts" in entry
|
||||
@@ -507,7 +507,7 @@ class TestInitProvisioning:
|
||||
|
||||
def test_log_auto_creates_directory(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "new_dir" / "engine.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "init_test"})
|
||||
assert log_file.parent.exists()
|
||||
|
||||
@@ -531,7 +531,7 @@ class TestInitProvisioning:
|
||||
def test_log_no_overwrite_on_append(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "test.jsonl"
|
||||
log_file.write_text('{"existing": true}\n')
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "new"})
|
||||
lines = log_file.read_text().strip().split("\n")
|
||||
assert len(lines) == 2
|
||||
@@ -666,7 +666,7 @@ class TestConfigDataContracts:
|
||||
|
||||
def test_data_keys_in_log_entry(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "test.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"ts": 1.0, "event": "Test", "hook": "test_hook", "exit_code": 0})
|
||||
entry = json.loads(log_file.read_text().strip())
|
||||
assert "ts" in entry
|
||||
@@ -710,7 +710,7 @@ class TestErrorResilienceExtended:
|
||||
|
||||
def test_missing_file_in_log_path(self, temp_test_dir, mock_logger):
|
||||
missing = temp_test_dir / "missing" / "nonexistent.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", missing):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=missing):
|
||||
_log({"event": "test_missing"})
|
||||
assert missing.exists()
|
||||
|
||||
@@ -895,13 +895,13 @@ class TestJsonHandlerNotApplicable:
|
||||
def test_log_default_factory(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "factory.jsonl"
|
||||
assert not log_file.exists()
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "factory_test"})
|
||||
assert log_file.exists()
|
||||
|
||||
def test_log_validate_json_output(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "validate.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "A", "ts": 1.0})
|
||||
_log({"event": "B", "ts": 2.0})
|
||||
for line in log_file.read_text().strip().split("\n"):
|
||||
@@ -916,13 +916,13 @@ class TestJsonHandlerNotApplicable:
|
||||
|
||||
def test_log_ensure_exists(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "new_dir" / "ensure.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"ensure": True})
|
||||
assert log_file.parent.exists()
|
||||
|
||||
def test_log_save_entry(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "save.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"saved": True, "value": 42})
|
||||
entry = json.loads(log_file.read_text().strip())
|
||||
assert entry["saved"] is True
|
||||
@@ -937,7 +937,7 @@ class TestJsonHandlerNotApplicable:
|
||||
|
||||
def test_log_operation_recorded(self, temp_test_dir, mock_logger):
|
||||
log_file = temp_test_dir / "ops.jsonl"
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics.LOG_FILE", log_file):
|
||||
with patch("aipass.hooks.apps.handlers.config.diagnostics._get_log_file", return_value=log_file):
|
||||
_log({"event": "PreToolUse", "hook": "test", "exit_code": 0})
|
||||
entry = json.loads(log_file.read_text().strip())
|
||||
assert entry["event"] == "PreToolUse"
|
||||
|
||||
@@ -1532,7 +1532,7 @@ class TestWriteDeliveryLog:
|
||||
|
||||
log_path = tmp_path / "delivery.jsonl"
|
||||
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._DELIVERY_LOG", log_path):
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._get_delivery_log", return_value=log_path):
|
||||
_write_delivery_log(
|
||||
"hello",
|
||||
["hello"],
|
||||
@@ -1552,7 +1552,7 @@ class TestWriteDeliveryLog:
|
||||
|
||||
log_path = tmp_path / "delivery.jsonl"
|
||||
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._DELIVERY_LOG", log_path):
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._get_delivery_log", return_value=log_path):
|
||||
_write_delivery_log(
|
||||
"**bold text**",
|
||||
["**bold text**"],
|
||||
@@ -1569,7 +1569,7 @@ class TestWriteDeliveryLog:
|
||||
|
||||
log_path = tmp_path / "delivery.jsonl"
|
||||
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._DELIVERY_LOG", log_path):
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._get_delivery_log", return_value=log_path):
|
||||
_write_delivery_log(
|
||||
"hello",
|
||||
["hello"],
|
||||
@@ -1585,7 +1585,7 @@ class TestWriteDeliveryLog:
|
||||
from aipass.hooks.apps.handlers.notification.telegram_response import _write_delivery_log
|
||||
|
||||
impossible = Path("/dev/null/impossible/log.jsonl")
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._DELIVERY_LOG", impossible):
|
||||
with patch(LOGGER_PATCH), patch(f"{MOD}._get_delivery_log", return_value=impossible):
|
||||
_write_delivery_log("hi", ["hi"], [{"idx": 0, "ok": True, "text": "hi"}], "s")
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user