diff --git a/.aipass/hooks.json b/.aipass/hooks.json index 96835ae7..a88cf9e9 100644 --- a/.aipass/hooks.json +++ b/.aipass/hooks.json @@ -38,6 +38,12 @@ "handler": "aipass.hooks.apps.handlers.prompt.navmap.handle", "matcher": "" }, + "compass_recall": { + "enabled": true, + "handler": "aipass.hooks.apps.handlers.prompt.compass_recall.handle", + "matcher": "", + "max_per_session": 10 + }, "auto_process": { "enabled": true, "handler": "aipass.hooks.apps.handlers.lifecycle.auto_process.handle", @@ -76,11 +82,6 @@ "enabled": true, "handler": "aipass.hooks.apps.handlers.security.registry_gate.handle", "matcher": "Bash|Edit|MultiEdit|Write|NotebookEdit" - }, - "engine_test_sound": { - "enabled": false, - "command": "python3 $AIPASS_HOME/.claude/hooks/engine_test_sound.py", - "matcher": "WebSearch" } }, diff --git a/.aipass/tier1_navmap.md b/.aipass/tier1_navmap.md index 1dd6b4c6..88d240e0 100644 --- a/.aipass/tier1_navmap.md +++ b/.aipass/tier1_navmap.md @@ -2,7 +2,7 @@ -AIPass is the system: autonomous agents (citizens) with identity, memory, and a mailbox, providing services to each other and to external projects. Each agent lives in a branch — its home and address. Everything routes through `drone`. +AIPass is the system: autonomous agents (citizens) with identity, memory, and a mailbox, providing services to each other and to external projects. Each agent lives in a branch — its home and address. Everything routes through `drone`. **AIPass is open source** — public repo on GitHub. Strangers read, clone, and scan this code; treat external findings as contributions. # Finding your way @@ -110,6 +110,6 @@ Your continuity across sessions. Save proactively — after milestones, decision # House rules - - Cross-platform, no hardcoded paths. Public repo — `pathlib`, never `/home/...`. + - Public repo — write as if it ships, because it does. No secrets in the tree, no hardcoded paths (`pathlib`, never `/home/...`), cross-platform. - No bare imports — always `from aipass..apps...`. - State lives in `.trinity/` and dashboards, never in prompts. Prompts are signposts; memories record; registries catalog. diff --git a/.claude/commands/prep.md b/.claude/commands/prep.md index 4f576fcb..26ed82fe 100644 --- a/.claude/commands/prep.md +++ b/.claude/commands/prep.md @@ -55,7 +55,12 @@ 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 -## 5. Loose Ends +## 5. Compass Review (Devpulse only) + +- Run ONE `drone @devpulse compass review` — it serves the oldest-unreviewed entry. Judge it: still true → confirm; superseded → archive it and note what replaced it; wrong → fix or archive. +- One entry per prep, every prep. This is the curation cadence — review only works if it actually runs (DPLAN-0246: all 127 entries sat unreviewed because nothing invoked it). + +## 6. Loose Ends - Flag anything in-flight: running background agents, dispatched branches waiting for replies, pending decisions - If anything can't survive compaction (e.g., agent IDs needed for resume), write it to local.json todos[] @@ -71,5 +76,6 @@ Prep complete: - Plans: [which ones updated] - Git: [branch, uncommitted count, suggestion] - Inbox: [count, action taken] +- Compass: [entry #N reviewed — verdict] - Loose ends: [any flagged] ``` diff --git a/CHANGELOG.md b/CHANGELOG.md index a512edc3..48eaffb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,162 @@ PyPI version — not the changelog header. --- +## [2026-07-16] + +### Added + +- **Compass ambient recall — Track 2 (DPLAN-0246/FPLAN-0332): rated decisions + surface unprompted.** On every user prompt, a new hooks handler + (`compass_recall`, registered in `.aipass/hooks.json` only) queries compass + FTS with the prompt text and injects matching rulings VERBATIM — + `[BAD] #56: ` — tidbits, never vibes. Three branches, one + pipeline, each piece behind a modules/-boundary API: devpulse's + `recall_decisions()` (side-effect-free scored candidates; rare-token + evidence scoring + a query-side stopword filter so greeting/filler words + can't fake relevance) + `mark_surfaced()` (counts only real injections); + @memory's pure `should_surface()` governance (promoted from the dormant + symbolic engine: threshold, 5/session cap, 10-message spacing — first + surface exempt, 300s cooldown, dedup; state-in/state-out, caller persists); + @hooks' 90-line handler + engine per-handler budget (errors never block a + prompt — `compass_recall_unreachable` log signature for @trigger's watcher). + Live acceptance matrix through the real bridge: topic-with-history prompts + recall the right ruling (a CI prompt surfaced the red-CI-never-parked + ruling), small talk and greetings stay silent, repeat prompts gate on + spacing. Review caught and fixed pre-ship: wrong payload key (`userInput` → + `prompt`), phantom `CLAUDE_CODE_SESSION_ID` env (session id is + stdin-payload-only), spacing gate blocking the first surface, and a trust + registry re-enrollment gap that silently disabled ALL project hooks for 20 + minutes after the hooks.json edit. 446 devpulse + 1011 memory + 1129 hooks + tests green; seedgo 31/31 on every touched module. + +- **Compass curation v2 Track 1 (DPLAN-0246/FPLAN-0331): supersedes links + + write-time conflict check.** A correcting compass entry now archives and + links what it replaces in one transaction (`compass add --supersedes N`); + query renders both directions ("supersedes #N" / "ARCHIVED — superseded by + #M") so a retracted decision can never masquerade as current truth. Every + `compass add` FTS-checks the new text against active entries and prints a + non-blocking "possible conflict with #X" advisory — flag-and-ask, no LLM, no + auto-resolve (boardroom ruling). New `compass note ` command (FTS + re-index proven by test), `--include-archived` query flag (the avoid-list is + finally searchable), dead `score` column removed from all code surfaces + (kept inert on disk — zero migration risk). Idempotent PRAGMA-checked + migration ran clean on the production store (128 rows, no loss); the four + fresh-eyes-audit archive pairs got their links backfilled. /prep now runs + one `compass review` per session — curation living in a path that already + runs, the lesson of all three compass eras. 435 devpulse tests green, + seedgo 31/31 on both touched modules. + +- **Close pipeline completes itself (DPLAN-0245): auto-vectorization + + crash-safe registry writes + drone timeout policy.** Closing a plan now + produces all side effects from one command — `post_close_runner` invokes + @memory's plan intake directly after archival (detached, loud on failure, + drains any backlog it finds), so plans can no longer silently pile up + unvectorized. Plan registry saves (@flow `save_registry` + mbank + `save_flow_registry`) now use the O_EXCL lockfile + atomic + tempfile-and-replace pattern, closing the same lost-update race class fixed + earlier in CLOSED_PLANS. @drone gained a 3-layer timeout policy: per-command + overrides (`@memory process-plans` 120s, `@flow close` 90s), a `--timeout N` + flag, 30s default — replacing the flat 30s guillotine that killed legitimate + long commands mid-pipeline; the timeout error now says how to override. + Proven end-to-end live: one `drone @flow close` on a throwaway plan yielded + archive + vectors + ledger + registry with zero manual steps, and the + auto-trigger swept a pre-existing backlog file on its first run. 730 flow + + 878 drone tests green, seedgo 100%. + +### Fixed + +- **CI seedgo gate back to 100% across all 17 branches.** The Track 2 compass + recall code left three branches at 99%: @hooks' compass_recall handler was + missing json_handler operation logging and had two silent catches (now + logged); @memory's governance module held its implementation in modules/ + (moved to handlers/governance/engine.py with modules/governance.py as the + thin re-export — the cross-branch import path is unchanged and live-E2E + verified through the real bridge); devpulse's README test count had drifted + (309 → 348). Audits re-run per branch: 100% overall, all suites green. + +- **Plan-number memory search hits the exact plan.** Searching a plan ID + ('DPLAN-0244', 'fplan 0332' — any case, dash or space) now pins the exact + plan as the top result at 100%, via a metadata lookup on the vector store's + source-file field instead of embedding similarity (which treats all plan IDs + as near-identical strings and never surfaced the target). Patrick ruling: + searching a plan number must return that plan first. Semantic search quality + for normal queries is unchanged. Also purged 193 junk vectors — throwaway + probe/flaky test plans from scratchpad sessions (dv4 batch, probe_test_plan, + throwaway_e2e_proof) that had leaked into the store. 1011 memory tests green. + +- **drone --timeout collision: router flag swallowed module flags.** The + DPLAN-0245 subprocess-timeout flag consumed the first `--timeout` token + anywhere in argv, so module-level flags silently vanished — watchdog's + `--timeout 1800` never arrived and long watches died at the 600s default + (live repro x2). Drone's flag is now namespaced `--drone-timeout`; plain + `--timeout` passes through untouched to the target module, with a regression + test pinning the passthrough. Per-command overrides intact. 879 drone tests + green, seedgo 100%. + +- **@memory command routing eaten by the new governance module.** The + governance module shipped in Track 2 had the wrong `handle_command` + signature (`args: list` instead of `command: str, args: list`) and always + returned True, so auto-discovery routed EVERY @memory command through it + first — `drone @memory search` answered "governance: unknown command 's'". + Fixed to the standard signature returning False for commands not its own; + search verified live (135 results). Library modules must decline commands + they don't own or they silently hijack the whole CLI. 1011 memory tests + green, seedgo 31/31. + ## [2026-07-15] +### Fixed + +- **Plan vectorization pipeline unwedged (DPLAN-0245): 57 closed plans were + silently missing from semantic memory since mid-June.** Vector IDs were pure + content hashes, so identical template boilerplate across different plans + produced duplicate IDs within one ChromaDB upsert — the store rejected the + entire batch, and the all-or-nothing intake retried the same failing batch + forever. Fixed in @memory: IDs are now salted with the source filename when + present (rollover hashes unchanged — no re-vectorization churn), in-batch + dedup as a safety net, and `process_plans()` now runs per-file with the + manifest saved after each success so a poison file can never wedge the queue + again. Backlog drained and verified: 229/229 archived plans vectorized, 1112 + chunks, formerly-lost plans answering semantic queries at 85%+ similarity. + 990 memory tests green. + +- **CLOSED_PLANS ledger append race (@flow): concurrent plan closes lost + entries.** `append_to_closed_plans` was an unlocked read-modify-write; the + S314 bulk sweep lost 18 of 21 entries to it (reconciled by hand). Now guarded + by an `O_CREAT|O_EXCL` lockfile with retry/backoff, and the previously + silent append failure is surfaced in close output and logs. 730 flow tests + green. + +- **Telegram routine read-timeouts no longer logged as errors (@skills, + Patrick ruling): ends the medic wake-loop.** A routine long-poll read + timeout (`socket.timeout` — an `OSError` subclass) slipped past the earlier + `URLError`-only guard into the network-outage path, logging ERROR once per + episode (~576 lines/30h) and waking @trigger's medic each time. The + `_is_routine_read_timeout` guard now covers the `OSError` handler too, and + the genuine-outage episode-start line is demoted ERROR→WARNING (backoff + self-heals; recovery already logs INFO; medic only fires on ERROR/CRITICAL). + Real failures still log ERROR. 825 telegram tests green. + +### Security + +- **Hook config trust model hardening (DPLAN-0244): closes a zero-interaction + RCE from untrusted `.aipass/hooks.json`.** The hook loader walked up from CWD + and trusted any `.aipass/hooks.json` it found; since the bridge is wired + globally in provider settings, a hostile repo shipping a `command`-type hook + could execute arbitrary shell on `SessionStart` with no user interaction. + Fixed with defense-in-depth. **Layer A (engine):** per-project configs may no + longer run `command`-type hooks (refused via an unconditionally-stamped + `_source` provenance flag), and handler paths are gated to the `aipass.*` + namespace. **Layer B (loader + CLI):** a trusted-project registry + (`~/.aipass/trusted_projects.json`, path + sha256) that the loader checks + fail-closed; on upgrade it bootstraps **only** the `$AIPASS_HOME` install + (never trust-on-first-use of an arbitrary directory); `aipass init`/`init + update` auto-enroll, and new `aipass trust`/`revoke` commands manage + enrollment. Both gates proven to block the attack independently via a live + acceptance test driving the real bridge with a real payload. 1105 hooks + + 133 aipass tests green. Origin: external scan (false positive at + `engine.py:37`) whose triage surfaced the real adjacent hole. + ### Added - **Supply-chain hardening pass (DPLAN-0243): commit signing + hash-pinned CI diff --git a/CLAUDE.md b/CLAUDE.md index e77b3513..5abc6868 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,6 +13,8 @@ These steps are sequential and dependent — run each ONCE, wait for the result, - Read: `.trinity/passport.json`, `.trinity/local.json`, `.trinity/observations.json`, `README.md` - Refresh: `drone @prax dashboard refresh @` — where `` is your branch name (CWD directory name) - Dashboard: Read `DASHBOARD.local.json` — act on what needs attention (new mail → check inbox, active plans → note them). This is your single status glance. + - announce ur current (PID) + Use drone commands for all operations. Never raw git, gh, file access, or python -m when drone provides it. diff --git a/src/aipass/aipass/.seedgo/bypass.json b/src/aipass/aipass/.seedgo/bypass.json index 26d8d28f..99cb67c7 100644 --- a/src/aipass/aipass/.seedgo/bypass.json +++ b/src/aipass/aipass/.seedgo/bypass.json @@ -375,6 +375,41 @@ "file": "shared/json_ops.py", "standard": "unused_function", "reason": "backup_json() is consumed by @spawn (cross-branch caller). Appears unused in @aipass-only scan but is a shared API." + }, + { + "file": "apps/modules/trust.py", + "standard": "encapsulation", + "reason": "Imports frozen trust_registry interface (enroll/revoke/is_trusted/read_registry) from @hooks by DPLAN-0244 design. Cross-branch import required — the registry module lives in hooks, consumers live in aipass." + }, + { + "file": "apps/modules/trust.py", + "standard": "json_structure", + "reason": "No JSON file operations — trust.py is a thin CLI wrapper that delegates all JSON I/O to the trust_registry module in @hooks. No json_handler needed." + }, + { + "file": "apps/modules/trust.py", + "standard": "introspection", + "reason": "aipass is binary-invoked: bare 'aipass trust' shows registry table; introspection via --info" + }, + { + "file": "apps/handlers/init/bootstrap.py", + "standard": "encapsulation", + "reason": "Imports enroll() from @hooks trust_registry (DPLAN-0244 frozen interface). Cross-branch import required — init must enroll projects in the trust registry after writing hooks.json." + }, + { + "file": "apps/handlers/init/bootstrap.py", + "standard": "handlers", + "reason": "Imports enroll() from @hooks trust_registry by DPLAN-0244 design. Cross-handler import required — bootstrap auto-enrolls projects after hooks.json creation/merge." + }, + { + "file": "tests/test_trust.py", + "standard": "architecture", + "reason": "Test file lives in tests/ by convention — not in apps/. Standard 3-layer structure applies to production code only." + }, + { + "file": "tests/test_trust.py", + "standard": "encapsulation", + "reason": "Tests import trust_registry directly to verify enrollment/revocation in isolation with monkeypatched REGISTRY_PATH." } ] } diff --git a/src/aipass/aipass/README.md b/src/aipass/aipass/README.md index f4b41e15..e54374e8 100644 --- a/src/aipass/aipass/README.md +++ b/src/aipass/aipass/README.md @@ -22,7 +22,8 @@ aipass/ │ │ ├── help_chat.py # README-backed Q&A (reads via readme_map handler) │ │ ├── init_flow.py # 10-stage guided setup │ │ ├── install.py # aipass install — one-command bootstrap (clone + setup + init) -│ │ └── profile.py # User profile read/write +│ │ ├── profile.py # User profile read/write +│ │ └── trust.py # Trust registry — aipass trust / aipass revoke │ ├── handlers/ │ │ ├── cross_os/ # Cross-OS pre-flight: gap_registry, preflight, run_record │ │ ├── handoff_platform/ # Platform-specific handoff detection @@ -56,6 +57,8 @@ aipass/ | `aipass init` | 10-stage guided setup (resumable) | | `aipass install` | One-command bootstrap — clone + setup.sh + hooks, then hand off to init (`--no-init`/`--with-init`/`--path`/`--here`) | | `aipass profile` | Show/edit user profile | +| `aipass trust [path]` | Show enrolled projects or enroll a project in the trust registry | +| `aipass revoke ` | Remove a project from the trust registry | | `aipass --version` | Version | ## Integration Points diff --git a/src/aipass/aipass/apps/handlers/init/bootstrap.py b/src/aipass/aipass/apps/handlers/init/bootstrap.py index ccdf7f09..2632afad 100644 --- a/src/aipass/aipass/apps/handlers/init/bootstrap.py +++ b/src/aipass/aipass/apps/handlers/init/bootstrap.py @@ -246,6 +246,22 @@ def _claude_settings(aipass_home: str | None = None) -> str: return json.dumps(data, indent=2, ensure_ascii=False) + "\n" +def _enroll_project(target: Path) -> None: + """Enroll a project in the trusted-project registry (DPLAN-0244). + + Lazy import to keep bootstrap.py free of prax/module-level deps. + """ + try: + from aipass.hooks.apps.handlers.config.trust_registry import enroll + + if enroll(str(target)): + logger.info("Enrolled project in trust registry: %s", target) + else: + logger.warning("Trust enrollment failed for %s", target) + except ImportError as exc: + logger.info("Trust registry unavailable, skipping enrollment: %s", exc) + + def _guard_init(target: Path) -> None: """Block init if target is inside an agent branch or existing project. @@ -362,6 +378,7 @@ def init_project(target: Path, project_name: str | None = None) -> dict: if template.is_file(): shutil.copy2(str(template), str(hooks_json_path)) created.append(str(hooks_json_path)) + _enroll_project(target) else: logger.info("hooks template not found at %s — skipping", template) @@ -573,6 +590,7 @@ def update_project(target: Path) -> dict: if existing_hooks != merged_hooks: hooks_json_path.write_text(merged_hooks_content, encoding="utf-8") updated.append(str(hooks_json_path)) + _enroll_project(target) else: already_current.append(str(hooks_json_path)) else: @@ -581,6 +599,7 @@ def update_project(target: Path) -> dict: encoding="utf-8", ) updated.append(str(hooks_json_path)) + _enroll_project(target) elif hooks_json_path.exists(): already_current.append(str(hooks_json_path)) diff --git a/src/aipass/aipass/apps/modules/trust.py b/src/aipass/aipass/apps/modules/trust.py new file mode 100644 index 00000000..cf4ccdd8 --- /dev/null +++ b/src/aipass/aipass/apps/modules/trust.py @@ -0,0 +1,122 @@ +# =================== AIPass ==================== +# Name: trust.py +# Description: Trust management — aipass trust / aipass revoke commands +# Version: 1.0.0 +# Created: 2026-07-15 +# Modified: 2026-07-15 +# ============================================= + +""" +aipass trust / revoke — manage the trusted-project registry (DPLAN-0244) + +Enrollment controls which projects have their .aipass/hooks.json loaded +by the hook engine. Projects created via `aipass init` auto-enroll; +these commands handle manual enrollment and revocation. +""" + +from __future__ import annotations + +from pathlib import Path + +from aipass.cli.apps.modules import console, error, success +from aipass.hooks.apps.handlers.config.trust_registry import ( + enroll, + read_registry, + revoke, +) +from aipass.prax import logger + +COMMAND = "trust" +_COMMAND_REVOKE = "revoke" + + +def print_introspection() -> None: + """Display the current trusted-project registry.""" + from rich.table import Table + + registry = read_registry() + projects = registry.get("projects", {}) + + console.print() + console.print("[bold cyan]aipass trust[/bold cyan] — trusted-project registry") + console.print() + + if not projects: + console.print("[dim]No projects enrolled.[/dim]") + else: + table = Table(show_header=True, header_style="bold yellow") + table.add_column("Project", style="cyan") + table.add_column("Hash", style="dim", max_width=24) + table.add_column("Enrolled") + for path, entry in projects.items(): + short_hash = entry.get("config_hash", "")[:18] + "..." + table.add_row(path, short_hash, entry.get("enrolled", "")) + console.print(table) + + console.print() + console.print("[dim]Use 'aipass trust ' to enroll or 'aipass revoke ' to remove.[/dim]") + console.print() + + +def print_help() -> None: + """Print usage help for the trust/revoke commands.""" + console.print() + console.print("[bold cyan]aipass trust / revoke[/bold cyan] — trusted-project registry") + console.print() + console.print("[yellow]USAGE:[/yellow]") + console.print(" [green]aipass trust[/green] [dim]# Show enrolled projects[/dim]") + console.print( + " [green]aipass trust [/green] [dim]# Enroll a project (requires .aipass/hooks.json)[/dim]" + ) + console.print(" [green]aipass revoke [/green] [dim]# Remove a project from the registry[/dim]") + console.print() + + +def _do_trust(args: list[str]) -> bool: + """Execute the trust enrollment for a given path.""" + target = Path(args[0]).resolve() + if not target.is_dir(): + error(f"Not a directory: {target}") + return True + hooks_path = target / ".aipass" / "hooks.json" + if not hooks_path.is_file(): + error(f"No .aipass/hooks.json found in {target}") + return True + if enroll(str(target)): + success(f"Enrolled {target}") + logger.info("[AIPASS] trust: enrolled %s", target) + else: + error(f"Failed to enroll {target}") + return True + + +def _do_revoke(args: list[str]) -> bool: + """Execute the revocation for a given path.""" + target = Path(args[0]).resolve() + if revoke(str(target)): + success(f"Revoked {target}") + logger.info("[AIPASS] revoke: removed %s", target) + else: + console.print(f"[dim]{target} was not in the registry.[/dim]") + return True + + +def handle_command(command: str, args: list[str]) -> bool: + """Route trust/revoke subcommands. Returns True if handled.""" + if command == COMMAND: + if not args: + print_introspection() + return True + if args[0] in ("--help", "-h", "help"): + print_help() + return True + if args[0] == "--info": + print_introspection() + return True + return _do_trust(args) + if command == _COMMAND_REVOKE: + if not args or args[0] in ("--help", "-h", "help"): + print_help() + return True + return _do_revoke(args) + return False diff --git a/src/aipass/aipass/tests/test_trust.py b/src/aipass/aipass/tests/test_trust.py new file mode 100644 index 00000000..072d3112 --- /dev/null +++ b/src/aipass/aipass/tests/test_trust.py @@ -0,0 +1,249 @@ +# =================== AIPass ==================== +# Name: test_trust.py +# Description: Tests for trust CLI commands and init enrollment (DPLAN-0244) +# Version: 1.0.0 +# Created: 2026-07-15 +# Modified: 2026-07-15 +# ============================================= + +"""Tests for trust/revoke CLI commands and init auto-enrollment. + +All tests use tmp dirs + monkeypatch REGISTRY_PATH so they never +touch the real ~/.aipass registry. +""" + +import pytest # pyright: ignore[reportMissingImports] + +from aipass.hooks.apps.handlers.config.trust_registry import ( + enroll, + is_trusted, + read_registry, + revoke, +) + + +@pytest.fixture(autouse=True) +def _isolate_registry(tmp_path, monkeypatch): + """Redirect REGISTRY_PATH to a tmp dir so tests never touch ~/.aipass.""" + fake_registry = tmp_path / "trusted_projects.json" + monkeypatch.setattr( + "aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", + fake_registry, + ) + + +# --------------------------------------------------------------------------- +# trust_registry direct tests +# --------------------------------------------------------------------------- + + +def test_enroll_project(tmp_path): + """enroll() registers a project with .aipass/hooks.json.""" + project = tmp_path / "myproject" + project.mkdir() + hooks_dir = project / ".aipass" + hooks_dir.mkdir() + hooks_file = hooks_dir / "hooks.json" + hooks_file.write_text('{"hooks_enabled": true}', encoding="utf-8") + + assert enroll(str(project)) is True + assert is_trusted(str(project)) is True + + +def test_enroll_no_hooks_json(tmp_path): + """enroll() returns False when .aipass/hooks.json is missing.""" + project = tmp_path / "empty" + project.mkdir() + assert enroll(str(project)) is False + + +def test_revoke_project(tmp_path): + """revoke() removes a previously enrolled project.""" + project = tmp_path / "myproject" + project.mkdir() + hooks_dir = project / ".aipass" + hooks_dir.mkdir() + hooks_file = hooks_dir / "hooks.json" + hooks_file.write_text('{"hooks_enabled": true}', encoding="utf-8") + + enroll(str(project)) + assert is_trusted(str(project)) is True + + assert revoke(str(project)) is True + assert is_trusted(str(project)) is False + + +def test_revoke_not_enrolled(tmp_path): + """revoke() returns False cleanly for a non-enrolled project.""" + project = tmp_path / "never_enrolled" + project.mkdir() + assert revoke(str(project)) is False + + +def test_is_trusted_hash_mismatch(tmp_path): + """is_trusted() returns False when hooks.json content changed after enrollment.""" + project = tmp_path / "myproject" + project.mkdir() + hooks_dir = project / ".aipass" + hooks_dir.mkdir() + hooks_file = hooks_dir / "hooks.json" + hooks_file.write_text('{"hooks_enabled": true}', encoding="utf-8") + + enroll(str(project)) + assert is_trusted(str(project)) is True + + hooks_file.write_text('{"hooks_enabled": false, "modified": true}', encoding="utf-8") + assert is_trusted(str(project)) is False + + +# --------------------------------------------------------------------------- +# trust CLI module tests +# --------------------------------------------------------------------------- + + +def test_trust_command_enrolls(tmp_path): + """aipass trust enrolls the project.""" + from aipass.aipass.apps.modules.trust import handle_command + + project = tmp_path / "proj" + project.mkdir() + hooks_dir = project / ".aipass" + hooks_dir.mkdir() + (hooks_dir / "hooks.json").write_text("{}", encoding="utf-8") + + assert handle_command("trust", [str(project)]) is True + assert is_trusted(str(project)) is True + + +def test_revoke_command_removes(tmp_path): + """aipass revoke removes enrollment.""" + from aipass.aipass.apps.modules.trust import handle_command + + project = tmp_path / "proj" + project.mkdir() + hooks_dir = project / ".aipass" + hooks_dir.mkdir() + (hooks_dir / "hooks.json").write_text("{}", encoding="utf-8") + + enroll(str(project)) + assert handle_command("revoke", [str(project)]) is True + assert is_trusted(str(project)) is False + + +def test_trust_command_no_hooks_json(tmp_path): + """aipass trust prints error when hooks.json is missing.""" + from aipass.aipass.apps.modules.trust import handle_command + + project = tmp_path / "bare" + project.mkdir() + assert handle_command("trust", [str(project)]) is True + assert is_trusted(str(project)) is False + + +def test_revoke_command_not_enrolled(tmp_path): + """aipass revoke handles non-enrolled project cleanly.""" + from aipass.aipass.apps.modules.trust import handle_command + + project = tmp_path / "ghost" + project.mkdir() + assert handle_command("revoke", [str(project)]) is True + + +def test_trust_command_help(): + """aipass trust --help returns True (handled).""" + from aipass.aipass.apps.modules.trust import handle_command + + assert handle_command("trust", ["--help"]) is True + assert handle_command("trust", []) is True + + +def test_trust_ignores_unrelated_command(): + """handle_command returns False for unrelated commands.""" + from aipass.aipass.apps.modules.trust import handle_command + + assert handle_command("doctor", []) is False + + +def test_trust_not_a_directory(tmp_path): + """aipass trust prints error.""" + from aipass.aipass.apps.modules.trust import handle_command + + fake = tmp_path / "not_a_dir.txt" + fake.write_text("hi", encoding="utf-8") + assert handle_command("trust", [str(fake)]) is True + assert is_trusted(str(fake)) is False + + +# --------------------------------------------------------------------------- +# init enrollment tests +# --------------------------------------------------------------------------- + + +def test_init_project_enrolls(tmp_path, monkeypatch): + """init_project auto-enrolls after copying hooks.json.""" + from aipass.aipass.apps.handlers.init.bootstrap import init_project + + monkeypatch.setattr( + "aipass.aipass.apps.handlers.init.bootstrap.is_throwaway_path", + lambda p: False, + ) + + aipass_home = tmp_path / "aipass_home" + aipass_home.mkdir() + aipass_dir = aipass_home / ".aipass" + aipass_dir.mkdir() + template = aipass_dir / "project_hooks.json" + template.write_text('{"hooks_enabled": true}', encoding="utf-8") + (aipass_home / "CLAUDE.md").write_text("# Test", encoding="utf-8") + monkeypatch.setattr( + "aipass.aipass.apps.handlers.init.bootstrap._detect_aipass_home", + lambda: str(aipass_home), + ) + + target = tmp_path / "newproject" + target.mkdir() + init_project(target, project_name="test") + + assert is_trusted(str(target.resolve())) is True + + +def test_init_update_rehashes(tmp_path, monkeypatch): + """init update re-enrolls after merging hooks.json (hash tracks new content).""" + from aipass.aipass.apps.handlers.init.bootstrap import init_project, update_project + + monkeypatch.setattr( + "aipass.aipass.apps.handlers.init.bootstrap.is_throwaway_path", + lambda p: False, + ) + + aipass_home = tmp_path / "aipass_home" + aipass_home.mkdir() + aipass_dir = aipass_home / ".aipass" + aipass_dir.mkdir() + template = aipass_dir / "project_hooks.json" + template.write_text('{"hooks_enabled": true}', encoding="utf-8") + (aipass_home / "CLAUDE.md").write_text("# Test", encoding="utf-8") + monkeypatch.setattr( + "aipass.aipass.apps.handlers.init.bootstrap._detect_aipass_home", + lambda: str(aipass_home), + ) + + target = tmp_path / "updproj" + target.mkdir() + init_project(target, project_name="test") + assert is_trusted(str(target.resolve())) is True + + old_reg = read_registry() + old_hash = old_reg["projects"][str(target.resolve())]["config_hash"] + + new_template = ( + '{"hooks_enabled": true, "SessionStart": ' + '{"new_hook": {"handler": "aipass.hooks.apps.handlers.test.handle", "enabled": true}}}' + ) + template.write_text(new_template, encoding="utf-8") + update_project(target) + + new_reg = read_registry() + new_hash = new_reg["projects"][str(target.resolve())]["config_hash"] + assert new_hash != old_hash + assert is_trusted(str(target.resolve())) is True diff --git a/src/aipass/devpulse/README.md b/src/aipass/devpulse/README.md index 44723600..e79e306d 100644 --- a/src/aipass/devpulse/README.md +++ b/src/aipass/devpulse/README.md @@ -44,7 +44,7 @@ src/aipass/devpulse/ │ │ └── watchdog/ # Agent, timer, schedule, registry │ └── plugins/ # Plugin extension point ├── devpulse_json/ # JSON handler storage (config, data, logs per module) -├── tests/ # 309 tests +├── tests/ # 348 tests ├── artifacts/ # Birth certificate, reports ├── dropbox/ # Received files, archived plans, install audit ├── docs/ # Transition notes diff --git a/src/aipass/devpulse/apps/handlers/compass/__init__.py b/src/aipass/devpulse/apps/handlers/compass/__init__.py index 7ec0cbc3..f5261dc7 100644 --- a/src/aipass/devpulse/apps/handlers/compass/__init__.py +++ b/src/aipass/devpulse/apps/handlers/compass/__init__.py @@ -23,9 +23,13 @@ from aipass.devpulse.apps.handlers.compass.store import ( VALID_STATUSES, add_decision, archive, + find_conflicts, + mark_surfaced, query_decisions, rate, + recall_decisions, review, + set_note, stats, ) @@ -36,8 +40,12 @@ __all__ = [ "VALID_STATUSES", "add_decision", "archive", + "find_conflicts", + "mark_surfaced", "query_decisions", "rate", + "recall_decisions", "review", + "set_note", "stats", ] diff --git a/src/aipass/devpulse/apps/handlers/compass/store.py b/src/aipass/devpulse/apps/handlers/compass/store.py index f700fdc7..b73b7d79 100644 --- a/src/aipass/devpulse/apps/handlers/compass/store.py +++ b/src/aipass/devpulse/apps/handlers/compass/store.py @@ -29,6 +29,7 @@ command, and maintenance UX are later phases. """ import logging +import re import sqlite3 from datetime import date from pathlib import Path @@ -57,6 +58,9 @@ VALID_SOURCES = ("devpulse", "user") VALID_STATUSES = ("active", "archived") # Columns we return / surface from the decisions table (everything useful). +# NOTE: ``score`` is deliberately absent — it is code-invisible (DPLAN-0246 +# seedgo ruling). The column stays physically on disk as inert NULL, but no +# Python surface (SELECTs, returned dicts) touches it. _DECISION_COLUMNS = ( "id", "created", @@ -66,10 +70,10 @@ _DECISION_COLUMNS = ( "note", "tags", "source", - "score", "status", "last_reviewed", "times_surfaced", + "supersedes", ) _SCHEMA = """ @@ -85,7 +89,8 @@ CREATE TABLE IF NOT EXISTS decisions ( score INTEGER, status TEXT NOT NULL DEFAULT 'active' CHECK(status IN ('active','archived')), last_reviewed TEXT, - times_surfaced INTEGER NOT NULL DEFAULT 0 + times_surfaced INTEGER NOT NULL DEFAULT 0, + supersedes INTEGER ); CREATE VIRTUAL TABLE IF NOT EXISTS decisions_fts USING fts5( @@ -133,6 +138,42 @@ def _verify_fts5(conn: sqlite3.Connection) -> None: ) from exc +def _migrate(conn: sqlite3.Connection) -> None: + """Apply idempotent schema migrations to an already-open DB. + + Adds the ``supersedes`` column to DBs created before it existed. Guarded by + a ``PRAGMA table_info`` pre-check so it is safe to run on every connect — + never a blind ``ALTER`` (DPLAN-0246 seedgo ruling: idempotent migration). + Fresh DBs already carry the column from ``_SCHEMA``; the pre-check makes + this a no-op for them. + """ + cols = {row["name"] for row in conn.execute("PRAGMA table_info(decisions)")} + if "supersedes" not in cols: + conn.execute("ALTER TABLE decisions ADD COLUMN supersedes INTEGER") + conn.commit() + logger.info("[compass] migration: added supersedes column") + + +# FTS5 MATCH treats characters like " * ( ) : - ^ and the words AND/OR/NOT as +# syntax. Untrusted text (a decision's own words) can therefore crash MATCH. +# We defuse it by extracting bare word tokens and OR-ing them as quoted string +# literals — no operator can survive, and quoting a bareword is exact. +_FTS_WORD = re.compile(r"\w+", re.UNICODE) + + +def _sanitize_fts_query(text: Optional[str]) -> Optional[str]: + """Turn arbitrary text into a safe FTS5 MATCH expression, or None. + + Returns an ``OR`` of the text's word tokens, each quoted as a string + literal so FTS5 syntax characters can never reach the parser. Returns None + when there are no usable tokens (caller should skip the search). + """ + tokens = _FTS_WORD.findall(text or "") + if not tokens: + return None + return " OR ".join(f'"{t}"' for t in tokens) + + def _resolve_db_path(db_path: Optional[Path | str]) -> Path: """Resolve the effective DB path, defaulting to the branch-root location.""" return Path(db_path) if db_path is not None else DEFAULT_DB_PATH @@ -141,8 +182,9 @@ def _resolve_db_path(db_path: Optional[Path | str]) -> Path: def _connect(db_path: Optional[Path | str]) -> sqlite3.Connection: """Open (and lazily initialise) the compass DB. - Creates parent directories on first use, verifies FTS5, ensures schema. - Rows come back as ``sqlite3.Row`` so we can build clean dicts. + Creates parent directories on first use, verifies FTS5, ensures schema, and + runs idempotent migrations. Rows come back as ``sqlite3.Row`` so we can + build clean dicts. """ path = _resolve_db_path(db_path) path.parent.mkdir(parents=True, exist_ok=True) @@ -151,6 +193,7 @@ def _connect(db_path: Optional[Path | str]) -> sqlite3.Connection: conn.execute("PRAGMA foreign_keys = ON") _verify_fts5(conn) conn.executescript(_SCHEMA) + _migrate(conn) return conn @@ -168,6 +211,7 @@ def add_decision( source: str = "devpulse", db_path: Optional[Path | str] = None, created: Optional[str] = None, + supersedes: Optional[int] = None, ) -> int: """Add a rated decision and return its new id. @@ -181,12 +225,16 @@ def add_decision( db_path: Optional DB path override (tests pass a temp path). created: Optional ISO date override; defaults to today. This is the ONLY place a "today" date is stamped. + supersedes: Optional id of the decision this entry corrects. When set, + the new entry links to it AND that entry is archived — atomically, + in one transaction. Errors cleanly (no write) if the id is unknown. Returns: The new row's integer id. Raises: - ValueError: On empty context/decision or invalid rating/source. + ValueError: On empty context/decision, invalid rating/source, or a + ``supersedes`` target that does not exist. """ if not context or not context.strip(): raise ValueError("context must be a non-empty string") @@ -201,22 +249,46 @@ def add_decision( conn = _connect(db_path) try: + # Validate the supersede target BEFORE any write so a bad id never + # leaves a partial insert behind. + if supersedes is not None: + target = conn.execute("SELECT 1 FROM decisions WHERE id = ?", (supersedes,)).fetchone() + if target is None: + raise ValueError(f"cannot supersede #{supersedes}: no decision with that id") + + # Insert + archive-the-target in ONE transaction (commit once at the + # end); any failure rolls the whole thing back — never half-applied. cur = conn.execute( """ - INSERT INTO decisions (created, context, decision, rating, note, tags, source) - VALUES (?, ?, ?, ?, ?, ?, ?) + INSERT INTO decisions (created, context, decision, rating, note, tags, source, supersedes) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) """, - (stamp, context.strip(), decision.strip(), rating, note, tags, source), + (stamp, context.strip(), decision.strip(), rating, note, tags, source, supersedes), ) - conn.commit() if cur.lastrowid is None: # pragma: no cover - sqlite always sets this on INSERT raise RuntimeError("compass: INSERT did not return a rowid") new_id = int(cur.lastrowid) + + if supersedes is not None: + conn.execute("UPDATE decisions SET status = 'archived' WHERE id = ?", (supersedes,)) + + conn.commit() + except Exception: + conn.rollback() + raise finally: conn.close() - logger.info("[compass] added decision id=%s rating=%s source=%s", new_id, rating, source) - json_handler.log_operation("compass_add", {"id": new_id, "rating": rating, "source": source}) + logger.info( + "[compass] added decision id=%s rating=%s source=%s supersedes=%s", + new_id, + rating, + source, + supersedes, + ) + json_handler.log_operation( + "compass_add", {"id": new_id, "rating": rating, "source": source, "supersedes": supersedes} + ) return new_id @@ -224,21 +296,28 @@ def query_decisions( query: str, rating: Optional[str] = None, limit: int = 5, + include_archived: bool = False, db_path: Optional[Path | str] = None, ) -> list[dict]: - """Search active decisions, ranked by FTS5 BM25 relevance. + """Search decisions, ranked by FTS5 BM25 relevance. - Increments ``times_surfaced`` for every returned row. + Increments ``times_surfaced`` for every returned row. Each result dict + carries a computed ``superseded_by`` field: the id of the row that + supersedes this one, or None. Combined with the ``supersedes`` column this + lets callers render both pointer directions. Args: query: FTS5 match query (keywords). rating: Optional exact rating filter (one of VALID_RATINGS). limit: Max rows to return (default 5). + include_archived: When True, lift the ``status = 'active'`` filter so + archived rows (the avoid-list) are searchable too. Default False — + unchanged active-only behaviour. db_path: Optional DB path override. Returns: A list of decision dicts (most relevant first). Each dict includes the - rating and all useful fields. + rating, all useful fields, and the computed ``superseded_by`` pointer. Raises: ValueError: On empty query, bad rating filter, or non-positive limit. @@ -256,9 +335,10 @@ def query_decisions( FROM decisions_fts f JOIN decisions d ON d.id = f.rowid WHERE decisions_fts MATCH ? - AND d.status = 'active' """ params: list = [query.strip()] + if not include_archived: + sql += " AND d.status = 'active'" if rating is not None: sql += " AND d.rating = ?" params.append(rating) @@ -269,24 +349,119 @@ def query_decisions( try: rows = conn.execute(sql, params).fetchall() results = [_row_to_dict(r) for r in rows] + for r in results: + r["superseded_by"] = None ids = [r["id"] for r in results] if ids: placeholders = ",".join("?" for _ in ids) + # Reverse-lookup: which returned rows are pointed AT by a superseder? + successors: dict = {} + for row in conn.execute( + f"SELECT id, supersedes FROM decisions WHERE supersedes IN ({placeholders})", + ids, + ): + successors[row["supersedes"]] = row["id"] conn.execute( f"UPDATE decisions SET times_surfaced = times_surfaced + 1 WHERE id IN ({placeholders})", ids, ) conn.commit() - # Reflect the increment in the returned dicts without a re-query. + # Reflect the increment + attach the reverse pointer without re-query. for r in results: r["times_surfaced"] = (r["times_surfaced"] or 0) + 1 + r["superseded_by"] = successors.get(r["id"]) finally: conn.close() - logger.info("[compass] query %r rating=%s -> %d hit(s)", query, rating, len(results)) + logger.info( + "[compass] query %r rating=%s include_archived=%s -> %d hit(s)", + query, + rating, + include_archived, + len(results), + ) return results +def find_conflicts( + context: str, + decision: str, + limit: int = 3, + db_path: Optional[Path | str] = None, +) -> list[dict]: + """Return ACTIVE decisions whose text overlaps a would-be new entry. + + A write-time, side-effect-free advisory helper: it does NOT increment + ``times_surfaced`` and never writes. The combined ``context + decision`` + text is sanitised (:func:`_sanitize_fts_query`) so no FTS5 syntax character + can crash the MATCH. Returns up to ``limit`` active hits by BM25 relevance, + or an empty list when the text has no usable tokens / nothing overlaps. + + Args: + context: The would-be new entry's context. + decision: The would-be new entry's decision. + limit: Max advisory hits to return (default 3). + db_path: Optional DB path override. + + Returns: + A list of active decision dicts (most relevant first), possibly empty. + """ + match = _sanitize_fts_query(f"{context or ''} {decision or ''}") + if match is None: + return [] + + select_cols = ", ".join(f"d.{c}" for c in _DECISION_COLUMNS) + sql = f""" + SELECT {select_cols} + FROM decisions_fts f + JOIN decisions d ON d.id = f.rowid + WHERE decisions_fts MATCH ? + AND d.status = 'active' + ORDER BY bm25(decisions_fts) ASC + LIMIT ? + """ + conn = _connect(db_path) + try: + rows = conn.execute(sql, (match, limit)).fetchall() + results = [_row_to_dict(r) for r in rows] + finally: + conn.close() + + logger.info("[compass] conflict-check -> %d active hit(s)", len(results)) + return results + + +def set_note( + decision_id: int, + note: Optional[str], + db_path: Optional[Path | str] = None, +) -> bool: + """Set (replace) the note on an existing decision. + + The FTS5 external-content ``decisions_au`` trigger re-indexes the row on + UPDATE, so the new note is immediately searchable. + + Args: + decision_id: Target decision id. + note: The note text to store (may be empty to clear). + db_path: Optional DB path override. + + Returns: + True if a row was updated, False if no such id. + """ + conn = _connect(db_path) + try: + cur = conn.execute("UPDATE decisions SET note = ? WHERE id = ?", (note, decision_id)) + conn.commit() + changed = cur.rowcount > 0 + finally: + conn.close() + + logger.info("[compass] note id=%s (changed=%s)", decision_id, changed) + json_handler.log_operation("compass_note", {"id": decision_id, "changed": changed}) + return changed + + def stats(db_path: Optional[Path | str] = None) -> dict: """Return decision counts by rating, by status, and the total. @@ -414,3 +589,174 @@ def review( logger.info("[compass] review surfaced id=%s stamped=%s", result["id"], stamp) json_handler.log_operation("compass_review", {"id": result["id"], "last_reviewed": stamp}) return result + + +# Ambient recall caps the OR-expansion of a raw prompt: beyond this many unique +# tokens the extra words add noise, not recall, and the MATCH string balloons. +_RECALL_MAX_TOKENS = 64 + +# Stopwords never reach the MATCH: in an OR-of-tokens query, high-frequency +# filler ("lets keep working on the...") outweighs topic words in BM25 and +# surfaces unrelated entries — proven live in the FPLAN-0332 acceptance run. +# Three categories, all query-side only (entry text is never filtered): +# grammatical stopwords; conversational filler verbs that open most prompts +# ("lets keep working on / need to fix"); greeting/small-talk words ("good +# morning", "how did it go last night"). In a technical store casual words are +# RARE (df 1-2), so rarity scoring alone cannot reject them — they must never +# become query tokens at all. The topic space is open; this filler set is +# closed and small, which is why filtering here works. +_RECALL_STOPWORDS = frozenset( + """a an and about again also are as at back be bit but by can could did do + does for from had has have how i if in is it its just me my no not of on + or our so still sure than thanks thank that the their then there these + they this to too u ur us was way we well were what when where which who + why will with would yes you your + add check doing done fix get go going keep lets look make need now see + should try use want work working write + day days good hello hey hi im ive last morning night ok okay please right + today tomorrow tonight week yesterday""".split() +) + + +def recall_decisions( + prompt_text: str, + limit: int = 3, + db_path: Optional[Path | str] = None, +) -> list[dict]: + """Return scored ambient-recall candidates for raw prompt text. + + The Track 2 read path (DPLAN-0246): a hooks handler passes the raw user + prompt; governance (@memory's ``should_surface``) judges the candidates. + Side-effect-free — ``times_surfaced`` is NOT incremented here, because a + candidate is not yet surfaced; the caller reports actual injections via + :func:`mark_surfaced` so the counter stays honest. + + The prompt is arbitrary text, never FTS5 syntax: unique word tokens (first + ``_RECALL_MAX_TOKENS``) are OR-ed as quoted literals, ACTIVE rows only, + ranked by BM25. Each result dict carries ``relevance`` — the BM25 magnitude + mapped to (0, 1) via ``m / (1 + m)``, higher = more relevant — so + governance thresholds live on a bounded scale. + + Args: + prompt_text: Raw prompt text (any content, any length). + limit: Max candidates to return (default 3). + db_path: Optional DB path override. + + Returns: + A list of active decision dicts (most relevant first), each with a + ``relevance`` float in (0, 1); empty when the prompt has no usable + tokens or nothing matches. + + Raises: + ValueError: On non-positive limit. + """ + if limit <= 0: + raise ValueError(f"limit must be a positive integer, got {limit!r}") + + tokens = _FTS_WORD.findall(prompt_text or "") + unique: list[str] = [] + seen: set[str] = set() + for t in tokens: + lowered = t.lower() + if lowered not in seen and lowered not in _RECALL_STOPWORDS: + seen.add(lowered) + unique.append(t) + if len(unique) >= _RECALL_MAX_TOKENS: + break + if not unique: + return [] + match = " OR ".join(f'"{t}"' for t in unique) + + select_cols = ", ".join(f"d.{c}" for c in _DECISION_COLUMNS) + sql = f""" + SELECT {select_cols}, bm25(decisions_fts) AS rank, + (d.context || ' ' || d.decision || ' ' || + COALESCE(d.note, '') || ' ' || COALESCE(d.tags, '')) AS _text + FROM decisions_fts f + JOIN decisions d ON d.id = f.rowid + WHERE decisions_fts MATCH ? + AND d.status = 'active' + ORDER BY bm25(decisions_fts) ASC + LIMIT ? + """ + conn = _connect(db_path) + try: + # Over-fetch: rare-token scoring below reorders, so BM25's top-N alone + # would let a filler-heavy row crowd out a topical one. + rows = conn.execute(sql, (match, max(limit * 3, 10))).fetchall() + + # Rarity cutoff: a token is evidence only if few entries contain it. + active_total = conn.execute("SELECT count(*) FROM decisions WHERE status = 'active'").fetchone()[0] + rare_cutoff = max(3, active_total // 10) + + # Document frequency per prompt token, ONE query against the FTS index. + df: dict[str, int] = {} + for t in unique: + df[t.lower()] = conn.execute( + "SELECT count(*) FROM decisions_fts f JOIN decisions d ON d.id = f.rowid " + "WHERE decisions_fts MATCH ? AND d.status = 'active'", + (f'"{t}"',), + ).fetchone()[0] + + results = [] + for row in rows: + item = _row_to_dict(row) + text = row["_text"].lower() + # Rare-token evidence: how many DISTINCTIVE prompt words this entry + # actually contains. Filler matches score zero — an entry with no + # rare-token overlap must not surface (FPLAN-0332 acceptance: a + # haiku prompt surfaced an unrelated ruling on BM25 alone). + matched_rare = sum( + 1 + for t in unique + if df[t.lower()] <= rare_cutoff and re.search(rf"\b{re.escape(t)}", text, re.IGNORECASE) + ) + item["relevance"] = matched_rare / (1.0 + matched_rare) + item["_bm25"] = row["rank"] + results.append(item) + + results.sort(key=lambda r: (-r["relevance"], r["_bm25"])) + results = results[:limit] + for r in results: + del r["_bm25"] + finally: + conn.close() + + logger.info("[compass] recall -> %d candidate(s)", len(results)) + return results + + +def mark_surfaced( + decision_ids: list[int], + db_path: Optional[Path | str] = None, +) -> int: + """Increment ``times_surfaced`` for decisions actually injected. + + The write half of the recall contract: :func:`recall_decisions` returns + candidates without side effects; whatever governance approves and the + caller truly injects gets counted here — never the merely-considered. + + Args: + decision_ids: Ids of the decisions that were injected. + db_path: Optional DB path override. + + Returns: + Number of rows updated (0 for an empty list). + """ + if not decision_ids: + return 0 + + conn = _connect(db_path) + try: + placeholders = ",".join("?" for _ in decision_ids) + cur = conn.execute( + f"UPDATE decisions SET times_surfaced = times_surfaced + 1 WHERE id IN ({placeholders})", + list(decision_ids), + ) + conn.commit() + updated = cur.rowcount + finally: + conn.close() + + logger.info("[compass] mark_surfaced -> %d row(s)", updated) + return updated diff --git a/src/aipass/devpulse/apps/modules/compass.py b/src/aipass/devpulse/apps/modules/compass.py index 11e02881..f9cd5ca0 100644 --- a/src/aipass/devpulse/apps/modules/compass.py +++ b/src/aipass/devpulse/apps/modules/compass.py @@ -18,11 +18,12 @@ This module is the thin command layer (FPLAN P2). It parses args, calls the No business logic lives here — that's the handler's job. Subcommands: - add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..] - query "question" [--rating R] [--limit N] + add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..] [--supersedes N] + query "question" [--rating R] [--limit N] [--include-archived] stats rate archive + note "text" review Every subcommand accepts ``--db PATH`` (passed through as ``db_path=``) for @@ -36,11 +37,19 @@ from typing import List, Optional from aipass.prax import logger from aipass.cli.apps.modules import err_console, error, warning from aipass.devpulse.apps.handlers import compass +from aipass.devpulse.apps.handlers.compass import mark_surfaced, recall_decisions from aipass.devpulse.apps.handlers.json import json_handler +# Public cross-branch recall API (DPLAN-0246 Track 2). Other branches import +# at the modules/ boundary ONLY (seedgo boardroom ruling): +# from aipass.devpulse.apps.modules.compass import recall_decisions, mark_surfaced +# recall_decisions(prompt_text, limit) -> scored candidates, side-effect-free; +# mark_surfaced(ids) counts only what the caller actually injected. +__all__ = ["handle_command", "mark_surfaced", "recall_decisions"] + console = err_console -_VALID_SUBCOMMANDS = ("add", "query", "stats", "rate", "archive", "review") +_VALID_SUBCOMMANDS = ("add", "query", "stats", "rate", "archive", "note", "review") # Console colour per rating — the rating is the signal, so make it pop. _RATING_STYLE = { @@ -59,6 +68,7 @@ HELP_TEXT = """\ compass stats Counts by rating/status compass rate Re-rate a decision compass archive Archive a decision + compass note "text" Set a decision's note compass review Surface one to review compass --help Show this help @@ -70,19 +80,43 @@ HELP_TEXT = """\ --note "..." Optional human observation. --tags a,b,c Optional comma-separated tags. --source S Optional. devpulse (default) or user. + --supersedes N Optional. Archive decision #N and link this entry as its + correction (atomic). At add time, overlapping active + entries are shown as a non-blocking advisory. + +[bold]Options (query):[/bold] + --rating R Optional exact-rating filter. + --limit N Optional max results (default 5). + --include-archived Also search archived (avoid-list) entries; archived hits + show their status + supersession pointer. [bold]Options (all subcommands):[/bold] --db PATH Use an alternate SQLite store (testing / power use). [bold]Examples:[/bold] drone @devpulse compass add "auth fork" "chose JWT over sessions" --rating good + drone @devpulse compass add "auth fork" "switch to sessions" --rating good --supersedes 4 drone @devpulse compass query "auth" --rating good --limit 3 + drone @devpulse compass query "auth" --include-archived drone @devpulse compass stats drone @devpulse compass rate 4 bad drone @devpulse compass archive 4 + drone @devpulse compass note 4 "revisited — this held up" drone @devpulse compass review -See DPLAN-0212 (design) and the compass handler (apps/handlers/compass/). +See DPLAN-0212 / DPLAN-0246 (design) and the compass handler (apps/handlers/compass/). +""" + + +_NOTE_HELP_TEXT = """\ +[bold]compass note[/bold] — set (replace) a decision's note + +Usage: + compass note "text" Set the note on decision # + compass note --help Show this help + +The note is re-indexed for search immediately — the FTS5 mirror stays in sync, +so the new note text is findable by 'compass query' right away. """ @@ -93,7 +127,7 @@ def print_introspection() -> None: console.print("[dim]Devpulse rated decision store. The truth-store of choices —[/dim]") console.print("[dim]each decision rated; the rating is the signal at a fork.[/dim]") console.print() - console.print("[yellow]Subcommands:[/yellow] [cyan]add, query, stats, rate, archive, review[/cyan]") + console.print("[yellow]Subcommands:[/yellow] [cyan]add, query, stats, rate, archive, note, review[/cyan]") console.print("[dim]Run 'compass --help' for full usage.[/dim]") console.print() @@ -141,6 +175,8 @@ def handle_command(command: str, args: List[str]) -> bool: return _handle_rate(sub_args) if subcommand == "archive": return _handle_archive(sub_args) + if subcommand == "note": + return _handle_note(sub_args) if subcommand == "review": return _handle_review(sub_args) @@ -179,6 +215,18 @@ def _extract_db_path(args: List[str]) -> tuple[List[str], Optional[str]]: return _extract_flag(args, "--db") +def _extract_bool_flag(args: List[str], flag: str) -> tuple[List[str], bool]: + """Pull a valueless boolean ``--flag`` out of args. + + Returns the remaining args (every occurrence of the flag removed) and True + if the flag was present, else False. Unlike ``_extract_flag`` this consumes + no following value. + """ + if flag in args: + return [a for a in args if a != flag], True + return args, False + + def _rating_tag(rating: str) -> str: """Render a coloured ``[RATING]`` tag for query/review output.""" style = _RATING_STYLE.get(rating, "bold white") @@ -198,13 +246,16 @@ def _handle_add(sub_args: List[str]) -> bool: rest, note = _extract_flag(rest, "--note") rest, tags = _extract_flag(rest, "--tags") rest, source = _extract_flag(rest, "--source") + rest, supersedes_raw = _extract_flag(rest, "--supersedes") except ValueError as exc: logger.warning("[compass] add arg-parse error: %s", exc) error(str(exc), suggestion="Use 'compass --help' for usage") return True if len(rest) < 2: - error('Usage: compass add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..]') + error( + 'Usage: compass add "context" "decision" --rating R [--note ..] [--tags a,b] [--source ..] [--supersedes N]' + ) return True if rating is None: error("compass add requires --rating", suggestion="One of: good | bad | impressive | interesting") @@ -213,6 +264,34 @@ def _handle_add(sub_args: List[str]) -> bool: context = rest[0] decision = rest[1] + supersedes: Optional[int] = None + if supersedes_raw is not None: + try: + supersedes = int(supersedes_raw) + except ValueError as exc: + logger.warning("[compass] add bad --supersedes %r: %s", supersedes_raw, exc) + error(f"--supersedes must be an integer, got {supersedes_raw!r}") + return True + + # Write-time conflict check — a NON-BLOCKING advisory (DPLAN-0246). Skipped + # when the writer already chose to supersede, and never allowed to block or + # crash the add. Only shown when NOT already superseding. + if supersedes is None: + try: + conflicts = compass.find_conflicts(context, decision, db_path=db_path) + except Exception as exc: # advisory must never break a write + logger.warning("[compass] conflict-check failed (non-blocking): %s", exc) + conflicts = [] + for c in conflicts: + cid = c.get("id") + excerpt = (c.get("context") or "").strip() + if len(excerpt) > 80: + excerpt = excerpt[:77] + "..." + console.print( + f"[yellow]possible conflict with #{cid}[/yellow]: {excerpt} " + f"[dim]— supersede? (--supersedes {cid})[/dim]" + ) + try: new_id = compass.add_decision( context, @@ -222,6 +301,7 @@ def _handle_add(sub_args: List[str]) -> bool: tags=tags, source=source if source is not None else "devpulse", db_path=db_path, + supersedes=supersedes, ) except ValueError as exc: logger.warning("[compass] add rejected: %s", exc) @@ -235,6 +315,8 @@ def _handle_add(sub_args: List[str]) -> bool: console.print(f" [cyan]note:[/cyan] {note}") if tags: console.print(f" [cyan]tags:[/cyan] {tags}") + if supersedes is not None: + console.print(f" [magenta]supersedes #{supersedes}[/magenta] [dim](archived)[/dim]") return True @@ -242,6 +324,7 @@ def _handle_query(sub_args: List[str]) -> bool: """Parse and dispatch ``compass query "question" [--rating R] [--limit N]``.""" try: rest, db_path = _extract_db_path(sub_args) + rest, include_archived = _extract_bool_flag(rest, "--include-archived") rest, rating = _extract_flag(rest, "--rating") rest, limit_raw = _extract_flag(rest, "--limit") except ValueError as exc: @@ -250,7 +333,7 @@ def _handle_query(sub_args: List[str]) -> bool: return True if not rest: - error('Usage: compass query "question" [--rating R] [--limit N]') + error('Usage: compass query "question" [--rating R] [--limit N] [--include-archived]') return True query_text = rest[0] @@ -265,7 +348,13 @@ def _handle_query(sub_args: List[str]) -> bool: return True try: - results = compass.query_decisions(query_text, rating=rating, limit=limit, db_path=db_path) + results = compass.query_decisions( + query_text, + rating=rating, + limit=limit, + include_archived=include_archived, + db_path=db_path, + ) except ValueError as exc: logger.warning("[compass] query rejected: %s", exc) error(str(exc)) @@ -294,6 +383,16 @@ def _render_query_results(query_text: str, rating: Optional[str], results: List[ console.print(f" [cyan]note:[/cyan] {r['note']}") if r.get("tags"): console.print(f" [cyan]tags:[/cyan] {r['tags']}") + # Supersession pointers — an archived hit must never masquerade as + # current truth, so flag its status + who replaced it (DPLAN-0246). + if r.get("status") == "archived": + superseded_by = r.get("superseded_by") + if superseded_by: + console.print(f" [bold yellow]ARCHIVED[/bold yellow] — superseded by #{superseded_by}") + else: + console.print(" [bold yellow]ARCHIVED[/bold yellow] (avoid-list)") + if r.get("supersedes"): + console.print(f" [magenta]supersedes #{r['supersedes']}[/magenta]") meta = f"source={r.get('source', '?')} status={r.get('status', '?')} surfaced={r.get('times_surfaced', 0)}" console.print(f" [dim]{meta}[/dim]") console.print() @@ -389,6 +488,44 @@ def _handle_archive(sub_args: List[str]) -> bool: return True +def _handle_note(sub_args: List[str]) -> bool: + """Dispatch ``compass note "text"`` — set a decision's note. + + Follows the subcommand-help convention: ``compass note --help`` prints the + per-subcommand help block; malformed input shows the Usage line. + """ + try: + rest, db_path = _extract_db_path(sub_args) + except ValueError as exc: + logger.warning("[compass] note arg-parse error: %s", exc) + error(str(exc)) + return True + + if rest and rest[0] in ("--help", "-h", "help"): + console.print(_NOTE_HELP_TEXT) + return True + + if len(rest) < 2: + error('Usage: compass note "text"') + return True + + try: + decision_id = int(rest[0]) + except ValueError as exc: + logger.warning("[compass] note bad id %r: %s", rest[0], exc) + error(f" must be an integer, got {rest[0]!r}") + return True + + note_text = rest[1] + changed = compass.set_note(decision_id, note_text, db_path=db_path) + if changed: + console.print(f"[green]Note set[/green] on [bold]#{decision_id}[/bold]") + console.print(f" [cyan]note:[/cyan] {note_text}") + else: + warning(f"No decision with id {decision_id} — nothing changed.") + return True + + def _handle_review(sub_args: List[str]) -> bool: """Dispatch ``compass review`` — surface one active decision to review.""" try: diff --git a/src/aipass/devpulse/tests/test_compass_command.py b/src/aipass/devpulse/tests/test_compass_command.py index f11f1696..f7c92a80 100644 --- a/src/aipass/devpulse/tests/test_compass_command.py +++ b/src/aipass/devpulse/tests/test_compass_command.py @@ -275,3 +275,176 @@ def test_flag_without_value_errors(capsys, db): assert compass_cmd.handle_command("compass", ["query", "x", "--rating"]) is True out = _output(capsys).lower() assert "rating" in out and "value" in out + + +# --------------------------------------------------------------------------- +# supersedes — atomic archive + link, both pointer directions +# --------------------------------------------------------------------------- + + +def test_add_supersedes_archives_and_links(capsys, db): + """add --supersedes N archives #N, links the new row, shows 'supersedes #N'.""" + old = _add(capsys, db, "old ctx sessions", "use sessions", "good") + capsys.readouterr() + assert ( + compass_cmd.handle_command( + "compass", + [ + "add", + "new ctx jwt", + "switch to jwt", + "--rating", + "good", + "--supersedes", + str(old), + "--db", + db, + ], + ) + is True + ) + out = _output(capsys) + assert f"supersedes #{old}" in out + + # The archived row is gone from the default (active-only) query... + q = _query_out(capsys, db, "sessions") + assert "0 result(s)" in q + + # ...but --include-archived surfaces it WITH its status + forward pointer. + q2 = _query_out(capsys, db, "sessions", "--include-archived") + assert "ARCHIVED" in q2.upper() + assert "superseded by #" in q2.lower() + + +def test_add_supersedes_bad_id_errors_no_write(capsys, db): + """add --supersedes to a missing id errors and writes nothing.""" + capsys.readouterr() + compass_cmd.handle_command( + "compass", + ["add", "ctx", "dec", "--rating", "good", "--supersedes", "9999", "--db", db], + ) + out = _output(capsys).lower() + assert "9999" in out + assert "total decisions: 0" in _stats_out(capsys, db).lower() + + +def test_add_supersedes_non_integer_errors(capsys, db): + """A non-integer --supersedes fails loud.""" + assert ( + compass_cmd.handle_command( + "compass", + ["add", "ctx", "dec", "--rating", "good", "--supersedes", "abc", "--db", db], + ) + is True + ) + out = _output(capsys).lower() + assert "supersedes" in out and "integer" in out + + +# --------------------------------------------------------------------------- +# write-time conflict advisory — non-blocking +# --------------------------------------------------------------------------- + + +def test_add_conflict_advisory_prints_but_does_not_block(capsys, db): + """An overlapping active row triggers an advisory; the add still succeeds.""" + _add(capsys, db, "caching layer strategy", "add redis caching", "good") + capsys.readouterr() + compass_cmd.handle_command( + "compass", + ["add", "caching approach again", "another caching layer", "--rating", "good", "--db", db], + ) + out = _output(capsys) + assert "possible conflict" in out.lower() + assert "--supersedes" in out # advisory hints the fix + assert "Added decision" in out # NON-BLOCKING: still added + + +def test_add_no_conflict_on_empty_store(capsys, db): + """First add on an empty store prints no advisory.""" + capsys.readouterr() + compass_cmd.handle_command("compass", ["add", "unique ctx", "unique dec", "--rating", "good", "--db", db]) + out = _output(capsys).lower() + assert "possible conflict" not in out + + +# --------------------------------------------------------------------------- +# note — set a note, prove it is immediately searchable +# --------------------------------------------------------------------------- + + +def test_note_command_sets_and_is_searchable(capsys, db): + """note "text" sets the note; a later query finds the new note text.""" + did = _add(capsys, db, "note cmd ctx", "note cmd dec", "good") + capsys.readouterr() + assert compass_cmd.handle_command("compass", ["note", str(did), "findme pterodactyl", "--db", db]) is True + out = _output(capsys).lower() + assert "note set" in out + + q = _query_out(capsys, db, "pterodactyl") + assert "1 result(s)" in q + + +def test_note_help(capsys): + """compass note --help prints per-subcommand usage.""" + assert compass_cmd.handle_command("compass", ["note", "--help"]) is True + out = _output(capsys).lower() + assert "note" in out and "usage" in out + + +def test_note_missing_id_warns(capsys, db): + """note on a non-existent id reports nothing changed, does not crash.""" + assert compass_cmd.handle_command("compass", ["note", "999", "text", "--db", db]) is True + out = _output(capsys).lower() + assert "999" in out and ("nothing changed" in out or "no decision" in out) + + +def test_note_missing_args_shows_usage(capsys, db): + """note with too few args shows usage.""" + assert compass_cmd.handle_command("compass", ["note", "5", "--db", db]) is True + out = _output(capsys).lower() + assert "usage" in out + + +def test_help_and_introspection_list_note(capsys): + """Both --help and bare introspection advertise the note subcommand.""" + compass_cmd.handle_command("compass", ["--help"]) + assert "note" in _output(capsys).lower() + compass_cmd.handle_command("compass", []) + assert "note" in _output(capsys).lower() + + +# --------------------------------------------------------------------------- +# --include-archived — archived hits must show status + supersession pointer +# --------------------------------------------------------------------------- + + +def test_include_archived_shows_archived_pointer(capsys, db): + """--include-archived surfaces an archived row flagged with its successor.""" + old = _add(capsys, db, "archived-visible ctx", "the old choice", "bad") + capsys.readouterr() + compass_cmd.handle_command( + "compass", + [ + "add", + "replacement ctx", + "the new choice", + "--rating", + "good", + "--supersedes", + str(old), + "--db", + db, + ], + ) + capsys.readouterr() + + # Default query hides the archived row. + q = _query_out(capsys, db, "old choice") + assert "0 result(s)" in q + + # With the flag it appears, unmistakably marked archived + superseded. + q2 = _query_out(capsys, db, "old choice", "--include-archived") + assert "1 result(s)" in q2 + assert "archived" in q2.lower() + assert "superseded by #" in q2.lower() diff --git a/src/aipass/devpulse/tests/test_compass_store.py b/src/aipass/devpulse/tests/test_compass_store.py index 4abfd957..907b8bc8 100644 --- a/src/aipass/devpulse/tests/test_compass_store.py +++ b/src/aipass/devpulse/tests/test_compass_store.py @@ -283,3 +283,252 @@ class TestInputValidation: compass.add_decision("ctx", "dec", "good", db_path=db) with pytest.raises(ValueError): compass.query_decisions("ctx", limit=0, db_path=db) + + +class TestSupersedes: + """supersedes column, atomic archive+link, idempotent migration (DPLAN-0246).""" + + def test_add_with_supersedes_archives_and_links(self, db): + """--supersedes links the corrector AND archives the target, atomically.""" + old = compass.add_decision("old auth ctx", "use sessions", "good", db_path=db) + new = compass.add_decision("new auth ctx", "switch to JWT", "good", db_path=db, supersedes=old) + # The corrector row links back to what it replaced. + hit = compass.query_decisions("JWT", db_path=db)[0] + assert hit["supersedes"] == old + # The old entry is archived → gone from the active query. + assert compass.query_decisions("sessions", db_path=db) == [] + # With include_archived it reappears, pointing FORWARD to its successor. + arch = compass.query_decisions("sessions", include_archived=True, db_path=db)[0] + assert arch["status"] == "archived" + assert arch["superseded_by"] == new + + def test_supersedes_nonexistent_raises_no_partial_write(self, db): + """A bad --supersedes id errors cleanly and leaves NO partial write.""" + with pytest.raises(ValueError): + compass.add_decision("ctx", "dec", "good", db_path=db, supersedes=9999) + assert compass.stats(db_path=db)["total"] == 0 + + def test_supersedes_defaults_none(self, db): + """A plain add has supersedes=None and superseded_by=None.""" + compass.add_decision("plain ctx", "plain dec", "good", db_path=db) + hit = compass.query_decisions("plain", db_path=db)[0] + assert hit["supersedes"] is None + assert hit["superseded_by"] is None + + def test_active_hit_shows_its_supersedes_pointer(self, db): + """An ACTIVE corrector still exposes its supersedes pointer on query.""" + old = compass.add_decision("legacy topic zzz", "old way", "bad", db_path=db) + compass.add_decision("current topic zzz", "new way", "good", db_path=db, supersedes=old) + hit = compass.query_decisions("current", db_path=db)[0] + assert hit["supersedes"] == old + assert hit["superseded_by"] is None # nothing supersedes the corrector + + def test_migration_idempotent_repeated_connects(self, db): + """Re-opening the DB re-runs migration harmlessly; column stays present.""" + compass.add_decision("ctx one", "dec one", "good", db_path=db) + for _ in range(3): + conn = store._connect(db) + try: + cols = {r["name"] for r in conn.execute("PRAGMA table_info(decisions)")} + finally: + conn.close() + assert "supersedes" in cols + + def test_migration_adds_column_to_legacy_db(self, db): + """A pre-supersedes DB gets the column added in via _connect migration.""" + # Build a legacy `decisions` table WITHOUT supersedes, as older builds had. + conn = sqlite3.connect(str(db)) + conn.execute( + "CREATE TABLE decisions (" + "id INTEGER PRIMARY KEY, created TEXT, context TEXT NOT NULL, " + "decision TEXT NOT NULL, rating TEXT NOT NULL, note TEXT, tags TEXT, " + "source TEXT, score INTEGER, status TEXT DEFAULT 'active', " + "last_reviewed TEXT, times_surfaced INTEGER DEFAULT 0)" + ) + conn.commit() + conn.close() + # Legacy table lacks the column... + conn = sqlite3.connect(str(db)) + pre = {r[1] for r in conn.execute("PRAGMA table_info(decisions)")} + conn.close() + assert "supersedes" not in pre + # ...a store connect migrates it in (CREATE IF NOT EXISTS is a no-op here). + conn = store._connect(db) + try: + post = {r["name"] for r in conn.execute("PRAGMA table_info(decisions)")} + finally: + conn.close() + assert "supersedes" in post + + +class TestFindConflicts: + """Write-time conflict check: FTS over ACTIVE rows, sanitized, side-effect-free.""" + + def test_finds_overlapping_active_row(self, db): + """A would-be entry surfaces an existing active row it overlaps.""" + compass.add_decision("caching strategy for the API", "add a redis caching layer", "good", db_path=db) + hits = compass.find_conflicts("caching approach", "use a caching layer", db_path=db) + assert len(hits) >= 1 + assert any("caching" in h["context"] for h in hits) + + def test_ignores_archived_rows(self, db): + """Conflict check searches active rows only — archived never surfaces.""" + did = compass.add_decision("archived topic xyzzy", "some decision", "good", db_path=db) + compass.archive(did, db_path=db) + assert compass.find_conflicts("xyzzy topic", "another decision", db_path=db) == [] + + def test_no_side_effects_on_times_surfaced(self, db): + """The advisory must NOT bump times_surfaced — it is not a real surface.""" + compass.add_decision("surfacing guard ctx", "a decision here", "good", db_path=db) + compass.find_conflicts("surfacing guard", "a decision", db_path=db) + hit = compass.query_decisions("surfacing", db_path=db)[0] + assert hit["times_surfaced"] == 1 # only the query above counted + + def test_sanitizes_fts_special_chars(self, db): + """Raw FTS5 syntax characters must not crash MATCH — sanitized to literals.""" + compass.add_decision("special ctx", "a normal decision", "good", db_path=db) + weird = 'broken " ( ) * : query -term AND OR NOT' + result = compass.find_conflicts(weird, "more * (text) ^caret", db_path=db) + assert isinstance(result, list) # no exception raised + + def test_empty_text_returns_empty(self, db): + """Text with no usable tokens yields no conflicts (and no crash).""" + assert compass.find_conflicts(" ", " ", db_path=db) == [] + + +class TestSetNote: + """note edits persist AND re-index immediately via the FTS5 UPDATE trigger.""" + + def test_set_note_updates_and_returns_true(self, db): + """set_note stores the note and reports the row was changed.""" + did = compass.add_decision("note ctx", "note dec", "good", db_path=db) + assert compass.set_note(did, "a fresh observation", db_path=db) is True + hit = compass.query_decisions("note ctx", db_path=db)[0] + assert hit["note"] == "a fresh observation" + + def test_note_edit_is_immediately_fts_searchable(self, db): + """PROOF: the decisions_au trigger re-indexes a note UPDATE for FTS.""" + did = compass.add_decision("indexing ctx", "indexing dec", "good", db_path=db) + # 'zebra' appears nowhere yet. + assert compass.query_decisions("zebra", db_path=db) == [] + compass.set_note(did, "mentions zebra now", db_path=db) + # Immediately findable through the freshly re-indexed note column. + found = compass.query_decisions("zebra", db_path=db) + assert len(found) == 1 + assert found[0]["id"] == did + + def test_set_note_missing_id_returns_false(self, db): + """set_note on a non-existent id returns False (no silent create).""" + assert compass.set_note(9999, "nope", db_path=db) is False + + +class TestScoreRemoved: + """score is gone from every Python surface (DPLAN-0246 seedgo ruling).""" + + def test_score_not_in_decision_columns(self): + """The code-level column list no longer names score.""" + assert "score" not in store._DECISION_COLUMNS + + def test_score_absent_from_query_dict(self, db): + """Query result dicts carry no score key.""" + compass.add_decision("score ctx", "score dec", "good", db_path=db) + hit = compass.query_decisions("score", db_path=db)[0] + assert "score" not in hit + + def test_score_absent_from_review_dict(self, db): + """Review result dicts carry no score key.""" + compass.add_decision("review score ctx", "dec", "good", db_path=db) + result = compass.review(db_path=db) + assert result is not None + assert "score" not in result + + +class TestRecall: + """recall_decisions/mark_surfaced — the Track 2 ambient-recall read path.""" + + def test_recall_returns_scored_active_candidates(self, db): + """Raw prompt text yields active hits, each with relevance in (0, 1).""" + compass.add_decision("vectorization pipeline ctx", "salt vector ids", "good", db_path=db) + hits = compass.recall_decisions("we are working on the vectorization pipeline", db_path=db) + assert hits and hits[0]["decision"] == "salt vector ids" + assert 0.0 < hits[0]["relevance"] < 1.0 + + def test_recall_is_side_effect_free(self, db): + """A recall does NOT bump times_surfaced — candidates are not surfacings.""" + compass.add_decision("recall counter ctx", "stay untouched", "good", db_path=db) + rid = compass.recall_decisions("recall counter", db_path=db)[0]["id"] + compass.recall_decisions("recall counter", db_path=db) + hit = compass.query_decisions("untouched", db_path=db)[0] + assert hit["id"] == rid + # query_decisions itself increments once; recalls added nothing. + assert hit["times_surfaced"] == 1 + + def test_recall_excludes_archived(self, db): + """Archived rows never come back as ambient candidates.""" + rid = compass.add_decision("archived recall ctx", "dead ruling", "bad", db_path=db) + compass.archive(rid, db_path=db) + assert compass.recall_decisions("archived recall dead ruling", db_path=db) == [] + + def test_recall_survives_fts_syntax_in_prompt(self, db): + """FTS5 syntax characters in a prompt cannot crash the MATCH.""" + compass.add_decision("syntax safety ctx", "quote all tokens", "good", db_path=db) + hits = compass.recall_decisions('safety AND (tokens) OR "quote" NEAR *:^-', db_path=db) + assert hits and hits[0]["decision"] == "quote all tokens" + + def test_recall_empty_prompt_returns_empty(self, db): + """No usable tokens → empty list, no error.""" + assert compass.recall_decisions("", db_path=db) == [] + assert compass.recall_decisions("()!@#$", db_path=db) == [] + + def test_recall_ranks_most_relevant_first(self, db): + """Denser overlap outranks a single shared token.""" + compass.add_decision("alpha beta gamma delta", "dense match", "good", db_path=db) + compass.add_decision("alpha unrelated topic here", "sparse match", "good", db_path=db) + hits = compass.recall_decisions("alpha beta gamma delta", limit=2, db_path=db) + assert hits[0]["decision"] == "dense match" + assert hits[0]["relevance"] > hits[1]["relevance"] + + def test_recall_invalid_limit_raises(self, db): + """Non-positive limit fails honestly.""" + with pytest.raises(ValueError): + compass.recall_decisions("anything", limit=0, db_path=db) + + def test_mark_surfaced_counts_only_injected(self, db): + """mark_surfaced increments exactly the ids the caller reports.""" + a = compass.add_decision("mark ctx one", "dec one xyzzy", "good", db_path=db) + b = compass.add_decision("mark ctx two", "dec two xyzzy", "good", db_path=db) + assert compass.mark_surfaced([a], db_path=db) == 1 + hits = {h["id"]: h for h in compass.query_decisions("xyzzy", limit=5, db_path=db)} + # query bumps both by 1; only a carries the extra mark_surfaced bump. + assert hits[a]["times_surfaced"] == 2 + assert hits[b]["times_surfaced"] == 1 + + def test_mark_surfaced_empty_list_is_noop(self, db): + """An empty id list returns 0 and touches nothing.""" + assert compass.mark_surfaced([], db_path=db) == 0 + + def test_recall_stopwords_do_not_drive_ranking(self, db): + """Filler words in the prompt cannot outrank topic words (FPLAN-0332).""" + compass.add_decision( + "docker sop ctx with the and on for filler heavy text", + "push dev first clone in container", + "good", + db_path=db, + ) + compass.add_decision( + "compass curation ctx", + "supersedes links archive corrections", + "good", + db_path=db, + ) + hits = compass.recall_decisions( + "lets keep working on the compass supersedes links and curation", + limit=2, + db_path=db, + ) + assert hits[0]["decision"] == "supersedes links archive corrections" + + def test_recall_all_stopword_prompt_returns_empty(self, db): + """A prompt made only of stopwords yields no candidates, no error.""" + compass.add_decision("some ctx", "some dec", "good", db_path=db) + assert compass.recall_decisions("what is it and how do we", db_path=db) == [] diff --git a/src/aipass/drone/apps/drone.py b/src/aipass/drone/apps/drone.py index 82c52c3e..89b07d54 100644 --- a/src/aipass/drone/apps/drone.py +++ b/src/aipass/drone/apps/drone.py @@ -51,6 +51,21 @@ INTERACTIVE_COMMANDS = ("monitor", "audit", "watchdog", "status") INTERACTIVE_BRANCHES = ("cli", "backup") +def _extract_timeout(args: list[str]) -> tuple[list[str], int | None]: + """Extract --drone-timeout N from an arg list. Returns (cleaned_args, timeout_or_None).""" + if "--drone-timeout" not in args: + return args, None + idx = args.index("--drone-timeout") + if idx + 1 >= len(args): + return args, None + try: + timeout = int(args[idx + 1]) + except ValueError: + logger.info("--drone-timeout value %r is not an integer, ignoring", args[idx + 1]) + return args, None + return args[:idx] + args[idx + 2 :], timeout + + # ============================================================================= # AUTO-DISCOVERY # ============================================================================= @@ -92,6 +107,7 @@ def show_help() -> None: table.add_row("list", "List registered custom commands") table.add_row("remove ", "Remove a custom command") table.add_row("rm [...]", "Contained safe-delete (project + tmp)") + table.add_row("--drone-timeout ", "Override subprocess timeout (default 30s)") table.add_row("--help", "Show this help") table.add_row("--version", "Show version") @@ -340,6 +356,7 @@ def _handle_custom_command(args: list[str]) -> int: target = cmd_data["target"] command = cmd_data["command"] cmd_args = list(cmd_data.get("args", [])) + remaining_args + cmd_args, explicit_timeout = _extract_timeout(cmd_args) module_name = target.lstrip("@").lower() interactive = command in INTERACTIVE_COMMANDS or module_name in INTERACTIVE_BRANCHES @@ -349,6 +366,7 @@ def _handle_custom_command(args: list[str]) -> int: target, command, args=cmd_args if cmd_args else None, + timeout=explicit_timeout, interactive=interactive, ) except (BranchNotFoundError, CommandExecutionError, RegistryError) as exc: @@ -412,6 +430,7 @@ def _handle_target(args: List[str]) -> int: """Handle `drone @target command [args]` or `drone @target --help`.""" target = args[0] rest = args[1:] + rest, explicit_timeout = _extract_timeout(rest) module_name = target.lstrip("@").lower() first_cmd = rest[0] if rest and rest[0] not in ("--help", "-h") else None @@ -470,6 +489,7 @@ def _handle_target(args: List[str]) -> int: target, command, args=cmd_args if cmd_args else None, + timeout=explicit_timeout, interactive=interactive, ) except (BranchNotFoundError, CommandExecutionError, RegistryError) as exc: diff --git a/src/aipass/drone/apps/handlers/executor.py b/src/aipass/drone/apps/handlers/executor.py index cecce70e..55a28150 100644 --- a/src/aipass/drone/apps/handlers/executor.py +++ b/src/aipass/drone/apps/handlers/executor.py @@ -21,6 +21,29 @@ from .exceptions import CommandExecutionError from aipass.drone.apps.handlers.json import json_handler +DEFAULT_TIMEOUT = 30 + +TIMEOUT_OVERRIDES: dict[str, dict[str, int]] = { + "memory": {"process-plans": 120}, + "flow": {"close": 90}, +} + + +def resolve_timeout(branch: str, command: str | None, explicit: int | None = None) -> int: + """Resolve subprocess timeout for a branch command. + + Priority: explicit flag > per-command policy > DEFAULT_TIMEOUT. + """ + if explicit is not None: + return explicit + branch_key = branch.lstrip("@").lower() + if command and branch_key in TIMEOUT_OVERRIDES: + cmd_timeout = TIMEOUT_OVERRIDES[branch_key].get(command) + if cmd_timeout is not None: + return cmd_timeout + return DEFAULT_TIMEOUT + + @dataclass class CommandResult: """Result of a routed command execution.""" @@ -82,7 +105,10 @@ def execute_command( return CommandResult(stdout="", stderr="", exit_code=130, branch="", command="") raise except subprocess.TimeoutExpired as e: - raise CommandExecutionError(f"Command timed out after {timeout}s: {' '.join(full_cmd)}") from e + raise CommandExecutionError( + f"Command timed out after {timeout}s: {' '.join(full_cmd)}\n" + f" Override with: drone @ --drone-timeout " + ) from e except FileNotFoundError as e: raise CommandExecutionError(f"Executable not found: {executable!r}") from e except OSError as e: diff --git a/src/aipass/drone/apps/modules/router.py b/src/aipass/drone/apps/modules/router.py index 68a30191..90a19d36 100644 --- a/src/aipass/drone/apps/modules/router.py +++ b/src/aipass/drone/apps/modules/router.py @@ -20,7 +20,7 @@ from typing import Dict, List, Optional from aipass.prax.apps.modules.logger import system_logger from aipass.cli.apps.modules import console -from aipass.drone.apps.handlers.executor import CommandResult +from aipass.drone.apps.handlers.executor import CommandResult, resolve_timeout from aipass.drone.apps.handlers.json import json_handler from aipass.drone.apps.handlers.router_handler import ( detect_caller_branch_name, @@ -90,28 +90,38 @@ def route_command( target: str, command: Optional[str] = None, args: Optional[List[str]] = None, - timeout: int = 30, + timeout: int | None = None, interactive: bool = False, ) -> CommandResult: """Route a command to a branch's entry point. Resolves @target to a path, then delegates to the handler for execution. When command is None, runs the branch with no args (introspection). + + Timeout resolution: explicit value > per-command policy > DEFAULT_TIMEOUT. """ branch_path = resolve_branch(target) branch_name = target.lstrip("@").lower() + resolved_timeout = resolve_timeout(branch_name, command, timeout) caller = detect_caller_branch_name(Path.cwd()) if not caller: caller = os.environ.get("AIPASS_BRANCH_NAME") caller_tag = f" [CALLER:{caller.upper()}]" if caller else "" - logger.info("Routing @%s%s → %s %s", branch_name, caller_tag, command or "(introspection)", args or []) + logger.info( + "Routing @%s%s → %s %s (timeout=%ds)", + branch_name, + caller_tag, + command or "(introspection)", + args or [], + resolved_timeout, + ) return execute_branch_command( branch_path=branch_path, branch_name=branch_name, command=command, args=args, - timeout=timeout, + timeout=resolved_timeout, interactive=interactive, ) @@ -147,7 +157,7 @@ def print_introspection(): def route_all( command: str, args: Optional[List[str]] = None, - timeout: int = 30, + timeout: int | None = None, ) -> Dict[str, CommandResult]: """Route the same command to ALL active branches in the registry.""" if args is None: diff --git a/src/aipass/drone/tests/test_activation.py b/src/aipass/drone/tests/test_activation.py index ccaf4e7c..7b29bd85 100644 --- a/src/aipass/drone/tests/test_activation.py +++ b/src/aipass/drone/tests/test_activation.py @@ -355,6 +355,7 @@ class TestHandleCustomCommand: "@seedgo", "audit", args=["aipass"], + timeout=None, interactive=True, ) @@ -380,6 +381,7 @@ class TestHandleCustomCommand: "@seedgo", "audit", args=["aipass", "@drone"], + timeout=None, interactive=True, ) @@ -616,6 +618,7 @@ class TestMainIntegration: "@seedgo", "audit", args=["aipass"], + timeout=None, interactive=True, ) @@ -642,6 +645,7 @@ class TestMainIntegration: "@seedgo", "audit", args=["aipass", "@drone"], + timeout=None, interactive=True, ) @@ -714,5 +718,6 @@ class TestMatchCommandIntegration: "@flow", "create", args=["--type=plan", "my-plan"], + timeout=None, interactive=False, ) diff --git a/src/aipass/drone/tests/test_cli_routing.py b/src/aipass/drone/tests/test_cli_routing.py index 872f437d..1f758972 100644 --- a/src/aipass/drone/tests/test_cli_routing.py +++ b/src/aipass/drone/tests/test_cli_routing.py @@ -903,3 +903,80 @@ class TestAipassIntercept: ): result = main() assert result == 0 + + +# --------------------------------------------------------------------------- +# _extract_timeout — --timeout flag parsing +# --------------------------------------------------------------------------- + + +class TestExtractTimeout: + """Tests for --drone-timeout flag extraction from arg lists.""" + + def test_no_flag(self) -> None: + """Args without --drone-timeout pass through unchanged.""" + from aipass.drone.apps.drone import _extract_timeout + + args = ["close", "FPLAN-0313"] + cleaned, timeout = _extract_timeout(args) + assert cleaned == ["close", "FPLAN-0313"] + assert timeout is None + + def test_flag_at_end(self) -> None: + """--drone-timeout N at end of args is extracted.""" + from aipass.drone.apps.drone import _extract_timeout + + cleaned, timeout = _extract_timeout(["process-plans", "--drone-timeout", "120"]) + assert cleaned == ["process-plans"] + assert timeout == 120 + + def test_flag_at_start(self) -> None: + """--drone-timeout N at start of args is extracted.""" + from aipass.drone.apps.drone import _extract_timeout + + cleaned, timeout = _extract_timeout(["--drone-timeout", "90", "close", "FPLAN-0313"]) + assert cleaned == ["close", "FPLAN-0313"] + assert timeout == 90 + + def test_flag_in_middle(self) -> None: + """--drone-timeout N in the middle of args is extracted.""" + from aipass.drone.apps.drone import _extract_timeout + + cleaned, timeout = _extract_timeout(["close", "--drone-timeout", "60", "FPLAN-0313"]) + assert cleaned == ["close", "FPLAN-0313"] + assert timeout == 60 + + def test_flag_without_value(self) -> None: + """--drone-timeout at end with no value returns None and leaves args.""" + from aipass.drone.apps.drone import _extract_timeout + + args = ["close", "--drone-timeout"] + cleaned, timeout = _extract_timeout(args) + assert cleaned == args + assert timeout is None + + def test_flag_non_integer_value(self) -> None: + """--drone-timeout with non-integer value returns None and leaves args.""" + from aipass.drone.apps.drone import _extract_timeout + + args = ["close", "--drone-timeout", "abc"] + cleaned, timeout = _extract_timeout(args) + assert cleaned == args + assert timeout is None + + def test_empty_args(self) -> None: + """Empty arg list returns empty with None timeout.""" + from aipass.drone.apps.drone import _extract_timeout + + cleaned, timeout = _extract_timeout([]) + assert cleaned == [] + assert timeout is None + + def test_plain_timeout_passes_through(self) -> None: + """--timeout (without drone- prefix) is NOT consumed — passes to target.""" + from aipass.drone.apps.drone import _extract_timeout + + args = ["watchdog", "agent", "@memory", "--timeout", "1800"] + cleaned, timeout = _extract_timeout(args) + assert cleaned == args + assert timeout is None diff --git a/src/aipass/drone/tests/test_executor.py b/src/aipass/drone/tests/test_executor.py index 1fe3b824..798e5001 100644 --- a/src/aipass/drone/tests/test_executor.py +++ b/src/aipass/drone/tests/test_executor.py @@ -8,7 +8,12 @@ from unittest.mock import patch import pytest from aipass.drone.apps.handlers.exceptions import CommandExecutionError -from aipass.drone.apps.handlers.executor import execute_command +from aipass.drone.apps.handlers.executor import ( + DEFAULT_TIMEOUT, + TIMEOUT_OVERRIDES, + execute_command, + resolve_timeout, +) # --------------------------------------------------------------------------- @@ -389,3 +394,63 @@ class TestShellSecurity: # The semicolon is treated as literal text, not a shell separator assert result.stdout.strip() == "hello; echo pwned" assert result.exit_code == 0 + + +# --------------------------------------------------------------------------- +# 11. resolve_timeout — policy resolution +# --------------------------------------------------------------------------- + + +class TestResolveTimeout: + """Timeout resolution: explicit > policy > default.""" + + def test_default_timeout(self): + """Unknown branch+command returns DEFAULT_TIMEOUT.""" + assert resolve_timeout("unknown", "whatever") == DEFAULT_TIMEOUT + + def test_policy_override(self): + """Known branch+command returns the policy value.""" + for branch, cmds in TIMEOUT_OVERRIDES.items(): + for cmd, expected in cmds.items(): + assert resolve_timeout(branch, cmd) == expected + + def test_explicit_wins_over_policy(self): + """Explicit timeout overrides the policy map.""" + branch = next(iter(TIMEOUT_OVERRIDES)) + cmd = next(iter(TIMEOUT_OVERRIDES[branch])) + assert resolve_timeout(branch, cmd, explicit=999) == 999 + + def test_explicit_wins_over_default(self): + """Explicit timeout overrides the default.""" + assert resolve_timeout("unknown", "whatever", explicit=42) == 42 + + def test_none_command_returns_default(self): + """None command (introspection) returns default.""" + assert resolve_timeout("memory", None) == DEFAULT_TIMEOUT + + def test_at_prefix_stripped(self): + """Leading @ on branch name is stripped before lookup.""" + for branch in TIMEOUT_OVERRIDES: + cmd = next(iter(TIMEOUT_OVERRIDES[branch])) + expected = TIMEOUT_OVERRIDES[branch][cmd] + assert resolve_timeout(f"@{branch}", cmd) == expected + + +# --------------------------------------------------------------------------- +# 12. Timeout error message includes --timeout hint +# --------------------------------------------------------------------------- + + +class TestTimeoutErrorMessage: + """Timeout error tells the caller how to override.""" + + def test_timeout_error_includes_override_hint(self, temp_test_dir: Path): + """The timeout error message mentions --timeout.""" + with pytest.raises(CommandExecutionError, match="--drone-timeout") as exc_info: + execute_command( + sys.executable, + ["-c", "import time; time.sleep(10)"], + cwd=str(temp_test_dir), + timeout=1, + ) + assert "--drone-timeout" in str(exc_info.value) diff --git a/src/aipass/flow/apps/handlers/mbank/process.py b/src/aipass/flow/apps/handlers/mbank/process.py index c512a8b1..7fd427e6 100644 --- a/src/aipass/flow/apps/handlers/mbank/process.py +++ b/src/aipass/flow/apps/handlers/mbank/process.py @@ -19,13 +19,16 @@ Key Functions: - verify_and_heal_orphaned_plans() - Orphan healing logic """ +# ruff: noqa: E402 from pathlib import Path _PKG_ROOT = Path(__file__).resolve().parents[4] # Standard imports import json +import os import shutil +import time from datetime import datetime, timezone from typing import Dict, List, Any @@ -42,6 +45,35 @@ from aipass.prax.apps.modules.logger import system_logger as logger FLOW_ROOT = _PKG_ROOT / "flow" FLOW_JSON_DIR = FLOW_ROOT / "flow_json" +MODULE_NAME = "mbank_process" +_LOCK_RETRIES = 10 +_LOCK_BACKOFF_BASE = 0.05 + + +def _acquire_lock(lock_path: Path) -> bool: + """Atomically acquire a lockfile via O_CREAT|O_EXCL with retry+backoff.""" + for attempt in range(_LOCK_RETRIES): + try: + fd = os.open(str(lock_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) + os.write(fd, str(os.getpid()).encode()) + os.close(fd) + return True + except FileExistsError: + logger.info("[%s] Lock contention on %s, retry %d", MODULE_NAME, lock_path, attempt + 1) + time.sleep(_LOCK_BACKOFF_BASE * (2**attempt)) + except OSError as exc: + logger.warning("[%s] Lock creation failed for %s: %s", MODULE_NAME, lock_path, exc) + return False + return False + + +def _release_lock(lock_path: Path) -> None: + """Remove lockfile, tolerating already-removed.""" + try: + lock_path.unlink(missing_ok=True) + except OSError as exc: + logger.warning("[%s] Could not release lock %s: %s", MODULE_NAME, lock_path, exc) + def _find_repo_root() -> Path: """Walk up from this file to find the repo root (contains AIPASS_REGISTRY.json).""" @@ -97,12 +129,22 @@ def load_flow_registry(registry_file: str | None = None) -> Dict[str, Any]: def save_flow_registry(registry: Dict[str, Any], registry_file: str | None = None) -> None: - """Save a plan registry.""" + """Save a plan registry with lockfile + atomic write.""" target = FLOW_JSON_DIR / registry_file if registry_file else REGISTRY_FILE + lock_path = target.with_suffix(".lock") + try: - registry["last_updated"] = datetime.now(timezone.utc).isoformat() - with open(target, "w", encoding="utf-8") as f: - json.dump(registry, f, indent=2, ensure_ascii=False) + if not _acquire_lock(lock_path): + raise OSError(f"Could not acquire lock for {target}") + + try: + registry["last_updated"] = datetime.now(timezone.utc).isoformat() + tmp_path = target.with_suffix(".tmp") + with open(tmp_path, "w", encoding="utf-8") as f: + json.dump(registry, f, indent=2, ensure_ascii=False) + os.replace(str(tmp_path), str(target)) + finally: + _release_lock(lock_path) except Exception as e: raise Exception(f"Failed to save flow registry: {e}") @@ -361,7 +403,10 @@ def is_template_content(content: str) -> bool: # today = datetime.now().strftime("%Y%m%d") # plan_num = plan_path.stem.replace("FPLAN-", "") # template_suffix = "-TEMP" if is_template else "" -# filename = f"{folder_context}-{analysis['type']}-{analysis['category']}-{analysis['action']}-FPLAN-{plan_num}{template_suffix}-{today}.md" +# filename = ( +# f"{folder_context}-{analysis['type']}-{analysis['category']}" +# f"-{analysis['action']}-FPLAN-{plan_num}{template_suffix}-{today}.md" +# ) # # filename = re.sub(r'[<>:"|?*]', '-', filename) # filename = re.sub(r'-+', '-', filename) @@ -627,7 +672,7 @@ def process_closed_plans() -> Dict[str, Any]: if archive_success: processed_count += 1 - # Vector intake handled by close_ops.py via drone @memory process-plans + # Vector intake triggered by post_close_runner via direct import results.append({"plan": plan_label, "status": "archived", "correlation_id": correlation_id}) else: error_count += 1 diff --git a/src/aipass/flow/apps/handlers/plan/append_closed_plan.py b/src/aipass/flow/apps/handlers/plan/append_closed_plan.py index 327af08d..f4ef2ce4 100644 --- a/src/aipass/flow/apps/handlers/plan/append_closed_plan.py +++ b/src/aipass/flow/apps/handlers/plan/append_closed_plan.py @@ -3,7 +3,7 @@ # Description: Closed Plans Local Registry Handler # Version: 0.1.0 # Created: 2026-03-03 -# Modified: 2026-03-03 +# Modified: 2026-07-15 # ============================================= """ @@ -13,8 +13,11 @@ Appends a closed plan entry to the branch's CLOSED_PLANS.local.json file. Creates the file if it doesn't exist. """ +# ruff: noqa: E402 import json +import os import re +import time from pathlib import Path # INFRASTRUCTURE IMPORT PATTERN @@ -27,6 +30,31 @@ from aipass.flow.apps.handlers.json import json_handler MODULE_NAME = "append_closed_plan" CLOSED_PLANS_FILE = "CLOSED_PLANS.local.json" +_LOCK_RETRIES = 10 +_LOCK_BACKOFF_BASE = 0.05 + + +def _acquire_append_lock(lock_path: Path) -> bool: + """Atomically acquire a lockfile via O_CREAT|O_EXCL with retry+backoff.""" + for attempt in range(_LOCK_RETRIES): + try: + fd = os.open(str(lock_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) + os.write(fd, str(os.getpid()).encode()) + os.close(fd) + return True + except FileExistsError: + logger.info("[%s] Lock contention on %s, retry %d", MODULE_NAME, lock_path, attempt + 1) + time.sleep(_LOCK_BACKOFF_BASE * (2**attempt)) + return False + + +def _release_append_lock(lock_path: Path) -> None: + """Remove lockfile, tolerating already-removed.""" + try: + lock_path.unlink(missing_ok=True) + except OSError as exc: + logger.warning("[%s] Could not release lock %s: %s", MODULE_NAME, lock_path, exc) + def append_to_closed_plans(plan_key: str, plan_info: dict, plan_location: Path) -> bool: """ @@ -61,27 +89,35 @@ def append_to_closed_plans(plan_key: str, plan_info: dict, plan_location: Path) "location": plan_info.get("relative_path", ""), } - # Read existing file or create new structure + # Locked read-modify-write to prevent lost updates under concurrent close closed_plans_path = plan_location / CLOSED_PLANS_FILE + lock_path = closed_plans_path.with_suffix(".lock") - if closed_plans_path.exists(): - with open(closed_plans_path, "r", encoding="utf-8") as f: - data = json.load(f) - else: - data = {"closed_plans": []} + if not _acquire_append_lock(lock_path): + logger.error( + f"[{MODULE_NAME}] Could not acquire lock for {closed_plans_path} after {_LOCK_RETRIES} retries" + ) + return False - # Check for duplicate plan_id before appending - existing_ids = {p.get("plan_id") for p in data.get("closed_plans", [])} - if plan_id in existing_ids: - logger.info(f"[{MODULE_NAME}] {plan_id} already in {CLOSED_PLANS_FILE} at {plan_location}, skipping") - return True + try: + if closed_plans_path.exists(): + with open(closed_plans_path, "r", encoding="utf-8") as f: + data = json.load(f) + else: + data = {"closed_plans": []} - # Append and write - data["closed_plans"].append(entry) + existing_ids = {p.get("plan_id") for p in data.get("closed_plans", [])} + if plan_id in existing_ids: + logger.info(f"[{MODULE_NAME}] {plan_id} already in {CLOSED_PLANS_FILE} at {plan_location}, skipping") + return True - with open(closed_plans_path, "w", encoding="utf-8") as f: - json.dump(data, f, indent=2, ensure_ascii=False) - f.write("\n") + data["closed_plans"].append(entry) + + with open(closed_plans_path, "w", encoding="utf-8") as f: + json.dump(data, f, indent=2, ensure_ascii=False) + f.write("\n") + finally: + _release_append_lock(lock_path) logger.info(f"[{MODULE_NAME}] Appended {plan_id} to {closed_plans_path}") json_handler.log_operation( diff --git a/src/aipass/flow/apps/handlers/plan/close_ops.py b/src/aipass/flow/apps/handlers/plan/close_ops.py index 06c0f6e4..87f59eb5 100644 --- a/src/aipass/flow/apps/handlers/plan/close_ops.py +++ b/src/aipass/flow/apps/handlers/plan/close_ops.py @@ -379,9 +379,22 @@ def close_plan_impl( try: from aipass.flow.apps.handlers.plan.append_closed_plan import append_to_closed_plans - append_to_closed_plans(plan_key, plan_info, plan_file.parent) + if not append_to_closed_plans(plan_key, plan_info, plan_file.parent): + logger.error(f"[{MODULE_NAME}] CLOSED_PLANS append failed for {plan_prefix}-{plan_key}") + messages.append( + { + "type": "warning", + "text": f" CLOSED_PLANS append failed for {plan_prefix}-{plan_key}", + } + ) except Exception as e: - logger.warning(f"[{MODULE_NAME}] CLOSED_PLANS update failed (non-critical): {e}") + logger.error(f"[{MODULE_NAME}] CLOSED_PLANS update failed: {e}") + messages.append( + { + "type": "warning", + "text": f" CLOSED_PLANS update failed: {e}", + } + ) # Fire trigger event for plan closure if trigger_fire_fn is not None: diff --git a/src/aipass/flow/apps/handlers/registry/save_registry.py b/src/aipass/flow/apps/handlers/registry/save_registry.py index bb40d9cd..7756811c 100644 --- a/src/aipass/flow/apps/handlers/registry/save_registry.py +++ b/src/aipass/flow/apps/handlers/registry/save_registry.py @@ -25,6 +25,8 @@ Usage: """ import json +import os +import time from pathlib import Path from datetime import datetime, timezone from typing import Dict, Any @@ -44,6 +46,35 @@ MODULE_NAME = "save_registry" FLOW_JSON_DIR = FLOW_ROOT / "flow_json" REGISTRY_FILE = FLOW_JSON_DIR / "fplan_registry.json" +_LOCK_RETRIES = 10 +_LOCK_BACKOFF_BASE = 0.05 + + +def _acquire_lock(lock_path: Path) -> bool: + """Atomically acquire a lockfile via O_CREAT|O_EXCL with retry+backoff.""" + for attempt in range(_LOCK_RETRIES): + try: + fd = os.open(str(lock_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) + os.write(fd, str(os.getpid()).encode()) + os.close(fd) + return True + except FileExistsError: + logger.info("[%s] Lock contention on %s, retry %d", MODULE_NAME, lock_path, attempt + 1) + time.sleep(_LOCK_BACKOFF_BASE * (2**attempt)) + except OSError as exc: + logger.warning("[%s] Lock creation failed for %s: %s", MODULE_NAME, lock_path, exc) + return False + return False + + +def _release_lock(lock_path: Path) -> None: + """Remove lockfile, tolerating already-removed.""" + try: + lock_path.unlink(missing_ok=True) + except OSError as exc: + logger.warning("[%s] Could not release lock %s: %s", MODULE_NAME, lock_path, exc) + + # ============================================= # HANDLER FUNCTION # ============================================= @@ -64,15 +95,30 @@ def save_registry(registry: Dict[str, Any], registry_file: str | None = None) -> Automatically updates the last_updated timestamp before saving. Creates the flow_json directory if it doesn't exist. + Uses a lockfile to serialize concurrent writes and atomic + tempfile+rename to prevent torn reads. """ target = FLOW_JSON_DIR / registry_file if registry_file else REGISTRY_FILE + lock_path = target.with_suffix(".lock") try: FLOW_JSON_DIR.mkdir(parents=True, exist_ok=True) - registry["_notice"] = "DO NOT MANUALLY EDIT — managed by flow close pipeline" - registry["last_updated"] = datetime.now(timezone.utc).isoformat() - with open(target, "w", encoding="utf-8") as f: - json.dump(registry, f, indent=2, ensure_ascii=False) + + if not _acquire_lock(lock_path): + logger.error("[%s] Could not acquire lock for %s after %d retries", MODULE_NAME, target, _LOCK_RETRIES) + return False + + try: + registry["_notice"] = "DO NOT MANUALLY EDIT — managed by flow close pipeline" + registry["last_updated"] = datetime.now(timezone.utc).isoformat() + + tmp_path = target.with_suffix(".tmp") + with open(tmp_path, "w", encoding="utf-8") as f: + json.dump(registry, f, indent=2, ensure_ascii=False) + os.replace(str(tmp_path), str(target)) + finally: + _release_lock(lock_path) + json_handler.log_operation( "registry_saved", { diff --git a/src/aipass/flow/apps/modules/post_close_runner.py b/src/aipass/flow/apps/modules/post_close_runner.py index 7d5c3403..942ca92a 100644 --- a/src/aipass/flow/apps/modules/post_close_runner.py +++ b/src/aipass/flow/apps/modules/post_close_runner.py @@ -32,7 +32,7 @@ if sys.platform == "win32": from pathlib import Path -from aipass.cli.apps.modules import console, error, warning +from aipass.cli.apps.modules import console, error, success, warning from aipass.flow.apps.handlers.json import json_handler from aipass.flow.apps.handlers.mbank.process import process_closed_plans from aipass.flow.apps.handlers.runner.lock_ops import acquire_lock, release_lock @@ -80,7 +80,25 @@ def handle_command(command: str, args: list) -> bool: try: process_closed_plans() - console.print("[green]Processing complete[/green]") + + try: + import importlib + + _plans_mod = importlib.import_module("aipass.memory.apps.handlers.intake.plans_processor") + result = _plans_mod.process_plans() + if result.get("success"): + count = result.get("files_processed", 0) + chunks = result.get("total_chunks", 0) + if count > 0: + success(f"Vectorized {count} plan(s) ({chunks} chunks)") + logger.info("[%s] Plan vectorization: %s", MODULE_NAME, result) + else: + logger.error("[%s] Plan vectorization failed: %s", MODULE_NAME, result.get("error", "unknown")) + error(f"Vectorization failed: {result.get('error', 'unknown')}") + except Exception as e: + logger.error("[%s] Plan vectorization error: %s", MODULE_NAME, e) + error(f"Vectorization error: {e}") + except Exception as e: logger.error(f"[{MODULE_NAME}] Background processing failed: {e}") error(f"Processing failed: {e}") @@ -130,6 +148,19 @@ if __name__ == "__main__": try: process_closed_plans() + + try: + import importlib + + _plans_mod = importlib.import_module("aipass.memory.apps.handlers.intake.plans_processor") + result = _plans_mod.process_plans() + if result.get("success"): + logger.info("[%s] Plan vectorization: %s", MODULE_NAME, result) + else: + logger.error("[%s] Plan vectorization failed: %s", MODULE_NAME, result.get("error", "unknown")) + except Exception as e: + logger.error("[%s] Plan vectorization error: %s", MODULE_NAME, e) + except Exception as e: logger.error(f"[{MODULE_NAME}] Background processing failed: {e}") finally: diff --git a/src/aipass/hooks/.seedgo/bypass.json b/src/aipass/hooks/.seedgo/bypass.json index 514cabc1..db381bd7 100644 --- a/src/aipass/hooks/.seedgo/bypass.json +++ b/src/aipass/hooks/.seedgo/bypass.json @@ -406,6 +406,11 @@ "standard": "json_structure", "reason": "Sound handler \u2014 no JSON operations, plays WAV files." }, + { + "file": "apps/handlers/config/trust_registry.py", + "standard": "unused_function", + "reason": "enroll() and revoke() are the public API consumed CROSS-BRANCH by @aipass CLI (init/trust/revoke commands, DPLAN-0244 phase 2). seedgo's intra-branch static analysis cannot see cross-branch callers. read_registry() also exported for @aipass CLI use." + }, { "file": "apps/handlers/config/loader.py", "standard": "json_structure", diff --git a/src/aipass/hooks/apps/handlers/config/loader.py b/src/aipass/hooks/apps/handlers/config/loader.py index 1648e53a..19118990 100644 --- a/src/aipass/hooks/apps/handlers/config/loader.py +++ b/src/aipass/hooks/apps/handlers/config/loader.py @@ -20,19 +20,39 @@ AIPASS_HOME = os.environ.get("AIPASS_HOME", "") def find_project_config() -> dict | None: - """Walk up from CWD looking for .aipass/hooks.json.""" + """Walk up from CWD looking for .aipass/hooks.json, with trust verification.""" + from aipass.hooks.apps.handlers.config.trust_registry import ( + REGISTRY_PATH, + bootstrap, + is_trusted, + ) + search = Path.cwd() home = Path.home() while search != home and search.parent != search: - config = search / ".aipass" / "hooks.json" - if config.exists(): + config_file = search / ".aipass" / "hooks.json" + if config_file.exists(): + project_dir = str(search) + + if not REGISTRY_PATH.exists(): + bootstrap() + + if not is_trusted(project_dir): + logger.warning( + "[HOOKS] project not enrolled in trust registry: %s (run: aipass init update)", + project_dir, + ) + return None + try: - raw = config.read_text(encoding="utf-8") + raw = config_file.read_text(encoding="utf-8") if AIPASS_HOME: raw = raw.replace("$AIPASS_HOME", AIPASS_HOME) - return json.loads(raw) + parsed = json.loads(raw) + parsed["_source"] = "project" + return parsed except (json.JSONDecodeError, OSError) as exc: - logger.error("[HOOKS] bad config %s: %s", config, exc) + logger.error("[HOOKS] bad config %s: %s", config_file, exc) return None search = search.parent return None diff --git a/src/aipass/hooks/apps/handlers/config/trust_registry.py b/src/aipass/hooks/apps/handlers/config/trust_registry.py new file mode 100644 index 00000000..6258d086 --- /dev/null +++ b/src/aipass/hooks/apps/handlers/config/trust_registry.py @@ -0,0 +1,138 @@ +# =================== AIPass ==================== +# Name: trust_registry.py +# Version: 1.0.0 +# Description: Trusted-project registry — DPLAN-0244 Layer B +# Branch: hooks +# Layer: apps/handlers/config +# Created: 2026-07-15 +# Modified: 2026-07-15 +# ============================================= + +"""Trusted-project registry for hook config loading. + +Single source of truth for which projects are trusted to have their +.aipass/hooks.json loaded by the hook engine. Registry lives at +~/.aipass/trusted_projects.json. @aipass CLI (init/trust/revoke) +imports this module for enrollment operations. +""" + +import hashlib +import json +import os +from pathlib import Path + +from aipass.hooks.apps.handlers.json import json_handler +from aipass.prax.apps.modules.logger import system_logger as logger + +REGISTRY_PATH = Path.home() / ".aipass" / "trusted_projects.json" + + +def _hash_file(path: Path) -> str: + """Compute sha256 of a file's contents.""" + data = path.read_bytes() + return f"sha256:{hashlib.sha256(data).hexdigest()}" + + +def read_registry() -> dict: + """Read the trusted-project registry. Returns empty registry if absent or corrupt.""" + if not REGISTRY_PATH.exists(): + return {"version": 1, "projects": {}} + try: + data = json_handler.read_json_file(REGISTRY_PATH) + if not isinstance(data.get("projects"), dict): + return {"version": 1, "projects": {}} + return data + except (json.JSONDecodeError, OSError) as exc: + logger.error("[HOOKS] bad trust registry %s: %s", REGISTRY_PATH, exc) + return {"version": 1, "projects": {}} + + +def _write_registry(registry: dict) -> None: + """Write the registry to disk, creating parent dirs if needed.""" + REGISTRY_PATH.parent.mkdir(parents=True, exist_ok=True) + json_handler.write_json_file(REGISTRY_PATH, registry) + + +def enroll(project_dir: str) -> bool: + """Enroll a project in the trusted registry. Returns True on success.""" + project_path = Path(project_dir).resolve() + config_path = project_path / ".aipass" / "hooks.json" + if not config_path.exists(): + logger.warning("[HOOKS] cannot enroll %s: no .aipass/hooks.json", project_path) + return False + config_hash = _hash_file(config_path) + registry = read_registry() + registry["projects"][str(project_path)] = { + "enrolled": _isoformat_now(), + "config_hash": config_hash, + "config_path": str(config_path), + } + _write_registry(registry) + json_handler.log_operation("enroll", {"project": str(project_path)}, module_name="trust_registry") + logger.info("[HOOKS] enrolled %s (hash=%s)", project_path, config_hash) + return True + + +def revoke(project_dir: str) -> bool: + """Remove a project from the trusted registry. Returns True if it was present.""" + project_path = str(Path(project_dir).resolve()) + registry = read_registry() + if project_path not in registry["projects"]: + return False + del registry["projects"][project_path] + _write_registry(registry) + json_handler.log_operation("revoke", {"project": project_path}, module_name="trust_registry") + logger.info("[HOOKS] revoked %s", project_path) + return True + + +def is_trusted(project_dir: str) -> bool: + """Check if a project is enrolled with a matching config hash.""" + project_path = str(Path(project_dir).resolve()) + registry = read_registry() + entry = registry["projects"].get(project_path) + if entry is None: + return False + config_path = Path(project_dir).resolve() / ".aipass" / "hooks.json" + if not config_path.exists(): + return False + current_hash = _hash_file(config_path) + return current_hash == entry.get("config_hash", "") + + +def bootstrap() -> bool: + """Bootstrap the registry with ONLY the AIPass install. Returns True on success. + + Called when the registry file does not exist. Enrolls the AIPass + install identified by $AIPASS_HOME — never the current CWD. + """ + aipass_home = os.environ.get("AIPASS_HOME", "") + if not aipass_home: + logger.warning("[HOOKS] registry absent and AIPASS_HOME not set — cannot bootstrap") + return False + aipass_path = Path(aipass_home).resolve() + config_path = aipass_path / ".aipass" / "hooks.json" + if not config_path.exists(): + logger.warning( + "[HOOKS] registry absent and AIPass hooks.json not found at %s", + config_path, + ) + return False + config_hash = _hash_file(config_path) + registry = {"version": 1, "projects": {}} + registry["projects"][str(aipass_path)] = { + "enrolled": _isoformat_now(), + "config_hash": config_hash, + "config_path": str(config_path), + } + _write_registry(registry) + json_handler.log_operation("bootstrap", {"aipass_home": str(aipass_path)}, module_name="trust_registry") + logger.info("[HOOKS] registry bootstrapped, enrolled AIPass install: %s", aipass_path) + return True + + +def _isoformat_now() -> str: + """Return current UTC time as ISO string.""" + from datetime import datetime, timezone + + return datetime.now(timezone.utc).isoformat() diff --git a/src/aipass/hooks/apps/handlers/prompt/compass_recall.py b/src/aipass/hooks/apps/handlers/prompt/compass_recall.py new file mode 100644 index 00000000..2fc17eb6 --- /dev/null +++ b/src/aipass/hooks/apps/handlers/prompt/compass_recall.py @@ -0,0 +1,117 @@ +# =================== AIPass ==================== +# Name: compass_recall.py +# Version: 1.0.0 +# Description: Ambient compass recall — surfaces rated decisions on relevant prompts +# Branch: hooks +# Layer: apps/handlers/prompt +# Created: 2026-07-16 +# Modified: 2026-07-16 +# ============================================= + +"""Queries compass FTS against the user's prompt and injects matching decisions +under governance rules. Never blocks the prompt on error.""" + +import json +import os +import tempfile +from pathlib import Path + +from aipass.prax.apps.modules.logger import system_logger as logger +from aipass.hooks.apps.handlers.json import json_handler + +_STATE_DIR = Path(tempfile.gettempdir()) + + +def _state_path(hook_data: dict | None = None) -> Path | None: + session_id = "" + if hook_data: + session_id = hook_data.get("session_id", "") + if not session_id: + session_id = os.environ.get("CLAUDE_CODE_SESSION_ID", "") + if not session_id: + return None + return _STATE_DIR / f"aipass-compass-recall-{session_id}.json" + + +def _load_state(hook_data: dict | None = None) -> dict: + path = _state_path(hook_data) + if path is None or not path.exists(): + return _fresh_state() + try: + return json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError) as exc: + logger.info("[HOOKS] compass_recall: state read failed: %s", exc) + return _fresh_state() + + +def _fresh_state() -> dict: + try: + from aipass.memory.apps.modules.governance import new_state + + return new_state() + except Exception as exc: + logger.info("[HOOKS] compass_recall: governance import failed: %s", exc) + return {"surfaces_count": 0, "messages_since_last": 0, "last_surface_time": 0.0, "surfaced_ids": []} + + +def _save_state(state: dict, hook_data: dict | None = None) -> None: + path = _state_path(hook_data) + if path is None: + return + try: + path.write_text(json.dumps(state), encoding="utf-8") + except OSError as exc: + logger.info("[HOOKS] compass_recall: state write failed: %s", exc) + + +def handle(hook_data: dict) -> dict: + """Surface relevant compass decisions into the prompt context.""" + try: + if not _state_path(hook_data): + return {"stdout": "", "exit_code": 0} + + state = _load_state(hook_data) + + from aipass.memory.apps.modules.governance import should_surface, record_message + + state = record_message(state) + + prompt_text = hook_data.get("prompt", "") + if not prompt_text or len(prompt_text) < 10: + _save_state(state, hook_data) + return {"stdout": "", "exit_code": 0} + + from aipass.devpulse.apps.modules.compass import recall_decisions, mark_surfaced + + candidates = recall_decisions(prompt_text, limit=3) + if not candidates: + _save_state(state, hook_data) + return {"stdout": "", "exit_code": 0} + + approved = [] + for c in candidates: + item_id = str(c["id"]) + relevance = c.get("relevance", 0.0) + surface, reason, new_st = should_surface(item_id, relevance, state) + if surface: + approved.append(c) + state = new_st + + _save_state(state, hook_data) + + if not approved: + return {"stdout": "", "exit_code": 0} + + lines = [] + for c in approved: + rating = c.get("rating", "good").upper() + lines.append(f"[{rating}] #{c['id']}: {c['decision']}") + + mark_surfaced([c["id"] for c in approved]) + json_handler.log_operation("compass_recall", {"count": len(approved)}) + + return {"stdout": "\n".join(lines), "exit_code": 0} + + except Exception as exc: + logger.info("[HOOKS] compass_recall_unreachable: %s", exc) + return {"stdout": "", "exit_code": 0} diff --git a/src/aipass/hooks/apps/modules/engine.py b/src/aipass/hooks/apps/modules/engine.py index 3c8d936f..b2583f40 100644 --- a/src/aipass/hooks/apps/modules/engine.py +++ b/src/aipass/hooks/apps/modules/engine.py @@ -14,6 +14,7 @@ import importlib import json import os import subprocess +import tempfile import time from pathlib import Path @@ -65,6 +66,18 @@ def _run_handler(handler_path: str, hook_data: dict) -> dict: start = time.monotonic() try: module_path, func_name = handler_path.rsplit(".", 1) + if not module_path.startswith("aipass."): + elapsed_ms = (time.monotonic() - start) * 1000 + logger.warning( + "[HOOKS] handler path refused (not in aipass.* namespace): %s", + handler_path, + ) + return { + "exit_code": -1, + "stdout": "", + "stderr": f"handler namespace refused: {handler_path}", + "elapsed_ms": round(elapsed_ms, 1), + } module = importlib.import_module(module_path) handler_func = getattr(module, func_name) result = handler_func(hook_data) @@ -89,6 +102,63 @@ def _matches(matcher: str, value: str) -> bool: return value in matcher.split("|") +_BUDGET_KEYS = ("max_per_session", "min_spacing_turns", "cooldown_seconds") + + +def _budget_state_path(session_id: str = "") -> Path | None: + if not session_id: + session_id = os.environ.get("CLAUDE_CODE_SESSION_ID", "") + if not session_id: + return None + return Path(tempfile.gettempdir()) / f"aipass-handler-budget-{session_id}.json" + + +def _load_budget_state(session_id: str = "") -> dict: + path = _budget_state_path(session_id) + if path is None or not path.exists(): + return {} + try: + return json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError) as exc: + logger.info("[HOOKS] budget: state read failed: %s", exc) + return {} + + +def _save_budget_state(state: dict, session_id: str = "") -> None: + path = _budget_state_path(session_id) + if path is None: + return + try: + path.write_text(json.dumps(state), encoding="utf-8") + except OSError as exc: + logger.info("[HOOKS] budget: state write failed: %s", exc) + + +def _check_budget(hook_name: str, budget_cfg: dict, budget_state: dict) -> tuple[bool, str]: + """Check if handler is within its per-session budget.""" + hs = budget_state.get(hook_name, {}) + fire_count = hs.get("fire_count", 0) + + max_fires = budget_cfg.get("max_per_session") + if max_fires is not None and fire_count >= max_fires: + return False, f"budget exhausted ({fire_count}/{max_fires})" + + if fire_count > 0: + min_spacing = budget_cfg.get("min_spacing_turns") + if min_spacing is not None: + turns_since = hs.get("turns_since_fire", 0) + if turns_since < min_spacing: + return False, f"spacing ({turns_since}/{min_spacing})" + + cooldown = budget_cfg.get("cooldown_seconds") + if cooldown is not None: + elapsed = time.time() - hs.get("last_fire_time", 0.0) + if elapsed < cooldown: + return False, f"cooldown ({int(cooldown - elapsed)}s)" + + return True, "ok" + + def dispatch(event_type: str, stdin_data: str, config: dict) -> tuple[str, int]: """Core dispatch — run hooks for event, return (merged_stdout, exit_code).""" if not config.get("hooks_enabled", True): @@ -111,6 +181,9 @@ def dispatch(event_type: str, stdin_data: str, config: dict) -> tuple[str, int]: outputs = [] total_start = time.monotonic() + budget_state = None + budget_dirty = False + payload_session_id = parsed.get("session_id", "") for hook_name, hook_def in event_hooks.items(): if not hook_def.get("enabled", True): @@ -137,6 +210,43 @@ def dispatch(event_type: str, stdin_data: str, config: dict) -> tuple[str, int]: ) continue + if command and not handler and config.get("_source") == "project": + logger.warning( + "[HOOKS] %s.%s REFUSED: command-type not allowed in per-project config", + event_type, + hook_name, + ) + _log( + { + "ts": time.time(), + "event": event_type, + "hook": hook_name, + "action": "refused_command_type", + } + ) + continue + + budget_cfg = {k: hook_def[k] for k in _BUDGET_KEYS if k in hook_def} + if budget_cfg: + if budget_state is None: + budget_state = _load_budget_state(payload_session_id) + hs = budget_state.setdefault(hook_name, {}) + hs["turns_since_fire"] = hs.get("turns_since_fire", 0) + 1 + budget_dirty = True + allowed, reason = _check_budget(hook_name, budget_cfg, budget_state) + if not allowed: + logger.info("[HOOKS] %s.%s budget: %s", event_type, hook_name, reason) + _log( + { + "ts": time.time(), + "event": event_type, + "hook": hook_name, + "action": "budget_suppressed", + "reason": reason, + } + ) + continue + if handler: result = _run_handler(handler, parsed) else: @@ -216,6 +326,15 @@ def dispatch(event_type: str, stdin_data: str, config: dict) -> tuple[str, int]: if result["stdout"]: outputs.append(result["stdout"]) + if budget_cfg and budget_state is not None: + hs = budget_state.setdefault(hook_name, {}) + hs["fire_count"] = hs.get("fire_count", 0) + 1 + hs["last_fire_time"] = time.time() + hs["turns_since_fire"] = 0 + budget_dirty = True + + if budget_dirty and budget_state is not None: + _save_budget_state(budget_state, payload_session_id) total_ms = (time.monotonic() - total_start) * 1000 logger.info("[HOOKS] %s complete: %d hooks %dms", event_type, len(outputs), total_ms) diff --git a/src/aipass/hooks/tests/test_compass_recall.py b/src/aipass/hooks/tests/test_compass_recall.py new file mode 100644 index 00000000..d9b4b35d --- /dev/null +++ b/src/aipass/hooks/tests/test_compass_recall.py @@ -0,0 +1,588 @@ +# =================== AIPass ==================== +# Name: test_compass_recall.py +# Version: 1.1.0 +# Description: Tests for compass recall prompt handler +# Branch: hooks +# Created: 2026-07-16 +# Modified: 2026-07-16 +# ============================================= + +"""Tests for handlers/prompt/compass_recall.py.""" + +import json +import os +from unittest.mock import patch + + +CANDIDATE_GOOD = { + "id": 56, + "rating": "good", + "decision": "Never hardcode config in prompts", + "context": "Prompt config management", + "note": "", + "tags": "config,prompts", + "relevance": 0.7, +} + +CANDIDATE_BAD = { + "id": 84, + "rating": "bad", + "decision": "Usage gap is not a bug", + "context": "Compass audit", + "note": "", + "tags": "compass", + "relevance": 0.5, +} + +CANDIDATE_LOW_RELEVANCE = { + "id": 99, + "rating": "good", + "decision": "Some low relevance decision", + "context": "Testing", + "note": "", + "tags": "test", + "relevance": 0.1, +} + +REAL_PAYLOAD = { + "session_id": "abc-123-def", + "transcript_path": "/tmp/transcript.jsonl", + "cwd": "/home/user/project", + "permission_mode": "default", + "hook_event_name": "UserPromptSubmit", + "prompt": "How should we handle prompt config?", +} + + +def _payload(prompt, session_id="test-session"): + """Build a realistic hook payload with documented keys.""" + return {"session_id": session_id, "prompt": prompt, "cwd": "/tmp"} + + +class TestCompassRecallHandler: + def test_surfaces_relevant_decision(self, tmp_path): + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[CANDIDATE_GOOD], + ), + patch( + "aipass.devpulse.apps.modules.compass.mark_surfaced", + return_value=1, + ) as mock_mark, + patch( + "aipass.memory.apps.modules.governance.should_surface", + return_value=( + True, + "Ready to surface", + { + "surfaces_count": 1, + "messages_since_last": 0, + "last_surface_time": 1000.0, + "surfaced_ids": ["56"], + }, + ), + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("How should we handle prompt config?")) + + assert result["exit_code"] == 0 + assert "[GOOD] #56:" in result["stdout"] + assert "Never hardcode config in prompts" in result["stdout"] + mock_mark.assert_called_once_with([56]) + + def test_formats_bad_rating(self, tmp_path): + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[CANDIDATE_BAD], + ), + patch( + "aipass.devpulse.apps.modules.compass.mark_surfaced", + return_value=1, + ), + patch( + "aipass.memory.apps.modules.governance.should_surface", + return_value=( + True, + "Ready", + { + "surfaces_count": 1, + "messages_since_last": 0, + "last_surface_time": 1000.0, + "surfaced_ids": ["84"], + }, + ), + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("Is the usage gap a real bug?")) + + assert "[BAD] #84:" in result["stdout"] + + def test_empty_when_no_candidates(self, tmp_path): + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[], + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("Some prompt about something")) + + assert result["exit_code"] == 0 + assert result["stdout"] == "" + + def test_empty_when_governance_suppresses(self, tmp_path): + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[CANDIDATE_GOOD], + ), + patch( + "aipass.memory.apps.modules.governance.should_surface", + return_value=( + False, + "Spacing not met", + { + "surfaces_count": 0, + "messages_since_last": 1, + "last_surface_time": 0.0, + "surfaced_ids": [], + }, + ), + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("How should we handle prompt config?")) + + assert result["exit_code"] == 0 + assert result["stdout"] == "" + + def test_empty_when_prompt_too_short(self, tmp_path): + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("Hi")) + + assert result["exit_code"] == 0 + assert result["stdout"] == "" + + def test_never_blocks_on_import_error(self, tmp_path): + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._state_path", + return_value=tmp_path / "state.json", + ), + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._load_state", + side_effect=Exception("DB locked"), + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("Some prompt about something important")) + + assert result["exit_code"] == 0 + assert result["stdout"] == "" + + def test_persists_governance_state(self, tmp_path): + updated_state = { + "surfaces_count": 1, + "messages_since_last": 0, + "last_surface_time": 1000.0, + "surfaced_ids": ["56"], + } + + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[CANDIDATE_GOOD], + ), + patch( + "aipass.devpulse.apps.modules.compass.mark_surfaced", + return_value=1, + ), + patch( + "aipass.memory.apps.modules.governance.should_surface", + return_value=(True, "Ready", updated_state), + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + handle(_payload("How should we handle prompt config?", session_id="test-persist")) + + state_file = tmp_path / "aipass-compass-recall-test-persist.json" + assert state_file.exists() + saved = json.loads(state_file.read_text()) + assert saved["surfaces_count"] == 1 + assert "56" in saved["surfaced_ids"] + + def test_multiple_candidates_partial_approval(self, tmp_path): + def mock_should_surface(item_id, relevance, state, config=None, *, current_time=None): + if item_id == "56": + new_st = {**state, "surfaces_count": 1, "surfaced_ids": list(state.get("surfaced_ids", [])) + ["56"]} + return True, "Ready", new_st + return False, "Below threshold", state + + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[CANDIDATE_GOOD, CANDIDATE_LOW_RELEVANCE], + ), + patch( + "aipass.devpulse.apps.modules.compass.mark_surfaced", + return_value=1, + ) as mock_mark, + patch( + "aipass.memory.apps.modules.governance.should_surface", + side_effect=mock_should_surface, + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("How should we handle prompt config?")) + + assert "[GOOD] #56:" in result["stdout"] + assert "#99" not in result["stdout"] + mock_mark.assert_called_once_with([56]) + + def test_empty_prompt_no_cross_branch_import(self, tmp_path): + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(_payload("")) + + assert result["exit_code"] == 0 + assert result["stdout"] == "" + + def test_no_session_id_degrades_safe(self): + """No session_id in payload or env = no injection, no crash.""" + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("CLAUDE_CODE_SESSION_ID", None) + result = handle({"prompt": "How should we handle prompt config?"}) + + assert result["exit_code"] == 0 + assert result["stdout"] == "" + + def test_real_documented_payload_shape(self, tmp_path): + """Surfaces from a payload using the official Claude Code hook keys.""" + with ( + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[CANDIDATE_GOOD], + ), + patch( + "aipass.devpulse.apps.modules.compass.mark_surfaced", + return_value=1, + ), + patch( + "aipass.memory.apps.modules.governance.should_surface", + return_value=( + True, + "Ready", + { + "surfaces_count": 1, + "messages_since_last": 0, + "last_surface_time": 1000.0, + "surfaced_ids": ["56"], + }, + ), + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle(REAL_PAYLOAD) + + assert result["exit_code"] == 0 + assert "[GOOD] #56:" in result["stdout"] + + def test_env_var_fallback_for_session_id(self, tmp_path): + """Falls back to CLAUDE_CODE_SESSION_ID env var if payload has no session_id.""" + with ( + patch.dict(os.environ, {"CLAUDE_CODE_SESSION_ID": "env-fallback"}), + patch( + "aipass.hooks.apps.handlers.prompt.compass_recall._STATE_DIR", + tmp_path, + ), + patch( + "aipass.devpulse.apps.modules.compass.recall_decisions", + return_value=[CANDIDATE_GOOD], + ), + patch( + "aipass.devpulse.apps.modules.compass.mark_surfaced", + return_value=1, + ), + patch( + "aipass.memory.apps.modules.governance.should_surface", + return_value=( + True, + "Ready", + { + "surfaces_count": 1, + "messages_since_last": 0, + "last_surface_time": 1000.0, + "surfaced_ids": ["56"], + }, + ), + ), + patch( + "aipass.memory.apps.modules.governance.record_message", + side_effect=lambda s: {**s, "messages_since_last": s.get("messages_since_last", 0) + 1}, + ), + ): + from aipass.hooks.apps.handlers.prompt.compass_recall import handle + + result = handle({"prompt": "How should we handle prompt config?"}) + + assert "[GOOD] #56:" in result["stdout"] + state_file = tmp_path / "aipass-compass-recall-env-fallback.json" + assert state_file.exists() + + +class TestEngineBudget: + def test_check_budget_allows_first_fire(self): + from aipass.hooks.apps.modules.engine import _check_budget + + allowed, reason = _check_budget("test_hook", {"max_per_session": 5}, {}) + assert allowed is True + + def test_check_budget_blocks_when_exhausted(self): + from aipass.hooks.apps.modules.engine import _check_budget + + state = {"test_hook": {"fire_count": 5}} + allowed, reason = _check_budget("test_hook", {"max_per_session": 5}, state) + assert allowed is False + assert "exhausted" in reason + + def test_check_budget_spacing_skipped_on_first_fire(self): + from aipass.hooks.apps.modules.engine import _check_budget + + state = {"test_hook": {"fire_count": 0, "turns_since_fire": 0}} + allowed, reason = _check_budget("test_hook", {"min_spacing_turns": 10}, state) + assert allowed is True + + def test_check_budget_spacing_enforced_after_fire(self): + from aipass.hooks.apps.modules.engine import _check_budget + + state = {"test_hook": {"fire_count": 1, "turns_since_fire": 3}} + allowed, reason = _check_budget("test_hook", {"min_spacing_turns": 10}, state) + assert allowed is False + assert "spacing" in reason + + def test_check_budget_spacing_passes_after_enough_turns(self): + from aipass.hooks.apps.modules.engine import _check_budget + + state = {"test_hook": {"fire_count": 1, "turns_since_fire": 10}} + allowed, reason = _check_budget("test_hook", {"min_spacing_turns": 10}, state) + assert allowed is True + + def test_check_budget_cooldown_enforced(self): + import time + + from aipass.hooks.apps.modules.engine import _check_budget + + state = {"test_hook": {"fire_count": 1, "last_fire_time": time.time() - 10}} + allowed, reason = _check_budget("test_hook", {"cooldown_seconds": 300}, state) + assert allowed is False + assert "cooldown" in reason + + def test_check_budget_cooldown_expired(self): + import time + + from aipass.hooks.apps.modules.engine import _check_budget + + state = {"test_hook": {"fire_count": 1, "last_fire_time": time.time() - 400}} + allowed, reason = _check_budget("test_hook", {"cooldown_seconds": 300}, state) + assert allowed is True + + def test_budget_state_persistence(self, tmp_path): + from aipass.hooks.apps.modules.engine import ( + _load_budget_state, + _save_budget_state, + ) + + state = {"compass_recall": {"fire_count": 2, "last_fire_time": 1000.0, "turns_since_fire": 5}} + + with patch("aipass.hooks.apps.modules.engine._budget_state_path", return_value=tmp_path / "budget.json"): + _save_budget_state(state) + loaded = _load_budget_state() + assert loaded["compass_recall"]["fire_count"] == 2 + + def test_budget_state_missing_returns_empty(self, tmp_path): + from aipass.hooks.apps.modules.engine import _load_budget_state + + with patch( + "aipass.hooks.apps.modules.engine._budget_state_path", + return_value=tmp_path / "nonexistent.json", + ): + assert _load_budget_state() == {} + + def test_dispatch_suppresses_over_budget_handler(self, tmp_path): + from aipass.hooks.apps.modules.engine import dispatch + + config = { + "hooks_enabled": True, + "UserPromptSubmit": { + "test_hook": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.prompt.compass_recall.handle", + "max_per_session": 0, + }, + }, + } + budget_file = tmp_path / "budget.json" + + with ( + patch("aipass.hooks.apps.modules.engine._budget_state_path", return_value=budget_file), + patch("aipass.hooks.apps.modules.engine._run_handler") as mock_run, + ): + dispatch("UserPromptSubmit", json.dumps({"session_id": "budget-test", "prompt": "test"}), config) + mock_run.assert_not_called() + + def test_dispatch_records_fire_on_output(self, tmp_path): + from aipass.hooks.apps.modules.engine import dispatch + + config = { + "hooks_enabled": True, + "UserPromptSubmit": { + "test_hook": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.prompt.compass_recall.handle", + "max_per_session": 10, + }, + }, + } + budget_file = tmp_path / "budget.json" + + with ( + patch("aipass.hooks.apps.modules.engine._budget_state_path", return_value=budget_file), + patch( + "aipass.hooks.apps.modules.engine._run_handler", + return_value={"exit_code": 0, "stdout": "[GOOD] #56: test", "stderr": "", "elapsed_ms": 5.0}, + ), + ): + dispatch("UserPromptSubmit", json.dumps({"session_id": "fire-test", "prompt": "test"}), config) + + assert budget_file.exists() + state = json.loads(budget_file.read_text()) + assert state["test_hook"]["fire_count"] == 1 + assert state["test_hook"]["turns_since_fire"] == 0 + + def test_dispatch_threads_payload_session_id(self, tmp_path): + """Budget state file is keyed by payload session_id, not env var.""" + from aipass.hooks.apps.modules.engine import dispatch + + config = { + "hooks_enabled": True, + "UserPromptSubmit": { + "test_hook": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.prompt.compass_recall.handle", + "max_per_session": 10, + }, + }, + } + + with ( + patch( + "aipass.hooks.apps.modules.engine._run_handler", + return_value={"exit_code": 0, "stdout": "output", "stderr": "", "elapsed_ms": 1.0}, + ), + ): + dispatch( + "UserPromptSubmit", + json.dumps({"session_id": "payload-sid", "prompt": "test"}), + config, + ) + + from aipass.hooks.apps.modules.engine import _budget_state_path + + path = _budget_state_path("payload-sid") + assert path is not None + assert "payload-sid" in str(path) diff --git a/src/aipass/hooks/tests/test_engine.py b/src/aipass/hooks/tests/test_engine.py index 365948d2..78272fbb 100644 --- a/src/aipass/hooks/tests/test_engine.py +++ b/src/aipass/hooks/tests/test_engine.py @@ -23,6 +23,7 @@ from aipass.hooks.apps.modules.engine import ( _log, ) from aipass.hooks.apps.handlers.config.loader import find_project_config +from aipass.hooks.apps.handlers.config.trust_registry import enroll class TestMatches: @@ -276,7 +277,13 @@ class TestFindProjectConfig: """Tests for find_project_config() CWD walk.""" def test_finds_config_in_cwd(self, hooks_config_file, temp_test_dir, mock_logger): - with patch("aipass.hooks.apps.modules.engine.Path.cwd", return_value=temp_test_dir): + reg_path = temp_test_dir / "registry.json" + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(temp_test_dir)) + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=temp_test_dir), + ): config = find_project_config() assert config is not None assert config["hooks_enabled"] is True @@ -295,9 +302,15 @@ class TestFindProjectConfig: "Stop": {"sound": {"enabled": True, "command": "python3 $AIPASS_HOME/hook.py", "matcher": ""}}, } (config_dir / "hooks.json").write_text(json.dumps(config)) - with patch("aipass.hooks.apps.modules.engine.Path.cwd", return_value=temp_test_dir): - with patch("aipass.hooks.apps.handlers.config.loader.AIPASS_HOME", "/test/path"): - result = find_project_config() + reg_path = temp_test_dir / "registry.json" + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(temp_test_dir)) + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=temp_test_dir), + patch("aipass.hooks.apps.handlers.config.loader.AIPASS_HOME", "/test/path"), + ): + result = find_project_config() assert result is not None assert "/test/path/hook.py" in result["Stop"]["sound"]["command"] @@ -502,7 +515,13 @@ class TestInitProvisioning: (config_dir / "hooks.json").write_text('{"hooks_enabled": true}') sub_dir = temp_test_dir / "deep" / "nested" / "path" sub_dir.mkdir(parents=True) - with patch("aipass.hooks.apps.modules.engine.Path.cwd", return_value=sub_dir): + reg_path = temp_test_dir / "registry.json" + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(temp_test_dir)) + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=sub_dir), + ): config = find_project_config() assert config is not None assert config["hooks_enabled"] is True @@ -721,6 +740,153 @@ class TestMockInfrastructure: assert hasattr(engine, "_run_hook") +class TestLayerATrustEnforcement: + """DPLAN-0244 Layer A: engine refuses command-type from project config, enforces handler namespace.""" + + def test_command_type_refused_from_project_config(self, mock_logger): + config = { + "hooks_enabled": True, + "_source": "project", + "PreToolUse": { + "evil_cmd": { + "enabled": True, + "command": "echo PWNED", + "matcher": "", + } + }, + } + with patch("aipass.hooks.apps.modules.engine._log") as mock_log: + with patch("aipass.hooks.apps.modules.engine._run_hook") as mock_run: + result = dispatch("PreToolUse", '{"tool_name":"Edit"}', config) + mock_run.assert_not_called() + assert result == ("", 0) + log_calls = [c[0][0] for c in mock_log.call_args_list] + assert any(e.get("action") == "refused_command_type" for e in log_calls if isinstance(e, dict)) + + def test_handler_namespace_enforced(self, mock_logger): + from aipass.hooks.apps.modules.engine import _run_handler + + result = _run_handler("evil.payload.handle", {}) + assert result["exit_code"] == -1 + assert "namespace refused" in result["stderr"] + + def test_handler_aipass_namespace_allowed(self, mock_logger): + from aipass.hooks.apps.modules.engine import _run_handler + + mock_handler = MagicMock(return_value={"exit_code": 0, "stdout": "ok"}) + mock_module = MagicMock() + mock_module.handle = mock_handler + with patch("importlib.import_module", return_value=mock_module): + result = _run_handler("aipass.hooks.apps.handlers.notification.stop_sound.handle", {}) + assert result["exit_code"] == 0 + + def test_command_type_allowed_from_default_config(self, mock_logger): + config = { + "hooks_enabled": True, + "_source": "default", + "Stop": { + "cmd_hook": { + "enabled": True, + "command": "echo allowed", + "matcher": "", + } + }, + } + with patch("aipass.hooks.apps.modules.engine._log"): + with patch("aipass.hooks.apps.modules.engine._run_hook") as mock_run: + mock_run.return_value = { + "exit_code": 0, + "stdout": "allowed", + "stderr": "", + "elapsed_ms": 5, + } + result = dispatch("Stop", "{}", config) + mock_run.assert_called_once() + assert "allowed" in result[0] + + def test_mixed_config_partial_refusal(self, mock_logger): + config = { + "hooks_enabled": True, + "_source": "project", + "UserPromptSubmit": { + "good_handler": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.notification.stop_sound.handle", + "matcher": "", + }, + "evil_cmd": { + "enabled": True, + "command": "echo PWNED", + "matcher": "", + }, + }, + } + mock_handler_func = MagicMock(return_value={"exit_code": 0, "stdout": "handler_ok"}) + mock_module = MagicMock() + mock_module.handle = mock_handler_func + with patch("aipass.hooks.apps.modules.engine._log"): + with patch("importlib.import_module", return_value=mock_module): + with patch("aipass.hooks.apps.modules.engine._run_hook") as mock_run: + result = dispatch("UserPromptSubmit", "{}", config) + mock_run.assert_not_called() + assert "handler_ok" in result[0] + assert result[1] == 0 + + def test_source_overwrite_not_merge(self, temp_test_dir, mock_logger): + config_dir = temp_test_dir / ".aipass" + config_dir.mkdir() + hostile_config = { + "hooks_enabled": True, + "_source": "provider", + "SessionStart": { + "evil": { + "enabled": True, + "command": "echo PWNED", + "matcher": "", + } + }, + } + (config_dir / "hooks.json").write_text(json.dumps(hostile_config)) + reg_path = temp_test_dir / "registry.json" + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(temp_test_dir)) + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=temp_test_dir), + ): + loaded = find_project_config() + assert loaded is not None + assert loaded["_source"] == "project" + with patch("aipass.hooks.apps.modules.engine._log"): + with patch("aipass.hooks.apps.modules.engine._run_hook") as mock_run: + result = dispatch("SessionStart", "{}", loaded) + mock_run.assert_not_called() + assert result == ("", 0) + + def test_command_without_source_defaults_allowed(self, mock_logger): + config = { + "hooks_enabled": True, + "Stop": { + "cmd_hook": { + "enabled": True, + "command": "echo ok", + "matcher": "", + } + }, + } + with patch("aipass.hooks.apps.modules.engine._log"): + with patch("aipass.hooks.apps.modules.engine._run_hook") as mock_run: + mock_run.return_value = { + "exit_code": 0, + "stdout": "ok", + "stderr": "", + "elapsed_ms": 5, + } + result = dispatch("Stop", "{}", config) + mock_run.assert_called_once() + assert "ok" in result[0] + + class TestJsonHandlerNotApplicable: """Hooks uses JSONL logging, not json_handler. These verify the log equivalent.""" diff --git a/src/aipass/hooks/tests/test_trust_registry.py b/src/aipass/hooks/tests/test_trust_registry.py new file mode 100644 index 00000000..5487fd0d --- /dev/null +++ b/src/aipass/hooks/tests/test_trust_registry.py @@ -0,0 +1,320 @@ +# =================== AIPass ==================== +# Name: test_trust_registry.py +# Version: 1.0.0 +# Description: Tests for trusted-project registry — DPLAN-0244 Layer B +# Branch: hooks +# Layer: tests +# Created: 2026-07-15 +# Modified: 2026-07-15 +# ============================================= + +"""Tests for trusted-project registry and loader trust integration.""" + +import json +from unittest.mock import patch + +from aipass.hooks.apps.handlers.config.trust_registry import ( + _hash_file, + bootstrap, + enroll, + is_trusted, + read_registry, + revoke, +) +from aipass.hooks.apps.handlers.config.loader import find_project_config + + +class TestRegistryHelpers: + """Unit tests for registry helper functions.""" + + def test_hash_file_deterministic(self, temp_test_dir): + f = temp_test_dir / "test.json" + f.write_text('{"hello": "world"}') + h1 = _hash_file(f) + h2 = _hash_file(f) + assert h1 == h2 + assert h1.startswith("sha256:") + + def test_hash_file_changes_on_content_change(self, temp_test_dir): + f = temp_test_dir / "test.json" + f.write_text('{"v": 1}') + h1 = _hash_file(f) + f.write_text('{"v": 2}') + h2 = _hash_file(f) + assert h1 != h2 + + def test_read_registry_absent(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "nonexistent.json" + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + result = read_registry() + assert result == {"version": 1, "projects": {}} + + def test_read_registry_valid(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + reg_data = { + "version": 1, + "projects": { + "/some/path": { + "enrolled": "2026-07-15T00:00:00", + "config_hash": "sha256:abc", + "config_path": "/some/path/.aipass/hooks.json", + } + }, + } + reg_path.write_text(json.dumps(reg_data)) + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + result = read_registry() + assert "/some/path" in result["projects"] + + def test_read_registry_corrupt(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + reg_path.write_text("{corrupt!!!") + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + result = read_registry() + assert result == {"version": 1, "projects": {}} + + +class TestEnrollRevoke: + """Unit tests for enroll() and revoke().""" + + def test_enroll_success(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + project = temp_test_dir / "myproject" + project.mkdir() + (project / ".aipass").mkdir() + (project / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + result = enroll(str(project)) + assert result is True + assert reg_path.exists() + data = json.loads(reg_path.read_text()) + assert str(project.resolve()) in data["projects"] + entry = data["projects"][str(project.resolve())] + assert entry["config_hash"].startswith("sha256:") + + def test_enroll_no_hooks_json(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + project = temp_test_dir / "empty_project" + project.mkdir() + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + result = enroll(str(project)) + assert result is False + + def test_revoke_success(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + project = temp_test_dir / "myproject" + project.mkdir() + (project / ".aipass").mkdir() + (project / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(project)) + result = revoke(str(project)) + assert result is True + data = json.loads(reg_path.read_text()) + assert str(project.resolve()) not in data["projects"] + + def test_revoke_nonexistent(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + result = revoke("/nonexistent/project") + assert result is False + + +class TestIsTrusted: + """Unit tests for is_trusted().""" + + def test_trusted_with_matching_hash(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + project = temp_test_dir / "myproject" + project.mkdir() + (project / ".aipass").mkdir() + (project / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(project)) + assert is_trusted(str(project)) is True + + def test_not_trusted_unregistered(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + assert is_trusted("/not/registered") is False + + def test_not_trusted_hash_mismatch(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + project = temp_test_dir / "myproject" + project.mkdir() + (project / ".aipass").mkdir() + hooks_file = project / ".aipass" / "hooks.json" + hooks_file.write_text('{"hooks_enabled": true}') + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(project)) + hooks_file.write_text('{"hooks_enabled": true, "tampered": true}') + assert is_trusted(str(project)) is False + + +class TestBootstrap: + """Tests for bootstrap() — enrolls ONLY AIPASS_HOME.""" + + def test_bootstrap_enrolls_aipass_home(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + aipass_dir = temp_test_dir / "aipass_install" + aipass_dir.mkdir() + (aipass_dir / ".aipass").mkdir() + (aipass_dir / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch.dict("os.environ", {"AIPASS_HOME": str(aipass_dir)}), + ): + result = bootstrap() + assert result is True + data = json.loads(reg_path.read_text()) + assert str(aipass_dir.resolve()) in data["projects"] + + def test_bootstrap_no_aipass_home(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch.dict("os.environ", {}, clear=True), + ): + result = bootstrap() + assert result is False + assert not reg_path.exists() + + def test_bootstrap_aipass_home_no_hooks_json(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + aipass_dir = temp_test_dir / "empty_install" + aipass_dir.mkdir() + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch.dict("os.environ", {"AIPASS_HOME": str(aipass_dir)}), + ): + result = bootstrap() + assert result is False + + def test_bootstrap_refuses_hostile_project(self, temp_test_dir, mock_logger): + """Security-critical: registry absent + first event in hostile CWD. + + Only AIPASS_HOME gets enrolled, hostile project is NOT enrolled. + """ + reg_path = temp_test_dir / "registry.json" + + aipass_dir = temp_test_dir / "real_aipass" + aipass_dir.mkdir() + (aipass_dir / ".aipass").mkdir() + (aipass_dir / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + + hostile_dir = temp_test_dir / "hostile_repo" + hostile_dir.mkdir() + (hostile_dir / ".aipass").mkdir() + (hostile_dir / ".aipass" / "hooks.json").write_text( + '{"hooks_enabled": true, "SessionStart": ' + '{"evil": {"enabled": true, "command": "touch /tmp/pwned", "matcher": ""}}}' + ) + + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch.dict("os.environ", {"AIPASS_HOME": str(aipass_dir)}), + ): + result = bootstrap() + assert result is True + + data = json.loads(reg_path.read_text()) + assert str(aipass_dir.resolve()) in data["projects"] + assert str(hostile_dir.resolve()) not in data["projects"] + + assert is_trusted(str(hostile_dir)) is False + assert is_trusted(str(aipass_dir)) is True + + +class TestLoaderTrustIntegration: + """Integration tests: loader.find_project_config() with registry.""" + + def test_registered_project_loads(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + project = temp_test_dir / "trusted_project" + project.mkdir() + (project / ".aipass").mkdir() + (project / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(project)) + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=project), + ): + config = find_project_config() + assert config is not None + assert config["hooks_enabled"] is True + assert config["_source"] == "project" + + def test_unregistered_project_skipped(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + reg_path.write_text('{"version": 1, "projects": {}}') + project = temp_test_dir / "unknown_project" + project.mkdir() + (project / ".aipass").mkdir() + (project / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=project), + ): + config = find_project_config() + assert config is None + + def test_hash_mismatch_skipped(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + project = temp_test_dir / "tampered_project" + project.mkdir() + (project / ".aipass").mkdir() + hooks_file = project / ".aipass" / "hooks.json" + hooks_file.write_text('{"hooks_enabled": true}') + with patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path): + enroll(str(project)) + hooks_file.write_text('{"hooks_enabled": true, "tampered": true}') + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=project), + ): + config = find_project_config() + assert config is None + + def test_loader_bootstraps_on_missing_registry(self, temp_test_dir, mock_logger): + reg_path = temp_test_dir / "registry.json" + aipass_dir = temp_test_dir / "aipass_install" + aipass_dir.mkdir() + (aipass_dir / ".aipass").mkdir() + (aipass_dir / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch.dict("os.environ", {"AIPASS_HOME": str(aipass_dir)}), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=aipass_dir), + ): + config = find_project_config() + assert config is not None + assert reg_path.exists() + + def test_loader_hostile_project_after_bootstrap(self, temp_test_dir, mock_logger): + """Full attack chain: hostile repo, registry absent, bootstrap fires.""" + reg_path = temp_test_dir / "registry.json" + + aipass_dir = temp_test_dir / "real_aipass" + aipass_dir.mkdir() + (aipass_dir / ".aipass").mkdir() + (aipass_dir / ".aipass" / "hooks.json").write_text('{"hooks_enabled": true}') + + hostile_dir = temp_test_dir / "hostile_repo" + hostile_dir.mkdir() + (hostile_dir / ".aipass").mkdir() + (hostile_dir / ".aipass" / "hooks.json").write_text( + '{"hooks_enabled": true, "SessionStart": ' + '{"evil": {"enabled": true, "command": "touch /tmp/pwned", "matcher": ""}}}' + ) + + with ( + patch("aipass.hooks.apps.handlers.config.trust_registry.REGISTRY_PATH", reg_path), + patch.dict("os.environ", {"AIPASS_HOME": str(aipass_dir)}), + patch("aipass.hooks.apps.handlers.config.loader.Path.cwd", return_value=hostile_dir), + ): + config = find_project_config() + assert config is None + assert reg_path.exists() + data = json.loads(reg_path.read_text()) + assert str(hostile_dir.resolve()) not in data["projects"] diff --git a/src/aipass/memory/.seedgo/bypass.json b/src/aipass/memory/.seedgo/bypass.json index 2bff3ff9..b01b259b 100644 --- a/src/aipass/memory/.seedgo/bypass.json +++ b/src/aipass/memory/.seedgo/bypass.json @@ -462,6 +462,16 @@ "functions": ["render_all_meta_tabs"], "reason": "Cross-branch public API — called by @spawn's build_replacements_dict to resolve {{*_META}} placeholders at branch creation." }, + { + "file": "apps/modules/governance.py", + "standard": "unused_function", + "reason": "Cross-branch public API — should_surface, record_message, new_state re-exported for @hooks compass_recall and @devpulse recall query." + }, + { + "file": "apps/handlers/governance/engine.py", + "standard": "unused_function", + "reason": "Implementation functions re-exported via modules/governance.py for cross-branch consumers." + }, { "file": "apps/modules/templates.py", "standard": "deep_nesting", diff --git a/src/aipass/memory/README.md b/src/aipass/memory/README.md index abe78592..c94b06d5 100644 --- a/src/aipass/memory/README.md +++ b/src/aipass/memory/README.md @@ -45,15 +45,17 @@ drone @memory watch # Auto-rollover watcher daemon (Ctrl+ memory/ ├── apps/ │ ├── memory.py # Entry point — auto-discovers modules -│ ├── modules/ # 6 modules +│ ├── modules/ # 7 modules +│ │ ├── governance.py # Surfacing governance — re-exports from handlers │ │ ├── lint.py # Entry limit violation scanner (read-only) │ │ ├── rollover.py # Rollover orchestration, status, sync-lines │ │ ├── search.py # Semantic query routing │ │ ├── symbolic.py # Fragmented memory extraction and search │ │ ├── templates.py # Template push, diff, status │ │ └── verify.py # Plan vectorization check -│ └── handlers/ # 14 handler groups +│ └── handlers/ # 15 handler groups │ ├── archive/ # indexer.py +│ ├── governance/ # engine.py — surfacing decision logic │ ├── intake/ # plans_processor.py, pool_processor.py │ ├── json/ # json_handler.py, memory_files.py, entry_limits.py, lint_handler.py, config_loader.py │ ├── learnings/ # manager.py diff --git a/src/aipass/memory/apps/handlers/governance/__init__.py b/src/aipass/memory/apps/handlers/governance/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/aipass/memory/apps/handlers/governance/engine.py b/src/aipass/memory/apps/handlers/governance/engine.py new file mode 100644 index 00000000..000482ed --- /dev/null +++ b/src/aipass/memory/apps/handlers/governance/engine.py @@ -0,0 +1,125 @@ +# =================== AIPass ==================== +# Name: engine.py +# Description: Surfacing governance engine — implementation +# Version: 1.0.0 +# Created: 2026-07-16 +# Modified: 2026-07-16 +# ============================================= + +""" +Surfacing Governance Engine + +Pure decision functions for controlling when recalled items should be +surfaced. Implementation logic — public API re-exported from +modules/governance.py for cross-branch consumers. +""" + +from typing import Any, Dict, Tuple + +from aipass.prax import logger +from aipass.memory.apps.handlers.json import json_handler + + +# ============================================================================= +# CONSTANTS — default config values +# ============================================================================= + +DEFAULT_CONFIG: Dict[str, Any] = { + "enabled": True, + "threshold": 0.3, + "max_surfaces_per_session": 5, + "min_messages_between": 10, + "cooldown_seconds": 300, +} + + +# ============================================================================= +# STATE FACTORY +# ============================================================================= + + +def new_state() -> Dict[str, Any]: + """Create a fresh governance state dict.""" + return { + "surfaces_count": 0, + "messages_since_last": 0, + "last_surface_time": 0.0, + "surfaced_ids": [], + } + + +# ============================================================================= +# CORE GOVERNANCE +# ============================================================================= + + +def should_surface( + item_id: str, + relevance_score: float, + state: Dict[str, Any], + config: Dict[str, Any] | None = None, + *, + current_time: float | None = None, +) -> Tuple[bool, str, Dict[str, Any]]: + """ + Decide whether an item should be surfaced, given current state. + + Pure function — does not mutate the input state dict. + """ + import time + + cfg = {**DEFAULT_CONFIG, **(config or {})} + now = current_time if current_time is not None else time.time() + + if not cfg.get("enabled", True): + return False, "Surfacing disabled", state + + threshold = cfg.get("threshold", 0.3) + if relevance_score < threshold: + return False, f"Below threshold ({relevance_score:.2f} < {threshold})", state + + max_surfaces = cfg.get("max_surfaces_per_session", 5) + if state.get("surfaces_count", 0) >= max_surfaces: + return False, f"Session budget exhausted ({max_surfaces}/{max_surfaces})", state + + min_messages = cfg.get("min_messages_between", 10) + messages_since = state.get("messages_since_last", 0) + last_time = state.get("last_surface_time", 0.0) + if last_time > 0 and messages_since < min_messages: + return False, f"Spacing not met ({messages_since}/{min_messages} messages)", state + + cooldown = cfg.get("cooldown_seconds", 300) + elapsed = now - last_time + if last_time > 0 and elapsed < cooldown: + remaining = int(cooldown - elapsed) + return False, f"Cooldown active ({remaining}s remaining)", state + + surfaced_ids = state.get("surfaced_ids", []) + if item_id in surfaced_ids: + return False, "Already surfaced this session", state + + updated = { + "surfaces_count": state.get("surfaces_count", 0) + 1, + "messages_since_last": 0, + "last_surface_time": now, + "surfaced_ids": list(surfaced_ids) + [item_id], + } + logger.info(f"[governance] Surfacing {item_id} (score={relevance_score:.2f}, surfaces={updated['surfaces_count']})") + json_handler.log_operation( + "governance_surface", + {"item_id": item_id, "relevance_score": relevance_score, "surfaces_count": updated["surfaces_count"]}, + ) + return True, "Ready to surface", updated + + +# ============================================================================= +# MESSAGE TRACKING +# ============================================================================= + + +def record_message(state: Dict[str, Any]) -> Dict[str, Any]: + """Record that a message was processed. Pure — returns updated state.""" + return { + **state, + "messages_since_last": state.get("messages_since_last", 0) + 1, + } diff --git a/src/aipass/memory/apps/handlers/intake/plans_processor.py b/src/aipass/memory/apps/handlers/intake/plans_processor.py index cb11e0cb..c3417c80 100644 --- a/src/aipass/memory/apps/handlers/intake/plans_processor.py +++ b/src/aipass/memory/apps/handlers/intake/plans_processor.py @@ -164,7 +164,7 @@ def _save_manifest(manifest: Dict[str, str]) -> None: # ============================================================================= -def _embed_texts(texts: List[str]) -> dict: +def _embed_texts(texts: List[str], timeout: int = 120) -> dict: """Encode texts via subprocess.""" input_data = json.dumps({"texts": texts}) try: @@ -173,7 +173,7 @@ def _embed_texts(texts: List[str]) -> dict: input=input_data, capture_output=True, text=True, - timeout=120, + timeout=timeout, ) if result.returncode != 0: return {"success": False, "error": result.stderr or "Embedding failed"} @@ -219,24 +219,17 @@ def process_plans() -> Dict[str, Any]: """ Process plan files from flow/processed_plans/ into vector storage. - Workflow: - 1. Load config to find plans directory - 2. Scan for unprocessed .md files - 3. Chunk each file into sections - 4. Embed all chunks via subprocess - 5. Store vectors in ChromaDB - 6. Update processed manifest + Processes each file independently so partial failure makes partial + progress — manifest is saved after every successful file. Returns: Dict with success, files_processed, total_chunks """ - # Load config plans_config = config_loader.section("plans") if not plans_config.get("enabled", False): return {"success": True, "skipped": True, "reason": "plans disabled"} - # Resolve plans directory (relative to repo root) plans_dir = plans_config.get("path", ".backup/processed_plans") repo_root = _find_repo_root() plans_path = Path(plans_dir) if Path(plans_dir).is_absolute() else repo_root / plans_dir @@ -246,7 +239,6 @@ def process_plans() -> Dict[str, Any]: if not plans_path.exists(): return {"success": True, "files_processed": 0, "total_chunks": 0, "reason": "plans dir not found"} - # Get plan files files = [] for ext in extensions: files.extend(plans_path.glob(f"*{ext}")) @@ -254,7 +246,6 @@ def process_plans() -> Dict[str, Any]: if not files: return {"success": True, "files_processed": 0, "total_chunks": 0} - # Load manifest to skip already-processed files manifest = _load_manifest() unprocessed = [f for f in files if f.name not in manifest] @@ -263,88 +254,73 @@ def process_plans() -> Dict[str, Any]: logger.info(f"[plans] Found {len(unprocessed)} unprocessed plan files") - errors = [] - - # -- Phase 1: Read all files, chunk them, collect texts + metadatas ---------- - all_texts: List[str] = [] - all_metadatas: List[Dict[str, str]] = [] - # Track which files produced chunks (for manifest update) - files_with_chunks: List[Path] = [] - # Files with 0 chunks still get marked in manifest (e.g. template content) - files_without_chunks: List[Path] = [] + errors: List[str] = [] + files_processed = 0 + total_chunks = 0 for plan_file in unprocessed: try: text = plan_file.read_text(encoding="utf-8") except Exception as e: - logger.warning(f"[plans_processor] Failed to read plan file {plan_file.name}: {e}") + logger.warning(f"[plans] Failed to read {plan_file.name}: {e}") errors.append(f"{plan_file.name}: read error: {e}") continue chunks = _chunk_plan_text(text, plan_file.name) if not chunks: - files_without_chunks.append(plan_file) + manifest[plan_file.name] = datetime.now().isoformat() + _save_manifest(manifest) continue - files_with_chunks.append(plan_file) - for c in chunks: - all_texts.append(c["text"]) - all_metadatas.append( - { - "source_file": plan_file.name, - "section": c["section"], - "processed_at": datetime.now().isoformat(), - "type": "plan", - } - ) + texts = [c["text"] for c in chunks] + metadatas = [ + { + "source_file": plan_file.name, + "section": c["section"], + "processed_at": datetime.now().isoformat(), + "type": "plan", + } + for c in chunks + ] - total_chunks = len(all_texts) - files_processed = 0 - - # Mark empty-chunk files in manifest immediately (nothing to embed) - for plan_file in files_without_chunks: - manifest[plan_file.name] = datetime.now().isoformat() - - # -- Phase 2: Batch embed + store (single subprocess each) ------------------ - if all_texts: - logger.info(f"[plans] Batch embedding {total_chunks} chunks from {len(files_with_chunks)} files") - - embed_result = _embed_texts(all_texts) + timeout = max(30, len(texts) * 3) + embed_result = _embed_texts(texts, timeout=timeout) if not embed_result.get("success"): - error_msg = f"batch embed error: {embed_result.get('error')}" - logger.error(f"[plans] {error_msg}") - errors.append(error_msg) - else: - embeddings = embed_result.get("embeddings", []) - if not embeddings: - errors.append("batch embed returned no embeddings") - else: - store_result = _store_vectors(embeddings, all_texts, all_metadatas, collection_name) - if not store_result.get("success"): - error_msg = f"batch store error: {store_result.get('error')}" - logger.error(f"[plans] {error_msg}") - errors.append(error_msg) - else: - # Success — mark all chunk-producing files in manifest - for plan_file in files_with_chunks: - manifest[plan_file.name] = datetime.now().isoformat() - files_processed = len(files_with_chunks) - logger.info(f"[plans] Batch complete: {files_processed} files, {total_chunks} chunks vectorized") + logger.warning(f"[plans] Embed failed for {plan_file.name}: {embed_result.get('error')}") + errors.append(f"{plan_file.name}: embed error: {embed_result.get('error')}") + continue - # Save manifest (includes empty-chunk files even if embedding failed) - _save_manifest(manifest) + embeddings = embed_result.get("embeddings", []) + if not embeddings: + errors.append(f"{plan_file.name}: embed returned no embeddings") + continue + + store_result = _store_vectors(embeddings, texts, metadatas, collection_name) + if not store_result.get("success"): + logger.warning(f"[plans] Store failed for {plan_file.name}: {store_result.get('error')}") + errors.append(f"{plan_file.name}: store error: {store_result.get('error')}") + continue + + manifest[plan_file.name] = datetime.now().isoformat() + _save_manifest(manifest) + files_processed += 1 + total_chunks += len(texts) + logger.info(f"[plans] {plan_file.name}: {len(texts)} chunks vectorized") + + if files_processed > 0: + logger.info(f"[plans] Complete: {files_processed} files, {total_chunks} chunks vectorized") result: Dict[str, Any] = { - "success": files_processed > 0 or (not errors and not files_with_chunks), + "success": files_processed > 0 or not errors, "files_processed": files_processed, - "total_chunks": total_chunks if files_processed > 0 else 0, + "total_chunks": total_chunks, } if errors: result["errors"] = errors json_handler.log_operation( "process_plans", - {"files_processed": files_processed, "total_chunks": result["total_chunks"], "success": result["success"]}, + {"files_processed": files_processed, "total_chunks": total_chunks, "success": result["success"]}, ) return result diff --git a/src/aipass/memory/apps/handlers/search/query_executor.py b/src/aipass/memory/apps/handlers/search/query_executor.py index 22fedf43..c2f74565 100644 --- a/src/aipass/memory/apps/handlers/search/query_executor.py +++ b/src/aipass/memory/apps/handlers/search/query_executor.py @@ -18,6 +18,7 @@ Purpose: layer to satisfy thin-module standard. """ +import re import subprocess import json import os @@ -210,6 +211,77 @@ def _filter_results(results: list, n_results: int) -> list: return filtered +# ============================================================================= +# PLAN-ID EXACT MATCHING +# ============================================================================= + +_PLAN_ID_RE = re.compile( + r"(?:^|\b)((?:d|f|p|td|a)plan)[\s\-_]*(\d{3,5})\b", + re.IGNORECASE, +) + + +def _extract_plan_id(query: str) -> str | None: + """Extract a normalized plan ID (e.g. 'FPLAN-0332') from a query string.""" + m = _PLAN_ID_RE.search(query) + if not m: + return None + prefix = m.group(1).upper() + number = m.group(2) + return f"{prefix}-{number}" + + +def _fetch_plan_by_metadata(plan_id: str, n_results: int) -> list: + """Fetch plan chunks directly from ChromaDB by source_file metadata.""" + input_data = { + "operation": "get_by_source", + "collection_name": "flow_plans", + "source_pattern": plan_id, + "n_results": n_results, + } + try: + result = subprocess.run( + [str(MEMORY_PYTHON), str(CHROMA_SUBPROCESS_SCRIPT)], + input=json.dumps(input_data), + capture_output=True, + text=True, + timeout=30, + ) + if result.returncode != 0: + return [] + data = json.loads(result.stdout) + if not data.get("success"): + return [] + return data.get("results", []) + except Exception as e: + logger.warning(f"[search] Plan metadata fetch failed: {e}") + return [] + + +def _pin_plan_id_matches(query: str, filtered: list, n_results: int) -> list: + """Pin exact plan-ID matches to the top of results. + + If the query contains a plan-ID pattern, fetch matching chunks directly + from ChromaDB metadata (bypassing embedding similarity) and pin them. + """ + plan_id = _extract_plan_id(query) + if not plan_id: + return filtered + + exact = _fetch_plan_by_metadata(plan_id, n_results) + if not exact: + return filtered + + for r in exact: + r["similarity"] = 1.0 + + logger.info(f"[search] Pinned {len(exact)} exact matches for {plan_id}") + + seen_ids = {r.get("id") for r in exact} + rest = [r for r in filtered if r.get("id") not in seen_ids] + return (exact + rest)[:n_results] + + # ============================================================================= # PUBLIC API # ============================================================================= @@ -273,6 +345,9 @@ def execute_search( # Step 3: Filter and score results filtered_results = _filter_results(raw_results, n_results) + # Step 4: Pin exact plan-ID matches to the top + filtered_results = _pin_plan_id_matches(query, filtered_results, n_results) + logger.info(f"[search] Filtered to {len(filtered_results)} relevant results") json_handler.log_operation( diff --git a/src/aipass/memory/apps/handlers/storage/chroma_subprocess.py b/src/aipass/memory/apps/handlers/storage/chroma_subprocess.py index 6a04dcc8..9efd166e 100755 --- a/src/aipass/memory/apps/handlers/storage/chroma_subprocess.py +++ b/src/aipass/memory/apps/handlers/storage/chroma_subprocess.py @@ -67,12 +67,30 @@ def _store_vectors(branch, memory_type, embeddings, documents, metadatas, db_pat embedding_function=None, ) - # Content-hash IDs prevent duplicates across rollover runs - ids = [f"{branch}_{memory_type}_{hashlib.sha256(doc.encode()).hexdigest()[:16]}" for doc in documents] + # Content-hash IDs — idempotent across runs. When metadata carries + # source_file (e.g. plans intake), salt the hash so identical boilerplate + # from different files gets distinct IDs and per-file provenance survives. + ids = [] + for doc, meta in zip(documents, metadatas): + salt = meta.get("source_file", "") if isinstance(meta, dict) else "" + hash_input = f"{salt}:{doc}" if salt else doc + ids.append(f"{branch}_{memory_type}_{hashlib.sha256(hash_input.encode()).hexdigest()[:16]}") # Chroma expects lists, not numpy arrays embeddings_list = [emb.tolist() if hasattr(emb, "tolist") else emb for emb in embeddings] + # Safety net: deduplicate within batch — ChromaDB rejects non-unique IDs + # in a single upsert call. + seen = {} + for i, doc_id in enumerate(ids): + seen[doc_id] = i + if len(seen) < len(ids): + unique_indices = sorted(seen.values()) + ids = [ids[i] for i in unique_indices] + embeddings_list = [embeddings_list[i] for i in unique_indices] + documents = [documents[i] for i in unique_indices] + metadatas = [metadatas[i] for i in unique_indices] + # Upsert: idempotent — same content gets same ID, no duplicates collection.upsert(embeddings=embeddings_list, documents=documents, metadatas=metadatas, ids=ids) @@ -134,6 +152,79 @@ def _check_plan(plan_label, db_path=None): return {"success": True, "found": match_count > 0, "count": match_count, "source_files": sorted(matching_files)} +def _get_by_source(collection_name, source_pattern, n_results=5, db_path=None): + """Fetch documents whose source_file metadata contains a pattern. + + Args: + collection_name: Name of the ChromaDB collection + source_pattern: Substring to match in source_file metadata + n_results: Maximum number of results to return + db_path: Optional path to Chroma database + + Returns: + Dict with success, results list (document, metadata, id) + """ + client = _get_client(db_path) + + try: + collection = client.get_collection(collection_name, embedding_function=None) + except Exception as e: + logger.warning(f"[chroma_subprocess] Collection '{collection_name}' not found in get_by_source: {e}") + return {"success": False, "error": f"Collection '{collection_name}' not found: {e}"} + + result = collection.get(include=["metadatas", "documents"]) + matches = [] + for i, meta in enumerate(result.get("metadatas", [])): + source = meta.get("source_file", "") + if source_pattern in source: + matches.append( + { + "collection": collection_name, + "document": result["documents"][i], + "metadata": meta, + "id": result["ids"][i], + "distance": 0.0, + } + ) + if len(matches) >= n_results: + break + + return {"success": True, "results": matches, "count": len(matches)} + + +def _delete_by_source(collection_name, source_pattern, db_path=None): + """Delete vectors whose source_file metadata contains a pattern. + + Args: + collection_name: Name of the ChromaDB collection + source_pattern: Substring to match in source_file metadata + db_path: Optional path to Chroma database + + Returns: + Dict with success, deleted count, and matched IDs + """ + client = _get_client(db_path) + + try: + collection = client.get_collection(collection_name, embedding_function=None) + except Exception as e: + logger.warning(f"[chroma_subprocess] Collection '{collection_name}' not found in delete_by_source: {e}") + return {"success": False, "error": f"Collection '{collection_name}' not found: {e}"} + + result = collection.get(include=["metadatas"]) + ids_to_delete = [] + for i, meta in enumerate(result.get("metadatas", [])): + source = meta.get("source_file", "") + if source_pattern in source: + ids_to_delete.append(result["ids"][i]) + + if not ids_to_delete: + return {"success": True, "deleted": 0, "ids": [], "message": "No matching vectors found"} + + collection.delete(ids=ids_to_delete) + return {"success": True, "deleted": len(ids_to_delete), "ids": ids_to_delete} + + def _search_vectors(query_embedding, branch=None, memory_type=None, n_results=5, db_path=None): """Search for similar vectors.""" client = _get_client(db_path) @@ -214,6 +305,19 @@ def main(): ) elif operation == "check_plan": result = _check_plan(plan_label=input_data.get("plan_label"), db_path=input_data.get("db_path")) + elif operation == "get_by_source": + result = _get_by_source( + collection_name=input_data.get("collection_name"), + source_pattern=input_data.get("source_pattern"), + n_results=input_data.get("n_results", 5), + db_path=input_data.get("db_path"), + ) + elif operation == "delete_by_source": + result = _delete_by_source( + collection_name=input_data.get("collection_name"), + source_pattern=input_data.get("source_pattern"), + db_path=input_data.get("db_path"), + ) else: result = {"success": False, "error": f"Unknown operation: {operation}"} diff --git a/src/aipass/memory/apps/modules/governance.py b/src/aipass/memory/apps/modules/governance.py new file mode 100644 index 00000000..e092967a --- /dev/null +++ b/src/aipass/memory/apps/modules/governance.py @@ -0,0 +1,80 @@ +# =================== AIPass ==================== +# Name: governance.py +# Description: Surfacing governance module — public API +# Version: 1.1.0 +# Created: 2026-07-16 +# Modified: 2026-07-16 +# ============================================= + +""" +Surfacing Governance Module — Public API + +Thin module re-exporting governance engine from handlers/governance/engine.py. +Cross-branch consumers import from here: + + from aipass.memory.apps.modules.governance import should_surface, record_message, new_state +""" + +import os +import sys + +if sys.platform == "win32": + os.environ.setdefault("PYTHONUTF8", "1") + for _stream in (sys.stdout, sys.stderr): + _reconfigure = getattr(_stream, "reconfigure", None) + if _reconfigure is not None: + _reconfigure(encoding="utf-8", errors="replace") + +from aipass.prax import logger # noqa: F401 +from aipass.memory.apps.handlers.json import json_handler +from aipass.memory.apps.handlers.governance.engine import ( + DEFAULT_CONFIG, + new_state, + record_message, + should_surface, +) + +__all__ = ["should_surface", "record_message", "new_state", "DEFAULT_CONFIG"] + + +# ============================================================================= +# MODULE ROUTING (handle_command for drone auto-discovery) +# ============================================================================= + + +def print_introspection() -> None: + """Display module introspection (seedgo standard).""" + from aipass.cli.apps.modules import console + + console.print() + console.print("[bold cyan]governance Module[/bold cyan]") + console.print("Pure surfacing governance — state-in/state-out decision functions") + console.print() + console.print("[yellow]Public API:[/yellow]") + console.print(" should_surface(item_id, relevance_score, state, config)") + console.print(" record_message(state)") + console.print(" new_state()") + console.print() + console.print("[dim]Library module — import from: aipass.memory.apps.modules.governance[/dim]") + + +def handle_command(command: str, args: list) -> bool: + """Entry point for drone module discovery — governance has no CLI surface.""" + if command != "governance": + return False + + json_handler.log_operation("governance_command", {"args": args}) + + if not args: + print_introspection() + return True + + if args[0] in ("--help", "-h", "help"): + print_introspection() + return True + + from aipass.cli.apps.modules import warning + + warning(f"governance: unknown subcommand '{args[0]}'") + print_introspection() + return True diff --git a/src/aipass/memory/tests/test_governance.py b/src/aipass/memory/tests/test_governance.py new file mode 100644 index 00000000..9b97c62f --- /dev/null +++ b/src/aipass/memory/tests/test_governance.py @@ -0,0 +1,252 @@ +"""Tests for the surfacing governance module (pure state-in/state-out API).""" + +import pytest + +from aipass.memory.apps.modules.governance import ( + DEFAULT_CONFIG, + new_state, + record_message, + should_surface, +) + + +# ============================================================================= +# new_state +# ============================================================================= + + +class TestNewState: + def test_returns_zeroed_state(self): + state = new_state() + assert state["surfaces_count"] == 0 + assert state["messages_since_last"] == 0 + assert state["last_surface_time"] == 0.0 + assert state["surfaced_ids"] == [] + + def test_returns_independent_copies(self): + s1 = new_state() + s2 = new_state() + s1["surfaced_ids"].append("x") + assert s2["surfaced_ids"] == [] + + +# ============================================================================= +# should_surface — rejection paths +# ============================================================================= + + +class TestShouldSurfaceRejections: + """Each rejection path tested independently with an otherwise-valid state.""" + + @pytest.fixture() + def ready_state(self): + """State that passes all checks when config is default.""" + return { + "surfaces_count": 0, + "messages_since_last": 20, + "last_surface_time": 0.0, + "surfaced_ids": [], + } + + def test_disabled(self, ready_state): + ok, reason, st = should_surface("item1", 0.8, ready_state, {"enabled": False}, current_time=1000.0) + assert ok is False + assert "disabled" in reason.lower() + assert st is ready_state + + def test_below_threshold(self, ready_state): + ok, reason, st = should_surface("item1", 0.1, ready_state, {"threshold": 0.3}, current_time=1000.0) + assert ok is False + assert "threshold" in reason.lower() + assert st is ready_state + + def test_budget_exhausted(self, ready_state): + ready_state["surfaces_count"] = 5 + ok, reason, st = should_surface("item1", 0.8, ready_state, {"max_surfaces_per_session": 5}, current_time=1000.0) + assert ok is False + assert "budget" in reason.lower() or "exhausted" in reason.lower() + assert st is ready_state + + def test_spacing_not_met(self, ready_state): + ready_state["messages_since_last"] = 3 + ready_state["last_surface_time"] = 1.0 + ok, reason, st = should_surface("item1", 0.8, ready_state, {"min_messages_between": 10}, current_time=1000.0) + assert ok is False + assert "spacing" in reason.lower() + assert st is ready_state + + def test_cooldown_active(self, ready_state): + ready_state["last_surface_time"] = 900.0 + ok, reason, st = should_surface("item1", 0.8, ready_state, {"cooldown_seconds": 300}, current_time=1000.0) + assert ok is False + assert "cooldown" in reason.lower() + assert st is ready_state + + def test_already_surfaced(self, ready_state): + ready_state["surfaced_ids"] = ["item1"] + ok, reason, st = should_surface("item1", 0.8, ready_state, current_time=1000.0) + assert ok is False + assert "already" in reason.lower() + assert st is ready_state + + +# ============================================================================= +# should_surface — happy path +# ============================================================================= + + +class TestShouldSurfaceHappy: + def test_surfaces_and_returns_updated_state(self): + state = { + "surfaces_count": 0, + "messages_since_last": 15, + "last_surface_time": 0.0, + "surfaced_ids": [], + } + ok, reason, updated = should_surface("compass-42", 0.75, state, current_time=5000.0) + assert ok is True + assert "ready" in reason.lower() + assert updated["surfaces_count"] == 1 + assert updated["messages_since_last"] == 0 + assert updated["last_surface_time"] == 5000.0 + assert "compass-42" in updated["surfaced_ids"] + + def test_does_not_mutate_input_state(self): + state = { + "surfaces_count": 0, + "messages_since_last": 15, + "last_surface_time": 0.0, + "surfaced_ids": [], + } + original_ids = state["surfaced_ids"] + should_surface("item1", 0.8, state, current_time=5000.0) + assert state["surfaces_count"] == 0 + assert state["surfaced_ids"] is original_ids + assert len(original_ids) == 0 + + def test_threshold_boundary_exact(self): + state = new_state() + state["messages_since_last"] = 10 + ok, _, _ = should_surface("x", 0.3, state, {"threshold": 0.3}, current_time=1000.0) + assert ok is True + + def test_cooldown_expired(self): + state = { + "surfaces_count": 0, + "messages_since_last": 15, + "last_surface_time": 500.0, + "surfaced_ids": [], + } + ok, _, _ = should_surface("x", 0.8, state, {"cooldown_seconds": 300}, current_time=801.0) + assert ok is True + + def test_first_surface_ignores_spacing(self): + """Fresh session: first prompt with high relevance surfaces immediately.""" + state = new_state() + state["messages_since_last"] = 1 + ok, reason, updated = should_surface("compass-1", 0.8, state, current_time=100.0) + assert ok is True + assert "ready" in reason.lower() + + for i in range(9): + updated = record_message(updated) + ok, reason, _ = should_surface("compass-2", 0.8, updated, current_time=100.0 + 400 + i) + assert ok is False + assert "spacing" in reason.lower() + + updated = record_message(updated) + ok, _, _ = should_surface("compass-2", 0.8, updated, current_time=600.0) + assert ok is True + + +# ============================================================================= +# State isolation — two independent states do not bleed +# ============================================================================= + + +class TestStateIsolation: + def test_two_states_independent(self): + s1 = new_state() + s1["messages_since_last"] = 20 + s2 = new_state() + s2["messages_since_last"] = 20 + + ok1, _, s1_updated = should_surface("a", 0.8, s1, current_time=1000.0) + ok2, _, s2_updated = should_surface("b", 0.9, s2, current_time=2000.0) + + assert ok1 is True + assert ok2 is True + assert s1_updated["surfaced_ids"] == ["a"] + assert s2_updated["surfaced_ids"] == ["b"] + assert s1_updated["last_surface_time"] == 1000.0 + assert s2_updated["last_surface_time"] == 2000.0 + + def test_chained_surfaces_accumulate(self): + state = new_state() + state["messages_since_last"] = 20 + + ok, _, state = should_surface("a", 0.8, state, current_time=1000.0) + assert ok is True + assert state["surfaces_count"] == 1 + + state["messages_since_last"] = 20 + ok, _, state = should_surface("b", 0.7, state, current_time=2000.0) + assert ok is True + assert state["surfaces_count"] == 2 + assert state["surfaced_ids"] == ["a", "b"] + + +# ============================================================================= +# record_message +# ============================================================================= + + +class TestRecordMessage: + def test_increments_counter(self): + state = new_state() + updated = record_message(state) + assert updated["messages_since_last"] == 1 + + def test_does_not_mutate_input(self): + state = new_state() + record_message(state) + assert state["messages_since_last"] == 0 + + def test_preserves_other_fields(self): + state = { + "surfaces_count": 3, + "messages_since_last": 5, + "last_surface_time": 100.0, + "surfaced_ids": ["x"], + } + updated = record_message(state) + assert updated["surfaces_count"] == 3 + assert updated["messages_since_last"] == 6 + assert updated["last_surface_time"] == 100.0 + assert updated["surfaced_ids"] == ["x"] + + +# ============================================================================= +# Config merging +# ============================================================================= + + +class TestConfigMerging: + def test_none_config_uses_defaults(self): + state = new_state() + state["messages_since_last"] = 20 + ok, _, _ = should_surface("x", 0.8, state, None, current_time=1000.0) + assert ok is True + + def test_partial_config_merges_with_defaults(self): + state = new_state() + state["messages_since_last"] = 20 + ok, reason, _ = should_surface("x", 0.25, state, {"threshold": 0.5}, current_time=1000.0) + assert ok is False + assert "threshold" in reason.lower() + + def test_default_config_values_match(self): + assert DEFAULT_CONFIG["threshold"] == 0.3 + assert DEFAULT_CONFIG["max_surfaces_per_session"] == 5 + assert DEFAULT_CONFIG["min_messages_between"] == 10 + assert DEFAULT_CONFIG["cooldown_seconds"] == 300 diff --git a/src/aipass/memory/tests/test_plans_processor.py b/src/aipass/memory/tests/test_plans_processor.py index d2728e7c..3803e348 100644 --- a/src/aipass/memory/tests/test_plans_processor.py +++ b/src/aipass/memory/tests/test_plans_processor.py @@ -501,7 +501,7 @@ class TestProcessPlans: monkeypatch.setattr( mod, "_embed_texts", - lambda texts: {"success": True, "embeddings": [[0.1, 0.2]] * len(texts)}, + lambda texts, timeout=120: {"success": True, "embeddings": [[0.1, 0.2]] * len(texts)}, ) monkeypatch.setattr( mod, @@ -547,7 +547,7 @@ class TestProcessPlans: monkeypatch.setattr( mod, "_embed_texts", - lambda texts: {"success": False, "error": "GPU out of memory"}, + lambda texts, timeout=120: {"success": False, "error": "GPU out of memory"}, ) mock_jh = MagicMock() @@ -584,7 +584,7 @@ class TestProcessPlans: monkeypatch.setattr( mod, "_embed_texts", - lambda texts: {"success": True, "embeddings": []}, + lambda texts, timeout=120: {"success": True, "embeddings": []}, ) mock_jh = MagicMock() @@ -620,7 +620,7 @@ class TestProcessPlans: monkeypatch.setattr( mod, "_embed_texts", - lambda texts: {"success": True, "embeddings": [[0.1, 0.2]] * len(texts)}, + lambda texts, timeout=120: {"success": True, "embeddings": [[0.1, 0.2]] * len(texts)}, ) monkeypatch.setattr( mod, diff --git a/src/aipass/skills/lib/telegram/apps/handlers/base_bot.py b/src/aipass/skills/lib/telegram/apps/handlers/base_bot.py index ec586f42..99cee968 100644 --- a/src/aipass/skills/lib/telegram/apps/handlers/base_bot.py +++ b/src/aipass/skills/lib/telegram/apps/handlers/base_bot.py @@ -380,7 +380,7 @@ class BaseBot: net_offline_since = now net_suppressed = 0 net_last_summary = now - logger.error("Telegram unreachable, backing off: %s", e) + logger.warning("Telegram unreachable, backing off: %s", e) else: net_suppressed += 1 if now - net_last_summary >= NETWORK_LOG_INTERVAL: @@ -471,6 +471,8 @@ class BaseBot: logger.error("Poll error: %s", e) return [] except (ConnectionError, OSError) as e: + if _is_routine_read_timeout(e): + return [] raise _NetworkPollError(str(e)) from e except Exception as e: logger.error("Unexpected poll error: %s", e) diff --git a/src/aipass/skills/lib/telegram/tests/test_network_backoff.py b/src/aipass/skills/lib/telegram/tests/test_network_backoff.py index b6fe1a74..cb67b082 100644 --- a/src/aipass/skills/lib/telegram/tests/test_network_backoff.py +++ b/src/aipass/skills/lib/telegram/tests/test_network_backoff.py @@ -164,6 +164,36 @@ class TestPollUpdatesErrorClassification: with pytest.raises(_NetworkPollError): bot.poll_updates(0) + def test_bare_socket_timeout_returns_empty(self, tmp_path, _patch_base_bot_deps): + """socket.timeout (subclass of OSError) with 'read operation' should be silenced, not raised.""" + bot = _make_bot(tmp_path, _patch_base_bot_deps) + import socket + + exc = socket.timeout("The read operation timed out") + with patch("aipass.skills.lib.telegram.apps.handlers.base_bot.urlopen", side_effect=exc): + result = bot.poll_updates(0) + assert result == [] + + def test_bare_socket_timeout_no_error_log(self, tmp_path, _patch_base_bot_deps): + bot = _make_bot(tmp_path, _patch_base_bot_deps) + import socket + + exc = socket.timeout("The read operation timed out") + with ( + patch("aipass.skills.lib.telegram.apps.handlers.base_bot.urlopen", side_effect=exc), + patch("aipass.skills.lib.telegram.apps.handlers.base_bot.logger") as mock_logger, + ): + bot.poll_updates(0) + mock_logger.error.assert_not_called() + + def test_bare_connect_timeout_raises_network_error(self, tmp_path, _patch_base_bot_deps): + """A connect timeout (not read) IS a network error.""" + bot = _make_bot(tmp_path, _patch_base_bot_deps) + exc = TimeoutError("Connection timed out") + with patch("aipass.skills.lib.telegram.apps.handlers.base_bot.urlopen", side_effect=exc): + with pytest.raises(_NetworkPollError): + bot.poll_updates(0) + def test_non_network_urlerror_logs_error(self, tmp_path, _patch_base_bot_deps): bot = _make_bot(tmp_path, _patch_base_bot_deps) exc = URLError("HTTP Error 502") @@ -247,7 +277,7 @@ class TestRunLoopNetworkBackoff: class TestLogOnceSemantics: - def test_first_failure_logs_error(self, tmp_path, _patch_base_bot_deps): + def test_first_failure_logs_warning(self, tmp_path, _patch_base_bot_deps): bot = _make_bot(tmp_path, _patch_base_bot_deps) call_count = 0 @@ -266,8 +296,10 @@ class TestLogOnceSemantics: ): bot.run() + warn_calls = [c for c in mock_logger.warning.call_args_list if "unreachable" in str(c)] + assert len(warn_calls) == 1 error_calls = [c for c in mock_logger.error.call_args_list if "unreachable" in str(c)] - assert len(error_calls) == 1 + assert len(error_calls) == 0 def test_subsequent_failures_suppressed(self, tmp_path, _patch_base_bot_deps): bot = _make_bot(tmp_path, _patch_base_bot_deps) @@ -288,9 +320,9 @@ class TestLogOnceSemantics: ): bot.run() - error_calls = [c for c in mock_logger.error.call_args_list if "unreachable" in str(c)] - # Only one "unreachable" error, not 10 - assert len(error_calls) == 1 + warn_calls = [c for c in mock_logger.warning.call_args_list if "unreachable" in str(c)] + # Only one "unreachable" warning, not 10 + assert len(warn_calls) == 1 def test_recovery_logs_info(self, tmp_path, _patch_base_bot_deps): bot = _make_bot(tmp_path, _patch_base_bot_deps)