diff --git a/.env.example b/.env.example index 6713097..7a09b54 100644 --- a/.env.example +++ b/.env.example @@ -23,3 +23,20 @@ PROJECTS_ROOT=/var/lib/handler/projects # CLAUDE_BIN=claude # MISE_BIN=mise # TMUX_BIN=tmux +# FORGE_BIN=forge +# GIT_BIN=git + +# Phase 2 (forge integration). Pin the forge version your base image installs; spawn +# verifies the injected forge matches and warns on drift. Leave unset to skip the check. +# FORGE_VERSION=1.2.3 + +# Branches a direct `git push` may not reach without a standing approval (comma-separated). +# Closes the "merge locally, push to main" path around the forge-merge approval gate. +# PROTECTED_BRANCHES=main,master + +# Per-project credentials are NOT set here — they live on each project's `credential_ref` +# as a POINTER (env:VAR / file:/path / cmd:...), resolved and injected only at spawn. +# The database never stores the raw token. Example, when registering a project: +# credential_ref = "env:LEEWORKS_TOKEN" (then export LEEWORKS_TOKEN where the control +# layer runs; it's injected as FORGE_TOKEN + +# the host-specific var, e.g. GITHUB_TOKEN) diff --git a/.gitignore b/.gitignore index 83972fa..c4971fa 100644 --- a/.gitignore +++ b/.gitignore @@ -216,3 +216,4 @@ __marimo__/ # Streamlit .streamlit/secrets.toml +.omc/ diff --git a/README.md b/README.md index 2525181..5da0730 100644 --- a/README.md +++ b/README.md @@ -9,11 +9,14 @@ Every agent process is a real `claude` binary invocation. There is no hard depen any particular git host or network layer: you bring your own Claude Code login, your own git remote, and your own network exposure. -> **Status: Phase 1 MVP.** The control layer, HTTP API, database, migrations, and -> verification hooks are implemented and tested (45 tests, SQLite). Live end-to-end agent -> spawning against a real `claude` binary + tmux is stubbed behind mockable seams and -> wired but not yet exercised against production binaries. See -> [`docs/PLAN.md`](docs/PLAN.md) for the full design and roadmap. +> **Status: Phase 2 (forge integration) implemented on top of the Phase 1 MVP.** The +> control layer, HTTP API, database, migrations, and verification/approval hooks are +> implemented and tested (106 tests, SQLite). Phase 2 adds credential resolution + +> injection, role-based forge-workflow skills, a hard approval gate, and a CI-status +> poller. Live end-to-end agent spawning against a real `claude` binary + tmux is stubbed +> behind mockable seams (`tmux`, `verify`, `forge`, `gitops`, `spawn.resume`) and wired +> but not yet exercised against production binaries. See [`docs/PLAN.md`](docs/PLAN.md) +> for the full design and roadmap. --- @@ -100,7 +103,9 @@ Configuration is entirely environment-driven (see [`.env.example`](.env.example) | `SHARED_CONTEXT_WRITE_TOKEN` | Higher-trust token gating `PUT /shared/context/:key` | falls back to `AUTH_TOKEN` | | `WEBHOOK_URL` | Generic target for the `Notification` hook (ntfy, Slack, …) | unset → no-op | | `PROJECTS_ROOT` | Base dir for per-project roots / worktrees | `./projects` | -| `CLAUDE_BIN` / `MISE_BIN` / `TMUX_BIN` | Binary overrides | `claude` / `mise` / `tmux` | +| `CLAUDE_BIN` / `MISE_BIN` / `TMUX_BIN` / `FORGE_BIN` / `GIT_BIN` | Binary overrides | `claude` / `mise` / `tmux` / `forge` / `git` | +| `FORGE_VERSION` | Pinned forge version verified at spawn (Phase 2) | unset → skip check | +| `PROTECTED_BRANCHES` | Branches a direct push needs an approval to reach (Phase 2) | `main,master` | ## Run @@ -143,17 +148,27 @@ curl -s -X POST $BASE/projects/leeworks-api/agents/api/resume -H "$TOKEN" \ The `handler` command manages agent processes (the write side): ```bash -handler spawn --project leeworks-api --name api --worktree feature/auth --task "add login" +handler spawn --project leeworks-api --name junior --role junior --worktree feat/auth --task "add login" handler list [--project leeworks-api] -handler attach --project leeworks-api --name api -handler kill --project leeworks-api --name api +handler attach --project leeworks-api --name junior +handler kill --project leeworks-api --name junior + +# Phase 2 — forge workflow +handler forge-init --project leeworks-api # write + commit the role skills +handler approve --branch feat/auth --pr 12 # senior agent records its verdict +handler reject --branch feat/auth --note "fix X" # (project/agent from env in-session) +handler poll-ci [--project leeworks-api] [--watch] # backfill CI verdicts ``` `spawn` refuses any project whose working directory has no `.mise.toml` with a -`[tasks.test]` task — the verification gate is a hard requirement, not a convention. It +`[tasks.test]` task — the verification gate is a hard requirement, not a convention — and +it also refuses to start if the project's `credential_ref` is configured but can't be +resolved, so a broken secret pointer fails fast instead of leaving an orphaned agent. It resolves the working directory (a subdirectory or a fresh git worktree, always under the -project root), writes a per-agent `.claude/settings.json` wiring the hooks, and launches a -`tmux` session with the agent's identity and `DATABASE_URL` injected into its environment. +project root), writes a per-agent `.claude/settings.json` wiring the hooks, resolves and +injects the project's credentials (see below), and launches a `tmux` session with the +agent's identity and `DATABASE_URL` injected into its environment. `--role` +(`junior`/`senior`/`deploy`) records which forge-workflow role the agent plays. ## API reference @@ -187,23 +202,69 @@ Wired into each agent as `python -m handler.hooks `: Never blocks the agent on delivery failure. Hook identity travels via environment variables injected at spawn (`HANDLER_AGENT_ID`, -`HANDLER_PROJECT_ID`, `HANDLER_AGENT_NAME`, `DATABASE_URL`), since hook stdin doesn't -carry it; the wiring itself lives in the generated `settings.json`. +`HANDLER_PROJECT_ID`, `HANDLER_AGENT_NAME`, `HANDLER_AGENT_ROLE`, `DATABASE_URL`), since +hook stdin doesn't carry it; the wiring itself lives in the generated `settings.json`. + +## Forge workflow (Phase 2) + +Handler doesn't give the operator forge commands. It **configures forge for the agents** +and lets them drive a role-based dev workflow themselves — the operator only sets a +project's `credential_ref` (and optionally a `FORGE_VERSION` pin). + +- **Three roles, three agents.** A `junior` agent writes the change and opens a PR; a + `senior` agent reviews it and records an approval; a `deploy` agent merges and ships it. + Each is a separate agent with its own tmux session and working dir, so review is a + genuine second context — not the author signing off on their own work. +- **Skills, committed into the repo.** `handler forge-init` writes role skills + (`forge-junior`, `forge-senior`, `forge-deploy`, plus an overview) into the managed + repo's `.claude/skills/` and commits them, so the workflow travels with the code and is + visible to humans. `forge` itself is already authenticated inside each agent, so it + works the same across GitHub/GitLab/Gitea/Forgejo/Bitbucket. +- **A hard approval gate.** A merge or deploy command (`forge … merge`, `mise run deploy`) + — and a direct `git push` to a protected branch (`main`/`master`, see `PROTECTED_BRANCHES`) + — is *denied* unless a standing `approved` record exists for the current branch, made by + a **different** agent than the one merging, and still pinned to the reviewed commit + (pushing new commits invalidates a stale approval). Same block-on-failure mechanism as + the test and push gates — the senior's `handler approve` is what unlocks it, no agent can + approve its own branch, and the protected-branch rule closes the "merge locally, push to + main" path around it. +- **CI follow-through.** When a push clears the local gates, Handler records the commit + with `ci_status = 'pending'`. The `handler poll-ci` poller then asks `forge ci list` for + the runs tied to that commit and backfills the authoritative verdict — one interface, + any forge, no inbound webhook. + +### Credentials — resolution, not storage (README 3.7) + +The database never stores a raw token. A project's `credential_ref` is a **pointer**: + +| Form | Meaning | +|---|---| +| `env:VAR_NAME` | read the value from an environment variable | +| `file:/path` | read (and strip) the value from a file | +| `cmd:some command` | run the command; its stdout is the value | + +At spawn the control layer resolves the pointer and injects the value into that one +agent's environment as `FORGE_TOKEN` (plus the host-specific `GITHUB_TOKEN` / +`GITEA_TOKEN` / … when the remote is recognized). A repo-local git credential helper is +installed that hands the same value back for HTTPS push/pull — so one secret services both +`forge` and `git`, and the raw token lives only in the process environment, never on disk +or in the database. ## Development ```bash -pytest # 45 tests, entirely on SQLite — no live claude/tmux/mise needed +pytest # 106 tests, entirely on SQLite — no live claude/tmux/mise/forge/git needed ruff check . # lint # or, via the project's own mise tasks: mise run verify # lint + test ``` The suite drives every API route through FastAPI's `TestClient`, exercises all four hook -types, and runs a real `alembic upgrade head` per test so the migration path itself is -covered. Three seams — `control.tmux`, `hooks.verify`, and `control.spawn.resume` — are -the mock points that stand in for live `claude`/`tmux`/`mise`, and the drop-in points for -wiring them up for real. +types plus the approval gate, credential resolution, the skills generator, and the CI +poller, and runs a real `alembic upgrade head` per test so the migration path itself is +covered. The seams — `control.tmux`, `control.forge`, `control.gitops`, `hooks.verify`, +and `control.spawn.resume` — are the mock points that stand in for live +`claude`/`tmux`/`mise`/`forge`/`git`, and the drop-in points for wiring them up for real. ## Project layout @@ -212,8 +273,9 @@ src/handler/ config.py # env-driven settings, shared by every entrypoint db/ # SQLAlchemy Core schema, engine, portable types, upsert, DAL api/ # FastAPI app, auth deps, pydantic schemas, routes - control/ # CLI, tmux/worktree/settings-gen seams, spawn orchestration - hooks/ # Stop/SessionEnd, PreToolUse gate, Notification, verify seam + control/ # CLI, tmux/worktree/settings-gen seams, spawn orchestration, + # forge/gitops seams, credentials, skills_gen, CI poller + hooks/ # Stop/SessionEnd, PreToolUse gate (push + approval), Notification migrations/ # Alembic env + versions tests/ # DB, API, hook, and control tests (SQLite) docs/PLAN.md # full design + phased roadmap (the original plan of action) @@ -221,10 +283,11 @@ docs/PLAN.md # full design + phased roadmap (the original plan of acti ## Roadmap -Phase 1 (this MVP) is the control layer + API. Still ahead: **Phase 2** forge integration -and credential resolution (PRs/issues/CI status via `forge`, one interface across GitHub / -GitLab / Gitea / Forgejo / Bitbucket), **Phase 3** a web UI, **Phase 4** optional -observability, and **Phase 5** open-source release. Details and design rationale live in +Phase 1 (the MVP) is the control layer + API. **Phase 2** (forge integration) is +implemented: credential resolution + injection, role-based forge-workflow skills, the hard +approval gate, and the CI-status poller — one interface across GitHub / GitLab / Gitea / +Forgejo / Bitbucket. Still ahead: **Phase 3** a web UI, **Phase 4** optional observability, +and **Phase 5** open-source release. Details and design rationale live in [`docs/PLAN.md`](docs/PLAN.md). ## License diff --git a/docs/PLAN.md b/docs/PLAN.md index e1d0427..d2ee6b7 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -1,7 +1,7 @@ # Remote Control Wrapper — Plan of Action **Status:** Living document — update phase checkboxes and status in place as work completes. Don't append new copies of this file; overwrite it, the way a checkmark gets overwritten. -**Last updated:** 2026-07-08 (revision 8: Phase 1 MVP vertical slice implemented — see checkboxes and README.md) +**Last updated:** 2026-07-08 (revision 9: Phase 2 forge integration implemented — credential resolution/injection, role-based forge-workflow skills, hard approval gate, CI poller; 94 tests. See checkboxes and README.md) --- @@ -210,10 +210,27 @@ agent spawning against a real `claude` binary + tmux is deferred behind mocked s **Definition of done:** run several projects side by side, each with its own agents, working directories, toolchain, and history, entirely through `curl` + a token, against either a live Postgres instance or the SQLite fallback — no Gitea, no Tailscale, no UI required, no state stored anywhere the control layer or API containers themselves live, nothing crosses a project boundary unless explicitly shared, no agent reaches `done` without a passing test run to show for it, and no push leaves that a local build already proved would fail. ### Phase 2 — Forge integration -- [ ] Repo-scoped actions via `forge`: branch creation, PR open, issue linking — one interface across GitHub/GitLab/Gitea/Forgejo/Bitbucket instead of a per-host integration -- [ ] Pin `forge` to a specific released version, not `@latest` — accepted risk given the project's youth, mitigated by not floating on a moving target -- [ ] Credential resolution (section 3.7): `credential_ref` → injected env var at spawn, shared by `forge` and git's credential helper -- [ ] Background poller reading back CI run results via `forge ci list` / `forge ci log`, backfilling `ci_status`/`ci_checked_at` on the log entry that recorded the push (section 3.6) + +Design decision (operator, 2026-07-08): forge is surfaced to the **agents as Claude Code +skills**, not to the operator as CLI/API commands. Handler configures forge (credential +resolution + version pin) and delivers a role-based workflow — **junior** writes and opens +a PR, **senior** reviews and records an approval, **deploy** merges and ships — as three +separate agents handing off through the DB + forge PR. A **hard approval gate** (same +block-on-failure mechanism as the test/push gates) refuses any merge/deploy on a branch +without a standing `approved` record made by a *different* agent. Live `forge`/`git` sit +behind mockable seams (`control.forge`, `control.gitops`), matching Phase 1. + +- [x] Repo-scoped actions via `forge`: branch creation, PR open, issue linking — delivered as committed role skills (`.claude/skills/forge-*`, `control.skills_gen`) the agents run against an already-authenticated `forge`; one interface across GitHub/GitLab/Gitea/Forgejo/Bitbucket +- [x] Pin `forge` to a specific released version, not `@latest` — `FORGE_VERSION` config, verified at spawn against `forge --version` (non-fatal warning on drift; the base image is the real pin) +- [x] Credential resolution (section 3.7): `credential_ref` (`env:`/`file:`/`cmd:`) → resolved at spawn and injected as `FORGE_TOKEN` + host-specific var, plus a git credential helper reading the same value from env (raw token never on disk / in the DB); resolution is a hard fail-fast gate before the agent row is written +- [x] Background poller reading back CI run results via `forge ci list` / `forge ci log`, backfilling `ci_status`/`ci_checked_at` on the log entry that recorded the push (section 3.6) — `control.poller`, `handler poll-ci` (one-shot sweep + optional `--watch`) +- [x] Hard approval gate (new, from the operator decision): `approvals` table + `PreToolUse` deny on `forge … merge` / `mise run deploy` — **and a direct `git push` to a protected branch** (`PROTECTED_BRANCHES`, default `main,master`) — unless a *different* agent has approved the current branch. Approvals are **pinned to the reviewed commit** (`approved_sha`), so new post-review commits invalidate a stale approval. Senior records verdicts via `handler approve` / `handler reject`. Credential helper is **scoped to the forge host** so the token is never offered to an arbitrary HTTPS URL. + +Reviewed by a separate `code-reviewer` pass; findings on gate bypass (local-merge→push, stale branch-scoped approvals, host-unscoped credential helper, an over-broad merge regex, and the CI `action_required` conclusion) were all addressed before completion. 106 tests. + +**Note:** fixed a latent migration bug found while adding migration `0002` — under Python 3.12+ pysqlite only flushes a DDL statement when a later statement forces it, so the final migration's DDL and the `alembic_version` stamp were being rolled back on close. `migrations/env.py` now commits explicitly after `run_migrations()`. (Latent in Phase 1 because each test starts from a fresh single-migration DB.) + +**Definition of done:** an operator registers a project with a `credential_ref`, runs `handler forge-init`, and spawns junior/senior/deploy agents; the junior opens a PR, the senior approves via `handler approve`, and only then can the deploy agent merge — enforced by the gate, not convention — with the push→CI verdict recorded back automatically, all against any forge `forge` supports and with no raw credential ever stored. ### Phase 3 — Production UI - [ ] Web frontend, API-backed only (same contract as `curl`) diff --git a/src/handler/config.py b/src/handler/config.py index d9fe355..eaa763d 100644 --- a/src/handler/config.py +++ b/src/handler/config.py @@ -36,6 +36,21 @@ class Settings(BaseSettings): claude_bin: str = "claude" mise_bin: str = "mise" tmux_bin: str = "tmux" + forge_bin: str = "forge" + git_bin: str = "git" + + # The pinned `forge` version (README 3.6 / Phase 2: pin, never float on @latest). + # When set, spawn verifies the injected forge matches and records a mismatch; when + # empty the check is skipped. Operators align this with what their base image installs. + forge_version: str = "" + + # Branches a direct `git push` may not reach without a standing approval — this closes + # the "merge locally, push to main" path around the forge-merge approval gate. + protected_branches: str = "main,master" + + @property + def protected_branch_set(self) -> set[str]: + return {b.strip() for b in self.protected_branches.split(",") if b.strip()} @property def effective_shared_write_token(self) -> str: diff --git a/src/handler/control/cli.py b/src/handler/control/cli.py index dcb4888..d18c5f6 100644 --- a/src/handler/control/cli.py +++ b/src/handler/control/cli.py @@ -1,7 +1,10 @@ -"""``handler`` CLI — spawn/list/attach/kill. +"""``handler`` CLI — the control layer's write side. -The DB is the source of truth for what agents exist; tmux is cross-checked for -liveness. All commands are project-namespaced. +Spawn/list/attach/kill manage agent processes. Phase 2 adds the forge-workflow control +commands: ``approve``/``reject`` (the senior agent records its verdict, which the deploy +gate checks), ``poll-ci`` (backfill CI verdicts), and ``forge-init`` (write the role +skills into a managed repo). The DB is the source of truth for what agents exist; tmux is +cross-checked for liveness. All commands are project-namespaced. """ from __future__ import annotations @@ -12,7 +15,7 @@ import sys from ..db import repository as repo from ..db.engine import connection -from . import spawn, tmux +from . import poller, skills_gen, spawn, tmux def _cmd_spawn(args: argparse.Namespace) -> int: @@ -23,12 +26,17 @@ def _cmd_spawn(args: argparse.Namespace) -> int: subdir=args.dir, worktree_branch=args.worktree, task=args.task, + role=args.role, ) except spawn.SpawnError as exc: print(f"error: {exc}", file=sys.stderr) return 1 print(f"spawned agent '{agent['name']}' (id={agent['id']}) in project '{args.project}'") print(f" working_dir: {agent['working_dir']}") + if args.role: + print(f" role: {args.role}") + if agent.get("forge_note"): + print(f" warning: {agent['forge_note']}", file=sys.stderr) print(f" tmux session: {tmux.session_name(args.project, args.name)}") return 0 @@ -44,7 +52,10 @@ def _cmd_list(args: argparse.Namespace) -> int: for agent in repo.list_agents(conn, project_id): session = tmux.session_name(project_id, agent["name"]) alive = "live" if session in live else "-" - print(f"{project_id}/{agent['name']}\t{agent['status']}\t{alive}\t{session}") + role = agent.get("role") or "-" + print( + f"{project_id}/{agent['name']}\t{role}\t{agent['status']}\t{alive}\t{session}" + ) return 0 @@ -68,6 +79,118 @@ def _cmd_kill(args: argparse.Namespace) -> int: return 0 +def _resolve_identity(args: argparse.Namespace) -> tuple[str, int] | None: + """Resolve (project_id, acting_agent_id) for approve/reject from flags or env. + + The senior agent runs these from inside its session, where ``HANDLER_PROJECT_ID`` / + ``HANDLER_AGENT_ID`` are set; flags override for manual/operator use. The acting + agent must exist — the approval FK and the gate's "different agent" check both rely + on a real agent id. + """ + project_id = args.project or os.environ.get("HANDLER_PROJECT_ID") + agent_id_raw = args.by_agent or os.environ.get("HANDLER_AGENT_ID") + if not project_id: + print("error: no project (pass --project or set HANDLER_PROJECT_ID)", file=sys.stderr) + return None + if not agent_id_raw: + print( + "error: no acting agent (pass --by-agent or set HANDLER_AGENT_ID)", + file=sys.stderr, + ) + return None + with connection() as conn: + agent = repo.get_agent_by_id(conn, int(agent_id_raw)) + if agent is None or agent["project_id"] != project_id: + print( + f"error: agent id={agent_id_raw} not found in project '{project_id}'", + file=sys.stderr, + ) + return None + return project_id, int(agent_id_raw) + + +def _record_verdict(args: argparse.Namespace, status: str) -> int: + resolved = _resolve_identity(args) + if resolved is None: + return 1 + project_id, agent_id = resolved + + # Pin an approval to the reviewed commit so later pushes to the branch invalidate it. + # Prefer an explicit --sha; otherwise read HEAD of the reviewing agent's working dir. + approved_sha = args.sha + if approved_sha is None and status == "approved": + from . import gitops + + with connection() as conn: + agent = repo.get_agent_by_id(conn, agent_id) + if agent is not None: + approved_sha = gitops.head_sha(agent["working_dir"]) + + with connection() as conn: + approval = repo.record_approval( + conn, + project_id=project_id, + branch=args.branch, + status=status, + approved_by_agent_id=agent_id, + pr_ref=args.pr, + note=args.note, + approved_sha=approved_sha, + ) + sha_note = f" @ {approved_sha[:12]}" if approved_sha else "" + print( + f"{status} branch '{args.branch}'{sha_note} in '{project_id}' " + f"(approval id={approval['id']}, by agent id={agent_id})" + ) + return 0 + + +def _cmd_approve(args: argparse.Namespace) -> int: + return _record_verdict(args, "approved") + + +def _cmd_reject(args: argparse.Namespace) -> int: + return _record_verdict(args, "rejected") + + +def _cmd_poll_ci(args: argparse.Namespace) -> int: + if args.watch: + for summary in poller.watch(project_id=args.project, interval=args.interval): + print( + f"sweep: checked={summary['checked']} resolved={summary['resolved']} " + f"pending={summary['pending']}" + ) + return 0 # pragma: no cover - watch loops until interrupted + summary = poller.sweep(project_id=args.project) + print( + f"checked={summary['checked']} resolved={summary['resolved']} " + f"pending={summary['pending']}" + ) + return 0 + + +def _cmd_forge_init(args: argparse.Namespace) -> int: + with connection() as conn: + project = repo.get_project(conn, args.project) + if project is None: + print(f"error: project '{args.project}' not registered", file=sys.stderr) + return 1 + root = project["root_dir"] + written = skills_gen.write_skills(root) + print(f"wrote {len(written)} forge skill file(s) under {root}/.claude/skills/") + if not args.no_commit: + from . import gitops + + rel = os.path.join(".claude", "skills") + ok_add, _ = gitops.add(root, [rel]) + ok_commit, out = gitops.commit(root, "chore: add handler forge-workflow skills") + if ok_add and ok_commit: + print("committed the skills into the repo") + else: + print(f"note: could not auto-commit ({out}); commit {rel} yourself", file=sys.stderr) + return 0 + + def build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser(prog="handler", description="Handler control layer") sub = parser.add_subparsers(dest="command", required=True) @@ -75,6 +198,7 @@ def build_parser() -> argparse.ArgumentParser: p_spawn = sub.add_parser("spawn", help="spawn an agent") p_spawn.add_argument("--project", required=True) p_spawn.add_argument("--name", required=True) + p_spawn.add_argument("--role", choices=["junior", "senior", "deploy"], help="workflow role") group = p_spawn.add_mutually_exclusive_group() group.add_argument("--worktree", metavar="BRANCH", help="git worktree on BRANCH") group.add_argument("--dir", metavar="SUBDIR", help="subdirectory under project root") @@ -95,6 +219,32 @@ def build_parser() -> argparse.ArgumentParser: p_kill.add_argument("--name", required=True) p_kill.set_defaults(func=_cmd_kill) + for verb, helptext, fn in ( + ("approve", "approve a branch for merge/deploy", _cmd_approve), + ("reject", "reject a branch (request changes)", _cmd_reject), + ): + p = sub.add_parser(verb, help=helptext) + p.add_argument("--branch", required=True, help="the branch being reviewed") + p.add_argument("--project", help="defaults to $HANDLER_PROJECT_ID") + p.add_argument( + "--by-agent", type=int, help="acting agent id; defaults to $HANDLER_AGENT_ID" + ) + p.add_argument("--pr", help="optional forge PR number/URL") + p.add_argument("--note", help="reason / review notes") + p.add_argument("--sha", help="pin the approval to this commit (defaults to HEAD)") + p.set_defaults(func=fn) + + p_poll = sub.add_parser("poll-ci", help="backfill CI verdicts for pending pushes") + p_poll.add_argument("--project", help="limit to one project") + p_poll.add_argument("--watch", action="store_true", help="loop instead of a single sweep") + p_poll.add_argument("--interval", type=float, default=30.0, help="seconds between sweeps") + p_poll.set_defaults(func=_cmd_poll_ci) + + p_forge = sub.add_parser("forge-init", help="write the forge-workflow skills into a repo") + p_forge.add_argument("--project", required=True) + p_forge.add_argument("--no-commit", action="store_true", help="write but don't git-commit") + p_forge.set_defaults(func=_cmd_forge_init) + return parser diff --git a/src/handler/control/credentials.py b/src/handler/control/credentials.py new file mode 100644 index 0000000..6601659 --- /dev/null +++ b/src/handler/control/credentials.py @@ -0,0 +1,167 @@ +"""Credential resolution + injection (README 3.7). + +The database never stores a raw token. ``projects.credential_ref`` is a *pointer* — +``env:VAR_NAME`` / ``file:/path`` / ``cmd:some command`` — mirroring forge's own +``--token-cmd`` pattern one level up. The control layer resolves it to an actual value +only at spawn time and injects it into that one agent's container environment; nothing +is persisted, nothing is baked into an image. + +One token, two consumers: ``forge`` reads the resolved token straight from the +environment (``GITHUB_TOKEN`` / ``GITEA_TOKEN`` / ``GITLAB_TOKEN`` per host, plus a +generic ``FORGE_TOKEN``), and git's HTTPS auth reads the same value through a credential +helper we install at spawn (see :func:`credential_env` and +:func:`git_credential_helper_value`). +""" + +from __future__ import annotations + +import os +import shlex +import subprocess +from urllib.parse import urlsplit + +# The env var Handler always injects and that the git credential helper reads back. +CANONICAL_TOKEN_ENV = "FORGE_TOKEN" + +# Map a remote-host match -> the token env var that host's forge/CLI conventions expect. +# FORGE_TOKEN is always set too, so an unknown host still works with forge. Matched +# against the parsed hostname (exact, or a dotted suffix) so a repo merely *named* +# "gitea" doesn't misfire. +_HOST_TOKEN_ENV = { + "github.com": "GITHUB_TOKEN", + "gitlab.com": "GITLAB_TOKEN", + "gitea.com": "GITEA_TOKEN", + "codeberg.org": "GITEA_TOKEN", + "bitbucket.org": "BITBUCKET_TOKEN", +} +# Substrings that, when present in the hostname itself, imply a self-hosted forge type. +_HOST_HINT_ENV = {"gitea": "GITEA_TOKEN", "forgejo": "GITEA_TOKEN", "gitlab": "GITLAB_TOKEN"} + + +class CredentialError(Exception): + """Raised when a ``credential_ref`` cannot be resolved to a value.""" + + +def resolve(credential_ref: str | None) -> str | None: + """Resolve a ``credential_ref`` pointer to an actual secret value. + + Returns ``None`` when no ref is configured (a project may not need credentials). + Raises :class:`CredentialError` when a configured ref cannot be resolved — a + misconfigured pointer is an error, not a silent "no token". + """ + if not credential_ref: + return None + ref = credential_ref.strip() + scheme, _, rest = ref.partition(":") + rest = rest.strip() + if not rest: + raise CredentialError(f"credential_ref '{ref}' has no value after '{scheme}:'") + + if scheme == "env": + value = os.environ.get(rest) + if value is None: + raise CredentialError(f"credential_ref env var '{rest}' is not set") + return value + + if scheme == "file": + try: + with open(os.path.expanduser(rest)) as fh: + return fh.read().strip() + except OSError as exc: + raise CredentialError(f"credential_ref file '{rest}' unreadable: {exc}") from exc + + if scheme == "cmd": + try: + result = subprocess.run( + shlex.split(rest), + capture_output=True, + text=True, + timeout=30, + ) + except (OSError, subprocess.SubprocessError) as exc: + raise CredentialError(f"credential_ref cmd '{rest}' failed: {exc}") from exc + if result.returncode != 0: + raise CredentialError( + f"credential_ref cmd '{rest}' exited {result.returncode}: " + f"{(result.stderr or '').strip()}" + ) + return result.stdout.strip() + + raise CredentialError( + f"credential_ref '{ref}' has unknown scheme '{scheme}' " + "(expected env:, file:, or cmd:)" + ) + + +def remote_host(git_remote: str | None) -> str | None: + """Parse the hostname from an https or scp-style (``git@host:path``) remote.""" + if not git_remote: + return None + remote = git_remote.strip() + if "://" in remote: + host = urlsplit(remote).hostname + return host.lower() if host else None + # scp-style: git@github.com:owner/repo.git + if "@" in remote and ":" in remote: + after_at = remote.split("@", 1)[1] + return after_at.split(":", 1)[0].lower() or None + return None + + +def _host_token_env(git_remote: str | None) -> str | None: + host = remote_host(git_remote) + if not host: + return None + for known, env_var in _HOST_TOKEN_ENV.items(): + if host == known or host.endswith("." + known): + return env_var + for hint, env_var in _HOST_HINT_ENV.items(): + if hint in host: + return env_var + return None + + +def git_credential_config(git_remote: str | None) -> tuple[str, str] | None: + """The scoped git config (key, value) that installs the credential helper. + + Scopes the helper to the forge's HTTPS base URL — ``credential.https://host.helper`` + — so the injected token is only ever offered to that host, never to an arbitrary + HTTPS URL the agent might touch. Returns ``None`` for ssh/unknown remotes, where no + HTTPS credential helper is needed (ssh uses deploy keys). + """ + if not git_remote or "://" not in git_remote: + return None + parts = urlsplit(git_remote.strip()) + if parts.scheme not in ("https", "http") or not parts.hostname: + return None + base = f"{parts.scheme}://{parts.hostname}" + return f"credential.{base}.helper", git_credential_helper_value() + + +def credential_env(token: str | None, git_remote: str | None) -> dict[str, str]: + """The environment variables to inject so forge + git both authenticate. + + Always sets ``FORGE_TOKEN`` (the generic name forge accepts and our git helper + reads); additionally sets the host-specific var (``GITHUB_TOKEN`` etc.) when the + remote host is recognized, so per-host tooling works with zero extra config. + """ + if not token: + return {} + env = {CANONICAL_TOKEN_ENV: token} + host_var = _host_token_env(git_remote) + if host_var: + env[host_var] = token + return env + + +def git_credential_helper_value() -> str: + """A git ``credential.helper`` value that hands back the injected token. + + Uses an inline shell helper reading ``$FORGE_TOKEN`` from the environment, so the + raw token lives only in the process environment — never written to disk. The + username ``x-access-token`` is the conventional throwaway that GitHub and most + self-hosted forges accept alongside a PAT-as-password. + """ + return ( + f'!f() {{ echo "username=x-access-token"; echo "password=${CANONICAL_TOKEN_ENV}"; }}; f' + ) diff --git a/src/handler/control/forge.py b/src/handler/control/forge.py new file mode 100644 index 0000000..d031f86 --- /dev/null +++ b/src/handler/control/forge.py @@ -0,0 +1,79 @@ +"""Thin ``forge`` wrapper — the mock seam for forge integration (README 3.6 / 3.7). + +Handler shells out to ``forge`` for exactly two jobs: verifying the pinned version at +spawn, and reading CI run status back for the poller. Everything else forge does +(branch creation, PR open, issue linking, merging) is driven by the *agents themselves* +through the generated skills, using the credentials Handler injects — Handler never +opens PRs on their behalf. Keeping our own forge use this small means one seam covers +it, and no live ``forge`` binary is needed in tests. + +``forge`` detects the forge type (GitHub/GitLab/Gitea/Forgejo/Bitbucket) from the git +remote, so ``ci list`` / ``ci log`` are the same two commands regardless of host. +""" + +from __future__ import annotations + +import json +import subprocess + +from ..config import get_settings + +_TIMEOUT = 120 # seconds; forge CI queries are quick metadata reads, not builds. + + +def _run(args: list[str], cwd: str) -> tuple[bool, str]: + forge = get_settings().forge_bin + try: + result = subprocess.run( + [forge, *args], + cwd=cwd, + capture_output=True, + text=True, + timeout=_TIMEOUT, + ) + except FileNotFoundError: + return False, f"'{forge}' not found" + except subprocess.TimeoutExpired: + return False, f"forge {' '.join(args)} timed out after {_TIMEOUT}s" + output = (result.stdout or "") + (result.stderr or "") + return result.returncode == 0, output.strip() + + +def check_version(cwd: str = ".") -> tuple[bool, str]: + """Return ``(matches_pin, reported_version_or_reason)``. + + ``matches_pin`` is True when the configured ``forge_version`` is empty (no pin to + enforce) or when ``forge --version`` reports a string containing it. + """ + pin = get_settings().forge_version + ok, out = _run(["--version"], cwd) + if not ok: + return False, out + if not pin: + return True, out + return (pin in out), out + + +def ci_list(cwd: str, sha: str) -> tuple[bool, list[dict]]: + """List CI runs for a commit via ``forge ci list``. + + Returns ``(ok, runs)``. ``forge`` is asked for JSON; a run dict is expected to carry + at least ``status``/``conclusion`` and an ``id``. On any failure ``ok`` is False and + the list is empty, so the poller simply leaves the entry ``pending`` and retries. + """ + ok, out = _run(["ci", "list", "--sha", sha, "--json"], cwd) + if not ok: + return False, [] + try: + data = json.loads(out) if out else [] + except json.JSONDecodeError: + return False, [] + if isinstance(data, dict): + # Some forges wrap the array, e.g. {"runs": [...]}. + data = data.get("runs") or data.get("data") or [] + return True, data if isinstance(data, list) else [] + + +def ci_log(cwd: str, run_id: str) -> tuple[bool, str]: + """Fetch a CI run's log via ``forge ci log `` (for surfacing why it failed).""" + return _run(["ci", "log", str(run_id)], cwd) diff --git a/src/handler/control/gitops.py b/src/handler/control/gitops.py new file mode 100644 index 0000000..fd7e357 --- /dev/null +++ b/src/handler/control/gitops.py @@ -0,0 +1,57 @@ +"""Thin ``git`` wrapper — the mock seam for the git operations Handler itself runs. + +Handler runs git for a few narrow, non-mutating-or-config-only jobs: reading the +current branch and HEAD sha (so the push/approval gates know *what* is being pushed or +merged), and installing a credential helper at spawn (README 3.7). Agents run their own +git for the actual work; this seam is only Handler's own use, kept behind one module so +tests never touch a real repo. +""" + +from __future__ import annotations + +import subprocess + +from ..config import get_settings + +_TIMEOUT = 30 + + +def _run(args: list[str], cwd: str) -> tuple[bool, str]: + git = get_settings().git_bin + try: + result = subprocess.run( + [git, *args], + cwd=cwd, + capture_output=True, + text=True, + timeout=_TIMEOUT, + ) + except FileNotFoundError: + return False, f"'{git}' not found" + except subprocess.TimeoutExpired: + return False, f"git {' '.join(args)} timed out" + output = (result.stdout or "") + (result.stderr or "") + return result.returncode == 0, output.strip() + + +def current_branch(cwd: str) -> str | None: + ok, out = _run(["rev-parse", "--abbrev-ref", "HEAD"], cwd) + return out if ok and out else None + + +def head_sha(cwd: str) -> str | None: + ok, out = _run(["rev-parse", "HEAD"], cwd) + return out if ok and out else None + + +def config_local(cwd: str, key: str, value: str) -> tuple[bool, str]: + """Set a repo-local git config key (used to install the credential helper).""" + return _run(["config", "--local", key, value], cwd) + + +def add(cwd: str, paths: list[str]) -> tuple[bool, str]: + return _run(["add", *paths], cwd) + + +def commit(cwd: str, message: str) -> tuple[bool, str]: + return _run(["commit", "-m", message], cwd) diff --git a/src/handler/control/poller.py b/src/handler/control/poller.py new file mode 100644 index 0000000..da07143 --- /dev/null +++ b/src/handler/control/poller.py @@ -0,0 +1,101 @@ +"""CI status backfill poller (README 3.6, Phase 2). + +CI is the authoritative build-and-deploy gate; Handler just closes the loop on it. When +the push gate clears a push, it records a log entry with ``push_sha`` and +``ci_status = 'pending'``. This poller — a control-layer process, not a hook, since CI +can take minutes — sweeps those pending entries, asks ``forge ci list`` for the runs tied +to each commit, and backfills ``ci_status`` / ``ci_checked_at`` once a run resolves. + +Same two forge commands regardless of host (``forge`` detects the forge from the remote), +so this is one poller against one interface, not per-host integration. It's built as a +one-shot :func:`sweep` (drive cadence with cron/systemd) plus an optional :func:`watch` +loop for setups with no external scheduler. +""" + +from __future__ import annotations + +import time + +from ..db import repository as repo +from ..db.engine import connection +from . import forge + +# A CI run's terminal "it failed" conclusions, however the forge spells them. +_FAIL_CONCLUSIONS = {"failure", "failed", "error", "cancelled", "canceled", "timed_out"} +# Explicit success *conclusions* (GitHub-style). A terminal-but-non-success conclusion +# such as ``action_required`` / ``neutral`` / ``stale`` is deliberately NOT here, so it +# stays pending rather than masquerading as a pass. +_SUCCESS_CONCLUSIONS = {"success", "succeeded", "passed"} +# Success-ish *statuses*, used only when a run reports no conclusion field at all +# (status-only forges); when a conclusion is present it must be judged on its own. +_SUCCESS_STATUSES = {"completed", "success", "passed"} + + +def _run_field(run: dict, *names: str) -> str: + for name in names: + value = run.get(name) + if isinstance(value, str) and value: + return value.lower() + return "" + + +def classify(runs: list[dict]) -> str: + """Reduce forge's CI runs for a commit to ``pass`` | ``fail`` | ``pending``. + + Conservative: any resolved failure -> ``fail``; all runs resolved and successful -> + ``pass``; anything still queued/running, or no runs yet, stays ``pending`` so the + poller keeps checking rather than declaring a verdict early. + """ + if not runs: + return "pending" + all_resolved = True + for run in runs: + conclusion = _run_field(run, "conclusion", "result") + status = _run_field(run, "status", "state") + if conclusion in _FAIL_CONCLUSIONS: + return "fail" + if conclusion: + # A conclusion is present: it must be an explicit success to count as done. + resolved = conclusion in _SUCCESS_CONCLUSIONS + else: + # No conclusion field (status-only forge): fall back to the status. + resolved = status in _SUCCESS_STATUSES + if not resolved: + all_resolved = False + return "pass" if all_resolved else "pending" + + +def sweep(project_id: str | None = None) -> dict: + """One pass over pending pushes. Returns counts: checked / resolved / still pending.""" + with connection() as conn: + entries = repo.get_pending_ci_entries(conn, project_id=project_id) + + checked = resolved = 0 + for entry in entries: + checked += 1 + ok, runs = forge.ci_list(entry["working_dir"], entry["push_sha"]) + if not ok: + # forge unavailable / remote hiccup — leave it pending and retry next sweep. + continue + verdict = classify(runs) + if verdict == "pending": + continue + with connection() as conn: + repo.update_ci_status(conn, entry["id"], verdict) + resolved += 1 + return {"checked": checked, "resolved": resolved, "pending": checked - resolved} + + +def watch(project_id: str | None = None, interval: float = 30.0, iterations: int | None = None): + """Loop :func:`sweep` forever (or ``iterations`` times, for tests), sleeping between. + + Yields each sweep's summary so a caller/test can observe progress without capturing + stdout. + """ + count = 0 + while iterations is None or count < iterations: + yield sweep(project_id=project_id) + count += 1 + if iterations is not None and count >= iterations: + break + time.sleep(interval) diff --git a/src/handler/control/skills_gen.py b/src/handler/control/skills_gen.py new file mode 100644 index 0000000..dd7f82d --- /dev/null +++ b/src/handler/control/skills_gen.py @@ -0,0 +1,150 @@ +"""Generate the role-based forge-workflow skills committed into a managed repo. + +Phase 2 surfaces forge to the *agents* as Claude Code skills, not to the operator as +CLI/API commands. The operator only configures credentials + the version pin; the agents +then drive the whole dev workflow — junior writes and opens a PR, senior reviews and +records an approval, deploy merges and ships once approved — using ``forge`` (already +authenticated by the injected credentials) and the ``handler`` approval commands. + +These skills are *committed into the managed repo* (``.claude/skills//SKILL.md``) +so they travel with the code and are visible to humans, not regenerated per agent. Each +role's agent naturally follows the skill matching its role; the hard approval gate in +``hooks.gate`` enforces the handoff regardless of what any skill says. +""" + +from __future__ import annotations + +import os + +# One skill per role, plus a short workflow overview. Kept as plain data so the writer is +# trivial and the content is easy to review/diff. Each entry is (dirname, front-matter +# name, description, body). +_SKILLS: list[tuple[str, str, str, str]] = [ + ( + "forge-workflow", + "forge-workflow", + "Overview of the junior -> senior -> deploy forge workflow this repo uses.", + """# Forge workflow (junior -> senior -> deploy) + +This repository is worked by three cooperating Handler agents, each a separate context: + +1. **junior** — writes the change on a feature branch and opens a pull request. +2. **senior** — reviews that pull request and records an approval (or requests changes). +3. **deploy** — merges the approved branch and deploys it. + +`forge` is already authenticated in this environment (Handler injected the project's +credentials), so it works the same whether the host is GitHub, GitLab, Gitea/Forgejo, or +Bitbucket. Handler enforces the handoffs with hard gates you cannot talk your way past: + +- A `git push` is blocked unless the tests **and** a throwaway image build pass locally. +- A **merge or deploy** — and a direct push to a protected branch (`main`/`master`) — is + blocked unless a *standing approval* exists for the current branch, made by a + **different** agent than the one merging. There is no self-approval, and the approval is + pinned to the reviewed commit: pushing new commits invalidates it and forces re-review. + +Follow the skill for your role: `forge-junior`, `forge-senior`, or `forge-deploy`. +""", + ), + ( + "forge-junior", + "forge-junior", + "Junior dev role: implement the change on a feature branch and open a PR.", + """# Role: junior developer + +You write the change. You do **not** merge or deploy it. + +1. Create a feature branch for the work: + ```bash + forge branch create feat/ # or: git checkout -b feat/ + ``` +2. Implement the change. Commit in small, coherent steps. +3. Before pushing, make sure the tests pass — the push gate will block you otherwise: + ```bash + mise run test + ``` +4. Push and open a pull request for review: + ```bash + git push -u origin feat/ + forge pr create --title "" --body "" + ``` +5. Your checkmark should now read *needs review*. Stop here. A **senior** agent picks it + up next — do not merge your own work; the approval gate will refuse it anyway. +""", + ), + ( + "forge-senior", + "forge-senior", + "Senior dev role: review the open PR and record an approval or request changes.", + """# Role: senior reviewer + +You review a junior's pull request and record a verdict. You do **not** write the feature +or deploy it. + +1. Find the branch/PR under review and check it out so you can read the real diff: + ```bash + forge pr list + forge pr checkout # or: git fetch && git checkout feat/ + ``` +2. Review thoroughly: correctness, tests, security, and that `mise run test` passes. +3. Record your verdict — this is what the deploy gate checks. Handler reads your identity + and project from the environment; name the branch you reviewed: + ```bash + # approve: + handler approve --branch feat/ --pr --note "" + # or request changes: + handler reject --branch feat/ --note "" + ``` +4. On approval, hand off to the **deploy** agent. On rejection, hand back to the junior. + +Your approval only counts because you are a *different* agent than the author — that +separation is the whole point of the gate. It is also pinned to the exact commit you +reviewed: if the junior pushes more commits afterwards, your approval no longer applies +and you must review again. +""", + ), + ( + "forge-deploy", + "forge-deploy", + "Deploy engineer role: merge the approved branch and deploy it.", + """# Role: deployment engineer + +You merge and ship an already-approved branch. You do **not** review or approve it. + +1. Check out the branch that was approved so you are *on* it (the gate checks the current + branch's approval): + ```bash + git fetch && git checkout feat/ + ``` +2. Merge and deploy. Both are gated — if there is no standing approval for this branch + from a different agent, Handler denies the command and tells you why: + ```bash + forge pr merge # blocked without an approval + mise run deploy # blocked without an approval + ``` +3. After the push/merge lands, CI does the authoritative build-and-deploy on its own + runner. Handler's poller records the CI verdict back onto the log entry for that + commit; you don't need to watch it manually. +""", + ), +] + + +def skill_files() -> dict[str, str]: + """Return ``relative_path -> file_contents`` for every generated skill file.""" + files: dict[str, str] = {} + for dirname, name, description, body in _SKILLS: + front = f"---\nname: {name}\ndescription: {description}\n---\n\n" + files[os.path.join(".claude", "skills", dirname, "SKILL.md")] = front + body + return files + + +def write_skills(repo_root: str) -> list[str]: + """Write the role skills under ``repo_root/.claude/skills/``; return written paths.""" + written: list[str] = [] + for rel_path, contents in skill_files().items(): + abs_path = os.path.join(repo_root, rel_path) + os.makedirs(os.path.dirname(abs_path), exist_ok=True) + with open(abs_path, "w") as fh: + fh.write(contents) + written.append(abs_path) + return written diff --git a/src/handler/control/spawn.py b/src/handler/control/spawn.py index 473cde3..63184e1 100644 --- a/src/handler/control/spawn.py +++ b/src/handler/control/spawn.py @@ -15,7 +15,7 @@ import tomllib from ..config import get_settings from ..db import repository as repo from ..db.engine import connection -from . import settings_gen, tmux, worktree +from . import credentials, forge, gitops, settings_gen, tmux, worktree class SpawnError(Exception): @@ -52,6 +52,21 @@ def _shell_quote(value: str) -> str: return "'" + value.replace("'", "'\\''") + "'" +def _install_git_credentials(working_dir: str, git_remote: str | None) -> None: + """Install a repo-local git credential helper that reads the injected token. + + The helper hands back ``$FORGE_TOKEN`` from the environment, so the raw value is never + written to disk (README 3.7: one secret servicing both forge and git), and it is + *scoped to the forge host* so the token is never offered to an arbitrary HTTPS URL. + A no-op for ssh/unknown remotes (deploy keys handle those). Best-effort — a + working_dir that isn't a git repo yet shouldn't block the spawn. + """ + cfg = credentials.git_credential_config(git_remote) + if cfg is not None: + key, value = cfg + gitops.config_local(working_dir, key, value) + + def spawn( project_id: str, name: str, @@ -59,6 +74,7 @@ def spawn( subdir: str | None = None, worktree_branch: str | None = None, task: str | None = None, + role: str | None = None, ) -> dict: """Create and launch an agent. Returns the agent row.""" with connection() as conn: @@ -72,11 +88,22 @@ def spawn( project["root_dir"], name, subdir=subdir, worktree_branch=worktree_branch ) - # Hard gate before any state is written or process launched. + # Hard gates before any state is written or process launched: the test task must + # exist, and a configured credential_ref must actually resolve — a broken + # pointer should fail fast, not leave an orphaned agent row behind. require_test_task(working_dir) + try: + token = credentials.resolve(project.get("credential_ref")) + except credentials.CredentialError as exc: + raise SpawnError(str(exc)) from exc agent = repo.create_agent( - conn, project_id=project_id, name=name, working_dir=working_dir, status="working" + conn, + project_id=project_id, + name=name, + working_dir=working_dir, + status="working", + role=role, ) settings_path = settings_gen.write_settings(working_dir) @@ -87,12 +114,34 @@ def spawn( "HANDLER_AGENT_ID": str(agent["id"]), "DATABASE_URL": get_settings().database_url, } + if role: + env["HANDLER_AGENT_ROLE"] = role + env.update(credentials.credential_env(token, project.get("git_remote"))) + if token: + _install_git_credentials(working_dir, project.get("git_remote")) + + # Verify the pinned forge version, if one is configured. Non-fatal: a version drift + # is recorded as a warning rather than blocking the spawn, since not every agent + # touches forge and the base image is the real pin (README 3.6, Phase 2). + forge_note = _check_forge_version(working_dir) + session = tmux.session_name(project_id, name) command = _claude_command(task, settings_path) tmux.new_session(session, cwd=working_dir, command=command, env=env) + agent = {**agent, "forge_note": forge_note} return agent +def _check_forge_version(working_dir: str) -> str | None: + pin = get_settings().forge_version + if not pin: + return None + ok, reported = forge.check_version(working_dir) + if ok: + return None + return f"forge version pin '{pin}' not satisfied: {reported}" + + def kill(project_id: str, name: str) -> None: with connection() as conn: agent = repo.get_agent_by_name(conn, project_id, name) diff --git a/src/handler/db/repository.py b/src/handler/db/repository.py index 6ce2cd3..bb23740 100644 --- a/src/handler/db/repository.py +++ b/src/handler/db/repository.py @@ -21,7 +21,7 @@ from typing import Any from sqlalchemy import Connection, select -from .tables import agents, checkmarks, log_entries, projects, shared_context +from .tables import agents, approvals, checkmarks, log_entries, projects, shared_context from .upsert import upsert_checkmark @@ -113,6 +113,53 @@ def get_shared_context_key(conn: Connection, key: str) -> dict | None: return _row_to_dict(row) +def get_agent_by_id(conn: Connection, agent_id: int) -> dict | None: + row = conn.execute(select(agents).where(agents.c.id == agent_id)).first() + return _row_to_dict(row) + + +def get_latest_approval(conn: Connection, project_id: str, branch: str) -> dict | None: + """The most recent approval/rejection for a branch — the deploy gate's input.""" + row = conn.execute( + select(approvals) + .where(approvals.c.project_id == project_id, approvals.c.branch == branch) + .order_by(approvals.c.id.desc()) + .limit(1) + ).first() + return _row_to_dict(row) + + +def get_pending_ci_entries( + conn: Connection, project_id: str | None = None, limit: int = 200 +) -> list[dict]: + """Log entries that recorded a push and still await a CI verdict (poller input). + + Joins through the agent so the poller knows which project/working-dir each push + belongs to. Scoped to one project when ``project_id`` is given. + """ + stmt = ( + select( + log_entries.c.id, + log_entries.c.push_sha, + log_entries.c.ci_status, + agents.c.id.label("agent_id"), + agents.c.project_id, + agents.c.working_dir, + ) + .select_from(log_entries.join(agents, log_entries.c.agent_id == agents.c.id)) + .where( + log_entries.c.ci_status == "pending", + log_entries.c.push_sha.is_not(None), + ) + .order_by(log_entries.c.id.asc()) + .limit(limit) + ) + if project_id is not None: + stmt = stmt.where(agents.c.project_id == project_id) + rows = conn.execute(stmt).all() + return [dict(r._mapping) for r in rows] + + # -------------------------------------------------------------------------- writes @@ -141,6 +188,7 @@ def create_agent( name: str, working_dir: str, status: str = "working", + role: str | None = None, ) -> dict: result = conn.execute( agents.insert().values( @@ -148,6 +196,7 @@ def create_agent( name=name, working_dir=working_dir, status=status, + role=role, created_at=_now(), ) ) @@ -183,6 +232,44 @@ def upsert_checkmark_row(conn: Connection, agent_id: int, **fields: Any) -> None upsert_checkmark(conn, values) +def record_approval( + conn: Connection, + project_id: str, + branch: str, + status: str, + approved_by_agent_id: int, + pr_ref: str | None = None, + note: str | None = None, + approved_sha: str | None = None, +) -> dict: + """Insert an approval/rejection record for a branch (the senior agent's verdict).""" + result = conn.execute( + approvals.insert().values( + project_id=project_id, + branch=branch, + approved_sha=approved_sha, + pr_ref=pr_ref, + status=status, + approved_by_agent_id=approved_by_agent_id, + note=note, + created_at=_now(), + ) + ) + approval_id = result.inserted_primary_key[0] + row = conn.execute(select(approvals).where(approvals.c.id == approval_id)).first() + return dict(row._mapping) + + +def update_ci_status(conn: Connection, log_entry_id: int, ci_status: str) -> bool: + """Backfill a resolved CI verdict onto the log entry that recorded the push.""" + result = conn.execute( + log_entries.update() + .where(log_entries.c.id == log_entry_id) + .values(ci_status=ci_status, ci_checked_at=_now()) + ) + return result.rowcount > 0 + + def set_shared_context(conn: Connection, key: str, value: str, agent_id: int | None) -> dict: """Upsert one shared-context key (the one table every project implicitly trusts).""" dialect = conn.dialect.name diff --git a/src/handler/db/tables.py b/src/handler/db/tables.py index ef27482..04983fb 100644 --- a/src/handler/db/tables.py +++ b/src/handler/db/tables.py @@ -12,6 +12,7 @@ from sqlalchemy import ( CheckConstraint, Column, ForeignKey, + Index, MetaData, String, Table, @@ -29,6 +30,7 @@ AGENT_STATUSES = ("working", "paused_for_input", "blocked", "done") GATE_STATUSES = ("pass", "fail", "unknown") CI_STATUSES = ("not_applicable", "pending", "pass", "fail") VISIBILITIES = ("project", "global") +APPROVAL_STATUSES = ("approved", "rejected") def _in(column: str, values: tuple[str, ...]) -> str: @@ -55,6 +57,9 @@ agents = Table( Column("name", String, nullable=False), # unique within a project, not globally Column("working_dir", String, nullable=False), Column("status", String, nullable=False), + # Optional workflow role (junior | senior | deploy) — informational, drives which + # forge skill an agent follows; the approval gate keys on identity, not role. + Column("role", String), Column("created_at", PortableTimestamp, nullable=False, server_default=func.now()), UniqueConstraint("project_id", "name", name="uq_agents_project_name"), CheckConstraint(_in("status", AGENT_STATUSES), name="ck_agents_status"), @@ -113,3 +118,24 @@ shared_context = Table( Column("set_by_agent_id", BigInteger, ForeignKey("agents.id")), Column("updated_at", PortableTimestamp, nullable=False, server_default=func.now()), ) + +# The record the hard approval gate checks (Phase 2). A senior agent writes one per +# branch it approves; the deploy gate refuses to merge/deploy a branch whose latest +# approval isn't ``approved`` and wasn't made by a *different* agent than the one pushing +# — so review is a genuine second context, never self-approval. ``approved_sha`` pins the +# approval to the reviewed commit so pushing new commits invalidates a stale approval. +approvals = Table( + "approvals", + metadata, + Column("id", PortableBigInt, primary_key=True, autoincrement=True), + Column("project_id", String, ForeignKey("projects.id"), nullable=False), + Column("branch", String, nullable=False), + Column("approved_sha", String), # the HEAD the reviewer signed off on, when known + Column("pr_ref", String), # optional forge PR number/URL, for traceability + Column("status", String, nullable=False), + Column("approved_by_agent_id", BigInteger, ForeignKey("agents.id"), nullable=False), + Column("note", String), + Column("created_at", PortableTimestamp, nullable=False, server_default=func.now()), + CheckConstraint(_in("status", APPROVAL_STATUSES), name="ck_approvals_status"), + Index("ix_approvals_project_branch", "project_id", "branch"), +) diff --git a/src/handler/hooks/gate.py b/src/handler/hooks/gate.py index 25bccf1..4a183ce 100644 --- a/src/handler/hooks/gate.py +++ b/src/handler/hooks/gate.py @@ -19,11 +19,16 @@ from datetime import UTC, datetime from sqlalchemy import Connection +from ..config import get_settings +from ..control import gitops from ..db import repository as repo from . import verify from .context import HookInput, Identity, emit _GIT_PUSH = re.compile(r"\bgit\s+push\b") +# Actions that ship code past review: a forge merge, or the canonical deploy task. Anchored +# to the actual subcommands so a PR title/commit message mentioning "merge" doesn't trip it. +_MERGE_DEPLOY = re.compile(r"\bforge\s+(?:pr\s+)?merge\b|\bmise\s+run\s+deploy\b") def _deny(reason: str) -> dict: @@ -118,15 +123,84 @@ def handle_git_push(conn: Connection, ident: Identity, hook_input: HookInput) -> if not build_ok: return _deny(f"Push blocked: image build failed.\n{build_out[-3000:]}") + # A direct push to a protected branch ships code without going through a forge merge, + # so it must clear the same approval gate — this closes the "merge locally, push main" + # path around the review requirement. + branch = gitops.current_branch(working_dir) + sha = gitops.head_sha(working_dir) + if branch in get_settings().protected_branch_set: + ok, reason = _approval_ok(conn, ident, branch, sha) + if not ok: + return _deny(f"Push to protected branch blocked: {reason}") + + # The push is cleared to leave. Record the commit + mark CI pending so the poller + # closes the loop on the authoritative CI verdict (README 3.6). + if sha: + repo.insert_log_entry( + conn, + agent_id=ident.agent_id, + status="working", + session_id=hook_input.session_id, + summary=f"push cleared local gates: {sha[:12]}", + push_sha=sha, + ci_status="pending", + ) return _allow("tests and image build passed") +def _approval_ok( + conn: Connection, ident: Identity, branch: str, current_sha: str | None +) -> tuple[bool, str]: + """Shared approval check: a standing ``approved`` for ``branch``, by a *different* + agent, still pinned to the current commit when the approval recorded a sha.""" + approval = repo.get_latest_approval(conn, ident.project_id, branch) + if approval is None or approval["status"] != "approved": + return False, ( + f"branch '{branch}' has no standing approval. A senior agent must approve it " + "before it can be merged or deployed." + ) + if approval["approved_by_agent_id"] == ident.agent_id: + return False, ( + f"branch '{branch}' was approved by this same agent. Review must come from a " + "different agent — no self-approval." + ) + approved_sha = approval.get("approved_sha") + if approved_sha and current_sha and approved_sha != current_sha: + return False, ( + f"the approval for '{branch}' was for commit {approved_sha[:12]}, but HEAD is " + f"now {current_sha[:12]}. The new commits must be re-reviewed." + ) + return True, f"branch '{branch}' approved by agent id={approval['approved_by_agent_id']}" + + +def handle_merge_deploy(conn: Connection, ident: Identity, hook_input: HookInput) -> dict: + """Hard approval gate: refuse to merge/deploy a branch without a standing approval. + + The approval must exist, be ``approved``, have been made by a *different* agent than + the one now merging — so 'senior approves' is a genuine second context and an agent + can't rubber-stamp its own work — and, when the approval pinned a commit, still match + the current HEAD so post-review commits don't ride in on a stale approval. + """ + working_dir = ident.working_dir or hook_input.cwd or "." + branch = gitops.current_branch(working_dir) + if branch is None: + return _deny( + "Approval gate: could not determine the current git branch, so this " + "merge/deploy cannot be checked against an approval. Aborting." + ) + ok, reason = _approval_ok(conn, ident, branch, gitops.head_sha(working_dir)) + return _allow(reason) if ok else _deny(f"Approval gate: {reason}") + + def handle(conn: Connection, ident: Identity, hook_input: HookInput) -> dict: tool = hook_input.tool_name + command = hook_input.tool_input.get("command", "") if tool == "Bash" else "" if tool == "AskUserQuestion": result = handle_ask_user_question(conn, ident, hook_input) - elif tool == "Bash" and _GIT_PUSH.search(hook_input.tool_input.get("command", "")): + elif tool == "Bash" and _GIT_PUSH.search(command): result = handle_git_push(conn, ident, hook_input) + elif tool == "Bash" and _MERGE_DEPLOY.search(command): + result = handle_merge_deploy(conn, ident, hook_input) else: # Not our concern — stay out of the way, let normal permission flow proceed. result = {} diff --git a/src/handler/migrations/env.py b/src/handler/migrations/env.py index ee67c8a..084c663 100644 --- a/src/handler/migrations/env.py +++ b/src/handler/migrations/env.py @@ -62,6 +62,11 @@ def run_migrations_online() -> None: ) with context.begin_transaction(): context.run_migrations() + # Explicit commit: on Python 3.12+ the pysqlite driver only flushes a DDL + # statement when a *later* statement forces it, so without this the final + # migration's DDL and the alembic_version stamp are rolled back on close. This + # is a no-op when the transaction was already committed. + connection.commit() if context.is_offline_mode(): diff --git a/src/handler/migrations/versions/0002_forge_approvals.py b/src/handler/migrations/versions/0002_forge_approvals.py new file mode 100644 index 0000000..dff7a11 --- /dev/null +++ b/src/handler/migrations/versions/0002_forge_approvals.py @@ -0,0 +1,57 @@ +"""forge phase 2: agent role + approvals + +Revision ID: 0002_forge_approvals +Revises: 0001_initial +Create Date: 2026-07-08 + +Adds the nullable ``agents.role`` column (junior | senior | deploy, informational) and +the ``approvals`` table the hard deploy/merge gate checks (README Phase 2). Hand-written +like 0001 so both dialects render correctly; SQLite supports ``ALTER TABLE ADD COLUMN`` +natively, so a plain ``op.add_column`` works on both backends. +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +from handler.db.types import PortableBigInt, PortableTimestamp + +revision: str = "0002_forge_approvals" +down_revision: str | None = "0001_initial" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +APPROVAL_STATUSES = "'approved', 'rejected'" + + +def upgrade() -> None: + op.add_column("agents", sa.Column("role", sa.String())) + + op.create_table( + "approvals", + sa.Column("id", PortableBigInt, primary_key=True, autoincrement=True), + sa.Column("project_id", sa.String(), sa.ForeignKey("projects.id"), nullable=False), + sa.Column("branch", sa.String(), nullable=False), + sa.Column("approved_sha", sa.String()), + sa.Column("pr_ref", sa.String()), + sa.Column("status", sa.String(), nullable=False), + sa.Column( + "approved_by_agent_id", + sa.BigInteger(), + sa.ForeignKey("agents.id"), + nullable=False, + ), + sa.Column("note", sa.String()), + sa.Column("created_at", PortableTimestamp, nullable=False, server_default=sa.func.now()), + sa.CheckConstraint(f"status IN ({APPROVAL_STATUSES})", name="ck_approvals_status"), + ) + op.create_index("ix_approvals_project_branch", "approvals", ["project_id", "branch"]) + + +def downgrade() -> None: + op.drop_index("ix_approvals_project_branch", table_name="approvals") + op.drop_table("approvals") + op.drop_column("agents", "role") diff --git a/tests/conftest.py b/tests/conftest.py index 5e22256..1e76c39 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -105,3 +105,45 @@ def fake_tmux(monkeypatch): monkeypatch.setattr(tmux, "list_sessions", list_sessions) return {"calls": calls, "live": live} + + +@pytest.fixture +def fake_gitops(monkeypatch): + """Fake the git seam: record config/add/commit, return a controllable branch/sha.""" + from handler.control import gitops + + state = {"branch": "feat/x", "sha": "abc123def456", "config": [], "add": [], "commit": []} + + def config_local(cwd, key, value): + state["config"].append({"cwd": cwd, "key": key, "value": value}) + return True, "" + + def add(cwd, paths): + state["add"].append({"cwd": cwd, "paths": paths}) + return True, "" + + def commit(cwd, message): + state["commit"].append({"cwd": cwd, "message": message}) + return True, "" + + monkeypatch.setattr(gitops, "current_branch", lambda cwd: state["branch"]) + monkeypatch.setattr(gitops, "head_sha", lambda cwd: state["sha"]) + monkeypatch.setattr(gitops, "config_local", config_local) + monkeypatch.setattr(gitops, "add", add) + monkeypatch.setattr(gitops, "commit", commit) + return state + + +@pytest.fixture +def fake_forge(monkeypatch): + """Fake the forge seam: controllable version check + CI runs.""" + from handler.control import forge + + state = {"version_ok": True, "version_out": "forge 1.2.3", "ci_ok": True, "runs": []} + + monkeypatch.setattr( + forge, "check_version", lambda cwd=".": (state["version_ok"], state["version_out"]) + ) + monkeypatch.setattr(forge, "ci_list", lambda cwd, sha: (state["ci_ok"], state["runs"])) + monkeypatch.setattr(forge, "ci_log", lambda cwd, run_id: (True, "log")) + return state diff --git a/tests/test_cli_phase2.py b/tests/test_cli_phase2.py new file mode 100644 index 0000000..1744d63 --- /dev/null +++ b/tests/test_cli_phase2.py @@ -0,0 +1,79 @@ +"""Phase 2 CLI: approve/reject (env identity), poll-ci, forge-init.""" + +from __future__ import annotations + +from handler.control import cli +from handler.db import repository as repo +from handler.db.engine import get_engine + + +def _seed_project_agent(role="senior", name="senior"): + with get_engine().begin() as conn: + repo.create_project(conn, "p", "/tmp/p") + return repo.create_agent(conn, "p", name, "/tmp/p/s", role=role) + + +def test_approve_via_cli_uses_env_identity(env, monkeypatch, capsys): + agent = _seed_project_agent() + monkeypatch.setenv("HANDLER_PROJECT_ID", "p") + monkeypatch.setenv("HANDLER_AGENT_ID", str(agent["id"])) + + rc = cli.main(["approve", "--branch", "feat/x", "--pr", "7", "--note", "lgtm"]) + assert rc == 0 + with get_engine().begin() as conn: + latest = repo.get_latest_approval(conn, "p", "feat/x") + assert latest["status"] == "approved" + assert latest["approved_by_agent_id"] == agent["id"] + assert latest["pr_ref"] == "7" + + +def test_reject_via_cli(env, monkeypatch): + agent = _seed_project_agent() + monkeypatch.setenv("HANDLER_PROJECT_ID", "p") + monkeypatch.setenv("HANDLER_AGENT_ID", str(agent["id"])) + assert cli.main(["reject", "--branch", "feat/x", "--note", "fix it"]) == 0 + with get_engine().begin() as conn: + assert repo.get_latest_approval(conn, "p", "feat/x")["status"] == "rejected" + + +def test_approve_without_identity_errors(env, monkeypatch, capsys): + _seed_project_agent() + monkeypatch.delenv("HANDLER_PROJECT_ID", raising=False) + monkeypatch.delenv("HANDLER_AGENT_ID", raising=False) + assert cli.main(["approve", "--branch", "feat/x"]) == 1 + assert "no project" in capsys.readouterr().err + + +def test_approve_rejects_unknown_agent(env, monkeypatch, capsys): + _seed_project_agent() + monkeypatch.setenv("HANDLER_PROJECT_ID", "p") + monkeypatch.setenv("HANDLER_AGENT_ID", "9999") + assert cli.main(["approve", "--branch", "feat/x"]) == 1 + assert "not found" in capsys.readouterr().err + + +def test_poll_ci_cli(env, fake_forge, capsys): + with get_engine().begin() as conn: + repo.create_project(conn, "p", "/tmp/p") + a = repo.create_agent(conn, "p", "a", "/tmp/p/a") + repo.insert_log_entry(conn, a["id"], status="working", push_sha="s", ci_status="pending") + fake_forge["runs"] = [{"conclusion": "success"}] + assert cli.main(["poll-ci"]) == 0 + assert "resolved=1" in capsys.readouterr().out + + +def test_forge_init_writes_and_commits(env, fake_gitops, capsys): + root = env["tmp"] / "proj" + root.mkdir(parents=True, exist_ok=True) + with get_engine().begin() as conn: + repo.create_project(conn, "proj", str(root)) + + assert cli.main(["forge-init", "--project", "proj"]) == 0 + assert (root / ".claude" / "skills" / "forge-junior" / "SKILL.md").exists() + # Auto-committed via the git seam. + assert len(fake_gitops["commit"]) == 1 + + +def test_forge_init_unknown_project_errors(env, capsys): + assert cli.main(["forge-init", "--project", "nope"]) == 1 + assert "not registered" in capsys.readouterr().err diff --git a/tests/test_control_spawn_forge.py b/tests/test_control_spawn_forge.py new file mode 100644 index 0000000..b50404c --- /dev/null +++ b/tests/test_control_spawn_forge.py @@ -0,0 +1,92 @@ +"""Phase 2 spawn wiring: credential injection, git helper, forge version note, role.""" + +from __future__ import annotations + +import pytest + +from handler.control import spawn +from handler.db import repository as repo +from handler.db.engine import get_engine + + +def _write_mise(root): + root.mkdir(parents=True, exist_ok=True) + (root / ".mise.toml").write_text("[tasks.test]\nrun = 'pytest'\n") + + +def _register(root, **kw): + with get_engine().begin() as conn: + repo.create_project(conn, "proj", str(root), **kw) + + +def test_spawn_injects_credentials_and_installs_helper(env, fake_tmux, fake_gitops, monkeypatch): + monkeypatch.setenv("PROJ_TOKEN", "s3cret") + root = env["tmp"] / "proj" + _write_mise(root) + _register(root, git_remote="https://github.com/me/proj.git", credential_ref="env:PROJ_TOKEN") + + spawn.spawn("proj", "junior", role="junior") + + call = fake_tmux["calls"]["new_session"][0] + # Token injected under the generic + host-specific names, never the raw ref stored. + assert call["env"]["FORGE_TOKEN"] == "s3cret" + assert call["env"]["GITHUB_TOKEN"] == "s3cret" + assert call["env"]["HANDLER_AGENT_ROLE"] == "junior" + # Git credential helper installed, scoped to the forge host (not global). + helper = [c for c in fake_gitops["config"] if c["key"].endswith(".helper")] + assert helper and helper[0]["key"] == "credential.https://github.com.helper" + assert "$FORGE_TOKEN" in helper[0]["value"] + + +def test_spawn_ssh_remote_installs_no_https_helper(env, fake_tmux, fake_gitops, monkeypatch): + monkeypatch.setenv("PROJ_TOKEN", "s3cret") + root = env["tmp"] / "proj" + _write_mise(root) + _register(root, git_remote="git@github.com:me/proj.git", credential_ref="env:PROJ_TOKEN") + spawn.spawn("proj", "junior", role="junior") + # ssh remote -> token still injected, but no HTTPS credential helper installed. + assert fake_tmux["calls"]["new_session"][0]["env"]["GITHUB_TOKEN"] == "s3cret" + assert fake_gitops["config"] == [] + + +def test_spawn_fails_fast_on_broken_credential_ref(env, fake_tmux, fake_gitops, monkeypatch): + monkeypatch.delenv("ABSENT_TOKEN", raising=False) + root = env["tmp"] / "proj" + _write_mise(root) + _register(root, credential_ref="env:ABSENT_TOKEN") + + with pytest.raises(spawn.SpawnError, match="not set"): + spawn.spawn("proj", "junior", role="junior") + # No agent row and no session left behind by the failed spawn. + with get_engine().begin() as conn: + assert repo.get_agent_by_name(conn, "proj", "junior") is None + assert fake_tmux["calls"]["new_session"] == [] + + +def test_spawn_without_credential_ref_injects_no_token(env, fake_tmux, fake_gitops): + root = env["tmp"] / "proj" + _write_mise(root) + _register(root) + spawn.spawn("proj", "api") + call = fake_tmux["calls"]["new_session"][0] + assert "FORGE_TOKEN" not in call["env"] + # No token -> no credential helper installed. + assert fake_gitops["config"] == [] + + +def test_spawn_reports_forge_version_mismatch(env, fake_tmux, fake_gitops, fake_forge, monkeypatch): + monkeypatch.setenv("FORGE_VERSION", "9.9.9") + from handler import config + from handler.db import engine + + config.get_settings.cache_clear() + engine.get_engine.cache_clear() + + root = env["tmp"] / "proj" + _write_mise(root) + _register(root) + fake_forge["version_ok"] = False + fake_forge["version_out"] = "forge 1.2.3" + + agent = spawn.spawn("proj", "api") + assert "9.9.9" in agent["forge_note"] diff --git a/tests/test_credentials.py b/tests/test_credentials.py new file mode 100644 index 0000000..71db260 --- /dev/null +++ b/tests/test_credentials.py @@ -0,0 +1,104 @@ +"""Credential resolution + env/helper derivation (README 3.7).""" + +from __future__ import annotations + +import pytest + +from handler.control import credentials + + +def test_resolve_none_returns_none(): + assert credentials.resolve(None) is None + assert credentials.resolve("") is None + + +def test_resolve_env(monkeypatch): + monkeypatch.setenv("MY_TOKEN", "secret-value") + assert credentials.resolve("env:MY_TOKEN") == "secret-value" + + +def test_resolve_env_missing_raises(monkeypatch): + monkeypatch.delenv("NOPE", raising=False) + with pytest.raises(credentials.CredentialError, match="not set"): + credentials.resolve("env:NOPE") + + +def test_resolve_file(tmp_path): + f = tmp_path / "tok" + f.write_text(" file-secret\n") + assert credentials.resolve(f"file:{f}") == "file-secret" + + +def test_resolve_file_missing_raises(tmp_path): + with pytest.raises(credentials.CredentialError, match="unreadable"): + credentials.resolve(f"file:{tmp_path / 'absent'}") + + +def test_resolve_cmd(): + assert credentials.resolve("cmd:printf hunter2") == "hunter2" + + +def test_resolve_cmd_failure_raises(): + with pytest.raises(credentials.CredentialError, match="exited"): + credentials.resolve("cmd:false") + + +def test_resolve_unknown_scheme_raises(): + with pytest.raises(credentials.CredentialError, match="unknown scheme"): + credentials.resolve("vault:secret/x") + + +def test_resolve_empty_value_raises(): + with pytest.raises(credentials.CredentialError, match="no value"): + credentials.resolve("env:") + + +def test_credential_env_always_sets_forge_token(): + env = credentials.credential_env("tok", None) + assert env == {"FORGE_TOKEN": "tok"} + + +def test_credential_env_adds_host_specific_var(): + gh = credentials.credential_env("tok", "https://github.com/me/repo.git") + assert gh["GITHUB_TOKEN"] == "tok" and gh["FORGE_TOKEN"] == "tok" + gitea = credentials.credential_env("tok", "https://gitea.example.com/me/repo.git") + assert gitea["GITEA_TOKEN"] == "tok" + + +def test_credential_env_empty_when_no_token(): + assert credentials.credential_env(None, "https://github.com/x") == {} + + +def test_git_credential_helper_reads_from_env(): + helper = credentials.git_credential_helper_value() + # Inline helper hands back the token from $FORGE_TOKEN, never a value on disk. + assert "$FORGE_TOKEN" in helper + assert helper.startswith("!") + + +def test_remote_host_parses_https_and_ssh(): + assert credentials.remote_host("https://github.com/me/repo.git") == "github.com" + assert credentials.remote_host("git@gitea.example.com:me/repo.git") == "gitea.example.com" + assert credentials.remote_host(None) is None + + +def test_host_token_env_not_fooled_by_repo_name(): + # A GitHub repo merely *named* 'gitea' must not be mapped to GITEA_TOKEN. + env = credentials.credential_env("tok", "https://github.com/me/gitea-mirror.git") + assert "GITEA_TOKEN" not in env and env["GITHUB_TOKEN"] == "tok" + + +def test_host_token_env_self_hosted_hint(): + env = credentials.credential_env("tok", "https://gitea.mycorp.internal/me/repo.git") + assert env["GITEA_TOKEN"] == "tok" + + +def test_git_credential_config_scoped_to_host(): + key, value = credentials.git_credential_config("https://github.com/me/repo.git") + assert key == "credential.https://github.com.helper" + assert "$FORGE_TOKEN" in value + + +def test_git_credential_config_none_for_ssh(): + assert credentials.git_credential_config("git@github.com:me/repo.git") is None + assert credentials.git_credential_config(None) is None diff --git a/tests/test_hook_gate_approval.py b/tests/test_hook_gate_approval.py new file mode 100644 index 0000000..afc945e --- /dev/null +++ b/tests/test_hook_gate_approval.py @@ -0,0 +1,128 @@ +"""Phase 2 gate behavior: the hard approval gate + push CI-pending recording.""" + +from __future__ import annotations + +from handler.db import repository as repo +from handler.hooks import gate, verify +from handler.hooks.context import HookInput, Identity + + +def _decision(result): + return result["hookSpecificOutput"]["permissionDecision"] + + +def _seed(conn, role=None, name="deploy"): + repo.create_project(conn, "p", "/tmp/p") + a = repo.create_agent(conn, "p", name, "/tmp/p/a", role=role) + return Identity(a["id"], "p", name, "/tmp/p/a") + + +def _merge_input(): + return HookInput( + {"tool_name": "Bash", "tool_input": {"command": "forge pr merge 7"}}, "pre_tool_use" + ) + + +def test_merge_denied_without_approval(conn, fake_gitops): + ident = _seed(conn) + result = gate.handle_merge_deploy(conn, ident, _merge_input()) + assert _decision(result) == "deny" + assert "no standing approval" in result["hookSpecificOutput"]["permissionDecisionReason"] + + +def test_merge_denied_when_latest_is_rejected(conn, fake_gitops): + ident = _seed(conn) + other = repo.create_agent(conn, "p", "senior", "/tmp/p/s", role="senior") + repo.record_approval(conn, "p", "feat/x", "rejected", other["id"]) + assert _decision(gate.handle_merge_deploy(conn, ident, _merge_input())) == "deny" + + +def test_merge_denied_on_self_approval(conn, fake_gitops): + ident = _seed(conn) + # Same agent approved the branch it now tries to merge — no self-approval. + repo.record_approval(conn, "p", "feat/x", "approved", ident.agent_id) + result = gate.handle_merge_deploy(conn, ident, _merge_input()) + assert _decision(result) == "deny" + assert "different agent" in result["hookSpecificOutput"]["permissionDecisionReason"] + + +def test_merge_allowed_with_different_agent_approval(conn, fake_gitops): + ident = _seed(conn) + senior = repo.create_agent(conn, "p", "senior", "/tmp/p/s", role="senior") + repo.record_approval(conn, "p", "feat/x", "approved", senior["id"]) + assert _decision(gate.handle_merge_deploy(conn, ident, _merge_input())) == "allow" + + +def test_deploy_task_is_also_gated(conn, fake_gitops): + ident = _seed(conn) + hi = HookInput( + {"tool_name": "Bash", "tool_input": {"command": "mise run deploy"}}, "pre_tool_use" + ) + # Routed through handle() to prove the matcher catches `mise run deploy`. + result = gate.handle(conn, ident, hi) + assert _decision(result) == "deny" + + +def test_merge_denied_when_branch_unknown(conn, fake_gitops): + ident = _seed(conn) + fake_gitops["branch"] = None + result = gate.handle_merge_deploy(conn, ident, _merge_input()) + assert _decision(result) == "deny" + assert "current git branch" in result["hookSpecificOutput"]["permissionDecisionReason"] + + +def test_merge_denied_when_approval_is_for_a_stale_commit(conn, fake_gitops): + ident = _seed(conn) + senior = repo.create_agent(conn, "p", "senior", "/tmp/p/s", role="senior") + # Approved an earlier commit; HEAD has since moved on. + repo.record_approval(conn, "p", "feat/x", "approved", senior["id"], approved_sha="oldsha") + fake_gitops["sha"] = "newsha" + result = gate.handle_merge_deploy(conn, ident, _merge_input()) + assert _decision(result) == "deny" + assert "re-reviewed" in result["hookSpecificOutput"]["permissionDecisionReason"] + + +def test_merge_allowed_when_approved_sha_matches_head(conn, fake_gitops): + ident = _seed(conn) + senior = repo.create_agent(conn, "p", "senior", "/tmp/p/s", role="senior") + fake_gitops["sha"] = "samesha" + repo.record_approval(conn, "p", "feat/x", "approved", senior["id"], approved_sha="samesha") + assert _decision(gate.handle_merge_deploy(conn, ident, _merge_input())) == "allow" + + +def test_push_records_ci_pending_on_allow(conn, fake_gitops, monkeypatch): + ident = _seed(conn, name="junior") + fake_gitops["branch"] = "feat/x" # not protected -> no approval needed to push + monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok")) + monkeypatch.setattr(verify, "run_build", lambda cwd: (True, "built")) + hi = HookInput({"tool_name": "Bash", "tool_input": {"command": "git push"}}, "pre_tool_use") + result = gate.handle_git_push(conn, ident, hi) + assert _decision(result) == "allow" + # A pending-CI log entry was recorded for the pushed commit. + pending = repo.get_pending_ci_entries(conn) + assert len(pending) == 1 + assert pending[0]["push_sha"] == fake_gitops["sha"] + + +def test_direct_push_to_protected_branch_needs_approval(conn, fake_gitops, monkeypatch): + # Closes the "merge locally, push to main" bypass around the forge-merge gate. + ident = _seed(conn) + fake_gitops["branch"] = "main" + monkeypatch.setattr(verify, "run_test", lambda cwd: (True, "ok")) + monkeypatch.setattr(verify, "run_build", lambda cwd: (True, "built")) + hi = HookInput( + {"tool_name": "Bash", "tool_input": {"command": "git push origin main"}}, "pre_tool_use" + ) + result = gate.handle_git_push(conn, ident, hi) + assert _decision(result) == "deny" + assert "protected branch" in result["hookSpecificOutput"]["permissionDecisionReason"] + # Nothing recorded as pending since the push was denied. + assert repo.get_pending_ci_entries(conn) == [] + + +def test_pr_title_mentioning_merge_is_not_gated(conn, fake_gitops): + # The approval gate must not trip on a PR title/commit message containing "merge". + ident = _seed(conn, name="junior") + cmd = 'forge pr create --title "add merge helper"' + hi = HookInput({"tool_name": "Bash", "tool_input": {"command": cmd}}, "pre_tool_use") + assert gate.handle(conn, ident, hi) == {} diff --git a/tests/test_poller.py b/tests/test_poller.py new file mode 100644 index 0000000..f0ac8c1 --- /dev/null +++ b/tests/test_poller.py @@ -0,0 +1,93 @@ +"""CI backfill poller: run classification + a full sweep against a faked forge.""" + +from __future__ import annotations + +from handler.control import poller +from handler.db import repository as repo + + +def test_classify_no_runs_is_pending(): + assert poller.classify([]) == "pending" + + +def test_classify_any_failure_is_fail(): + runs = [{"conclusion": "success"}, {"conclusion": "failure"}] + assert poller.classify(runs) == "fail" + + +def test_classify_all_success_is_pass(): + runs = [{"status": "completed", "conclusion": "success"}] + assert poller.classify(runs) == "pass" + + +def test_classify_running_stays_pending(): + runs = [{"status": "in_progress", "conclusion": None}] + assert poller.classify(runs) == "pending" + + +def test_classify_tolerates_alternate_spellings(): + assert poller.classify([{"conclusion": "succeeded"}]) == "pass" + assert poller.classify([{"conclusion": "canceled"}]) == "fail" + + +def test_classify_action_required_stays_pending_not_pass(): + # A terminal-but-non-success conclusion must NOT be reported as a pass. + runs = [{"status": "completed", "conclusion": "action_required"}] + assert poller.classify(runs) == "pending" + + +def test_classify_status_only_forge_success(): + # No conclusion field at all: fall back to the status. + assert poller.classify([{"status": "completed"}]) == "pass" + + +def _seed_pending(conn): + repo.create_project(conn, "p", "/tmp/p") + a = repo.create_agent(conn, "p", "a", "/tmp/p/a") + return repo.insert_log_entry( + conn, a["id"], status="working", push_sha="sha1", ci_status="pending" + ) + + +def test_sweep_backfills_pass(engine, fake_forge): + with engine.begin() as conn: + entry_id = _seed_pending(conn) + fake_forge["runs"] = [{"status": "completed", "conclusion": "success"}] + + summary = poller.sweep() + assert summary == {"checked": 1, "resolved": 1, "pending": 0} + with engine.begin() as conn: + assert repo.get_pending_ci_entries(conn) == [] + row = [e for e in repo.get_log(conn, 1) if e["id"] == entry_id][0] + assert row["ci_status"] == "pass" + assert row["ci_checked_at"] is not None + + +def test_sweep_leaves_pending_when_unresolved(engine, fake_forge): + with engine.begin() as conn: + _seed_pending(conn) + fake_forge["runs"] = [{"status": "in_progress"}] + summary = poller.sweep() + assert summary["resolved"] == 0 + with engine.begin() as conn: + assert len(repo.get_pending_ci_entries(conn)) == 1 + + +def test_sweep_leaves_pending_when_forge_unavailable(engine, fake_forge): + with engine.begin() as conn: + _seed_pending(conn) + fake_forge["ci_ok"] = False + summary = poller.sweep() + assert summary["resolved"] == 0 + with engine.begin() as conn: + assert len(repo.get_pending_ci_entries(conn)) == 1 + + +def test_watch_runs_bounded_iterations(engine, fake_forge): + with engine.begin() as conn: + _seed_pending(conn) + fake_forge["runs"] = [{"conclusion": "failure"}] + summaries = list(poller.watch(iterations=1, interval=0)) + assert len(summaries) == 1 + with engine.begin() as conn: + assert repo.get_log(conn, 1)[-1]["ci_status"] == "fail" diff --git a/tests/test_repository_phase2.py b/tests/test_repository_phase2.py new file mode 100644 index 0000000..30d4b7d --- /dev/null +++ b/tests/test_repository_phase2.py @@ -0,0 +1,67 @@ +"""Phase 2 DAL: agent role, approvals, and CI backfill helpers.""" + +from __future__ import annotations + +from handler.db import repository as repo + + +def test_agent_role_is_stored(conn): + repo.create_project(conn, "p", "/tmp/p") + a = repo.create_agent(conn, "p", "senior", "/tmp/p/senior", role="senior") + assert a["role"] == "senior" + assert repo.get_agent_by_id(conn, a["id"])["role"] == "senior" + + +def test_approval_record_and_latest(conn): + repo.create_project(conn, "p", "/tmp/p") + junior = repo.create_agent(conn, "p", "junior", "/tmp/p/j", role="junior") + senior = repo.create_agent(conn, "p", "senior", "/tmp/p/s", role="senior") + + assert repo.get_latest_approval(conn, "p", "feat/x") is None + + repo.record_approval(conn, "p", "feat/x", "rejected", junior["id"], note="nit") + latest = repo.record_approval(conn, "p", "feat/x", "approved", senior["id"], pr_ref="7") + got = repo.get_latest_approval(conn, "p", "feat/x") + # Latest wins (by insertion order). + assert got["id"] == latest["id"] + assert got["status"] == "approved" + assert got["approved_by_agent_id"] == senior["id"] + assert got["pr_ref"] == "7" + + +def test_approval_scoped_by_project_and_branch(conn): + repo.create_project(conn, "p", "/tmp/p") + a = repo.create_agent(conn, "p", "s", "/tmp/p/s") + repo.record_approval(conn, "p", "feat/x", "approved", a["id"]) + assert repo.get_latest_approval(conn, "p", "feat/other") is None + + +def test_pending_ci_entries_and_backfill(conn): + repo.create_project(conn, "p", "/tmp/p") + a = repo.create_agent(conn, "p", "a", "/tmp/p/a") + # A push-recording entry (pending) and a normal entry (not_applicable). + pending_id = repo.insert_log_entry( + conn, a["id"], status="working", push_sha="deadbeef", ci_status="pending" + ) + repo.insert_log_entry(conn, a["id"], status="working", summary="no push") + + entries = repo.get_pending_ci_entries(conn) + assert [e["id"] for e in entries] == [pending_id] + assert entries[0]["push_sha"] == "deadbeef" + assert entries[0]["project_id"] == "p" + assert entries[0]["working_dir"] == "/tmp/p/a" + + assert repo.update_ci_status(conn, pending_id, "pass") is True + # No longer pending once resolved. + assert repo.get_pending_ci_entries(conn) == [] + assert repo.get_log(conn, a["id"])[-1]["ci_status"] in ("pass", "not_applicable") + + +def test_pending_ci_entries_scoped_to_project(conn): + repo.create_project(conn, "p1", "/tmp/p1") + repo.create_project(conn, "p2", "/tmp/p2") + a1 = repo.create_agent(conn, "p1", "a", "/tmp/p1/a") + a2 = repo.create_agent(conn, "p2", "a", "/tmp/p2/a") + repo.insert_log_entry(conn, a1["id"], status="working", push_sha="s1", ci_status="pending") + repo.insert_log_entry(conn, a2["id"], status="working", push_sha="s2", ci_status="pending") + assert [e["project_id"] for e in repo.get_pending_ci_entries(conn, project_id="p1")] == ["p1"] diff --git a/tests/test_skills_gen.py b/tests/test_skills_gen.py new file mode 100644 index 0000000..37c9614 --- /dev/null +++ b/tests/test_skills_gen.py @@ -0,0 +1,32 @@ +"""Role-based forge skills generation (committed into the managed repo).""" + +from __future__ import annotations + +from handler.control import skills_gen + + +def test_skill_files_cover_every_role(): + files = skills_gen.skill_files() + paths = set(files) + for role in ("forge-workflow", "forge-junior", "forge-senior", "forge-deploy"): + assert f".claude/skills/{role}/SKILL.md" in paths + + +def test_skill_files_have_frontmatter(): + for _, contents in skills_gen.skill_files().items(): + assert contents.startswith("---\nname: ") + assert "description:" in contents + + +def test_senior_skill_references_the_approval_command(): + senior = skills_gen.skill_files()[".claude/skills/forge-senior/SKILL.md"] + assert "handler approve" in senior + assert "handler reject" in senior + + +def test_write_skills_materializes_files(tmp_path): + written = skills_gen.write_skills(str(tmp_path)) + assert len(written) == 4 + junior = tmp_path / ".claude" / "skills" / "forge-junior" / "SKILL.md" + assert junior.exists() + assert "junior developer" in junior.read_text()