orchestrator/agent-team/agent_team/nodes/builders.py

"""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)$"
            # Parity with the CI-side denylist (case-insensitive): bare policy
            # docs, any path naming "iam", Terraform, and CDK stack files —
            # these were box-side gaps a diff could use to skip the cross-review.
            r"|(^|/)policy[^/]*\.json$"
            r"|(^|/)[^/]*iam[^/]*$"
            r"|\.tf$"
            r"|(^|/).+[-_]stack\.(ts|py)$",
            re.IGNORECASE,
        ),
        "modifies IAM/policy/Terraform/CDK IaC",
    ),
    (
        re.compile(r"^\.github/actions/.+", re.IGNORECASE),
        "modifies a composite GitHub Action (.github/actions/**)",
    ),
    (
        re.compile(r"\.(pem|key)$", re.IGNORECASE),
        "modifies key material (*.pem / *.key)",
    ),
)

# 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
# ---------------------------------------------------------------------------

# A unified diff is parsed **per file section**, each section delimited by its
# ``diff --git a/<src> b/<dest>`` header. That header carries BOTH the source and
# destination path for every change kind — modify, add, delete, mode-change,
# rename, and copy — so reading it (not only the ``+++ b/`` body line) is what
# lets the scan see deletes, mode-only changes, and ``copy to`` targets that have
# no ``+++`` line or whose ``+++`` is ``/dev/null``. The ``---``/``+++`` and
# rename/copy ``from``/``to`` lines refine the section's source/dest when present.
_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*$")
_COPY_FROM_RE = re.compile(r"^copy from (.+?)\s*$")
_COPY_TO_RE = re.compile(r"^copy 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 every path it would create/modify/move/remove.

    The diff is walked **per file section**, each delimited by its ``diff --git
    a/<src> b/<dest>`` header. Because that header carries the source and the
    destination for *every* change kind — including deletes (``+++ /dev/null``),
    mode-only changes (no ``+++`` line at all), and ``copy to`` targets — reading
    it closes the bypasses that a ``+++``-only scan misses. Within a section the
    ``--- a/`` / ``+++ b/`` lines and the rename/copy ``from``/``to`` lines refine
    the source/destination when present (a rename/copy ``to`` is authoritative for
    the destination and links it to its source). BOTH the section source and
    destination are emitted as targets, so removing or moving a file *away from* a
    denied/out-of-scope path is flagged too. A path that fails
    :func:`_canonicalize` is surfaced as an unsafe target. Pure header parsing —
    it never executes the diff.
    """
    targets: list[_DiffTarget] = []
    seen: set[tuple[str, str | None]] = set()

    # Per-section accumulators; flushed at each new ``diff --git`` and at EOF.
    src_raw: str | None = None
    dst_raw: str | None = None
    move_from: str | None = None

    def flush() -> None:
        nonlocal src_raw, dst_raw, move_from
        if src_raw is None and dst_raw is None:
            return
        rf = _canonicalize(move_from) if move_from else None
        # Source side: catches deletes and renames/copies away from a denied or
        # out-of-scope path. Destination side: catches creates/modifies/mode
        # changes/copy targets, linked to its source for a clear violation note.
        _emit_target(targets, seen, src_raw)
        _emit_target(targets, seen, dst_raw, rename_from=rf)
        src_raw = dst_raw = move_from = None

    for line in diff.splitlines():
        git = _DIFF_GIT_RE.match(line)
        if git:
            flush()
            src_raw, dst_raw = git.group(1), git.group(2)
            continue

        for pattern in (_RENAME_TO_RE, _COPY_TO_RE):
            m = pattern.match(line)
            if m:
                dst_raw = m.group(1)  # authoritative destination for the section
                break
        else:
            for pattern in (_RENAME_FROM_RE, _COPY_FROM_RE):
                m = pattern.match(line)
                if m:
                    move_from = m.group(1)
                    break
            else:
                minus = _MINUS_RE.match(line)
                if minus and minus.group(1).strip() != _DEV_NULL:
                    src_raw = minus.group(1)
                    continue
                plus = _PLUS_RE.match(line)
                # Ignore hunk body lines starting with "+++"; a real header is
                # "+++ b/path" or "+++ /dev/null". A /dev/null target means a
                # delete, so the destination stays the diff --git path.
                if plus and plus.group(1).strip() != _DEV_NULL:
                    dst_raw = plus.group(1)

    flush()
    return targets


def _emit_target(
    targets: list[_DiffTarget],
    seen: set[tuple[str, str | None]],
    raw: str | None,
    *,
    rename_from: str | None = None,
) -> None:
    """Canonicalize ``raw`` and append it as a target (unsafe paths flagged).

    ``/dev/null`` and empty values are dropped (no real path). A path that fails
    :func:`_canonicalize` (absolute / parent-escaping) is appended as an unsafe
    target so the scan rejects it rather than letting an indirection bypass the
    denylist.
    """
    if raw is None:
        return
    stripped = raw.strip()
    if not stripped or stripped == _DEV_NULL:
        return
    canon = _canonicalize(stripped)
    if canon is None:
        _append_unique(targets, seen, _DiffTarget(path=stripped), unsafe=True)
    else:
        _append_unique(
            targets,
            seen,
            _DiffTarget(path=canon, rename_from=rename_from),
            unsafe=False,
        )


# 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