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/decisions.py
Adam Moussa 48a81c0802 fix(agent-team): centralize safe decision mapping + remove free-text abandon hair-trigger
Security-review follow-up (LOGIC-01/02/05, all confirmed correctness).

- New transport-neutral `decisions.normalize_decision(raw, *, allow_abandon)` is
  the single source of truth: approve-allowlist→approve; abandon-allowlist→abandon
  ONLY when allow_abandon; everything else (prose, empty, abandon-verbs when
  disallowed) → request_changes with the full reply as notes; idempotent on an
  already-formed decision dict. slack_adapter.map_plan_decision is now a thin
  wrapper (default allow_abandon=True, no caller churn).
- LOGIC-01/02: graph._parse_decision now delegates to normalize_decision (was:
  any unrecognized verb → abandon → FAILED). The graph is now the universal safe
  backstop, so EVERY writer that bypassed the listener mapping — operator CLI
  answer_on_behalf (raw), Coordinator.submit_answer (raw), the recovery sweep —
  loops back on prose instead of silently FAILing the task. Explicit abandon
  still abandons (preserves the confirmed-button path).
- LOGIC-05: the Slack FREE-TEXT reply path maps with allow_abandon=False, so a
  bare "cancel"/"stop"/"abandon" typed in-thread → request_changes (never
  terminal abandon); abandon stays reachable only via the confirm-guarded button.

Tests: graph unrecognized→loops-back (not FAILED), operator raw-prose→request_
changes, free-text destructive verbs→request_changes vs button→abandon,
normalizer idempotency. 1484 passed.
2026-06-24 11:48:04 -04:00

107 lines
5.4 KiB
Python

"""Transport-neutral plan-decision normalizer (the ONE safe mapping).
THE LOAD-BEARING B3 SAFETY MAPPING, in a single transport-neutral home so BOTH
the graph (:func:`agent_team.graph._parse_decision`) and the transports (the
Slack listener / adapter) call ONE function rather than each re-deriving the
allowlist. Keeping it here avoids a graph→transport import: the graph is the
universal chokepoint and depends only on this leaf module, and the Slack adapter
keeps a thin :func:`~agent_team.transport.slack_adapter.map_plan_decision`
wrapper that delegates here.
A plan-gate decision can be WRITTEN by several paths — the Slack listener (which
pre-maps), but ALSO the operator CLI (``answer_on_behalf`` → ``answer_question``,
RAW) and ``Coordinator.submit_answer`` (RAW). On resume, whatever was stored
reaches the graph's decision parser. Previously the graph mapped any UNRECOGNIZED
verb to ``abandon`` → terminal FAILED, so an operator (or any non-listener
writer) answering change-request prose silently FAILED the task. Centralizing the
safe mapping here and calling it AT THE GRAPH makes every writer safe.
``allow_abandon`` is the LOGIC-05 distinction. A destructive verb
(``cancel``/``stop``/``kill``/``reject``/``abandon``) is honoured as an abandon
ONLY when the caller vouches that the input is a *confirmed* destructive intent
— i.e. the Slack Block Kit "Abandon" button, which is guarded by a confirm
dialog. A bare destructive verb typed as FREE TEXT in a thread (``allow_abandon
=False``) must NOT terminally fail the task with no confirmation; it normalizes
to ``request_changes`` carrying the full reply as notes.
"""
from __future__ import annotations
from typing import Any
__all__ = [
"ABANDON_VERBS",
"APPROVE_VERBS",
"VALID_DECISIONS",
"normalize_decision",
]
# Approve-allowlist: a reply that is exactly one of these (case/whitespace
# insensitive) is an approval; anything else is never an accidental approve.
APPROVE_VERBS: frozenset[str] = frozenset(
{"approve", "approved", "yes", "ok", "lgtm", "ship"}
)
# Abandon-allowlist (DESTRUCTIVE). A bare verb here abandons the task →
# terminal FAILED, BUT ONLY when ``allow_abandon=True`` — i.e. the input is a
# CONFIRMED destructive intent (the confirm-dialog-guarded Block Kit Abandon
# button, or the graph chokepoint honouring an explicit prior decision). The
# SAME verbs typed as FREE TEXT (``allow_abandon=False``) are NOT honoured as a
# destructive abandon (LOGIC-05): they fall through to ``request_changes`` so a
# casually-typed "cancel" never terminally fails a task with no confirmation.
ABANDON_VERBS: frozenset[str] = frozenset(
{"abandon", "reject", "cancel", "stop", "kill"}
)
# The three valid structured decisions a normalized result can carry.
VALID_DECISIONS: frozenset[str] = frozenset({"approve", "request_changes", "abandon"})
def normalize_decision(raw: Any, *, allow_abandon: bool) -> dict[str, str]:
"""Map a raw plan-gate reply to a structured ``{"decision","notes"}`` dict.
The single transport-neutral normalizer. Behavior:
* **Idempotent** — if ``raw`` is already a dict carrying a valid
``"decision"`` in :data:`VALID_DECISIONS`, return it normalized (decision
lowercased/stripped, notes coerced to ``str``) WITHOUT re-mapping. This
lets the graph chokepoint accept an already-decided value (e.g. the
listener's pre-mapped ``request_changes`` with notes) unchanged, while a
RAW string from another writer is mapped below. An ``abandon`` dict is
honoured here regardless of ``allow_abandon`` because it is an explicit,
already-formed decision, not raw free text.
* **String** — normalize (``str`` → strip → lowercase):
* in :data:`APPROVE_VERBS` → ``approve`` (notes ``""``);
* if ``allow_abandon`` AND in :data:`ABANDON_VERBS` → ``abandon``
(notes ``""``);
* **everything else** — prose, empty/whitespace, AND abandon-verbs when
``allow_abandon=False`` — → ``request_changes`` carrying the FULL
ORIGINAL reply (original casing preserved) as ``notes``. This is the
safe default: arbitrary input is a change request, never an accidental
approve or (unconfirmed) abandon.
The reply is opaque DATA throughout — never executed or interpreted beyond
this verb match.
"""
# Idempotency: an already-formed decision dict passes through normalized.
if isinstance(raw, dict):
decision = str(raw.get("decision", "") or "").strip().lower()
if decision in VALID_DECISIONS:
return {
"decision": decision,
"notes": str(raw.get("notes", "") or ""),
}
# A dict WITHOUT a recognized decision falls through to the string path
# using its stringified form (defensive; should not occur in practice).
original = "" if raw is None else str(raw)
verb = original.strip().lower()
if verb in APPROVE_VERBS:
return {"decision": "approve", "notes": ""}
if allow_abandon and verb in ABANDON_VERBS:
return {"decision": "abandon", "notes": ""}
# Everything else → request_changes, carrying the full ORIGINAL reply (not
# the lowercased form) so the notes preserve the human's exact wording. A
# bare destructive verb typed as free text lands here when allow_abandon is
# False (LOGIC-05): it requests changes, never a silent terminal abandon.
return {"decision": "request_changes", "notes": original.strip()}