This repository has been archived on 2026-08-04. You can view files and clone it, but cannot push or open issues or pull requests.
orchestrator/agent-team/agent_team/nodes/builders.py
Adam Moussa 15a416d31a Add Plane-2 leaf scaffold (pipeline graph, nodes, HITL, transports, CI)
Consolidates the 18 leaf modules from the r720-plane2-scaffold workflow onto
the foundation commit. Full suite: 535 passed, 1 skipped; ruff + format clean.

Built (pre-deployment scaffold only — nothing provisioned/enabled):
- LangGraph pipeline graph.py (INTAKE->CLARIFY->PLAN, interrupt()/resume, checkpointer-injectable)
- nodes: clarifier (98% gate), planner, review_loop (GPT-4.1), builders->candidate diff, verifier
- §3.3.1 HITL: ledger ops, resume_worker, deadline_timer, recovery sweep, responder
- transports: slack / github / claude_code adapters
- ci_gate (pure-code pass/fail), operator_cli, run-team.py entry, P1 sim harness
- ci/agent-team-apply-verify.yml (split untrusted/privileged jobs) — authored, disabled

KNOWN OPEN FINDINGS (verifier/cross-review, not yet fixed — see follow-up):
- builders denylist: 4 execution-proven bypasses (delete, mode-change, copy-to, out-of-scope delete)
- §3.3.1 CAS: BEGIN IMMEDIATE outside try/except; shared-connection txn nesting unsafe under concurrency
- operator_cli: missing re-deliver/force-resume; audit-after-mutate ordering gap
- ci yaml: GPT-4.1 cross-review PASS w/ 4 FIX items (symlink path escape, etc.)
- P1 sim harness models the ledger layer, not real LangGraph interrupt/resume; P1 exit criteria not yet truly proven

Deploy-gated (NOT done): IAM/step-ca/Roles Anywhere/confluence-bot provisioning,
/sh-security-review sign-off, live Slack/CI, rsync, live dry-runs, Adam approval.
2026-06-17 15:16:12 -04:00

548 lines
21 KiB
Python

"""Plane-2 builders node — approved plan -> candidate diff (design §3.3.2, §7.1 P3).
This is the LangGraph **builders** stage (design §3.3): it turns the approved,
review-cleared plan into a **candidate diff**. Per D2/D11 the box has no write
token; builders do NOT write to repos — they emit the diff as data for the
org-CI apply/verify workflow. This leaf owns the **box-side half** of the
§3.3.2 CI-as-verifier trust boundary:
* **Box-side trust-control-surface denylist (boundary #2).** Before a diff can
advance to the build/verify path, this node rejects any candidate diff that
touches the trust-control surface — ``.github/workflows/**``, IAM/policy/
permission IaC (CDK/SAM), branch-protection / ``CODEOWNERS`` / Dependabot
config, or any file **outside the task's declared scope**. The match is not
naive: it canonicalizes paths (resolving ``.`` / ``..`` and rejecting absolute
or parent-escaping paths), and it inspects **rename targets** so a rename into
a denied path cannot slip through. A violation parks the task for mandatory
human + GPT cross-review (it is the mandatory-cross-review surface regardless,
per CLAUDE.md) and never auto-advances.
* **Diff integrity hash (boundary #3).** The accepted diff is hashed with the
same content-hash primitive the foundation uses
(:func:`agent_team.state_store.compute_content_hash`) and the hash is recorded
on the task record (``diff_hash``). CI verifies this hash matches the ledger
before applying the patch, so a tampered/substituted diff fails closed.
The CI-side enforcement (the credential-less untrusted job, the CI hard-fail
guard, the pure-code pass/fail gate, branch protection — boundaries #1/#4/#5)
lives in the CI workflow, NOT here; this node is the box-side pre-check plus the
hash the CI gate keys against.
The actual diff synthesis (Claude spec via the §3.1 billing seam + DeepSeek
mechanical edits) is delegated to an **injectable** ``DiffBuilder`` so this leaf
stays unit-testable and dependency-free; the real SDK/orchestrator wiring is
bound by the coordinator at startup. The default builder uses
:func:`agent_team.billing.claude_invoke` so an un-wired environment fails loudly
via the foundation's unconfigured-invoker contract rather than silently
producing nothing.
This module imports the committed foundation contracts verbatim; it redefines
none of them.
"""
from __future__ import annotations
import posixpath
import re
from dataclasses import dataclass, field
from typing import Any, Mapping, Protocol
from agent_team.billing import ClaudeResult, claude_invoke
from agent_team.state_store import compute_content_hash
from agent_team.task_model import Phase, PipelineState, TaskStatus
__all__ = [
"DENYLIST_REASONS",
"BuildError",
"DiffBuilder",
"TrustBoundaryViolation",
"build_candidate_diff",
"builders_node",
"default_diff_builder",
"iter_diff_target_paths",
"scan_trust_control_surface",
]
# ---------------------------------------------------------------------------
# Trust-control-surface denylist (design §3.3.2 boundary #2)
# ---------------------------------------------------------------------------
#
# Patterns are matched against POSIX-canonicalized, repo-relative paths (see
# ``_canonicalize``). Each entry is (compiled regex, human reason) so a
# violation report names *why* a path is denied. The patterns intentionally
# over-match toward rejection: a denied diff is escalated to human + GPT
# cross-review, never silently dropped, so false positives cost a review, not a
# security hole.
_DENY_PATTERNS: tuple[tuple[re.Pattern[str], str], ...] = (
(
re.compile(r"^\.github/workflows/.+"),
"modifies a GitHub Actions workflow (.github/workflows/**)",
),
(
re.compile(r"(^|/)CODEOWNERS$"),
"modifies CODEOWNERS",
),
(
re.compile(r"(^|/)\.github/dependabot\.ya?ml$|^dependabot\.ya?ml$"),
"modifies Dependabot configuration",
),
(
re.compile(r"(^|/)\.github/settings\.ya?ml$"),
"modifies repo/branch-protection settings (.github/settings.yml)",
),
# IAM / policy / permission IaC (CDK/SAM and raw policy docs). These are the
# mandatory-cross-review surface regardless of the pipeline (CLAUDE.md).
(
re.compile(
r"(^|/)(template|samconfig)\.ya?ml$"
r"|(^|/)serverless\.ya?ml$"
),
"modifies SAM/serverless IaC (templates carry IAM policies)",
),
(
re.compile(
r"(^|/)cdk\.json$"
r"|(^|/).+\.(iam|policy)\.(json|ya?ml)$"
r"|(^|/)(iam|policies|policy)/.+\.(json|ya?ml)$"
),
"modifies IAM/policy IaC",
),
)
# A stable, importable description of every denylist reason, useful for callers
# (reports, tests) that want to enumerate the surface without re-deriving it.
DENYLIST_REASONS: tuple[str, ...] = tuple(reason for _, reason in _DENY_PATTERNS)
class BuildError(Exception):
"""Raised when the builders node cannot produce a usable candidate diff.
Signals a malformed approved plan or an empty/garbled diff from the
injected builder — i.e. the node cannot proceed, distinct from a *policy*
rejection (:class:`TrustBoundaryViolation`), which is a successful scan that
found a forbidden change.
"""
@dataclass
class TrustBoundaryViolation:
"""A single trust-control-surface denylist hit (design §3.3.2 boundary #2).
``path`` is the canonicalized repo-relative path that tripped the check;
``reason`` is the human-readable denylist rule (or an out-of-scope / unsafe
-path explanation). ``rename_from`` is set when the violation is a rename
whose *target* lands in a denied/out-of-scope location, so a rename cannot
launder a forbidden path.
"""
path: str
reason: str
rename_from: str | None = None
class DiffBuilder(Protocol):
"""Injectable diff-synthesis seam (design §3.3 builders stage).
A ``DiffBuilder`` turns the approved ``plan`` into a unified-diff string.
The real implementation wires Claude (spec, via the §3.1 billing seam) and
DeepSeek (mechanical edits, via the local orchestrator); tests pass a stub.
It MUST return a unified diff and MUST NOT perform any repo writes (D2/D11).
"""
def __call__(
self, *, plan: Mapping[str, Any], config: Mapping[str, Any] | None
) -> str:
"""Return the candidate unified diff for ``plan``."""
...
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.
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.
"""
prompt = _render_build_prompt(plan)
result: ClaudeResult = claude_invoke(prompt, config=config)
return result.text
def _render_build_prompt(plan: Mapping[str, Any]) -> str:
"""Render the approved plan into a builder instruction prompt.
Kept deliberately small and deterministic: the plan is the source of truth
and the builder's job is to emit a unified diff implementing it without
touching the trust-control surface (§3.3.2).
"""
title = str(plan.get("title", "(untitled task)"))
scope = plan.get("scope") or []
phases = plan.get("phases") or []
scope_lines = "\n".join(f" - {p}" for p in scope) or " (no scope declared)"
phase_lines = (
"\n".join(f" {i + 1}. {p}" for i, p in enumerate(phases)) or " (none)"
)
return (
"Implement the approved plan below as a single unified diff (git "
"format). Touch ONLY files within the declared scope. Do NOT modify "
"CI workflows, IAM/policy IaC, branch-protection, CODEOWNERS, or "
"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"
)
# ---------------------------------------------------------------------------
# Diff parsing + canonicalization
# ---------------------------------------------------------------------------
# Matches the "+++ b/<path>" (and "--- a/<path>") target lines of a unified
# diff, plus git "rename to"/"rename from" lines. We read targets from the
# header so the scan sees exactly the paths the patch would create/modify.
_PLUS_RE = re.compile(r"^\+\+\+ (?:b/)?(.+?)\s*$")
_MINUS_RE = re.compile(r"^--- (?:a/)?(.+?)\s*$")
_DIFF_GIT_RE = re.compile(r"^diff --git a/(.+?) b/(.+?)\s*$")
_RENAME_FROM_RE = re.compile(r"^rename from (.+?)\s*$")
_RENAME_TO_RE = re.compile(r"^rename to (.+?)\s*$")
# /dev/null appears as the source of an add or target of a delete; it is never a
# real repo path and must not be scanned/scoped as one.
_DEV_NULL = "/dev/null"
@dataclass
class _DiffTarget:
"""An internal record of one path the diff would create/modify/rename.
``path`` is the canonicalized destination; ``rename_from`` is the prior
canonical path when this target is the destination of a git rename.
"""
path: str
rename_from: str | None = None
def _canonicalize(raw: str) -> str | None:
"""Canonicalize a repo-relative diff path; return ``None`` if it is unsafe.
Strips a leading ``a/`` / ``b/`` prefix, normalizes ``.``/``..`` segments
with :func:`posixpath.normpath`, and rejects anything that escapes the repo
root (absolute paths, or a normalized path beginning with ``..``). Returning
``None`` signals an *unsafe* path that the scan treats as a violation rather
than silently letting an indirection bypass the denylist (§3.3.2 boundary
#2: "it resolves symlinks and canonicalizes paths").
"""
path = raw.strip()
if not path or path == _DEV_NULL:
return None
for prefix in ("a/", "b/"):
if path.startswith(prefix):
path = path[len(prefix) :]
break
# Normalize backslashes to forward slashes so a Windows-style separator
# cannot smuggle a segment past the POSIX normalizer.
path = path.replace("\\", "/")
if posixpath.isabs(path):
return None
normalized = posixpath.normpath(path)
if normalized == "." or normalized.startswith("../") or normalized == "..":
return None
return normalized
def iter_diff_target_paths(diff: str) -> list[_DiffTarget]:
"""Parse a unified diff into the set of target paths it would write.
Reads the ``+++ b/<path>`` header lines (the destinations a patch creates or
modifies) and git ``rename to`` lines (carrying the matching ``rename
from``), canonicalizing each. A path that fails :func:`_canonicalize` is
surfaced as an unsafe target (path preserved raw, marked via a sentinel) so
the scan rejects it. Pure header parsing — it never executes the diff.
"""
targets: list[_DiffTarget] = []
seen: set[tuple[str, str | None]] = set()
pending_rename_from: str | None = None
for line in diff.splitlines():
rename_from = _RENAME_FROM_RE.match(line)
if rename_from:
pending_rename_from = _canonicalize(rename_from.group(1))
continue
rename_to = _RENAME_TO_RE.match(line)
if rename_to:
canon = _canonicalize(rename_to.group(1))
target = _DiffTarget(
path=canon if canon is not None else rename_to.group(1).strip(),
rename_from=pending_rename_from,
)
_append_unique(targets, seen, target, unsafe=canon is None)
pending_rename_from = None
continue
plus = _PLUS_RE.match(line)
if plus:
# Ignore hunk body lines that merely start with "+++"; a real header
# is "+++ b/path" or "+++ /dev/null". _canonicalize maps /dev/null
# to None which we drop (a delete has no created target).
raw = plus.group(1)
if raw.strip() == _DEV_NULL:
continue
canon = _canonicalize(raw)
if canon is None:
# Unsafe (absolute / parent-escaping) target line.
_append_unique(
targets,
seen,
_DiffTarget(path=raw.strip()),
unsafe=True,
)
else:
_append_unique(targets, seen, _DiffTarget(path=canon), unsafe=False)
return targets
# Marks a target whose path could not be safely canonicalized. Stored on the
# _DiffTarget via a parallel set keyed by identity is overkill; instead we use a
# reserved reason string the scanner recognizes.
_UNSAFE_PATH_SENTINEL = "\x00unsafe\x00"
def _append_unique(
targets: list[_DiffTarget],
seen: set[tuple[str, str | None]],
target: _DiffTarget,
*,
unsafe: bool,
) -> None:
"""Append ``target`` if its (path, rename_from) pair is new; tag unsafe."""
if unsafe:
# Tag the rename_from slot with the sentinel so the scanner can flag it
# without changing the public _DiffTarget shape.
target = _DiffTarget(path=target.path, rename_from=_UNSAFE_PATH_SENTINEL)
key = (target.path, target.rename_from)
if key in seen:
return
seen.add(key)
targets.append(target)
def _path_denied(path: str) -> str | None:
"""Return the denylist reason if ``path`` is on the trust-control surface."""
for pattern, reason in _DENY_PATTERNS:
if pattern.search(path):
return reason
return None
def _in_scope(path: str, scope: tuple[str, ...]) -> bool:
"""Return ``True`` if ``path`` falls under one of the declared scope prefixes.
Scope entries are canonicalized directory/file prefixes. A path is in scope
if it equals a scope entry or sits beneath a scope directory (prefix match
on a ``/`` boundary). An empty scope means "nothing is in scope", so every
path is rejected as out-of-scope — the design treats an undeclared scope as
a hard stop, not a wildcard (§3.3.2: "files outside the task's declared
scope").
"""
for entry in scope:
if path == entry or path.startswith(entry + "/"):
return True
return False
def _normalize_scope(scope: Any) -> tuple[str, ...]:
"""Canonicalize the declared scope into a tuple of safe path prefixes.
Unsafe scope entries (absolute / parent-escaping) are dropped, so a
malformed scope can only *shrink* what is allowed, never widen it.
"""
if not scope:
return ()
out: list[str] = []
for entry in scope:
canon = _canonicalize(str(entry))
if canon is not None and canon not in out:
out.append(canon)
return tuple(out)
def scan_trust_control_surface(
diff: str, *, scope: Any
) -> list[TrustBoundaryViolation]:
"""Scan a candidate diff for trust-control-surface violations (§3.3.2 #2).
Returns every violation found (empty list == clean). A target violates the
boundary if it is (a) an unsafe/uncanonicalizable path, (b) on the denylist
(``.github/workflows/**``, IAM/policy IaC, CODEOWNERS, branch-protection,
Dependabot), or (c) outside the task's declared ``scope`` — including a
rename whose *destination* is denied/out-of-scope, so a rename cannot
launder a forbidden path. The scan is pure header parsing; it never executes
the patch.
"""
declared_scope = _normalize_scope(scope)
violations: list[TrustBoundaryViolation] = []
for target in iter_diff_target_paths(diff):
rename_from = target.rename_from
is_unsafe = rename_from == _UNSAFE_PATH_SENTINEL
if is_unsafe:
rename_from = None
if is_unsafe:
violations.append(
TrustBoundaryViolation(
path=target.path,
reason="unsafe path (absolute or escapes the repo root)",
rename_from=None,
)
)
continue
denied_reason = _path_denied(target.path)
if denied_reason is not None:
violations.append(
TrustBoundaryViolation(
path=target.path,
reason=denied_reason,
rename_from=rename_from,
)
)
continue
if not _in_scope(target.path, declared_scope):
violations.append(
TrustBoundaryViolation(
path=target.path,
reason="outside the task's declared scope",
rename_from=rename_from,
)
)
return violations
# ---------------------------------------------------------------------------
# Build result + the node entrypoint
# ---------------------------------------------------------------------------
@dataclass
class _BuildOutcome:
"""Internal result of :func:`build_candidate_diff` before state assembly."""
diff: str
diff_hash: str
violations: list[TrustBoundaryViolation] = field(default_factory=list)
@property
def clean(self) -> bool:
return not self.violations
def build_candidate_diff(
plan: Mapping[str, Any],
*,
builder: DiffBuilder | None = None,
config: Mapping[str, Any] | None = None,
) -> _BuildOutcome:
"""Synthesize a candidate diff, scan it, and hash it (§3.3.2 #2/#3).
Calls the injected ``builder`` (default :func:`default_diff_builder`) to turn
the approved ``plan`` into a unified diff, runs the box-side trust-control
-surface scan against the plan's declared ``scope``, and computes the diff
integrity hash via the foundation's
:func:`agent_team.state_store.compute_content_hash`. The hash is always
computed (CI keys against it) but a non-empty violation list means the diff
must NOT auto-advance — the caller parks it for human + GPT cross-review.
Raises :class:`BuildError` if the plan is not a mapping or the builder
returns an empty/whitespace-only diff (nothing to build).
"""
if not isinstance(plan, Mapping):
raise BuildError("approved plan must be a mapping")
diff_builder = builder if builder is not None else default_diff_builder
diff = diff_builder(plan=plan, config=config)
if not isinstance(diff, str) or not diff.strip():
raise BuildError("diff builder produced an empty candidate diff")
diff_hash = compute_content_hash(diff.encode("utf-8"))
violations = scan_trust_control_surface(diff, scope=plan.get("scope"))
return _BuildOutcome(diff=diff, diff_hash=diff_hash, violations=violations)
def _format_violations(violations: list[TrustBoundaryViolation]) -> str:
"""Render violations into a single human-readable park reason."""
lines = []
for v in violations:
if v.rename_from:
lines.append(f"{v.rename_from} -> {v.path}: {v.reason}")
else:
lines.append(f"{v.path}: {v.reason}")
return "; ".join(lines)
def builders_node(
state: PipelineState,
*,
builder: DiffBuilder | None = None,
config: Mapping[str, Any] | None = None,
) -> dict[str, Any]:
"""LangGraph builders node: approved plan -> candidate diff (§3.3, §3.3.2).
Reads the approved ``plan`` from ``state``, produces a candidate diff via the
injected (or default) :class:`DiffBuilder`, runs the §3.3.2 box-side
trust-control-surface scan, and writes the result back as a **partial**
:class:`agent_team.task_model.PipelineState` update (the graph state is
``total=False``):
* **Clean diff** — writes ``candidate_diff`` + ``diff_hash``, sets
``current_phase`` to ``VERIFY`` and ``status`` to ``ACTIVE`` so the org-CI
apply/verify stage runs next (the box never builds locally, D2/D11).
* **Violation(s)** — does NOT advance to verify. Records the diff + hash
(provenance, per §3.3.2: a denylist-touching diff is an ALARM), sets
``current_phase`` to ``PARKED`` and ``status`` to ``PARKED``, and writes a
``park_reason`` naming the violations so the coordinator escalates to
mandatory human review + GPT cross-review. It is never auto-built.
The node never raises for a *policy* rejection (that is an expected outcome);
it raises :class:`BuildError` only when there is no usable plan/diff at all.
The graph-state enum-valued keys are written as their ``.value`` strings to
match the :class:`PipelineState` ``TypedDict`` (str-typed), mirroring
:func:`agent_team.task_model.task_to_dict`. The return type is a plain
``dict`` (a structural superset of the partial ``PipelineState`` update) so
the park path can carry an extra ``park_reason`` annotation without
redefining the foundation ``TypedDict``.
"""
plan = state.get("plan")
if not plan:
raise BuildError("builders_node requires an approved plan in state")
outcome = build_candidate_diff(plan, builder=builder, config=config)
update: dict[str, Any] = {
"candidate_diff": outcome.diff,
"diff_hash": outcome.diff_hash,
}
if outcome.clean:
update["current_phase"] = Phase.VERIFY.value
update["status"] = TaskStatus.ACTIVE.value
else:
update["current_phase"] = Phase.PARKED.value
update["status"] = TaskStatus.PARKED.value
update["park_reason"] = (
"trust-control-surface violation (mandatory human + GPT "
f"cross-review): {_format_violations(outcome.violations)}"
)
return update