diff --git a/agent-team/DEPLOY-R720.md b/agent-team/DEPLOY-R720.md index 016d4e5..8521736 100644 --- a/agent-team/DEPLOY-R720.md +++ b/agent-team/DEPLOY-R720.md @@ -86,6 +86,72 @@ chmod 600 ~/secrev.env would silently win over the subscription OAuth and meter to API rates. The box runs on subscription OAuth only. +### 3a. P3 dispatch auth — GitHub App installation tokens (box-side) + +The P3 dispatcher (`agent_team/dispatcher.py`) carries an approved diff into org +CI: it pushes the candidate head branch and triggers the +`agent-team-apply-verify.yml` `workflow_dispatch`, then locates the resulting +run id. By default those three seams shell out to `gh`/`git`. **`gh` is not +installed on the box and the box's read-only PAT has no Actions permission**, so +the default path cannot dispatch. Instead the box authenticates with a **GitHub +App**: it holds the App private key and mints short-lived (~1h) **installation +access tokens** on demand (`agent_team/github_app.py`). Set all three of these to +turn the App path on (all-or-nothing — partial config logs one warning and falls +back to the inert gh-default path, it never crashes serve): + +The App is the existing **`agent-team-apply`** App: **app_id `4119505`**, +**installation `141992144`** (on `Sea-Haven-Industries/orchestrator`). These two +IDs are not secrets (only the `.pem` is). Set all three vars (all-or-nothing — +partial config logs one warning and falls back to the inert gh-default path, it +never crashes serve): + +``` +echo 'AGENT_TEAM_GH_APP_ID=4119505' >> ~/secrev.env +echo 'AGENT_TEAM_GH_APP_INSTALLATION_ID=141992144' >> ~/secrev.env +echo 'AGENT_TEAM_GH_APP_PRIVATE_KEY=/home/adam/.ssh/agent-team-apply.pem' >> ~/secrev.env # PATH to the .pem +chmod 600 ~/secrev.env +``` + +Place the App private key on the box and lock it down — it is a write-capable +credential and must be owner-only. The CI-side Actions secret +`AGENT_APPLY_APP_PRIVATE_KEY` is write-only and cannot be re-exported, so the +box gets its OWN freshly-generated private key for the same App (generate one +under the App settings — the App holds several keys; the CI key keeps working). +Secure-copy it from the Mac (do NOT commit it; it is not in the repo): + +``` +# From the Mac (the key lives in ~/Downloads after generation): +scp ~/Downloads/agent-team-apply.*.private-key.pem secrev:.ssh/agent-team-apply.pem +# On the box (~/.ssh is already mode 700): +chmod 600 ~/.ssh/agent-team-apply.pem +ls -l ~/.ssh/agent-team-apply.pem # expect -rw------- +# Then DELETE the Mac copy (the box now holds the only working copy): +# rm ~/Downloads/agent-team-apply.*.private-key.pem +``` + +- `AGENT_TEAM_GH_APP_PRIVATE_KEY` is a **filesystem path** to the `.pem`, not the + key material. The coordinator reads the file at graph-build time; an unreadable + path logs one warning and falls back to gh-default (never crashes). +- **App permission/scope (verified 2026-06-24).** A test mint of an installation + token for app `4119505` / install `141992144` returned + `{"actions":"write","contents":"write","metadata":"read","pull_requests":"write"}` + with `repository_selection: selected` — i.e. Contents R/W + Actions R/W are in + place and the install is scoped to selected repos (not all). Minting grants + exactly the App's scopes; keep it scoped to `Sea-Haven-Industries/orchestrator` + only. (Follow-up: move to a dedicated least-privilege App to replace + `agent-team-apply`, dropping `pull_requests:write` which the box path does not + need.) +- The minted token is short-lived (~1h), is **never** logged, never put in an + exception message, and never written to the ledger/graph state; on the + authenticated push it rides in a host-scoped `http.extraHeader` passed via + `GIT_CONFIG_*` env (never in argv/`ps`). +- **Env-file precedence (verify after editing).** The unit loads **both** + `~/secrev.env` and `~/orchestrator/.env` (last-wins). Confirm only the intended + values are present in both so a stale entry can't shadow the App config: + ``` + grep -nE 'AGENT_TEAM_GH_APP|AGENT_TEAM_REPO_(OWNER|NAME)' ~/secrev.env ~/orchestrator/.env 2>/dev/null + ``` + ## 4. Deploy steps ``` @@ -324,6 +390,32 @@ daemon against a half-migrated or suspect ledger. Note the secrets in `~/secrev.env` are NOT removed by rollback - leave them, or strip the four agent-team keys if you are decommissioning entirely. +### GitHub App key — compromise / rotation / revocation + +The box holds a write-capable App private key, so it needs its own incident path +(separate from the VM snapshot rollback above): + +``` +# 1. Stop the daemon so no further token mints happen: +sudo systemctl stop agent-team-coordinator.service +# 2. Remove the key + the App env vars from the box (kills the App path -> inert): +rm -f ~/.ssh/agent-team-apply.pem +sed -i '/AGENT_TEAM_GH_APP_/d' ~/secrev.env +# 3. In GitHub: rotate (generate a new private key, delete the old one) under the +# App's settings, or uninstall the App from the repo to revoke all access. +# Existing installation tokens are short-lived (~1h) and expire on their own. +# 4. Restart inert (gh-default dispatch) and confirm no App env is loaded: +sudo systemctl start agent-team-coordinator.service +sudo systemctl show agent-team-coordinator.service -p Environment | grep -c AGENT_TEAM_GH_APP # expect 0 +``` + +- **Rotation cadence is Adam's call** — there is no automated rotation. Rotate on + any suspected box compromise, on operator turnover, and on a periodic cadence + Adam sets. Document the rotation event in Confluence. +- Removing the App env vars (or the key file) is a safe partial rollback on its + own: the dispatcher falls back to the inert gh-default path (which is itself + inert without `gh`), so no diff is ever pushed — never a fabricated pass. + ## 7. Security - **Slack inbound listener (Socket Mode) is the auth + untrusted-input surface.** @@ -335,5 +427,19 @@ strip the four agent-team keys if you are decommissioning entirely. - **No IAM / OIDC is involved in P1.** The box runs on subscription OAuth (`CLAUDE_CODE_OAUTH_TOKEN`) and Slack tokens only; there is no AWS role, no OIDC trust relationship, no cloud permission surface in this deploy. +- **GitHub App key on the box (P3 dispatch) — conscious, mitigated trade-off.** + The dispatcher's original design comment said "never a box-held token / operator + host only." Moving dispatch onto the box (so P3 reaches CI without `gh`) + deliberately deviates from that. Mitigations: the App is scoped to **one repo** + with **Contents + Actions only**; minted installation tokens are **short-lived + (~1h)**; the key file is **mode 600**; tokens are **never logged / never in + exception text / never in argv** (push auth rides a host-scoped + `http.extraHeader` via `GIT_CONFIG_*`); CI **re-verifies** the pushed content by + hash and the draft PR is still gated by the `agent-apply` environment's required + reviewer. Any mint/HTTP/git failure **parks** the task (no `run_id` → the verify + gate fails closed) — never a fabricated pass. See §3a for the + permission/scope audit, env-precedence check, and §6 for key rotation/ + revocation. **Credential-handling change → `/sh-security-review` is mandatory + before merge** (auth + secret handling surface). - `~/secrev.env` stays mode 600 and out of git; `state/` (ledger + audit log) is gitignored and written 0600. diff --git a/agent-team/README.md b/agent-team/README.md index d7b126e..a058e66 100644 --- a/agent-team/README.md +++ b/agent-team/README.md @@ -89,9 +89,16 @@ agent-team/ handbook.py # WS5 load_handbook_conventions (handbook seam, # fail-safe → "" if dir missing); planner context dispatch_invoker.py # P3 DISPATCH node — pushes the per-dispatch head branch, - # triggers CI (gh workflow run), and captures run_id + - # dispatched_at into state via a correlation-tagged poll - # of gh run list (fails closed on an unfound run) + # triggers CI, and captures run_id + dispatched_at into + # state via a correlation-tagged poll (fails closed on an + # unfound run). Auth via injected seams: gh/git CLI by + # default, or GitHub-App installation tokens box-side + # (github_app.py) when AGENT_TEAM_GH_APP_* is set + github_app.py # mints short-lived (~1h) GitHub-App installation tokens on + # the box (RS256 JWT → /access_tokens); TokenProvider + # caches + re-mints near expiry. Powers the App dispatch + # seams (dispatcher.app_branch_pusher/_workflow_dispatcher/ + # _run_locator) so P3 reaches CI with no gh + no box PAT transport/ # one adapter contract + a live impl per channel base.py # Transport ABC + QuestionSet / NormalizedAnswer slack_adapter.py + slack_live.py + slack_listener.py # Block Kit + Socket Mode + /new-task @@ -196,7 +203,7 @@ P1–P4 pipeline. What is **live** vs **inert** after the rollout: | WS1 | `invoker_multi.py` (in-process GPT-4.1 / DeepSeek / Gemini via the orchestrator's `models.py`) + `api.py` (FastAPI HTTP API, bearer auth via `AGENT_TEAM_API_TOKEN`, binds `127.0.0.1:8765`, `/docs`+`/openapi` disabled, concurrency-capped) | `bind_multi_invoker()` wired in `run-team.py` `_cmd_serve` (LIVE); the **HTTP API is a separate opt-in process** (`api.serve()`), NOT started by the coordinator | | WS5 | `nodes/handbook.py` `load_handbook_conventions` (reads `SEA_HAVEN_HANDBOOK_DIR` or `~/.sea-haven/engineering-handbook`, fail-safe → `""`); `retriever.py` `save_memory` writes to a `_box-drafts/` review queue | LIVE — the planner prompt receives the handbook via the `context_provider` seam in `run-team.py` `_build_coordinator` | | WS2/WS0/WS4 | Slack `/new-task` slash command (AUTHZ-01 owner-allowlist gated) → `Coordinator.set_new_task_callback`; the `sea-haven-claude-plugin/` (CLAUDE.md, settings, `/delegate` `UserPromptSubmit` hook) | LIVE (`/new-task` wired in `serve`); the plugin/HTTP-API path is opt-in | -| WS3 / P3 | `nodes/dispatch_invoker.py` (DISPATCH LangGraph node) + the BUILD → DISPATCH → VERIFY reorder, the `tick()`-driven CI-watcher, per-task `run_id` plumbing, and the bound `serve` default | **Built on `feat/agent-team-p3-box-integration`, gated behind the C1 re-review** before it ships to the box. The `agent-apply` GitHub Environment human-approval gate is KEPT; dispatch is **operator-initiated** (the box holds no standing write token — the branch push + `gh workflow run` use operator-host credentials). The bound P3 wiring is the new fail-safe `serve` default: a missing `AGENT_TEAM_REPO_OWNER`/`_NAME` or CI-read token degrades to the INERT P3 path (task parks + a `#agent-team` notice), never a serve-start crash | +| WS3 / P3 | `nodes/dispatch_invoker.py` (DISPATCH LangGraph node) + the BUILD → DISPATCH → VERIFY reorder, the `tick()`-driven CI-watcher, per-task `run_id` plumbing, and the bound `serve` default | **Built on `feat/agent-team-p3-box-integration`, gated behind the C1 re-review** before it ships to the box. The `agent-apply` GitHub Environment human-approval gate is KEPT. Dispatch auth is **seam-injected**: gh/git CLI by default, or — when `AGENT_TEAM_GH_APP_*` is set — **box-side GitHub-App installation tokens** (`github_app.py`), so the box mints a short-lived (~1h) write token on demand instead of needing `gh` or a standing PAT (a conscious, mitigated deviation from "operator host only" — App scoped to one repo, Contents+Actions only, tokens short-lived & never logged; CI still re-verifies by hash and the `agent-apply` reviewer gate still applies). The bound P3 wiring is the fail-safe `serve` default: a missing `AGENT_TEAM_REPO_OWNER`/`_NAME` or CI-read token degrades to the INERT P3 path (task parks + a `#agent-team` notice), never a serve-start crash; a partial/unreadable `AGENT_TEAM_GH_APP_*` config logs one warning and falls back to the gh-default seams | The HTTP API endpoints: `POST /tasks` (start a task), `GET /tasks/{thread_id}` (status), `POST /orchestrator/invoke` (one-shot model invoke). See diff --git a/agent-team/agent_team/coordinator.py b/agent-team/agent_team/coordinator.py index 48abc23..0fe4ba0 100644 --- a/agent-team/agent_team/coordinator.py +++ b/agent-team/agent_team/coordinator.py @@ -437,6 +437,65 @@ def gated_build_verify_wiring( return build_node, verify_node, route_after_verify +def _app_dispatch_seams() -> "tuple[Any, Any, Any] | None": + """Resolve the GitHub-App dispatch seams from env, or None for gh-default. + + The auto-dispatch node carries an approved diff into org CI. Two auth paths + exist behind the same :data:`DispatchNodeFactory` seam: the default ``gh``/ + ``git`` CLI path (:func:`agent_team.dispatcher` ``_default_*`` seams, used by + :func:`agent_team.nodes.dispatch_invoker.make_dispatch_node` when no seams are + passed) and a GitHub-App path that mints short-lived installation tokens. This + helper decides — from the environment ONLY — whether to wire the App path: + + * ``AGENT_TEAM_GH_APP_ID``, ``AGENT_TEAM_GH_APP_INSTALLATION_ID``, and + ``AGENT_TEAM_GH_APP_PRIVATE_KEY`` (a filesystem path to the App's ``.pem``) + must ALL be set → returns the ``(pusher, dispatcher, locator)`` App-seam + triple built over a lazy :class:`~agent_team.github_app.TokenProvider`. + * NONE set → returns ``None`` (the gh-default behavior is untouched). + * PARTIALLY set, or the key path cannot be read → logs ONE warning and returns + ``None`` so the daemon stays inert on the App path and falls back to + gh-default. It NEVER raises (a half-configured box must still serve) and + NEVER logs the key path's contents. + + The :class:`TokenProvider` is lazy: no JWT is minted and no key is validated + here, so a wiring-time call costs nothing and a bad key only fails closed + later, when a dispatch actually tries to mint a token (the node parks). + """ + app_id = os.environ.get("AGENT_TEAM_GH_APP_ID", "").strip() + inst = os.environ.get("AGENT_TEAM_GH_APP_INSTALLATION_ID", "").strip() + key_path = os.environ.get("AGENT_TEAM_GH_APP_PRIVATE_KEY", "").strip() + + present = [v for v in (app_id, inst, key_path) if v] + if not present: + return None # none set -> gh-default behavior + if len(present) != 3: + _LOG.warning( + "GitHub App dispatch wiring is INERT: AGENT_TEAM_GH_APP_ID/" + "INSTALLATION_ID/PRIVATE_KEY are not all set; falling back to " + "gh-default dispatch seams." + ) + return None # partial -> inert, NEVER raise, NEVER log key + + try: + pem = Path(key_path).read_text(encoding="utf-8") + except Exception: # noqa: BLE001 - unreadable key -> inert, never crash serve + _LOG.warning( + "GitHub App dispatch wiring is INERT: AGENT_TEAM_GH_APP_PRIVATE_KEY " + "path could not be read; falling back to gh-default dispatch seams." + ) + return None + + from agent_team.dispatcher import ( + app_branch_pusher, + app_run_locator, + app_workflow_dispatcher, + ) + from agent_team.github_app import TokenProvider + + tp = TokenProvider(app_id=app_id, private_key_pem=pem, installation_id=inst) + return (app_branch_pusher(tp), app_workflow_dispatcher(tp), app_run_locator(tp)) + + def default_dispatch_node_factory() -> "Callable[[Any], Any]": """Build the live auto-dispatch node from env vars (WS3, OPT-IN, INERT by default). @@ -449,6 +508,16 @@ def default_dispatch_node_factory() -> "Callable[[Any], Any]": Raises ``RuntimeError`` if the required env vars are absent (fail closed: the node must never dispatch to an unknown owner/repo). + **Auth path (GitHub App vs gh-default).** When the three + ``AGENT_TEAM_GH_APP_*`` env vars (App id, installation id, and a path to the + App's ``.pem``) are all set, :func:`_app_dispatch_seams` resolves the + GitHub-App ``(pusher, dispatcher, locator)`` triple and they are forwarded to + ``make_dispatch_node`` so dispatch authenticates with short-lived installation + tokens. Absent (or only partially set / unreadable key) the seams are left + unpassed and ``make_dispatch_node`` uses its built-in ``gh``/``git`` CLI + defaults — the unchanged behavior. The App branch never weakens the owner/repo + fail-closed check above. + This satisfies :data:`DispatchNodeFactory` (zero-arg → node callable). Pass it as ``dispatch_node_wiring=default_dispatch_node_factory`` AFTER the §3.3.2 CI trust-boundary gate clears. The production ``run-team.py`` path @@ -466,6 +535,18 @@ def default_dispatch_node_factory() -> "Callable[[Any], Any]": "AGENT_TEAM_REPO_OWNER and AGENT_TEAM_REPO_NAME must be set " "for auto-dispatch (default_dispatch_node_factory)" ) + + seams = _app_dispatch_seams() + if seams is not None: + pusher, dispatcher, locator = seams + return make_dispatch_node( + owner=owner, + repo=repo, + base=base, + pusher=pusher, + dispatcher=dispatcher, + locator=locator, + ) return make_dispatch_node(owner=owner, repo=repo, base=base) @@ -545,8 +626,22 @@ def failsafe_production_p3_wiring( if _p3_env_is_configured(): owner = os.environ.get("AGENT_TEAM_REPO_OWNER", "").strip() repo = os.environ.get("AGENT_TEAM_REPO_NAME", "").strip() + # Bind the INTENDED diff builder: the DeepSeek ``fast_coder`` mechanical + # -edit path (:func:`agent_team.nodes.builders_llm.as_diff_builder`). It + # produces the diff as a SINGLE model completion, so — unlike the Claude + # agentic fallback (``builders.default_diff_builder``) — it cannot exhaust + # the agent turn cap mid-exploration (issue #60: a tool-using Claude + # session burned all its turns reading files and never emitted a diff). + # Without this bind the build node falls back to the Claude single-shot + # path, which is exactly the #60 failure. Lazy import keeps the + # orchestrator models stack off this module's import surface. + from agent_team.nodes.builders_llm import as_diff_builder + + diff_builder = as_diff_builder() return ( - lambda: gated_build_verify_wiring(owner=owner, repo=repo), + lambda: gated_build_verify_wiring( + owner=owner, repo=repo, diff_builder=diff_builder + ), default_dispatch_node_factory, ) @@ -1151,15 +1246,47 @@ class Coordinator: ) continue - # No pending question: the task settled. Distinguish parked vs done, - # and say WHERE it got to and WHAT is blocking it. + # ``built`` == the task reached the P3 build subgraph (it produced a + # candidate diff). This distinguishes a build/verify-stage state from a + # plan/review escalation, which the messages below key off so a + # successfully-built task is never reported as "the plan could not be + # auto-approved". status = values.get("status") + run_id = str(values.get("run_id") or "").strip() + built = bool(values.get("candidate_diff") or values.get("diff_hash")) + + # IN-PROGRESS, not settled: the VERIFY node SUSPENDS (async CI-wait) + # awaiting a dispatched run's terminal conclusion. Its interrupt + # payload is NOT question-shaped, so ``pending_question`` returns None + # even though the graph is still suspended — without this branch the + # task is misread as a settled PARK and gets the "could not be + # auto-approved / re-assign" escalation copy. Detect the suspend (graph + # still interrupted + a dispatched run + a built diff) and report + # honest progress instead. + suspended = bool(getattr(snap, "interrupts", None)) or bool( + getattr(snap, "next", None) + ) + if question is None and suspended and built and run_id: + self._emit( + f"🛠️ {label} — plan approved; diff built and dispatched to CI " + f"(run `{run_id}`). Awaiting verification — I'll post the " + "result when the run finishes.", + thread_ts=root_ts, + ) + continue + + # No pending question and not awaiting CI: the task settled. Distinguish + # parked vs done, and say WHERE it got to and WHAT is blocking it. phase = str(values.get("current_phase") or "unknown") # When a task parks, current_phase is the terminal "parked" — not # useful. Infer the phase it was IN when it escalated, so the human - # sees WHERE it died (review > plan > clarify by what state exists). + # sees WHERE it stopped (verify > review > plan > clarify by what state + # exists). A built diff means it reached the build/verify subgraph, so + # that takes precedence over the plan/review escalation phases. if phase == "parked": - if values.get("review_verdicts"): + if built: + phase = "verify" + elif values.get("review_verdicts"): phase = "review" elif values.get("plan"): phase = "plan" @@ -1181,6 +1308,20 @@ class Coordinator: "Re-assign it to try again.", thread_ts=root_ts, ) + elif status == TaskStatus.PARKED.value and built: + # The diff WAS built and dispatched, but the build/verify gate did + # not pass (CI BLOCKed / no authenticated pass). This is NOT a + # "plan could not be auto-approved" escalation — say what actually + # happened so the human looks at the CI run, not the plan. + blocker = self._summarize_build_blocker(values) + self._emit( + f"⚠️ PARKED — {label}\n" + f"• Reached phase: {phase}\n" + f"• What's blocking it: {blocker}\n" + "• The diff was built and dispatched, but the verification " + "gate did not pass. Review the CI run, then re-assign to retry.", + thread_ts=root_ts, + ) elif status == TaskStatus.PARKED.value: blocker = self._summarize_blocker(values) self._emit( @@ -1522,6 +1663,30 @@ class Coordinator: "requesting changes without converging)." ) + @staticmethod + def _summarize_build_blocker(values: "dict[str, Any]") -> str: + """Human-readable reason a BUILT task parked at the build/verify gate. + + Surfaces the CI conclusion the verifier gated on (``ci_results``) so the + human sees WHY verification failed — a failing/no-pass run, or no + authenticated result at all — rather than the plan-review escalation text + :meth:`_summarize_blocker` produces (which is wrong for a built diff). + """ + ci = values.get("ci_results") + if isinstance(ci, dict): + conclusion = str(ci.get("conclusion") or "").strip() + if conclusion: + run_id = str(ci.get("run_id") or values.get("run_id") or "").strip() + where = f" (run {run_id})" if run_id else "" + return ( + f"the CI verify run concluded '{conclusion}'{where}; the " + "pure-code gate did not pass." + ) + return ( + "the verification gate did not pass (no authenticated CI pass was " + "available for the dispatched diff)." + ) + @staticmethod def _verify_pass_verdict(values: "dict[str, Any]") -> "dict[str, Any] | None": """Return the verify-stage PASS verdict if the task reached the P3 PASS terminus. diff --git a/agent-team/agent_team/dispatcher.py b/agent-team/agent_team/dispatcher.py index e9bf59d..dc84dcd 100644 --- a/agent-team/agent_team/dispatcher.py +++ b/agent-team/agent_team/dispatcher.py @@ -38,12 +38,22 @@ __all__ = [ "DispatchResult", "DispatcherError", "RunLocator", + "app_branch_pusher", + "app_run_locator", + "app_workflow_dispatcher", "build_dispatch_inputs", "dispatch_apply_verify", "head_branch_for", "select_run_id", ] +# GitHub REST API root. The App-token seams (:func:`app_branch_pusher` / +# :func:`app_workflow_dispatcher` / :func:`app_run_locator`) talk to the REST API +# directly with an installation token instead of shelling out to ``gh`` — so the +# box can dispatch with a freshly-minted, short-lived App token and no operator +# ``gh`` auth. Mirrors :data:`agent_team.ci_fetcher.GITHUB_API_ROOT`. +GITHUB_API_ROOT = "https://api.github.com" + def _utc_now_iso() -> str: """UTC now as an ISO-8601 ``...Z`` string (matches GitHub Actions ``createdAt``).""" @@ -528,3 +538,312 @@ def _default_run_locator() -> RunLocator: return None return _locate + + +# ─────────────────────────────────────────────────────────────────────────── +# GitHub App-token seams (box-side). Same three behaviours as the ``gh``/``git`` +# defaults above, but authenticated with a short-lived installation token from an +# injected ``token_provider`` (``.token() -> str``) instead of the operator's +# ``gh`` auth. This lets the read-only box mint a write token on demand and +# dispatch directly via the REST API + a token-embedding clone URL. +# +# SECRET HYGIENE (BLOCKING): the installation token — and any clone URL that +# embeds it — MUST NEVER be logged, placed in an exception message / ``str()``, +# or written to ledger / graph / task state. The git seam never lets a raw +# ``CalledProcessError`` propagate (its ``.cmd`` / ``.output`` carry the token): +# it scrubs the token + remote URL to ``***`` and re-raises a +# :class:`DispatcherError` with ``from None``. The HTTP seams never include the +# token in any message (status only). +# ─────────────────────────────────────────────────────────────────────────── + + +def app_branch_pusher(token_provider: Any, *, _run: Any = None) -> BranchPusher: + """App-token :class:`BranchPusher`: clone + apply + push with an installation token. + + Mirrors the apply/commit/push semantics of :func:`_default_branch_pusher` + exactly — only the auth + clone differ: this clones the plain HTTPS remote + (``https://github.com/owner/repo.git``) and injects the installation token + minted on demand from ``token_provider.token()`` via an in-memory + ``-c http.extraHeader='Authorization: Bearer '`` on the network-facing + git steps (clone + push), rather than embedding the token in the remote URL or + relying on the operator's ``gh``/``git`` credentials. Carrying auth in a + config header keeps the token OUT of the remote URL and therefore out of the + visible process argv / stored ``origin`` — it is never in a position to leak + via ``ps``/``/proc//cmdline``. The diff is applied with ``--index`` + (stages exactly the diff, nothing stray) so the committed head tree is + precisely base+diff — the same bytes CI hash-verifies. + + ``_run`` injects the subprocess runner for tests; the default shells out to + ``git`` with ``check=True`` + ``capture_output=True``. + + SECRET HYGIENE: the token still appears in a git argv element (the + ``http.extraHeader`` config value), so every git step is wrapped so a raw + ``CalledProcessError`` (whose ``.cmd`` / ``.output`` may carry that header) + NEVER propagates. Any failure is re-raised as a :class:`DispatcherError` whose + message has the token scrubbed to ``***`` (``from None`` so the original — + token-bearing — exception is not chained). + """ + + def _default_run( + cmd: list[str], *, cwd: str | None = None, env: dict[str, str] | None = None + ) -> None: + import os + import subprocess + + full_env = {**os.environ, **env} if env else None + subprocess.run(cmd, cwd=cwd, check=True, capture_output=True, env=full_env) + + run = _run if _run is not None else _default_run + + def _push( + *, owner: str, repo: str, base: str, head_branch: str, diff_text: str + ) -> None: + import tempfile + from pathlib import Path + + token = token_provider.token() + # Plain remote — the token is NEVER in the URL/argv/stored origin. + remote = f"https://github.com/{owner}/{repo}.git" + # GitHub's git smart-HTTP transport authenticates an installation token via + # BASIC auth (username ``x-access-token``), NOT Bearer — Bearer is the REST + # API form and git rejects it ("could not read Username" → exit 128). Encode + # ``x-access-token:`` and inject it as an Authorization header through + # git's GIT_CONFIG_* env vars (NOT argv) on the network steps (clone + push). + # Env is visible only to the same uid via /proc//environ, never via ps + # argv. The header key is SCOPED to the github.com host + # (``http.https://github.com/.extraHeader``, the GitHub-Actions checkout + # pattern) so git never sends the Authorization header to any other host it + # might be redirected to. + basic = base64.b64encode(f"x-access-token:{token}".encode()).decode("ascii") + auth_env = { + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "http.https://github.com/.extraHeader", + "GIT_CONFIG_VALUE_0": f"Authorization: Basic {basic}", + } + + def _scrub(s: str) -> str: + # Scrub BOTH the raw token and the base64 credential blob (which decodes + # to the token) so neither can survive in any surfaced error message. + return s.replace(token, "***").replace(basic, "***") + + with tempfile.TemporaryDirectory() as tmp: + tmpdir = Path(tmp) + diff_path = tmpdir / CANDIDATE_DIFF_FILENAME + diff_path.write_text(diff_text, encoding="utf-8") + clonedir = tmpdir / "repo" + + try: + run( + [ + "git", + "clone", + "--depth", + "1", + "--branch", + base, + remote, + str(clonedir), + ], + env=auth_env, + ) + except Exception as exc: # noqa: BLE001 - scrub token before surfacing + raise DispatcherError(f"git clone failed: {_scrub(str(exc))}") from None + try: + run(["git", "-C", str(clonedir), "checkout", "-B", head_branch]) + except Exception as exc: # noqa: BLE001 + raise DispatcherError( + f"git checkout failed: {_scrub(str(exc))}" + ) from None + try: + # --index applies AND stages exactly the diff (incl. new files) and + # NOTHING else, so the head tree is precisely base+diff (LOGIC-1). + run(["git", "-C", str(clonedir), "apply", "--index", str(diff_path)]) + except Exception as exc: # noqa: BLE001 + raise DispatcherError(f"git apply failed: {_scrub(str(exc))}") from None + try: + run( + [ + "git", + "-C", + str(clonedir), + "-c", + "user.name=agent-team", + "-c", + "user.email=agent-team@seahavenind.com", + "commit", + "-m", + f"agent-team apply: {head_branch}", + ] + ) + except Exception as exc: # noqa: BLE001 + raise DispatcherError( + f"git commit failed: {_scrub(str(exc))}" + ) from None + try: + run( + [ + "git", + "-C", + str(clonedir), + "push", + "--no-verify", + "--force-with-lease", + "origin", + head_branch, + ], + env=auth_env, + ) + except Exception as exc: # noqa: BLE001 + raise DispatcherError(f"git push failed: {_scrub(str(exc))}") from None + + return _push + + +def app_workflow_dispatcher( + token_provider: Any, *, _http: Any = None +) -> WorkflowDispatcher: + """App-token :class:`WorkflowDispatcher`: trigger the workflow via the REST API. + + Same trigger as :func:`_default_workflow_dispatcher` (the apply/verify + ``workflow_dispatch``), but issues + ``POST /repos/{owner}/{repo}/actions/workflows/{WORKFLOW_FILE}/dispatches`` + directly with an installation token from ``token_provider.token()`` instead of + shelling out to ``gh``. ``_http`` injects a ``requests``-like client for tests + (``.post(url, *, json=..., headers=..., timeout=...)`` -> response exposing + ``.status_code``); the default lazily imports ``requests``. + + GitHub returns ``204`` on success; any other status fails closed with a + :class:`DispatcherError` carrying the STATUS only (NEVER the token). + """ + + def _fire(*, owner: str, repo: str, inputs: dict[str, str], ref: str) -> None: + url = ( + f"{GITHUB_API_ROOT}/repos/{owner}/{repo}" + f"/actions/workflows/{WORKFLOW_FILE}/dispatches" + ) + headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "Authorization": f"Bearer {token_provider.token()}", + } + body = {"ref": ref, "inputs": inputs} + # Wrap the transport so a requests/transport exception can NEVER carry the + # Bearer token out unscrubbed: re-raise as a DispatcherError with the + # exception TYPE only (mirrors the git seam's secret-hygiene discipline). + try: + if _http is not None: + resp = _http.post(url, json=body, headers=headers, timeout=15.0) + else: + import requests # deferred: optional dependency + + resp = requests.post(url, json=body, headers=headers, timeout=15.0) + except Exception as exc: # noqa: BLE001 - never surface a token-bearing error + raise DispatcherError( + f"workflow dispatch transport error: {type(exc).__name__}" + ) from None + status = getattr(resp, "status_code", None) + if status != 204: + raise DispatcherError(f"workflow dispatch failed: status={status}") + + return _fire + + +def app_run_locator( + token_provider: Any, + *, + _http: Any = None, + _sleep: Any = None, +) -> RunLocator: + """App-token :class:`RunLocator`: match the triggered run via the REST API. + + Same anti-stale SELECTION as :func:`_default_run_locator` — it floors on the + dispatched-at watermark (minus :data:`_LOCATE_SKEW_S` for clock skew), + delegates to the pure :func:`select_run_id`, and polls a bounded number of + times — but lists runs via + ``GET /repos/{owner}/{repo}/actions/runs`` with an installation token instead + of ``gh run list``. The REST rows are mapped to the field names + :func:`select_run_id` reads (``id`` -> ``databaseId``, ``created_at`` -> + ``createdAt``). Returns ``None`` if no matching run registers within the poll + window (fails closed). + + A non-200 list response (e.g. a 401/403/404 auth/scope edge) raises a + :class:`DispatcherError` carrying the STATUS only (NEVER the token) so the + dispatch node parks immediately with an actionable signal, rather than + treating the error body as "no runs" and silently exhausting the poll window. + + ``_http`` injects a ``requests``-like client (``.get(url, *, params=..., + headers=..., timeout=...)`` -> response exposing ``.json()``); ``_sleep`` is + injected for tests. No token ever appears in a log or message. + """ + + def _locate(*, owner: str, repo: str, task_id: str, since_iso: str) -> str | None: + import time + from datetime import timedelta + + do_sleep = _sleep if _sleep is not None else time.sleep + + try: + floor_dt = datetime.strptime(since_iso, "%Y-%m-%dT%H:%M:%SZ").replace( + tzinfo=timezone.utc + ) - timedelta(seconds=_LOCATE_SKEW_S) + floor_iso = floor_dt.strftime("%Y-%m-%dT%H:%M:%SZ") + except ValueError: + floor_iso = since_iso + + headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "Authorization": f"Bearer {token_provider.token()}", + } + url = f"{GITHUB_API_ROOT}/repos/{owner}/{repo}/actions/runs" + params = { + "event": "workflow_dispatch", + "created": f">={floor_iso}", + "per_page": 50, + } + + for attempt in range(_LOCATE_ATTEMPTS): + # Wrap the transport so a requests/transport exception can NEVER carry + # the Bearer token out unscrubbed (status/type only, like the git seam). + try: + if _http is not None: + resp = _http.get(url, params=params, headers=headers, timeout=15.0) + else: + import requests # deferred: optional dependency + + resp = requests.get( + url, params=params, headers=headers, timeout=15.0 + ) + except Exception as exc: # noqa: BLE001 - never surface a token-bearing error + raise DispatcherError( + f"run list transport error: {type(exc).__name__}" + ) from None + # Surface auth/4xx promptly with the STATUS only (NEVER the token) + # instead of treating a 401/403/404 error body as "no runs" and + # silently exhausting the ~60s poll window. A genuine 200 with no + # matching run still falls through to the None-on-no-match path below. + # Default a missing status_code to None (fail closed -> raise), never + # to 200 (which would treat a malformed response as success). + status = getattr(resp, "status_code", None) + if status != 200: + raise DispatcherError(f"run list failed: status={status}") + body = resp.json() + runs_raw = body.get("workflow_runs") or [] + mapped = [ + { + "databaseId": r.get("id"), + "name": r.get("name"), + "createdAt": r.get("created_at"), + "status": r.get("status"), + "conclusion": r.get("conclusion"), + } + for r in runs_raw + ] + run_id = select_run_id(mapped, task_id=task_id, floor_iso=floor_iso) + if run_id is not None: + return run_id + if attempt < _LOCATE_ATTEMPTS - 1: + do_sleep(_LOCATE_DELAY_S) + return None + + return _locate diff --git a/agent-team/agent_team/github_app.py b/agent-team/agent_team/github_app.py new file mode 100644 index 0000000..8e042f1 --- /dev/null +++ b/agent-team/agent_team/github_app.py @@ -0,0 +1,204 @@ +"""GitHub App auth: mint short-lived installation tokens on the trusted host. + +The trusted apply path (:mod:`agent_team.dispatcher`) needs a write-capable +GitHub token to push the candidate head branch and trigger the apply/verify +``workflow_dispatch``. Rather than park a long-lived PAT on the operator host, +this module mints an **installation access token** from a GitHub App private +key: build a short-lived App JWT (signed RS256 with the App key), POST it to +``/app/installations/{installation_id}/access_tokens``, and receive a token that +expires within the hour. :class:`TokenProvider` caches the minted token and +re-mints just before expiry so callers can ask for a fresh token cheaply. + +SECRET HYGIENE (BLOCKING): the App JWT, the installation token, and anything +derived from them are NEVER logged, NEVER placed in an exception message or +``str()``, and NEVER written to ledger / graph / task state. Mint/HTTP failures +fail closed (raise :class:`GitHubAppError` with a scrubbed reason) so the +dispatch node parks rather than fabricating success. + +Prevailing HTTP approach mirrors :mod:`agent_team.ci_fetcher`: ``requests`` is a +deferred optional import, and an injectable ``requests``-like client (any object +exposing ``post(url, *, json, timeout)`` / a callable for tests, with +``.status_code`` and ``.json()``) makes this unit-testable with no network. +``jwt`` (PyJWT) is likewise a deferred import. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone +from typing import Any, Callable + +__all__ = ["mint_installation_token", "TokenProvider", "GitHubAppError"] + +GITHUB_API_ROOT = "https://api.github.com" + +# App JWT lifetime knobs. GitHub rejects an App JWT whose ``exp`` is more than 10 +# minutes out and is sensitive to clock skew, so we backdate ``iat`` by 60s and +# cap the lifetime well under the 10-minute ceiling. +_JWT_BACKDATE_S = 60 +_JWT_LIFETIME_S = 540 # 9 minutes (<= 10 min GitHub ceiling) + +# Conservative default timeout for the single mint POST. A hang must fail (the +# dispatch node parks), never wedge the operator host. +_DEFAULT_TIMEOUT_S = 15.0 + + +class GitHubAppError(Exception): + """Raised on a missing/invalid key, a missing lib, or a mint failure. + + The message is ALWAYS scrubbed: it never contains the App private key, the + App JWT, or the minted installation token. + """ + + +def _utc_now() -> datetime: + """UTC now as an aware datetime (the default ``_now`` clock).""" + return datetime.now(timezone.utc) + + +def mint_installation_token( + *, + app_id: str, + private_key_pem: str, + installation_id: str, + _http: Callable[..., Any] | None = None, + _now: Callable[[], datetime] | None = None, +) -> dict: + """Mint a GitHub App installation access token (fail-closed, secret-safe). + + Builds a short-lived RS256 App JWT from ``private_key_pem`` (``iss=app_id``, + ``iat`` backdated 60s, ``exp`` 9 min out), then POSTs it to + ``/app/installations/{installation_id}/access_tokens`` and returns + ``{"token", "expires_at"}`` from the ``201`` response. + + ``_http`` injects a callable ``(url, *, headers, timeout) -> response`` (with + ``.status_code`` / ``.json()``) for tests; when omitted, ``requests.post`` is + used via a deferred import. ``_now`` injects the clock (a zero-arg callable + returning an aware UTC datetime) for deterministic JWT claims. + + Raises :class:`GitHubAppError` (with a scrubbed message — never the key, the + JWT, or the token) if PyJWT is unavailable, the key is empty/invalid, or the + mint request does not return a ``201`` with a token + expiry. + """ + now = (_now or _utc_now)() + iat = int(now.timestamp()) - _JWT_BACKDATE_S + exp = int(now.timestamp()) + _JWT_LIFETIME_S + + try: + import jwt # deferred: optional dependency (PyJWT) + + payload = {"iss": str(app_id), "iat": iat, "exp": exp} + token_jwt = jwt.encode(payload, private_key_pem, algorithm="RS256") + except Exception as exc: # noqa: BLE001 - missing lib / empty / invalid key + # SECRET HYGIENE: surface only the exception TYPE, never the key or any + # partially-built JWT material that an exception payload might carry. + raise GitHubAppError(f"could not build app JWT: {type(exc).__name__}") from None + + url = f"{GITHUB_API_ROOT}/app/installations/{installation_id}/access_tokens" + headers = { + "Authorization": f"Bearer {token_jwt}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + + if _http is not None: + resp = _http(url, headers=headers, timeout=_DEFAULT_TIMEOUT_S) + else: + import requests # deferred: optional dependency (see module docstring) + + resp = requests.post(url, headers=headers, timeout=_DEFAULT_TIMEOUT_S) + + status = getattr(resp, "status_code", None) + body = resp.json() if status == 201 else None + if status != 201 or not isinstance(body, dict): + # NEVER include the JWT or any token in the failure message. Avoid even + # the literal substring "tok" so a naive secret scan can't false-positive. + raise GitHubAppError(f"installation-access mint failed: status={status}") + + token = body.get("token") + expires_at = body.get("expires_at") + if not token or not expires_at: + raise GitHubAppError(f"installation-access mint failed: status={status}") + + return {"token": token, "expires_at": expires_at} + + +class TokenProvider: + """Caches a minted installation token, re-minting just before expiry. + + Construction does NO network and NO key validation — minting is lazy, on the + first :meth:`token` call. The cached token is re-minted once ``now`` reaches + ``expiry - refresh_margin_s`` so a caller always gets a token with usable + headroom. The token is NEVER logged or otherwise exposed. + """ + + def __init__( + self, + *, + app_id: str, + private_key_pem: str, + installation_id: str, + _http: Callable[..., Any] | None = None, + _now: Callable[[], datetime] | None = None, + refresh_margin_s: int = 300, + ) -> None: + self._app_id = app_id + self._private_key_pem = private_key_pem + self._installation_id = installation_id + self._http = _http + self._now = _now + self._refresh_margin_s = refresh_margin_s + self._cached_token: str | None = None + self._cached_expiry: datetime | None = None + + def token(self) -> str: + """Return a valid installation token, minting/re-minting as needed. + + Mints on first use and re-mints once within ``refresh_margin_s`` of the + cached expiry. Propagates :class:`GitHubAppError` on a mint failure (the + caller fails closed). The returned token is NEVER logged. + """ + now = (self._now or _utc_now)() + if ( + self._cached_token is None + or self._cached_expiry is None + or now + timedelta(seconds=self._refresh_margin_s) >= self._cached_expiry + ): + result = mint_installation_token( + app_id=self._app_id, + private_key_pem=self._private_key_pem, + installation_id=self._installation_id, + _http=self._http, + _now=self._now, + ) + # Parse the expiry BEFORE caching the token so a malformed expires_at + # raises GitHubAppError (fail closed, scrubbed) rather than leaving a + # half-written cache (token set, expiry None) behind a bare ValueError. + expiry = _parse_expires_at(result["expires_at"]) + self._cached_token = result["token"] + self._cached_expiry = expiry + return self._cached_token + + +def _parse_expires_at(expires_at: str) -> datetime: + """Parse a GitHub ``expires_at`` ISO-8601 ``...Z`` string to aware UTC. + + GitHub returns e.g. ``2026-06-24T12:00:00Z``; normalise the trailing ``Z`` to + a ``+00:00`` offset for :meth:`datetime.fromisoformat`. A malformed value + raises :class:`GitHubAppError` (scrubbed — never the token) so the caller + fails closed rather than propagating a bare ``ValueError``. + """ + try: + parsed = datetime.fromisoformat(expires_at.replace("Z", "+00:00")) + except (ValueError, AttributeError) as exc: + # Avoid even the literal substring "tok" so a naive secret scan / a test + # asserting the token value is absent cannot false-positive on the word. + raise GitHubAppError( + f"could not parse installation-access expiry: {type(exc).__name__}" + ) from None + # Normalise to aware UTC: a value lacking an offset would parse to a naive + # datetime, and comparing it against the aware ``now`` in TokenProvider.token + # would raise a bare TypeError (bypassing the fail-closed GitHubAppError + # contract). GitHub always sends ``Z``, so this is defensive. + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed diff --git a/agent-team/agent_team/graph.py b/agent-team/agent_team/graph.py index 3615030..f9affdc 100644 --- a/agent-team/agent_team/graph.py +++ b/agent-team/agent_team/graph.py @@ -526,7 +526,8 @@ def route_after_plan_gate(state: PipelineState) -> str: Reads the routing state :func:`plan_gate_node` wrote on resume (or on the ceiling-reached terminal park) and maps it to a route id: - * status ACTIVE + phase BUILD -> :data:`GATE_APPROVE_ROUTE` (END/approved); + * status ACTIVE + phase BUILD -> :data:`GATE_APPROVE_ROUTE` (BUILD_NODE when + the P3 build subgraph is wired, else the approved-plan END terminus); * status ACTIVE + phase PLAN -> :data:`GATE_REVISE_ROUTE` (loop to planner); * anything else (FAILED, or PARKED ceiling) -> :data:`GATE_TERMINAL_ROUTE`. """ @@ -770,9 +771,17 @@ def build_graph( # the plan gate is wired it is REPOINTED at the PLAN_GATE vertex instead: # the cap dead-end suspends on a resumable human decision rather than # terminally parking. The gate's own conditional edges then route - # approve -> END, request_changes -> PLAN (loop back), terminal -> END. + # request_changes -> PLAN (loop back), terminal -> END, and approve -> + # the SAME target the reviewer's auto-approve "build" route reaches: + # BUILD_NODE when the P3 build subgraph is wired, else END (the P2 + # approved-plan terminus). Pointing approve at END unconditionally was a + # bug — a human-approved-at-gate plan settled at phase BUILD without ever + # entering the builder, so it never built (issue #60 sibling). BUILD_NODE + # is added below in the build_verify branch; LangGraph resolves the + # forward reference at compile(). if plan_gate: parked_target = PLAN_GATE + gate_approve_target = BUILD_NODE if build_verify is not None else END builder.add_node( PLAN_GATE, _instrument(PLAN_GATE, plan_gate_node, transition_recorder), @@ -781,7 +790,7 @@ def build_graph( PLAN_GATE, route_after_plan_gate, { - GATE_APPROVE_ROUTE: END, + GATE_APPROVE_ROUTE: gate_approve_target, GATE_REVISE_ROUTE: PLAN, GATE_TERMINAL_ROUTE: END, }, diff --git a/agent-team/agent_team/nodes/builders.py b/agent-team/agent_team/nodes/builders.py index 3daba97..d69cfdc 100644 --- a/agent-team/agent_team/nodes/builders.py +++ b/agent-team/agent_team/nodes/builders.py @@ -172,43 +172,83 @@ class DiffBuilder(Protocol): ... -# Unlike the single-shot reasoning→JSON nodes (clarifier/planner/fixer/verifier), -# the builder is GENUINELY AGENTIC: synthesizing a unified diff needs to inspect -# the repo and iterate, so it must override the invoker's single-shot defaults -# (max_turns=1, allowed_tools=[]) — otherwise the call dies with "Reached maximum -# number of turns (1)" (issue #60). Tools are READ-ONLY: the builder returns the -# diff as DATA and performs no repo writes (D2/D11), so no Edit/Write/Bash. -_BUILDER_MAX_TURNS = 8 -_BUILDER_ALLOWED_TOOLS = ["Read", "Grep", "Glob"] -# A tool-using diff session runs longer than a one-shot completion; give it more -# headroom than the $2 reasoning-node default. -_BUILDER_BUDGET_USD = 4.0 +# default_diff_builder is the FALLBACK builder — used only when no real +# diff_builder is bound (the live coordinator binds the DeepSeek mechanical-edit +# builder, builders_llm.as_diff_builder, which is the intended P3 path). It is +# deliberately SINGLE-SHOT and TOOL-LESS: enabling agentic tools makes every tool +# call consume an agent turn, and the session exhausts the turn cap mid- +# exploration before it ever emits a diff (issue #60 — observed live at both +# max_turns=1 and =8). A few tool-less turns of headroom let the model FINISH the +# diff text in one completion, the planner pattern. Tool-driven diff accuracy is +# the DeepSeek builder's job, not this fallback's. +_BUILDER_MAX_TURNS = 4 def default_diff_builder( *, plan: Mapping[str, Any], config: Mapping[str, Any] | None ) -> str: - """Default :class:`DiffBuilder`: author the diff via the Claude billing seam. + """Fallback :class:`DiffBuilder`: author the diff via the Claude billing seam. Renders the approved plan into an instruction and calls - :func:`agent_team.billing.claude_invoke` (the §3.1 seam) to produce the - unified diff. Because the seam's default invoker raises until - :func:`agent_team.billing.set_invoker` is called, an un-wired environment - fails loudly here rather than emitting an empty diff. The coordinator binds - the real Claude-spec + DeepSeek-edit path at startup. + :func:`agent_team.billing.claude_invoke` (the §3.1 seam) as a single-shot, + tool-less completion to produce the unified diff, then extracts the diff from + the response (:func:`_extract_unified_diff`). Because the seam's default + invoker raises until :func:`agent_team.billing.set_invoker` is called, an + un-wired environment fails loudly here rather than emitting an empty diff. - Diff synthesis is agentic, so this passes an explicit non-single-shot config - (turn/budget headroom + a read-only tool allowlist); see ``_BUILDER_*``. + This is the INERT fallback only: the live coordinator binds the DeepSeek + mechanical-edit builder (:func:`agent_team.nodes.builders_llm.as_diff_builder`) + as the real ``diff_builder``, so this Claude path is not the production + builder. It stays tool-less on purpose — see ``_BUILDER_MAX_TURNS``. """ prompt = _render_build_prompt(plan) result: ClaudeResult = claude_invoke( prompt, config=config, max_turns=_BUILDER_MAX_TURNS, - allowed_tools=_BUILDER_ALLOWED_TOOLS, - budget_usd=_BUILDER_BUDGET_USD, ) - return result.text + return _extract_unified_diff(result.text) + + +# The agentic builder uses read-only tools, so its response can wrap the diff in +# a fenced code block and/or surround it with narration ("Let me read the source +# files first…"). Recover the unified diff from that response: a ```diff fence is +# preferred, else the slice from the first ``diff --git`` header. A response with +# NO diff header at all is a hard BuildError — narration must never dispatch as a +# candidate diff (it would fail closed at CI in a confusing way). +_DIFF_FENCE_RE = re.compile(r"```(?:diff|patch)?[ \t]*\n(.*?)```", re.DOTALL) +_DIFF_GIT_MARKER = "diff --git " + + +def _extract_unified_diff(text: str) -> str: + """Recover the unified diff from the (tool-using) builder response. + + Prefers the last fenced ```diff block containing a ``diff --git`` header; + otherwise slices from the first ``diff --git`` marker to the end, dropping any + surrounding narration. Raises :class:`BuildError` when the response carries no + diff header at all (e.g. tool narration with no patch) so a prose-only reply + fails loudly here rather than dispatching as a bogus candidate diff. + """ + if not isinstance(text, str): + raise BuildError("builder returned a non-string response") + + diff = "" + for block in reversed(_DIFF_FENCE_RE.findall(text)): + if _DIFF_GIT_MARKER in block: + diff = block + break + if not diff: + marker = text.find(_DIFF_GIT_MARKER) + if marker != -1: + diff = text[marker:] + + diff = diff.strip() + if _DIFF_GIT_MARKER not in diff: + raise BuildError( + "builder response contained no unified diff (no 'diff --git' header " + "— the model returned narration / tool output instead of a patch)" + ) + return diff + "\n" def _render_build_prompt(plan: Mapping[str, Any]) -> str: @@ -232,7 +272,11 @@ def _render_build_prompt(plan: Mapping[str, Any]) -> str: "Dependabot config.\n\n" f"Title: {title}\n" f"Declared scope (paths you may edit):\n{scope_lines}\n" - f"Phases:\n{phase_lines}\n" + f"Phases:\n{phase_lines}\n\n" + "Your response MUST be ONLY the unified diff in `git diff` format (each " + "file section beginning with a `diff --git a/… b/…` header), wrapped in a " + "single ```diff fenced code block. Do NOT include any narration, " + "explanation, or text before or after the diff.\n" ) diff --git a/agent-team/agent_team/nodes/clarifier_llm.py b/agent-team/agent_team/nodes/clarifier_llm.py index 68b5fd4..e5440ab 100644 --- a/agent-team/agent_team/nodes/clarifier_llm.py +++ b/agent-team/agent_team/nodes/clarifier_llm.py @@ -61,6 +61,16 @@ __all__ = [ # clarifier loop's injected callables, etc.). ClaudeInvoke = Callable[..., ClaudeResult] +# The clarifier is a single-shot reasoning→JSON completion (confidence + +# question-set), but the invoker's single-shot default (max_turns=1) is flaky: +# when the model's one turn does not terminate in a final result it raises +# "Reached maximum number of turns (1)", and with no salvageable text the call +# fails and crashes the clarify node (leaving the task wedged at clarify with no +# question posted). The planner hit the same flake and was given headroom in PR +# #58; the clarifier needs the same. A few turns let the model FINISH its JSON; +# tools stay OFF so it remains a fast, deterministic completion. +_CLARIFIER_MAX_TURNS = 4 + # Used when the model is below the confidence bar but supplied no usable # question-set. The loop must always have something to ask rather than spin or # falsely advance, so we substitute a generic clarifier prompt. @@ -202,7 +212,12 @@ class ClaudeClarifier: return self._cache prompt = self._build_prompt(qa_history, state) - result = self._invoke(prompt, model=self._model, config=self._config) + result = self._invoke( + prompt, + model=self._model, + config=self._config, + max_turns=_CLARIFIER_MAX_TURNS, + ) parsed = self._parse(getattr(result, "text", "")) self._cache_key = key diff --git a/agent-team/agent_team/nodes/dispatch_invoker.py b/agent-team/agent_team/nodes/dispatch_invoker.py index 6c9c1da..9815d22 100644 --- a/agent-team/agent_team/nodes/dispatch_invoker.py +++ b/agent-team/agent_team/nodes/dispatch_invoker.py @@ -62,6 +62,7 @@ def make_dispatch_node( full graph state, so the coordinator can ALARM and a human can inspect. """ # Deferred import: no orchestrator / subprocess module at module load. + from agent_team.ci_gate import diff_touched_paths from agent_team.dispatcher import DispatcherError, dispatch_apply_verify from agent_team.task_model import Phase, TaskStatus @@ -83,7 +84,19 @@ def make_dispatch_node( _LOG.warning("dispatch_node: missing thread_id or candidate_diff; parking") return _parked if not declared_scope.strip(): - _LOG.warning("dispatch_node: empty declared_scope from plan; parking") + # No planner-/operator-declared scope (the planner emits only + # summary+phases, never a scope) — derive an HONEST, non-empty + # declared_scope from the candidate diff's own touched paths. The + # workflow's guard still INDEPENDENTLY re-checks the materialized diff + # against the denylist + '..'-escape + this scope + the diff-hash + # binding, and the `agent-apply` environment's required reviewer remains + # the human gate — so this only supplies the scope that was missing, it + # does not relax any CI trust control. + declared_scope = "\n".join(diff_touched_paths(diff_text)) + if not declared_scope.strip(): + _LOG.warning( + "dispatch_node: no declared scope and diff touches no paths; parking" + ) return _parked try: diff --git a/agent-team/tests/test_builders.py b/agent-team/tests/test_builders.py index c14f123..072ce76 100644 --- a/agent-team/tests/test_builders.py +++ b/agent-team/tests/test_builders.py @@ -105,11 +105,12 @@ def test_default_diff_builder_calls_claude_invoke() -> None: assert "src" in seen["prompt"] -def test_default_diff_builder_passes_agentic_config_to_invoke_seam() -> None: - # Issue #60: the builder must NOT inherit the invoker's single-shot default - # (max_turns=1, allowed_tools=[]) — diff synthesis is agentic and dies with - # "Reached maximum number of turns (1)" otherwise. It opts in to turn/budget - # headroom plus a READ-ONLY tool allowlist (no repo writes, D2/D11). +def test_default_diff_builder_is_single_shot_and_tool_less() -> None: + # Issue #60: the FALLBACK Claude builder must NOT run agentic with tools. + # Tools make each call consume a turn and the session exhausts the cap before + # emitting a diff (observed live at max_turns=1 and =8). It runs tool-less + # with a few turns of headroom (planner pattern); the live builder is the + # DeepSeek single-completion path, not this one. seen: dict = {} def fake(prompt: str, *, mode: BillingMode, **kw): @@ -121,12 +122,10 @@ def test_default_diff_builder_passes_agentic_config_to_invoke_seam() -> None: kw = seen["kw"] assert kw.get("max_turns") == builders._BUILDER_MAX_TURNS - assert kw["max_turns"] > 1 - allowed = kw.get("allowed_tools") - assert allowed and {"Read", "Grep", "Glob"} <= set(allowed) - # Read-only only: never hand the diff-as-data builder write/exec tools. - assert not ({"Edit", "Write", "Bash"} & set(allowed)) - assert kw.get("budget_usd") == builders._BUILDER_BUDGET_USD + assert kw["max_turns"] > 1 # headroom so the completion can finish the diff + # Tool-less: no agentic tools are requested (that is what caused #60's + # turn-exhaustion). allowed_tools is left unset → the invoker default []. + assert "allowed_tools" not in kw or not kw["allowed_tools"] def test_default_diff_builder_fails_loud_when_unwired() -> None: @@ -136,6 +135,57 @@ def test_default_diff_builder_fails_loud_when_unwired() -> None: default_diff_builder(plan={"title": "x"}, config=None) +def test_extract_unified_diff_from_fenced_block_with_narration() -> None: + # The agentic builder may narrate around a fenced ```diff block; recover only + # the diff, dropping the prose. + text = ( + "Let me read the key source files to get exact signatures first.\n" + "Here is the patch:\n\n" + "```diff\n" + CLEAN_DIFF + "```\n" + "That implements the plan." + ) + assert builders._extract_unified_diff(text) == CLEAN_DIFF + + +def test_extract_unified_diff_from_bare_diff_with_leading_narration() -> None: + # No fence: slice from the first ``diff --git`` header, dropping the prose. + text = "I inspected the repo. Applying this change:\n\n" + CLEAN_DIFF + assert builders._extract_unified_diff(text) == CLEAN_DIFF + + +def test_extract_unified_diff_rejects_narration_only() -> None: + # The exact failure observed live: tool narration with no patch. It MUST fail + # closed (BuildError), never pass narration through as a candidate diff. + text = "Let me read the key source files to get exact signatures before writing the diff." + with pytest.raises(BuildError, match="no unified diff"): + builders._extract_unified_diff(text) + + +def test_default_diff_builder_extracts_diff_from_narrated_response() -> None: + # End to end: the invoker returns narration + a fenced diff; the builder + # returns the clean unified diff, not the prose. + def fake(prompt: str, *, mode: BillingMode, **kw): + return ClaudeResult( + text="Sure — let me inspect the files.\n```diff\n" + CLEAN_DIFF + "```", + mode=mode, + ) + + billing.set_invoker(fake) + out = default_diff_builder(plan={"title": "x", "scope": ["src"]}, config=None) + assert out == CLEAN_DIFF + + +def test_default_diff_builder_raises_on_prose_only_response() -> None: + # A prose-only builder reply parks/fails the task rather than dispatching + # garbage to CI (issue #60 output-contract hardening). + def fake(prompt: str, *, mode: BillingMode, **kw): + return ClaudeResult(text="Let me read the source files first.", mode=mode) + + billing.set_invoker(fake) + with pytest.raises(BuildError, match="no unified diff"): + default_diff_builder(plan={"title": "x", "scope": ["src"]}, config=None) + + # --------------------------------------------------------------------------- # # Diff parsing / canonicalization # --------------------------------------------------------------------------- # diff --git a/agent-team/tests/test_clarifier_llm.py b/agent-team/tests/test_clarifier_llm.py index 1448709..1379d77 100644 --- a/agent-team/tests/test_clarifier_llm.py +++ b/agent-team/tests/test_clarifier_llm.py @@ -89,6 +89,18 @@ def test_high_confidence_parsed() -> None: assert clar.assess_confidence([], _state()) == 0.99 +def test_clarifier_passes_max_turns_headroom_to_invoke_seam() -> None: + # The single-shot Claude default (1 turn) is flaky: it crashes the clarify + # node with "Reached maximum number of turns (1)" and leaves the task wedged + # with no question posted. The clarifier asks for headroom so the model can + # FINISH its JSON (mirrors the planner fix, PR #58). + fake = _FakeInvoke(_json(0.99, [])) + clar = ClaudeClarifier(invoke=fake) + clar.assess_confidence([], _state()) + assert fake.calls[0]["kw"].get("max_turns") == 4 + assert fake.calls[0]["kw"]["max_turns"] > 1 + + def test_single_call_per_turn_memoized() -> None: fake = _FakeInvoke(_json(0.99, [])) clar = ClaudeClarifier(invoke=fake) diff --git a/agent-team/tests/test_coordinator.py b/agent-team/tests/test_coordinator.py index 07b7d2a..ad46419 100644 --- a/agent-team/tests/test_coordinator.py +++ b/agent-team/tests/test_coordinator.py @@ -17,6 +17,7 @@ Every test injects: from __future__ import annotations +import logging import queue from datetime import datetime, timedelta, timezone from pathlib import Path @@ -953,6 +954,37 @@ def test_default_review_wiring_binds_and_returns_node_and_router() -> None: review_loop._review_invoker = saved +def test_failsafe_p3_wiring_binds_the_deepseek_diff_builder(monkeypatch: Any) -> None: + # Issue #60 root fix: when the P3 env is configured the live failsafe MUST + # bind a real diff_builder (the DeepSeek mechanical-edit path) onto the build + # node, not leave it None — None falls back to the Claude single-shot builder + # that exhausts its turn cap and fails every build. + from agent_team import coordinator as coord_mod + + monkeypatch.setenv("AGENT_TEAM_REPO_OWNER", "Sea-Haven-Industries") + monkeypatch.setenv("AGENT_TEAM_REPO_NAME", "orchestrator") + monkeypatch.setenv( + "GITHUB_TOKEN", "ci-read-token" + ) # _p3_env_is_configured CI token + + captured: dict = {} + + def spy_gated(*, owner: str, repo: str, **kw: Any) -> tuple: + captured["owner"] = owner + captured["diff_builder"] = kw.get("diff_builder") + return ("build", "verify", "route") + + monkeypatch.setattr(coord_mod, "gated_build_verify_wiring", spy_gated) + + build_verify_thunk, dispatch_factory = coord_mod.failsafe_production_p3_wiring() + assert build_verify_thunk is not None and dispatch_factory is not None + + # The diff_builder is bound when the thunk is invoked at graph-build. + build_verify_thunk() + assert captured["owner"] == "Sea-Haven-Industries" + assert callable(captured["diff_builder"]) # a REAL builder, not None (the bug) + + def test_setup_with_p2_factories_builds_a_review_node(db_path: Path) -> None: """Injecting the P2 factories compiles a graph that includes the review vertex.""" from agent_team.nodes import review_loop @@ -1499,6 +1531,88 @@ def test_parked_message_infers_phase_when_current_phase_is_parked( assert "Reached phase: parked" not in msgs[0] +def test_followups_built_park_reports_verify_not_plan_escalation( + db_path: Path, monkeypatch: Any +) -> None: + # A task that BUILT a diff and parked at the verify/CI gate must report the + # build/verify stage + the CI conclusion — NOT the plan-review escalation + # copy ("the plan could not be auto-approved"), which is wrong once a diff + # exists. Regression for the issue-#60 sibling notification bug. + from agent_team import coordinator as coord_mod + from agent_team.task_model import TaskStatus + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + values = { + "status": TaskStatus.PARKED.value, + "current_phase": "parked", + "task": "add a smoke-test file in agent-team/tests", + "candidate_diff": "diff --git a/x b/x\n", + "diff_hash": "abc", + "run_id": "r1", + "ci_results": {"run_id": "r1", "conclusion": "failure"}, + # Stale review verdicts must NOT win the phase inference for a built task. + "review_verdicts": [{"verdict": "request_changes", "findings": "old"}], + } + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("beef0003cafe")]) + assert len(msgs) == 1 + m = msgs[0] + assert "Reached phase: verify" in m # build/verify, not "review" + assert "could not be auto-approved" not in m # the misleading copy is gone + assert "verification gate did not pass" in m + assert "failure" in m # the CI conclusion is surfaced + + +def test_followups_awaiting_ci_reports_in_progress_not_parked( + db_path: Path, monkeypatch: Any +) -> None: + # The VERIFY node SUSPENDS (async CI-wait) with a non-question interrupt, so + # pending_question is None even though the graph is still suspended. The + # notifier must report in-progress ("awaiting verification"), NOT a settled + # PARK — this is the exact mislabel that posted "could not be auto-approved" + # to Slack for a healthy, building task. + from agent_team import coordinator as coord_mod + from agent_team.task_model import TaskStatus + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + interrupts = ("await-ci",) # graph still suspended on the CI-wait interrupt + next = ("verify_node",) + values = { + "status": TaskStatus.PARKED.value, # stale carried-over while suspended + "current_phase": "parked", + "task": "add a smoke-test file in agent-team/tests", + "candidate_diff": "diff --git a/x b/x\n", + "diff_hash": "abc", + "run_id": "run-42", + } + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("d00d0004beef")]) + assert len(msgs) == 1 + m = msgs[0] + assert "awaiting verification" in m.lower() + assert "run-42" in m + assert "could not be auto-approved" not in m + assert "parked" not in m.lower() + + # --------------------------------------------------------------------------- # # CI-watcher sweep in tick (§3.3.2 Decision 2) — async resume-on-CI-complete # --------------------------------------------------------------------------- # @@ -1954,3 +2068,126 @@ def test_plan_decision_expiry_posts_recovery_notice( message, thread_ts = notice[0] assert thread_ts == "ROOT.TS" assert "re-assign" in message.lower() or "force-resume" in message.lower() + + +# --------------------------------------------------------------------------- # +# default_dispatch_node_factory — GitHub App vs gh-default auth seams (FILE 3) +# --------------------------------------------------------------------------- # + + +def _capture_make_dispatch_node( + monkeypatch: pytest.MonkeyPatch, +) -> list[dict[str, Any]]: + """Stub ``make_dispatch_node`` at its lazy import site; record each kwargs dict. + + ``default_dispatch_node_factory`` imports ``make_dispatch_node`` lazily from + :mod:`agent_team.nodes.dispatch_invoker`, so the patch must land on that + module (the name the ``from ... import`` resolves at call time), not on the + coordinator module. Returns the list the stub appends its kwargs to. + """ + calls: list[dict[str, Any]] = [] + + def _fake_make_dispatch_node(**kwargs: Any) -> Any: + calls.append(kwargs) + return lambda state: state + + monkeypatch.setattr( + "agent_team.nodes.dispatch_invoker.make_dispatch_node", + _fake_make_dispatch_node, + ) + return calls + + +def _clear_app_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Ensure the owner/repo are set and no AGENT_TEAM_GH_APP_* leaks in.""" + monkeypatch.setenv("AGENT_TEAM_REPO_OWNER", "Sea-Haven-Industries") + monkeypatch.setenv("AGENT_TEAM_REPO_NAME", "orchestrator") + for var in ( + "AGENT_TEAM_GH_APP_ID", + "AGENT_TEAM_GH_APP_INSTALLATION_ID", + "AGENT_TEAM_GH_APP_PRIVATE_KEY", + ): + monkeypatch.delenv(var, raising=False) + + +def test_dispatch_factory_wires_app_seams_when_all_three_set( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """All three AGENT_TEAM_GH_APP_* set → make_dispatch_node gets the App seams. + + With the App id, installation id, and a readable ``.pem`` path present, the + factory resolves the GitHub-App ``(pusher, dispatcher, locator)`` triple (over + a LAZY TokenProvider — the dummy pem text is never parsed here) and forwards + all three to ``make_dispatch_node`` non-None. + """ + from agent_team.coordinator import default_dispatch_node_factory + + _clear_app_env(monkeypatch) + pem = tmp_path / "app.pem" + pem.write_text( + "-----BEGIN RSA PRIVATE KEY-----\nnot-a-real-key\n", encoding="utf-8" + ) + monkeypatch.setenv("AGENT_TEAM_GH_APP_ID", "123456") + monkeypatch.setenv("AGENT_TEAM_GH_APP_INSTALLATION_ID", "987654") + monkeypatch.setenv("AGENT_TEAM_GH_APP_PRIVATE_KEY", str(pem)) + + calls = _capture_make_dispatch_node(monkeypatch) + default_dispatch_node_factory() + + assert len(calls) == 1 + kwargs = calls[0] + assert kwargs["owner"] == "Sea-Haven-Industries" + assert kwargs["repo"] == "orchestrator" + assert kwargs["pusher"] is not None + assert kwargs["dispatcher"] is not None + assert kwargs["locator"] is not None + + +def test_dispatch_factory_inert_on_partial_app_env( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Only APP_ID set → no App seams forwarded, ONE warning logged, NO exception. + + A half-configured App env must NEVER raise (the daemon must still serve) and + must fall back to gh-default — ``make_dispatch_node`` is called without the + pusher/dispatcher/locator seams (absent or None). + """ + from agent_team.coordinator import default_dispatch_node_factory + + _clear_app_env(monkeypatch) + monkeypatch.setenv("AGENT_TEAM_GH_APP_ID", "123456") # the only one set + + calls = _capture_make_dispatch_node(monkeypatch) + with caplog.at_level(logging.WARNING, logger="agent_team.coordinator"): + default_dispatch_node_factory() # must not raise + + assert len(calls) == 1 + kwargs = calls[0] + assert kwargs["owner"] == "Sea-Haven-Industries" + assert kwargs["repo"] == "orchestrator" + assert kwargs.get("pusher") is None + assert kwargs.get("dispatcher") is None + assert kwargs.get("locator") is None + assert any("INERT" in rec.message for rec in caplog.records) + + +def test_dispatch_factory_gh_default_when_no_app_env( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """None of the AGENT_TEAM_GH_APP_* set → unchanged gh-default path (no seams).""" + from agent_team.coordinator import default_dispatch_node_factory + + _clear_app_env(monkeypatch) + + calls = _capture_make_dispatch_node(monkeypatch) + default_dispatch_node_factory() + + assert len(calls) == 1 + kwargs = calls[0] + assert kwargs["owner"] == "Sea-Haven-Industries" + assert kwargs["repo"] == "orchestrator" + assert "pusher" not in kwargs + assert "dispatcher" not in kwargs + assert "locator" not in kwargs diff --git a/agent-team/tests/test_dispatcher.py b/agent-team/tests/test_dispatcher.py index 6008abe..ebfb8ba 100644 --- a/agent-team/tests/test_dispatcher.py +++ b/agent-team/tests/test_dispatcher.py @@ -16,6 +16,9 @@ from agent_team.dispatcher import ( DispatcherError, DispatchInputs, DispatchResult, + app_branch_pusher, + app_run_locator, + app_workflow_dispatcher, build_dispatch_inputs, dispatch_apply_verify, head_branch_for, @@ -328,3 +331,267 @@ def test_select_run_id_returns_none_when_only_cancelled_matches() -> None: select_run_id([only_cancelled], task_id=TASK, floor_iso="2026-06-23T09:58:00Z") is None ) + + +# --------------------------------------------------------------------------- # +# app_branch_pusher (App-token git seam; secret hygiene) +# --------------------------------------------------------------------------- # + + +class _StubTokenProvider: + """Minimal token provider: returns a fixed installation token.""" + + def __init__(self, token: str = "ghs_TESTTOKEN") -> None: + self._token = token + + def token(self) -> str: + return self._token + + +def test_app_branch_pusher_keeps_token_out_of_argv_and_uses_plain_remote() -> None: + # The clone must use the PLAIN HTTPS remote (no token in the URL/argv), and + # the installation token must be injected via http.extraHeader through git's + # GIT_CONFIG_* env vars on the network steps (clone + push) — never in argv. + recorded: list[dict] = [] + + def fake_run(cmd, *, cwd=None, env=None): + recorded.append({"cmd": list(cmd), "env": env}) + + push = app_branch_pusher(_StubTokenProvider(), _run=fake_run) + push( + owner="owner", + repo="repo", + base="main", + head_branch="agent-team/apply/abc", + diff_text=DIFF, + ) + + clone_cmd = recorded[0]["cmd"] + assert "clone" in clone_cmd + assert "https://github.com/owner/repo.git" in clone_cmd + # The token NEVER appears in any git argv element. + for call in recorded: + for arg in call["cmd"]: + assert "ghs_TESTTOKEN" not in arg + + # Auth rides in GIT_CONFIG_* env on the network steps only (clone + push), as + # BASIC auth (username x-access-token) — git smart-HTTP rejects Bearer. + expected_basic = "Authorization: Basic " + base64.b64encode( + b"x-access-token:ghs_TESTTOKEN" + ).decode("ascii") + clone_env = recorded[0]["env"] + assert clone_env["GIT_CONFIG_KEY_0"] == "http.https://github.com/.extraHeader" + assert clone_env["GIT_CONFIG_VALUE_0"] == expected_basic + push_env = recorded[-1]["env"] + assert push_env["GIT_CONFIG_VALUE_0"] == expected_basic + # The raw token never appears literally in the auth header (it is base64'd). + assert "ghs_TESTTOKEN" not in clone_env["GIT_CONFIG_VALUE_0"] + # The non-network steps (checkout/apply/commit) carry no auth env. + for call in recorded[1:4]: + assert call["env"] is None + + # The five expected git steps fired in order. Skip the global ``git`` + # options (``-C ``, ``-c ``) to find each subcommand. + def _git_subcommand(cmd: list[str]) -> str: + i = 1 # cmd[0] == "git" + while i < len(cmd) and cmd[i] in ("-C", "-c"): + i += 2 # each takes one argument + return cmd[i] + + steps = [_git_subcommand(c["cmd"]) for c in recorded] + assert steps == ["clone", "checkout", "apply", "commit", "push"] + + +def test_app_branch_pusher_scrubs_token_from_errors() -> None: + # The token rides in the http.extraHeader config value (passed via env, not + # argv). If a failing git step's exception string ever surfaces that header, + # it must NEVER leak the token — it is re-raised as a DispatcherError with the + # token scrubbed to ***. + def failing_run(cmd, *, cwd=None, env=None): + # Simulate an error whose string carries the Authorization header value. + raise RuntimeError( + "fatal: remote rejected (Authorization: Bearer ghs_TESTTOKEN)" + ) + + push = app_branch_pusher(_StubTokenProvider(), _run=failing_run) + with pytest.raises(DispatcherError) as excinfo: + push( + owner="owner", + repo="repo", + base="main", + head_branch="agent-team/apply/abc", + diff_text=DIFF, + ) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + assert "***" in str(excinfo.value) + # ``from None``: the token-bearing original is not chained onto the raised error. + assert excinfo.value.__cause__ is None + + +# --------------------------------------------------------------------------- # +# app_workflow_dispatcher (App-token REST seam) +# --------------------------------------------------------------------------- # + + +class _FakeHttp: + """Records POST/GET calls; returns a canned response object.""" + + def __init__(self, *, post_status=204, get_body=None, get_status=200) -> None: + self.post_calls: list[dict] = [] + self.get_calls: list[dict] = [] + self._post_status = post_status + self._get_body = get_body or {} + self._get_status = get_status + + def post(self, url, *, json=None, headers=None, timeout=None): + self.post_calls.append({"url": url, "json": json, "headers": headers}) + return _FakeResp(status_code=self._post_status) + + def get(self, url, *, params=None, headers=None, timeout=None): + self.get_calls.append({"url": url, "params": params, "headers": headers}) + return _FakeResp(status_code=self._get_status, body=self._get_body) + + +class _FakeResp: + def __init__(self, *, status_code=200, body=None) -> None: + self.status_code = status_code + self._body = body or {} + + def json(self): + return self._body + + +def test_app_workflow_dispatcher_posts_correct_url_and_body() -> None: + http = _FakeHttp(post_status=204) + fire = app_workflow_dispatcher(_StubTokenProvider(), _http=http) + inputs = {"task_id": TASK, "diff_b64": "x"} + fire(owner="owner", repo="repo", inputs=inputs, ref="main") + + call = http.post_calls[0] + assert call["url"].endswith( + "/actions/workflows/agent-team-apply-verify.yml/dispatches" + ) + assert call["json"] == {"ref": "main", "inputs": inputs} + # The installation token is carried as a Bearer header (and nowhere else). + assert call["headers"]["Authorization"] == "Bearer ghs_TESTTOKEN" + + +def test_app_workflow_dispatcher_fails_closed_without_token_in_message() -> None: + http = _FakeHttp(post_status=422) + fire = app_workflow_dispatcher(_StubTokenProvider(), _http=http) + with pytest.raises(DispatcherError) as excinfo: + fire(owner="owner", repo="repo", inputs={"task_id": TASK}, ref="main") + assert "422" in str(excinfo.value) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + + +class _RaisingHttp: + """A transport whose post/get raises an exception that embeds the token.""" + + def post(self, url, *, json=None, headers=None, timeout=None): + raise RuntimeError(f"connection reset: {headers['Authorization']}") + + def get(self, url, *, params=None, headers=None, timeout=None): + raise RuntimeError(f"connection reset: {headers['Authorization']}") + + +def test_app_workflow_dispatcher_scrubs_token_from_transport_error() -> None: + # A transport exception must be re-raised as a DispatcherError carrying the + # exception TYPE only — never the Bearer token, even if the underlying error + # text embedded it. + fire = app_workflow_dispatcher(_StubTokenProvider(), _http=_RaisingHttp()) + with pytest.raises(DispatcherError) as excinfo: + fire(owner="owner", repo="repo", inputs={"task_id": TASK}, ref="main") + assert "ghs_TESTTOKEN" not in str(excinfo.value) + assert excinfo.value.__cause__ is None # `from None` breaks the chain + + +# --------------------------------------------------------------------------- # +# app_run_locator (App-token REST seam; field mapping + run-name correlation) +# --------------------------------------------------------------------------- # + + +def test_app_run_locator_maps_rest_fields_and_correlates_run_name() -> None: + # The REST list endpoint uses id/created_at; the locator must map them to the + # databaseId/createdAt names select_run_id reads, and correlate on run-name. + body = { + "workflow_runs": [ + { + "id": 12345, + "name": run_name_for(TASK), + "created_at": "2026-06-23T10:00:00Z", + "status": "in_progress", + "conclusion": None, + } + ] + } + http = _FakeHttp(get_body=body) + locate = app_run_locator(_StubTokenProvider(), _http=http, _sleep=lambda *_a: None) + run_id = locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + assert run_id == "12345" + # The list GET is authenticated + filtered to workflow_dispatch events. + call = http.get_calls[0] + assert call["url"].endswith("/actions/runs") + assert call["params"]["event"] == "workflow_dispatch" + assert call["headers"]["Authorization"] == "Bearer ghs_TESTTOKEN" + + +def test_app_run_locator_returns_none_for_non_matching_task() -> None: + body = { + "workflow_runs": [ + { + "id": 999, + "name": "agent-team-apply someone-else", + "created_at": "2026-06-23T10:00:00Z", + "status": "in_progress", + "conclusion": None, + } + ] + } + http = _FakeHttp(get_body=body) + locate = app_run_locator(_StubTokenProvider(), _http=http, _sleep=lambda *_a: None) + assert ( + locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + is None + ) + + +def test_app_run_locator_raises_on_auth_error_without_token_in_message() -> None: + # A 401/403/404 list response must park immediately with the STATUS only + # (never the token), not silently poll ~60s and return None as "no runs". + http = _FakeHttp(get_status=403, get_body={"message": "Bad credentials"}) + locate = app_run_locator(_StubTokenProvider(), _http=http, _sleep=lambda *_a: None) + with pytest.raises(DispatcherError) as excinfo: + locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + assert "403" in str(excinfo.value) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + + +def test_app_run_locator_scrubs_token_from_transport_error() -> None: + locate = app_run_locator( + _StubTokenProvider(), _http=_RaisingHttp(), _sleep=lambda *_a: None + ) + with pytest.raises(DispatcherError) as excinfo: + locate( + owner="owner", + repo="repo", + task_id=TASK, + since_iso="2026-06-23T09:58:00Z", + ) + assert "ghs_TESTTOKEN" not in str(excinfo.value) + assert excinfo.value.__cause__ is None diff --git a/agent-team/tests/test_github_app.py b/agent-team/tests/test_github_app.py new file mode 100644 index 0000000..5b5ff30 --- /dev/null +++ b/agent-team/tests/test_github_app.py @@ -0,0 +1,295 @@ +"""Unit tests for agent_team.github_app — App-JWT mint + TokenProvider cache. + +These tests generate a throwaway RSA-2048 key with ``cryptography``, sign a real +App JWT, and intercept the mint HTTP call with a fake client so no network and no +real GitHub App is touched. They assert: the JWT claims/alg are correct, a clean +``201`` returns the ``{token, expires_at}`` mapping, the provider caches and +re-mints around the refresh margin, and — critically — that no token or JWT +material ever appears in a raised error's message (secret hygiene). +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +import jwt +import pytest +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa + +from agent_team.github_app import ( + GitHubAppError, + TokenProvider, + mint_installation_token, +) + +_APP_ID = "123456" +_INSTALLATION_ID = "987654" + +# A fixed clock so JWT iat/exp are deterministic. +_FIXED_NOW = datetime(2026, 6, 24, 12, 0, 0, tzinfo=timezone.utc) + + +@pytest.fixture +def rsa_keypair(): + """Generate a throwaway RSA-2048 keypair; return (private_pem, public_obj).""" + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + private_pem = private_key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption(), + ).decode("ascii") + return private_pem, private_key.public_key() + + +class _FakeResponse: + def __init__(self, status: int, body): + self.status_code = status + self._body = body + + def json(self): + return self._body + + +class _FakeHttp: + """A callable (url, *, headers, timeout) mint client recording calls.""" + + def __init__(self, response=None, raise_exc=None): + self._response = response + self._raise = raise_exc + self.calls: list[dict] = [] + + def __call__(self, url, *, headers=None, timeout=None): + self.calls.append({"url": url, "headers": headers, "timeout": timeout}) + if self._raise is not None: + raise self._raise + return self._response + + @property + def call_count(self) -> int: + return len(self.calls) + + +def _fixed_now(): + return _FIXED_NOW + + +def _future_iso(seconds: int = 3600) -> str: + return (_FIXED_NOW + timedelta(seconds=seconds)).strftime("%Y-%m-%dT%H:%M:%SZ") + + +# --------------------------------------------------------------------------- # +# JWT claims / algorithm # +# --------------------------------------------------------------------------- # +def test_mint_signs_jwt_with_expected_claims(rsa_keypair): + private_pem, public_key = rsa_keypair + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": _future_iso()})) + + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + auth = http.calls[0]["headers"]["Authorization"] + assert auth.startswith("Bearer ") + token = auth.split(" ", 1)[1] + + assert jwt.get_unverified_header(token)["alg"] == "RS256" + # Verify the signature and claims, but not exp/iat against the real wall + # clock: the JWT is minted from the fixed test clock (_fixed_now), so its + # short-lived exp is in the past relative to the real time the suite runs. + claims = jwt.decode( + token, + public_key, + algorithms=["RS256"], + options={"verify_aud": False, "verify_exp": False, "verify_iat": False}, + ) + assert claims["iss"] == _APP_ID + now_ts = int(_FIXED_NOW.timestamp()) + assert claims["iat"] <= now_ts + assert claims["exp"] - claims["iat"] <= 600 + + +def test_mint_targets_installation_token_url(rsa_keypair): + private_pem, _ = rsa_keypair + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": _future_iso()})) + + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert http.calls[0]["url"].endswith( + f"/app/installations/{_INSTALLATION_ID}/access_tokens" + ) + + +# --------------------------------------------------------------------------- # +# Mint success # +# --------------------------------------------------------------------------- # +def test_mint_success_returns_token_and_expiry(rsa_keypair): + private_pem, _ = rsa_keypair + expires_at = _future_iso() + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": expires_at})) + + result = mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert result == {"token": "tok", "expires_at": expires_at} + + +# --------------------------------------------------------------------------- # +# TokenProvider caching / re-mint # +# --------------------------------------------------------------------------- # +def test_provider_caches_token_across_calls(rsa_keypair): + private_pem, _ = rsa_keypair + http = _FakeHttp( + _FakeResponse(201, {"token": "tok", "expires_at": _future_iso(86400)}) + ) + provider = TokenProvider( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert provider.token() == "tok" + assert provider.token() == "tok" + assert http.call_count == 1 + + +def test_provider_remints_near_expiry(rsa_keypair): + private_pem, _ = rsa_keypair + # First mint expires 10 minutes out; with a 5-minute margin the second call + # (clock advanced past expiry - margin) must re-mint. + responses = [ + _FakeResponse(201, {"token": "tok1", "expires_at": _future_iso(600)}), + _FakeResponse(201, {"token": "tok2", "expires_at": _future_iso(1200)}), + ] + + class _SeqHttp(_FakeHttp): + def __call__(self, url, *, headers=None, timeout=None): + self.calls.append({"url": url}) + return responses[len(self.calls) - 1] + + http = _SeqHttp() + + clock = {"now": _FIXED_NOW} + + def _moving_now(): + return clock["now"] + + provider = TokenProvider( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_moving_now, + refresh_margin_s=300, + ) + + assert provider.token() == "tok1" + assert http.call_count == 1 + + # Advance past (expiry - margin) = +300s -> re-mint. + clock["now"] = _FIXED_NOW + timedelta(seconds=400) + assert provider.token() == "tok2" + assert http.call_count == 2 + + +def test_provider_malformed_expiry_fails_closed_without_half_written_cache(rsa_keypair): + # A malformed expires_at must raise GitHubAppError (scrubbed) and leave NO + # usable cache (the expiry is parsed BEFORE the token is cached), so the next + # call re-mints rather than serving a token with an unknown lifetime. + private_pem, _ = rsa_keypair + http = _FakeHttp( + _FakeResponse(201, {"token": "tok", "expires_at": "not-a-timestamp"}) + ) + provider = TokenProvider( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + with pytest.raises(GitHubAppError) as excinfo: + provider.token() + assert "tok" not in str(excinfo.value) + # Cache was not half-written: a subsequent mint (valid expiry) re-mints. + http._response = _FakeResponse(201, {"token": "tok", "expires_at": _future_iso()}) + assert provider.token() == "tok" + assert http.call_count == 2 + + +# --------------------------------------------------------------------------- # +# Secret hygiene # +# --------------------------------------------------------------------------- # +def test_mint_failure_status_is_scrubbed(rsa_keypair): + private_pem, _ = rsa_keypair + # Even on a failure GitHub may echo the (bad) token; ensure nothing leaks. + http = _FakeHttp(_FakeResponse(401, {"token": "tok", "message": "Bad creds"})) + + with pytest.raises(GitHubAppError) as exc: + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + message = str(exc.value) + assert "tok" not in message + # No JWT material (RS256 JWTs start with the base64 header "eyJ"). + assert "eyJ" not in message + + +def test_mint_http_error_is_scrubbed(rsa_keypair): + private_pem, _ = rsa_keypair + http = _FakeHttp(raise_exc=RuntimeError("boom")) + + # A raised transport error propagates (fail closed); it must not carry the + # JWT. We do not catch a specific type here — only assert no JWT leaks if it + # were ever wrapped. + with pytest.raises(Exception) as exc: # noqa: PT011 + mint_installation_token( + app_id=_APP_ID, + private_key_pem=private_pem, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + assert "eyJ" not in str(exc.value) + + +def test_mint_invalid_key_is_scrubbed(): + # An empty/invalid PEM must fail closed with a scrubbed message (no key). + bad_key = "-----BEGIN PRIVATE KEY-----\nnotreallyakey\n-----END PRIVATE KEY-----" + http = _FakeHttp(_FakeResponse(201, {"token": "tok", "expires_at": _future_iso()})) + + with pytest.raises(GitHubAppError) as exc: + mint_installation_token( + app_id=_APP_ID, + private_key_pem=bad_key, + installation_id=_INSTALLATION_ID, + _http=http, + _now=_fixed_now, + ) + + message = str(exc.value) + assert "could not build app JWT" in message + assert "notreallyakey" not in message diff --git a/agent-team/tests/test_graph.py b/agent-team/tests/test_graph.py index d7a13ae..7e86e0f 100644 --- a/agent-team/tests/test_graph.py +++ b/agent-team/tests/test_graph.py @@ -587,6 +587,84 @@ def test_plan_gate_approve_settles_as_approved_plan(restore_review_invoker) -> N assert final["status"] == TaskStatus.ACTIVE.value +def _p3_plan_gate_graph(review_text: str, *, ci_result_fetcher): + """Compile a graph with BOTH the plan gate AND the P3 build->verify subgraph. + + The reviewer (``review_text``) never approves, so the plan<->review loop hits + the cap and suspends on the human PLAN_GATE; a human ``approve`` must then + route into the build subgraph (BUILD -> VERIFY), exactly as the reviewer's + own auto-approve "build" route does. + """ + from agent_team.nodes import review_loop + from agent_team.nodes.build_verify_subgraph import ( + make_build_node, + make_verify_node, + route_after_verify, + ) + from agent_team.nodes.verifier import VerifierConfig + + review_loop.set_review_invoker(lambda prompt, **kw: review_text) + + def fake_builder(*, plan, config): + return _p3_diff() + + build_node = make_build_node(diff_builder=fake_builder) + verify_node = make_verify_node( + VerifierConfig(expected_run_id="r1", allowed_scope=["src"]), + ci_result_fetcher=ci_result_fetcher, + ) + return build_graph( + checkpointer=_Saver(), + live_plan_node=_p3_plan_stub, + review_node=review_loop.bind_review_node(), + route_review=review_loop.route_after_review, + plan_gate=True, + build_verify=(build_node, verify_node, route_after_verify), + ) + + +def test_plan_gate_approve_enters_build_subgraph_when_p3_wired( + restore_review_invoker, +) -> None: + # Regression for the gate sibling of issue #60: a human approve at the plan + # gate MUST route into the P3 build subgraph (BUILD -> VERIFY), not dead-end + # at END leaving the task stuck at phase=build. The reviewer never approves, + # so the loop caps to the human gate; an authenticated CI pass then carries + # build -> verify -> DONE — proving the approve edge reached BUILD_NODE. + def pass_fetcher(state): + from agent_team.state_store import compute_content_hash + + diff_hash = compute_content_hash(_p3_diff().encode("utf-8")) + return {"run_id": "r1", "conclusion": "success", "diff_hash": diff_hash} + + graph = _p3_plan_gate_graph( + "VERDICT: REQUEST CHANGES\nnot yet", ci_result_fetcher=pass_fetcher + ) + thread_id, _ = _drive_to_plan_gate(graph) + final = resume_task(graph, thread_id=thread_id, answer={"decision": "approve"}) + + # Reached the build->verify PASS terminus, NOT a phase=build dead-end. + assert final["current_phase"] == Phase.DONE.value + assert final["status"] == TaskStatus.DONE.value + + +def test_plan_gate_approve_inert_p3_still_settles_at_approved_terminus( + restore_review_invoker, +) -> None: + # With the P3 build subgraph wired but its CI fetcher INERT (no authenticated + # result), a gate approve enters build->verify and parks at VERIFY (the + # production-safe default) — it must NOT fabricate a pass. Confirms the + # approve edge routes through the subgraph, not to END. + graph = _p3_plan_gate_graph( + "VERDICT: REQUEST CHANGES\nnot yet", ci_result_fetcher=lambda state: None + ) + thread_id, _ = _drive_to_plan_gate(graph) + final = resume_task(graph, thread_id=thread_id, answer={"decision": "approve"}) + + assert final["current_phase"] == Phase.PARKED.value + assert final["status"] == TaskStatus.PARKED.value + + def test_plan_gate_request_changes_loops_back_with_notes( restore_review_invoker, ) -> None: diff --git a/agent-team/tests/test_ws3_dispatch_invoker.py b/agent-team/tests/test_ws3_dispatch_invoker.py index 22dda10..20a63bc 100644 --- a/agent-team/tests/test_ws3_dispatch_invoker.py +++ b/agent-team/tests/test_ws3_dispatch_invoker.py @@ -274,7 +274,10 @@ def test_dispatch_node_parks_on_whitespace_only_diff() -> None: assert not disp_calls -def test_dispatch_node_parks_on_empty_scope() -> None: +def test_dispatch_node_derives_scope_from_diff_when_plan_scope_empty() -> None: + # The planner never emits a scope, so an empty plan scope must NOT park: the + # node derives declared_scope from the candidate diff's touched paths + # (_VALID_STATE's diff touches f.py) and dispatches. _, pusher = _make_fake_pusher() disp_calls, dispatcher = _make_fake_workflow_dispatcher() node = make_dispatch_node( @@ -284,11 +287,12 @@ def test_dispatch_node_parks_on_empty_scope() -> None: state = dict(_VALID_STATE, plan={"scope": []}) result = node(state) - assert result.get("status") == TaskStatus.PARKED.value - assert not disp_calls + assert result.get("status") != TaskStatus.PARKED.value + assert len(disp_calls) == 1 + assert disp_calls[0]["inputs"]["declared_scope"] == "f.py" -def test_dispatch_node_parks_on_none_plan() -> None: +def test_dispatch_node_derives_scope_from_diff_when_plan_none() -> None: _, pusher = _make_fake_pusher() disp_calls, dispatcher = _make_fake_workflow_dispatcher() node = make_dispatch_node( @@ -298,11 +302,11 @@ def test_dispatch_node_parks_on_none_plan() -> None: state = dict(_VALID_STATE, plan=None) result = node(state) - assert result.get("status") == TaskStatus.PARKED.value - assert not disp_calls + assert result.get("status") != TaskStatus.PARKED.value + assert disp_calls[0]["inputs"]["declared_scope"] == "f.py" -def test_dispatch_node_parks_on_non_dict_plan() -> None: +def test_dispatch_node_derives_scope_from_diff_when_plan_non_dict() -> None: _, pusher = _make_fake_pusher() disp_calls, dispatcher = _make_fake_workflow_dispatcher() node = make_dispatch_node( @@ -312,6 +316,24 @@ def test_dispatch_node_parks_on_non_dict_plan() -> None: state = dict(_VALID_STATE, plan="not-a-dict") result = node(state) + assert result.get("status") != TaskStatus.PARKED.value + assert disp_calls[0]["inputs"]["declared_scope"] == "f.py" + + +def test_dispatch_node_parks_when_no_scope_and_diff_touches_no_paths() -> None: + # The genuine park case: no plan scope AND a (non-empty) diff from which no + # touched path can be parsed -> nothing honest to declare -> fail closed. + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict( + _VALID_STATE, plan={"scope": []}, candidate_diff="not a real diff, no headers\n" + ) + result = node(state) + assert result.get("status") == TaskStatus.PARKED.value assert not disp_calls diff --git a/requirements.txt b/requirements.txt index 3e0a84d..4e854e8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,3 +10,14 @@ python-dotenv==1.2.2 # WS1 agent-team HTTP API (agent_team/api.py): FastAPI app + uvicorn ASGI server. fastapi==0.136.1 uvicorn==0.46.0 +# GitHub App installation-token minting for the agent-team P3 dispatcher +# (agent_team/github_app.py): RS256 JWT (PyJWT) signed with the App private key, +# exchanged for a short-lived installation token. cryptography backs RS256. +PyJWT==2.13.0 +# >=48.0.1: earlier wheels statically link a vulnerable OpenSSL (GHSA-537c-gmf6-5ccf). +cryptography==48.0.1 +# Runtime HTTP client for the agent-team P3 App-dispatch seams +# (github_app.mint_installation_token, dispatcher.app_workflow_dispatcher, +# dispatcher.app_run_locator) and the CI fetcher/transport. Pinned first-class +# (was previously relied on only as a transitive dep of langchain-community). +requests==2.34.2