From c8b2e586de624b1004a04ce238d5894fbd990575 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Thu, 12 Nov 2026 18:07:17 +0000 Subject: [PATCH] fix(orchestrator): hub gate view reads recorded parents through gates.opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit W5 moved recorded parents into a journal that gates.opened() loads into a TEMP table; the W3 hub opened gates.db raw, so parents_of() raised and the gate view stayed empty. opened() and journal() take the kept directory explicitly (CLI: watch.KEPT, hub: its /kept mount), opened() closes the connection when setup raises, and the W3 tests use opened() plus a test for a parent from the kept journal. 🎯 Fix: 95 % 💥 Break: 3 % 💸 Cost: 100 % ⏱️ Time: 100 % 🛡️ Security: 100 % 🏆 Score: 92 🚦 Gate: ✅ 90/5, cost and time <= 100, security 100 % Co-Authored-By: Claude Opus 5.5 --- orchestrator/kvb_orchestrator/gating.py | 15 ++++----- skills/kvb-check-gate/scripts/gates.py | 42 ++++++++++++++++--------- tests/e2e/test_workbench_w3_e2e.py | 2 +- tests/test_hub_gates.py | 19 ++++++++--- 4 files changed, 51 insertions(+), 27 deletions(-) diff --git a/orchestrator/kvb_orchestrator/gating.py b/orchestrator/kvb_orchestrator/gating.py index 3862a96..8e9e153 100644 --- a/orchestrator/kvb_orchestrator/gating.py +++ b/orchestrator/kvb_orchestrator/gating.py @@ -7,7 +7,6 @@ import time from contextlib import closing from pathlib import Path from types import ModuleType -from urllib.parse import quote from .errors import ToolError from .fields import decode, expect @@ -31,9 +30,10 @@ def shown_command(argv: list[str]) -> str: def gates_module(script: Path) -> ModuleType | None: """Returns: - The installed gates.py as a module, for its resolve() and - parents_of(), which read only the database; loaded again after an - install changed it, None when it is missing or does not load. + The installed gates.py as a module, for its opened(), resolve() and + parents_of(), which only read the database and the parent journals; + loaded again after an install changed it, None when it is missing or + does not load. """ try: changed = script.stat().st_mtime_ns @@ -71,13 +71,14 @@ def read_gates(gates: ModuleType | None, db: Path, scan: dict) -> dict: Returns: {repo: {branch: {parents, rules, resolved}}} as `gates.py show --json` - prints it, read with the database opened read-only and without git; - {} without the script or a readable database. + prints it, read through gates.opened() with the database read-only, + the kept parent journal from the same mount and without git; {} + without the script or a readable database. """ if gates is None or not db.is_file(): return {} try: - with closing(sqlite3.connect(f"file:{quote(str(db))}?mode=ro", uri=True)) as connection: + with closing(gates.opened(db, False, db.parent)) as connection: return {repo["path"]: {entry["name"]: branch_gates(gates, connection, repo["path"], entry["name"]) for entry in repo["branches"]} for repo in scan["repos"]} except sqlite3.Error: return {} diff --git a/skills/kvb-check-gate/scripts/gates.py b/skills/kvb-check-gate/scripts/gates.py index 8101c9e..45bb5ea 100755 --- a/skills/kvb-check-gate/scripts/gates.py +++ b/skills/kvb-check-gate/scripts/gates.py @@ -169,9 +169,9 @@ def current_branch(worktree: str) -> str | None: return named.stdout.strip() if named.returncode == 0 else None -def journal(origin: Path, kept: Path) -> list[dict]: +def journal(origin: Path, kept_dir: Path, kept: Path) -> list[dict]: merged: dict[str, dict] = {} - for base, path in ((watch.KEPT, kept), (watch.ROOT, origin)): + for base, path in ((kept_dir, kept), (watch.ROOT, origin)): if base == watch.ROOT and watch.guard_root(hook=True) is not None: continue try: @@ -233,11 +233,21 @@ def receipted(repo: str, tree: str, level: str) -> bool: cutoff = time.time() - RECEIPT_SECONDS return any( row.get("repo") == repo and row.get("tree") == tree and fresh(row, cutoff) and meets(row, level) - for row in journal(watch.RECEIPTS, watch.KEPT_RECEIPTS) + for row in journal(watch.RECEIPTS, watch.KEPT, watch.KEPT_RECEIPTS) ) -def opened(path: Path, write: bool) -> sqlite3.Connection: +def opened(path: Path, write: bool, kept_dir: Path) -> sqlite3.Connection: + """Args: + path: gates.db; read-only unless write, in memory when it is missing. + write: open it for rule and parent changes. + kept_dir: the directory holding the kept gate-parents.jsonl: watch.KEPT + on the host, the read-only mount of it in the hub. + + Returns: + The connection with the TEMP table recorded filled from the parent + journals, which parents_of(), resolve() and show() read. + """ if write: path.parent.mkdir(parents=True, exist_ok=True) db = sqlite3.connect(path) @@ -245,14 +255,18 @@ def opened(path: Path, write: bool) -> sqlite3.Connection: db = sqlite3.connect(f"{path.resolve().as_uri()}?mode=ro", uri=True) else: db = sqlite3.connect(":memory:") - if write or not path.exists(): - db.executescript(SCHEMA) - db.execute("CREATE TEMP TABLE recorded (repo TEXT, branch TEXT, parent TEXT)") - db.executemany("INSERT INTO recorded VALUES (?, ?, ?)", [ - (row["repo"], row["branch"], row["parent"]) - for row in journal(watch.PARENTS, watch.KEPT_PARENTS) - if all(isinstance(row.get(key), str) for key in ("repo", "branch", "parent")) and row["branch"] != row["parent"] - ]) + try: + if write or not path.exists(): + db.executescript(SCHEMA) + db.execute("CREATE TEMP TABLE recorded (repo TEXT, branch TEXT, parent TEXT)") + db.executemany("INSERT INTO recorded VALUES (?, ?, ?)", [ + (row["repo"], row["branch"], row["parent"]) + for row in journal(watch.PARENTS, kept_dir, kept_dir / watch.KEPT_PARENTS.name) + if all(isinstance(row.get(key), str) for key in ("repo", "branch", "parent")) and row["branch"] != row["parent"] + ]) + except BaseException: + db.close() + raise return db @@ -517,7 +531,7 @@ def run_hook(name: str, argv: list[str], db_path: Path) -> int: record_after_checkout(here) return 0 targets = hook_targets(name, here) - db = opened(db_path, False) + db = opened(db_path, False, watch.KEPT) refused = [text for branch, tree, action in targets if (text := refusal(db, here, branch, tree, action))] for text in refused: print(text, file=sys.stderr) @@ -631,7 +645,7 @@ def main() -> int: verb = getattr(args, "verb", None) if args.command in ("tree", "quick", "hooks", "hook", "receipt") or verb == "record": return journal_command(args) - db = opened(args.db, args.command in ("rule", "parents")) + db = opened(args.db, args.command in ("rule", "parents"), watch.KEPT) repo = root(args.repo) if getattr(args, "repo", None) else None if args.command == "resolve": level, source = resolve(db, repo, args.branch, args.action) diff --git a/tests/e2e/test_workbench_w3_e2e.py b/tests/e2e/test_workbench_w3_e2e.py index 77c9a18..75e994f 100644 --- a/tests/e2e/test_workbench_w3_e2e.py +++ b/tests/e2e/test_workbench_w3_e2e.py @@ -115,7 +115,7 @@ class TestWorkbenchArtifacts(HubCase): next(item for item in data["repos"][0]["items"] if item["branch"] == "feature/kept")["skip"] = None self.tree_file.write_text(json.dumps(data), encoding="utf-8") self.repo = f"{root}/repos/github.com/me/demo" - db = gates.connect(self.kept / "gates.db") + db = gates.opened(self.kept / "gates.db", True, self.kept) db.executemany("INSERT INTO rules VALUES (?, ?, ?, ?, ?)", [(self.repo, "main", "commit", "test", 0.0), (self.repo, "feature/done", "pr", "e2e", 0.0)]) db.executemany("INSERT INTO parents VALUES (?, ?, ?, ?)", [(self.repo, "feature", "main", "recorded"), (self.repo, "feature/done", "feature", "derived"), (self.repo, "feature/done", "feature/kept", "operator"), (self.repo, "feature/kept", "main", "derived")]) db.commit() diff --git a/tests/test_hub_gates.py b/tests/test_hub_gates.py index d83d43c..01e0702 100644 --- a/tests/test_hub_gates.py +++ b/tests/test_hub_gates.py @@ -3,7 +3,6 @@ from __future__ import annotations import importlib.util import json import os -import sqlite3 import subprocess import sys import time @@ -131,7 +130,7 @@ class TestRunnerGates(HubCase): self.queue([rule("commit", "quality-high", branch="main", repo=repo), parent("main", branch="feature/x", repo=repo)]) self.run_gates_runner(["--gates", str(GATES)]) self.assertEqual(self.finished(), [("done", 0), ("done", 0)]) - with closing(sqlite3.connect(self.db)) as db: + with closing(gates.opened(self.db, False, self.db.parent)) as db: self.assertEqual(gates.resolve(db, repo, "feature/x", "commit"), ("quality-high", "main")) self.assertEqual(gates.parents_of(db, repo, "feature/x"), [("main", "operator")]) self.assertEqual([call for call in self.scripts.calls if call[0].endswith("operator.py")], []) @@ -163,7 +162,7 @@ class TestRunnerGates(HubCase): class TestHubGateView(HubCase): def write_db(self, repo: str) -> Path: db_path = self.kept / "gates.db" - db = gates.connect(db_path) + db = gates.opened(db_path, True, self.kept) db.executemany("INSERT INTO rules VALUES (?, ?, ?, ?, ?)", [(repo, "main", "commit", "test", 0.0), (repo, "main", "merge", "quality-high", 0.0), (repo, "feature", "pr", "e2e", 0.0), (repo, "feature/kept", "merge", "quick", 0.0)]) db.executemany("INSERT INTO parents VALUES (?, ?, ?, ?)", [(repo, "feature", "main", "recorded"), (repo, "feature/done", "feature", "derived"), (repo, "feature/done", "feature/kept", "operator"), (repo, "feature/kept", "main", "derived")]) db.commit() @@ -182,7 +181,7 @@ class TestHubGateView(HubCase): self.assertEqual(status, 200, body) shown = json.loads(body)["gates"] self.assertEqual(sorted(shown[repo]), ["feature", "feature/done", "feature/kept", "main"]) - with closing(sqlite3.connect(f"file:{db_path}?mode=ro", uri=True)) as db, mock.patch.object(gates, "branches", return_value={name: "0" * 40 for name in shown[repo]}): + with closing(gates.opened(db_path, False, self.kept)) as db, mock.patch.object(gates, "branches", return_value={name: "0" * 40 for name in shown[repo]}): self.assertEqual(shown[repo], gates.show(db, [repo])[repo]) done = shown[repo]["feature/done"] self.assertEqual(done["parents"], [{"parent": "feature", "source": "derived"}, {"parent": "feature/kept", "source": "operator"}]) @@ -190,6 +189,16 @@ class TestHubGateView(HubCase): self.assertEqual(db_path.read_bytes(), before) self.assertEqual(sorted(path.name for path in self.kept.iterdir()), ["gates.db"]) + def test_a_parent_in_the_kept_journal_of_the_mount_shows_as_recorded(self): + root = self.write_tree() + repo = f"{root}/repos/github.com/me/demo" + self.write_db(repo) + row = {"id": "1-1", "repo": repo, "branch": "main", "parent": "feature/kept"} + (self.kept / "gate-parents.jsonl").write_text(json.dumps(row) + "\n", encoding="utf-8") + status, _, body = self.get("/api/tree") + self.assertEqual(status, 200, body) + self.assertEqual(json.loads(body)["gates"][repo]["main"]["parents"], [{"parent": "feature/kept", "source": "recorded"}]) + def test_without_a_database_or_with_a_broken_one_the_tree_carries_no_gates(self): self.write_tree() self.assertEqual(json.loads(self.get("/api/tree")[2])["gates"], {}) @@ -199,7 +208,7 @@ class TestHubGateView(HubCase): def test_a_stored_level_outside_levels_or_an_older_gates_py_without_parents_of_keeps_the_tree_up_without_gates(self): root = self.write_tree() repo = f"{root}/repos/github.com/me/demo" - db = gates.connect(self.write_db(repo)) + db = gates.opened(self.write_db(repo), True, self.kept) db.execute("UPDATE rules SET level = 'bogus' WHERE branch = 'main' AND action = 'commit'") db.commit() status, _, body = self.get("/api/tree")