Harden controller review and runtime gates

.opencode/agents/approver.md

@@ -8,11 +8,6 @@ permission:
   task: deny
   bash:
     "*": deny
-    "git status*": allow
-    "git diff*": allow
-    "git log*": allow
-    "git show*": allow
-    "git rev-parse*": allow
     "/home/overseer/.local/share/nestquest-controller/scripts/nq-approve-merge *": allow
   external_directory:
     "*": ask

.opencode/agents/orchestrator.md

@@ -30,8 +30,10 @@ The NESTQUEST_CONTROL_HOME environment variable names the installed launchers
 (normally /home/overseer/.local/share/nestquest-controller).
 For implementation, run "$NESTQUEST_CONTROL_HOME/scripts/nq-agent" with role
 developer, a task worktree, title, and task prompt file. For independent
-review, run "$NESTQUEST_CONTROL_HOME/scripts/nq-review" in that task worktree,
-against the feature branch and the exact current head. The developer may merge
+review, run "$NESTQUEST_CONTROL_HOME/scripts/nq-review" with the worktree,
+feature base ref, exact head ref, review prompt file, PR number, and an output
+file under /tmp/opencode. It publishes an exact-head Gitea approval only after
+Claude approves. The developer may merge
 a task PR into its feature branch only after independent approval of the exact
 head and verification of every task gate. For feature-to-dev approval and
 merge, run "$NESTQUEST_CONTROL_HOME/scripts/nq-agent" with role approver in

ORCHESTRATION.md

@@ -12,11 +12,13 @@ open for the NestQuest checkout.
 `scripts/nq-agent` launches developer and approver sessions with fixed agent
 definitions and no `--auto` flag. Headless OpenCode runs use `--pure` so the
 global idle-notification plugin does not notify on every controller tick.
-The approver's shell permission permits read-only Git commands and only the
-installed `nq-approve-merge` broker for Gitea. That broker requires exact-head
-Claude approval evidence, the `dev` target, a mergeable PR, and passing
-configured checks before it requests a squash merge. `scripts/nq-review` launches the independent
-Claude review in the task worktree and checks the exact head before review.
+The approver's shell permission permits only the installed
+`nq-approve-merge` broker for Gitea. That broker requires exact-head
+Claude approval evidence, an exact-head Gitea approval by `review_agent`,
+the `dev` target, a mergeable PR, and passing configured checks before it
+requests a squash merge. `scripts/nq-review` launches the independent
+Claude review in the task worktree, checks the head again afterward, saves
+the output under `/tmp/opencode`, and posts the Gitea approval.
 The approver verifies the reviewed head again before a merge.
 
 The production service uses a stable copy of this configuration and scripts
@@ -25,13 +27,20 @@ which task branch is checked out in the primary NestQuest directory. Install
 that copy, run `scripts/nq-controller --check`, and verify all three agents
 with `opencode debug agent <name>` while `OPENCODE_CONFIG_DIR` points at the
 installed `.opencode` directory. Install `deploy/nestquest-controller.service`
-under `/etc/systemd/system/`, then enable it. The service waits for an open
+under `~/.config/systemd/user/`, then enable it with `systemctl --user`.
+The host has lingering enabled for the `overseer` user. The service waits for an open
 interactive NestQuest session to close before starting work.
 
 The CTXD preflight reads the configured HTTP/MCP server's NestQuest prompt.
 It deliberately does not inspect `/home/overseer/.ctx/ctxd.db`, which belongs
 to a different local store and lacks this project.
 
+All four agent credentials are currently readable by the same Unix account.
+The Gitea review and merge gates enforce normal workflow sequencing, but
+they are not a hard isolation boundary against a process running as that
+account. Strong isolation requires separate OS identities and protected
+credentials for the controller, reviewer, and approver.
+
 A tick has an explicit six-hour timeout. On timeout or an OpenCode error, the
 next tick must reconcile external state before acting. Three consecutive
 failed ticks stop the service. A BLOCKED verdict also stops it. QUIESCENT

deploy/nestquest-controller.service

@@ -1,13 +1,8 @@
 [Unit]
 Description=NestQuest OpenCode controller
-After=network-online.target
-Wants=network-online.target
-StartLimitIntervalSec=600
-StartLimitBurst=5
 
 [Service]
 Type=simple
-User=overseer
 WorkingDirectory=/mnt/ai-storage/projects/NestQuest
 ExecStartPre=/home/overseer/.local/share/nestquest-controller/scripts/nq-controller --check
 ExecStart=/home/overseer/.local/share/nestquest-controller/scripts/nq-controller
@@ -19,4 +14,4 @@ KillMode=control-group
 Environment=PYTHONUNBUFFERED=1
 
 [Install]
-WantedBy=multi-user.target
+WantedBy=default.target

orchestration_tests/test_nq_approve_merge.py

@@ -2,6 +2,7 @@ from pathlib import Path
 import runpy
 import tempfile
 import unittest
+from unittest.mock import patch
 
 
 APPROVE = runpy.run_path(str(Path(__file__).resolve().parents[1] / "scripts/nq-approve-merge"))
@@ -22,6 +23,19 @@ class MergeGateTests(unittest.TestCase):
             with self.assertRaisesRegex(RuntimeError, "exact head"):
                 APPROVE["validate_review"](review, HEAD)
 
+    def test_requires_distinct_gitea_reviewer_on_exact_commit(self):
+        records = [{"state": "APPROVED", "commit_id": HEAD, "user": {"login": "review_agent"}}]
+        with patch.dict(APPROVE["has_platform_review"].__globals__, {"request": lambda *_: records}):
+            self.assertTrue(APPROVE["has_platform_review"](149, HEAD, "unused"))
+            self.assertFalse(APPROVE["has_platform_review"](149, "b" * 40, "unused"))
+            records[0]["user"]["login"] = "orchestrator-agent"
+            self.assertFalse(APPROVE["has_platform_review"](149, HEAD, "unused"))
+            records[:] = [
+                {"id": 1, "state": "APPROVED", "commit_id": HEAD, "user": {"login": "review_agent"}},
+                {"id": 2, "state": "REQUEST_CHANGES", "commit_id": HEAD, "user": {"login": "review_agent"}},
+            ]
+            self.assertFalse(APPROVE["has_platform_review"](149, HEAD, "unused"))
+
 
 if __name__ == "__main__":
     unittest.main()

orchestration_tests/test_nq_controller.py

@@ -19,6 +19,8 @@ class ControllerProtocolTests(unittest.TestCase):
     def test_error_event_does_not_publish_provider_message(self):
         event = {"type": "error", "error": {"name": "APIError", "data": {"message": "secret provider response"}}}
         self.assertEqual(CONTROL["parse_event"](json.dumps(event)), (None, "APIError"))
+        self.assertEqual(CONTROL["parse_event"]("null"), (None, None))
+        self.assertEqual(CONTROL["parse_event"]("[]"), (None, None))
 
     def test_tick_uses_final_text(self):
         event = json.dumps({"type": "text", "part": {"type": "text", "text": "ORCHESTRATOR_STATE: QUIESCENT\n"}})
@@ -53,6 +55,11 @@ class ControllerProtocolTests(unittest.TestCase):
             self.assertTrue(CONTROL["interactive_nestquest_session"](project, proc, database))
             (entry / "cmdline").write_bytes(b"opencode\0--dir\0" + str(root).encode() + b"\0")
             self.assertFalse(CONTROL["interactive_nestquest_session"](project, proc, database))
+            with sqlite3.connect(database) as db:
+                db.execute("DROP TABLE session")
+            (entry / "cmdline").write_bytes(b"opencode\0-s\0ses_test\0")
+            with self.assertRaisesRegex(RuntimeError, "lookup failed"):
+                CONTROL["interactive_nestquest_session"](project, proc, database)
 
 
 if __name__ == "__main__":

orchestration_tests/test_nq_review.py

@@ -1,43 +1,17 @@
 from pathlib import Path
-import os
-import subprocess
-import tempfile
+import runpy
 import unittest
 
-
-SCRIPT = Path(__file__).resolve().parents[1] / "scripts/nq-review"
-
+REVIEW = runpy.run_path(str(Path(__file__).resolve().parents[1] / "scripts/nq-review"))
+HEAD = "a" * 40
 
 class ReviewLauncherTests(unittest.TestCase):
-    def test_pins_head_and_model_in_worktree(self):
-        with tempfile.TemporaryDirectory() as directory:
-            root = Path(directory)
-            repo = root / "repo"
-            repo.mkdir()
-            subprocess.run(["git", "init", "-q", str(repo)], check=True)
-            subprocess.run(["git", "-C", str(repo), "config", "user.email", "test@example.invalid"], check=True)
-            subprocess.run(["git", "-C", str(repo), "config", "user.name", "Test"], check=True)
-            (repo / "README").write_text("initial\n")
-            subprocess.run(["git", "-C", str(repo), "add", "README"], check=True)
-            subprocess.run(["git", "-C", str(repo), "commit", "-qm", "initial"], check=True)
-            head = subprocess.check_output(["git", "-C", str(repo), "rev-parse", "HEAD"], text=True).strip()
-            prompt = root / "review.txt"
-            prompt.write_text("Review this change.")
-            stub = root / "claude"
-            stub.write_text("#!/bin/sh\nprintf '%s\\n' \"$PWD\" \"$@\"\n")
-            stub.chmod(0o700)
-            env = os.environ.copy()
-            env["CLAUDE_BIN"] = str(stub)
-            result = subprocess.run([str(SCRIPT), str(repo), head, head, str(prompt)], env=env, text=True, capture_output=True)
-            self.assertEqual(result.returncode, 0, result.stderr)
-            self.assertIn(str(repo), result.stdout)
-            self.assertIn("--model\nclaude-sonnet-5", result.stdout)
-            self.assertIn("--effort\nhigh", result.stdout)
-            self.assertIn(head, result.stdout)
-
-            result = subprocess.run([str(SCRIPT), str(repo), head, "HEAD~1", str(prompt)], env=env, text=True, capture_output=True)
-            self.assertNotEqual(result.returncode, 0)
-
+    def test_exact_head_verdict(self):
+        self.assertEqual(REVIEW["verdict"](f"REVIEWED_HEAD: {HEAD}\nREVIEW_VERDICT: APPROVED\n", HEAD), "APPROVED")
+        with self.assertRaisesRegex(RuntimeError, "one verdict"):
+            REVIEW["verdict"](f"REVIEWED_HEAD: {'b' * 40}\nREVIEW_VERDICT: APPROVED\n", HEAD)
+        with self.assertRaisesRegex(RuntimeError, "one verdict"):
+            REVIEW["verdict"](f"REVIEWED_HEAD: {HEAD}\nREVIEW_VERDICT: APPROVED\nREVIEW_VERDICT: CHANGES_REQUIRED\n", HEAD)
 
 if __name__ == "__main__":
     unittest.main()

scripts/nq-approve-merge

@@ -25,7 +25,7 @@ def token_from_file() -> str:
     raise RuntimeError("GITEA_APPROVER_AGENT_TOKEN is unavailable")
 
 
-def request(path: str, token: str, payload: dict | None = None) -> dict:
+def request(path: str, token: str, payload: dict | None = None) -> dict | list:
     body = None if payload is None else json.dumps(payload).encode()
     headers = {"Authorization": "token " + token, "Accept": "application/json"}
     if body is not None:
@@ -68,6 +68,22 @@ def validate_review(path: Path, head: str) -> None:
         raise RuntimeError("independent review does not approve this exact head")
 
 
+def has_platform_review(pr_number: int, head: str, token: str) -> bool:
+    reviews = request(f"/pulls/{pr_number}/reviews", token)
+    if not isinstance(reviews, list):
+        raise RuntimeError("Gitea returned invalid review records")
+    by_reviewer = [
+        review for review in reviews
+        if isinstance(review, dict)
+        and (review.get("user") or {}).get("login") == "review_agent"
+        and review.get("commit_id") == head
+    ]
+    if not by_reviewer:
+        return False
+    latest = max(by_reviewer, key=lambda review: int(review.get("id") or 0))
+    return latest.get("state") == "APPROVED"
+
+
 def main() -> int:
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument("operation", choices=("inspect", "merge"))
@@ -87,6 +103,8 @@ def main() -> int:
     if not FULL_SHA.fullmatch(args.reviewed_head):
         parser.error("reviewed head must be a full SHA")
     validate_review(args.review_file, args.reviewed_head)
+    if not has_platform_review(args.pr, args.reviewed_head, token):
+        raise RuntimeError("Gitea lacks an exact-head approval by review_agent")
     if before["base"] != "dev" or before["head"] != args.reviewed_head:
         raise RuntimeError("PR target or current head differs from the approved review")
     if before["state"] != "open" or before["merged"] or before["mergeable"] is not True:

scripts/nq-controller

@@ -54,8 +54,8 @@ def interactive_nestquest_session(
     """Avoid racing a user-owned OpenCode TUI for the same repository."""
     try:
         db = sqlite3.connect(f"file:{database}?mode=ro", uri=True)
-    except sqlite3.Error:
-        return False
+    except sqlite3.Error as error:
+        raise RuntimeError("OpenCode session database is unavailable") from error
     try:
         for entry in proc_root.iterdir():
             if not entry.name.isdecimal() or int(entry.name) == os.getpid():
@@ -65,7 +65,7 @@ def interactive_nestquest_session(
                 args = [a.decode(errors="replace") for a in args if a]
                 if not args or Path(args[0]).name != "opencode":
                     continue
-                if any(a in ("run", "debug", "serve", "web") for a in args[1:]):
+                if len(args) > 1 and args[1] in ("run", "debug", "serve", "web"):
                     continue
                 session = None
                 for flag in ("-s", "--session"):
@@ -81,6 +81,10 @@ def interactive_nestquest_session(
                         return True
                 elif (entry / "cwd").resolve() == project.resolve():
                     return True
+            except sqlite3.Error as error:
+                raise RuntimeError("OpenCode session lookup failed") from error
+            except TypeError as error:
+                raise RuntimeError("OpenCode session directory is invalid") from error
             except (OSError, PermissionError):
                 continue
     finally:
@@ -93,6 +97,8 @@ def parse_event(raw: str) -> tuple[str | None, str | None]:
         event = json.loads(raw)
     except json.JSONDecodeError:
         return None, None
+    if not isinstance(event, dict):
+        return None, None
     kind = event.get("type")
     part = event.get("part") or {}
     if kind == "text" and isinstance(part, dict):
@@ -192,11 +198,16 @@ def main() -> int:
             ctxd_version = check_ctxd()
         except (OSError, KeyError, ValueError, RuntimeError) as error:
             print(f"CTXD preflight failed: {type(error).__name__}", file=sys.stderr)
-            return 77
+            return 1
         print(f"ready: project={project} configuration={config_dir} binary={binary} ctxd_prompt_version={ctxd_version}")
         return 0
     if args.check_interactive:
-        print("interactive NestQuest session open" if interactive_nestquest_session(project) else "no interactive NestQuest session")
+        try:
+            active = interactive_nestquest_session(project)
+        except RuntimeError:
+            print("OpenCode session state unavailable", file=sys.stderr)
+            return 77
+        print("interactive NestQuest session open" if active else "no interactive NestQuest session")
         return 0
 
     args.state_dir.mkdir(mode=0o700, parents=True, exist_ok=True)
@@ -213,7 +224,15 @@ def main() -> int:
         env["NESTQUEST_CONTROL_HOME"] = str(control_home)
         failures = 0
         while True:
-            if interactive_nestquest_session(project):
+            try:
+                interactive = interactive_nestquest_session(project)
+            except RuntimeError:
+                print("OpenCode session state unavailable; waiting", file=sys.stderr, flush=True)
+                if args.once:
+                    return 77
+                time.sleep(60)
+                continue
+            if interactive:
                 print("interactive NestQuest session still open; waiting", flush=True)
                 if args.once:
                     return 75

scripts/nq-review

@@ -1,34 +1,110 @@
-#!/usr/bin/env bash
-set -euo pipefail
+#!/usr/bin/env python3
+"""Run Claude on an exact PR head and publish an independent Gitea approval."""
+from __future__ import annotations
+import argparse
+import json
+import os
+from pathlib import Path
+import re
+import subprocess
+import sys
+import urllib.error
+import urllib.request
 
-if [[ $# -ne 4 ]]; then
-  echo "usage: nq-review WORKTREE BASE_REF HEAD_REF REVIEW_PROMPT_FILE" >&2
-  exit 64
-fi
+API = "https://code.cubecraftlabs.com/api/v1"
+REPO = "/repos/Maniacs_Mansion/NestQuest"
+CREDENTIALS = Path("/home/overseer/.llms/credentials.env")
+CLAUDE = "/home/overseer/.local/bin/claude"
+SHA = re.compile(r"^[0-9a-f]{40}$")
 
-worktree=$1
-base_ref=$2
-head_ref=$3
-prompt_file=$4
-[[ -d "$worktree" && -f "$prompt_file" ]] || { echo "missing worktree or prompt" >&2; exit 66; }
+def git(worktree: Path, *args: str) -> str:
+    return subprocess.check_output(["git", "-C", str(worktree), *args], text=True).strip()
 
-cd "$worktree"
-base_commit=$(git rev-parse --verify "$base_ref^{commit}")
-head_commit=$(git rev-parse --verify "$head_ref^{commit}")
-[[ "$(git rev-parse HEAD)" == "$head_commit" ]] || { echo "worktree HEAD differs from reviewed head" >&2; exit 65; }
-git merge-base --is-ancestor "$base_commit" "$head_commit" || { echo "base is not an ancestor of head" >&2; exit 65; }
+def token() -> str:
+    for line in CREDENTIALS.read_text().splitlines():
+        if line.startswith("GITEA_REVIEW_AGENT_TOKEN="):
+            return line.split("=", 1)[1].strip().strip("\"'")
+    raise RuntimeError("review_agent credential is unavailable")
 
-review_prompt=$(cat "$prompt_file")
-review_prompt+=$'\n\nReview only the delta from the specified base to this exact head. Base: '
-review_prompt+="$base_commit"
-review_prompt+=$'. Head: '
-review_prompt+="$head_commit"
-review_prompt+=$'. Verify the current HEAD before issuing a verdict. Read only; do not modify files. End with two plain lines, REVIEWED_HEAD: <full 40-character SHA> and REVIEW_VERDICT: APPROVED or REVIEW_VERDICT: CHANGES_REQUIRED.'
+def api(path: str, credential: str, payload: dict | None = None) -> dict:
+    body = None if payload is None else json.dumps(payload).encode()
+    headers = {"Authorization": "token " + credential, "Accept": "application/json"}
+    if body is not None:
+        headers["Content-Type"] = "application/json"
+    call = urllib.request.Request(API + path, data=body, headers=headers)
+    try:
+        with urllib.request.urlopen(call, timeout=20) as response:
+            return json.loads(response.read())
+    except urllib.error.HTTPError as error:
+        raise RuntimeError(f"Gitea returned HTTP {error.code}") from None
 
-claude_bin=${CLAUDE_BIN:-/home/overseer/.local/bin/claude}
-exec "$claude_bin" -p "$review_prompt" \
-  --model claude-sonnet-5 \
-  --effort high \
-  --permission-mode plan \
-  --allowedTools Read Grep Glob \
-    'Bash(git diff:*)' 'Bash(git log:*)' 'Bash(git show:*)' 'Bash(git status:*)'
+def verdict(output: str, head: str) -> str:
+    reviewed = re.findall(r"(?m)^REVIEWED_HEAD:\s*([0-9a-f]{40})\s*$", output)
+    decisions = re.findall(r"(?m)^REVIEW_VERDICT:\s*(APPROVED|CHANGES_REQUIRED)\s*$", output)
+    if reviewed != [head] or len(decisions) != 1:
+        raise RuntimeError("Claude did not return one verdict for the exact reviewed head")
+    return decisions[0]
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("worktree", type=Path)
+    parser.add_argument("base_ref")
+    parser.add_argument("head_ref")
+    parser.add_argument("prompt_file", type=Path)
+    parser.add_argument("pr", type=int)
+    parser.add_argument("output_file", type=Path)
+    args = parser.parse_args()
+    if not args.worktree.is_dir() or not args.prompt_file.is_file() or args.pr < 1:
+        parser.error("valid worktree, prompt file, and PR number required")
+    if not args.output_file.resolve().is_relative_to(Path("/tmp/opencode").resolve()):
+        parser.error("review output must be under /tmp/opencode")
+    base = git(args.worktree, "rev-parse", "--verify", args.base_ref + "^{commit}")
+    head = git(args.worktree, "rev-parse", "--verify", args.head_ref + "^{commit}")
+    if not SHA.fullmatch(head) or git(args.worktree, "rev-parse", "HEAD") != head:
+        raise RuntimeError("worktree HEAD differs from requested head")
+    subprocess.run(["git", "-C", str(args.worktree), "merge-base", "--is-ancestor", base, head], check=True)
+    prompt = args.prompt_file.read_text() + (
+        f"\n\nReview only the delta from base {base} to exact head {head}. "
+        "Verify current HEAD before issuing a verdict. Read only; do not modify files. "
+        "End with two plain lines: REVIEWED_HEAD: <full 40-character SHA> "
+        "and REVIEW_VERDICT: APPROVED or REVIEW_VERDICT: CHANGES_REQUIRED."
+    )
+    result = subprocess.run(
+        [CLAUDE, "-p", prompt, "--model", "claude-sonnet-5", "--effort", "high",
+         "--permission-mode", "plan", "--allowedTools", "Read", "Grep", "Glob",
+         "Bash(git diff:*)", "Bash(git log:*)", "Bash(git show:*)", "Bash(git status:*)"],
+        cwd=args.worktree, text=True, capture_output=True, timeout=1200,
+    )
+    args.output_file.parent.mkdir(parents=True, exist_ok=True)
+    args.output_file.write_text(result.stdout)
+    os.chmod(args.output_file, 0o600)
+    if result.returncode:
+        raise RuntimeError(f"Claude exited {result.returncode}: {result.stderr[-300:]}")
+    decision = verdict(result.stdout, head)
+    if git(args.worktree, "rev-parse", "HEAD") != head:
+        raise RuntimeError("worktree HEAD changed during review")
+    print(result.stdout, end="")
+    if decision != "APPROVED":
+        return 2
+    credential = token()
+    if api("/user", credential).get("login") != "review_agent":
+        raise RuntimeError("review credential belongs to wrong account")
+    pr = api(f"{REPO}/pulls/{args.pr}", credential)
+    if pr.get("state") != "open" or (pr.get("head") or {}).get("sha") != head:
+        raise RuntimeError("PR head changed before review publication")
+    published = api(
+        f"{REPO}/pulls/{args.pr}/reviews", credential,
+        {"body": "Independent Claude Sonnet 5 review completed for this exact commit.",
+         "commit_id": head, "event": "APPROVED"},
+    )
+    if published.get("state") != "APPROVED" or published.get("commit_id") != head:
+        raise RuntimeError("Gitea did not confirm exact-head approval")
+    return 0
+
+if __name__ == "__main__":
+    try:
+        raise SystemExit(main())
+    except (OSError, ValueError, RuntimeError, subprocess.CalledProcessError,
+            subprocess.TimeoutExpired) as error:
+        print(f"review gate failed: {error}", file=sys.stderr)
+        raise SystemExit(1)