From 464003503a779c496a7722ed05d2d2f9782bb52b Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 12:52:15 -0400 Subject: [PATCH 01/19] feat(operator-queue): platform alert registry and sweep planner (#3246) C1 of #3246 (one pending row per platform-alert subject). Adds the stdlib-only leaf services/platform_alerts.py, which both migration tracks and the service graph will import: - Kind registry for every in-repo platform emitter, with its id prefix and lifetime class: 14 days by default (OPERATOR_PLATFORM_ALERT_LIFETIME_DAYS), a fixed 30-day net for edge-triggered kinds with a clear hook, or none for the four a person must act on. EXTERNAL_PREFIXES keeps role-drift- out; register() takes kinds from outside this module. - Snooze window after a person ends an alert: 7 days (OPERATOR_PLATFORM_ALERT_SNOOZE_DAYS), per the operator's T5 ruling. - Key belt and subject_for(): ":", id-shaped keys kept, anything else sha256[:16]; event kinds have no subject. - derive_legacy_subject(): reads every pre-#3246 id shape back to its subject, or to "known kind, no subject" when the id does not say which condition (git-bloat-, rows missing their context key). Unknown, gate and external prefixes are never derived. - plan_sweep(): pure planner for the upgrade sweep. Keeps the newest pending row per (agent, subject) (created_at, then id; NULL oldest; a row that already holds a subject keeps it), ends the rest, stamps survivors, gives subject-less known kinds a lifetime stamp, stamps person-ended rows inside the snooze window, and plans nothing on its own output. Nothing calls the leaf yet; the migration (C2) and the seam (C4) do. Tests: tests/unit/test_3246_platform_alerts_leaf.py (78), including a bare-interpreter load by path. Mutation-checked: survivor min instead of max, greedy sid regex, no snooze window, and reversed prefix order each go red. Co-Authored-By: Claude Opus 5.5 --- src/backend/services/platform_alerts.py | 521 +++++++++++++++++++ tests/registry.json | 15 +- tests/unit/test_3246_platform_alerts_leaf.py | 451 ++++++++++++++++ 3 files changed, 985 insertions(+), 2 deletions(-) create mode 100644 src/backend/services/platform_alerts.py create mode 100644 tests/unit/test_3246_platform_alerts_leaf.py diff --git a/src/backend/services/platform_alerts.py b/src/backend/services/platform_alerts.py new file mode 100644 index 0000000000..9cb923076f --- /dev/null +++ b/src/backend/services/platform_alerts.py @@ -0,0 +1,521 @@ +"""Platform alerts: one pending row per subject (#3246). + +A platform-raised alert describes a CONDITION ("this subscription is near its +limit", "this agent's circuit is dormant"). The operator queue used to store +each reading of it as a separate MESSAGE, so a new reading filed a new pending +row and nothing ended the old ones. This module is the one place that knows +what a platform alert is about: its SUBJECT, `":"`, which the +database can see and which carries at most one pending row. + +This file is the **leaf**: the kind registry, the key belt, the derivation of a +subject from a pre-#3246 row's `request_id` (+ context), and the pure upgrade +sweep planner. It is **stdlib-only at module level** — both migration tracks +(`db/migrations.py`, loaded by file path in `test_schema_parity.py`, and the +Alembic revision) import it, and so does `operator_queue_service`, which +`ask_service` imports. A module-level import of the service graph here would +close that ring (#3246 plan, E1/E2). Anything that needs the database imports +it inside the function. + +Lifetimes (T4): a kind declares one of three classes — + * ``LIFETIME_DEFAULT`` — 14 days from the last reading, env + ``OPERATOR_PLATFORM_ALERT_LIFETIME_DAYS``; + * ``LIFETIME_NET`` — a fixed 30 days, for edge-triggered kinds that have a + clear hook, where expiry is only a net against a clear lost to a crash; + * ``None`` — a person must act; the row never expires on its own. + +Snooze (T5, operator ruling 2026-10-05): after a person ends an alert, a new +reading of the same subject files nothing for 7 days (env +``OPERATOR_PLATFORM_ALERT_SNOOZE_DAYS``) unless it raises the priority or +changes one of the kind's ``material_keys``. The window is read here so the +sweep and the seam agree on it. +""" +import hashlib +import json +import logging +import os +import re +from dataclasses import dataclass, field +from datetime import datetime, timedelta, timezone +from typing import Any, Callable, Dict, Iterable, Mapping, Optional, Tuple + +logger = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Lifetime and snooze +# --------------------------------------------------------------------------- + +LIFETIME_DEFAULT = "default" +LIFETIME_NET = "net" +_LIFETIME_CLASSES = (LIFETIME_DEFAULT, LIFETIME_NET, None) + +DEFAULT_LIFETIME_DAYS = 14 +LIFETIME_ENV = "OPERATOR_PLATFORM_ALERT_LIFETIME_DAYS" +NET_LIFETIME_DAYS = 30 + +DEFAULT_SNOOZE_DAYS = 7 +SNOOZE_ENV = "OPERATOR_PLATFORM_ALERT_SNOOZE_DAYS" + + +def _positive_int_env(name: str, default: int) -> int: + raw = os.getenv(name) + if raw is None: + return default + try: + value = int(raw.strip()) + except (TypeError, ValueError): + value = 0 + if value < 1: + logger.warning( + "[platform_alerts] %s=%r is not a positive whole number of days; " + "using the default %d", name, raw, default, + ) + return default + return value + + +def default_lifetime_days() -> int: + """The default lifetime in days (env knob; bad value → default + WARNING).""" + return _positive_int_env(LIFETIME_ENV, DEFAULT_LIFETIME_DAYS) + + +def snooze_window() -> timedelta: + """How long a person's ending of an alert silences new readings of it.""" + return timedelta(days=_positive_int_env(SNOOZE_ENV, DEFAULT_SNOOZE_DAYS)) + + +# --------------------------------------------------------------------------- +# Registry +# --------------------------------------------------------------------------- + +CLEARS_HOOK = "hook" # the emitter ends the row when the condition clears +CLEARS_LIFETIME_ONLY = "lifetime_only" # nothing observes the clear; the lifetime ends it +CLEARS_PERSON = "person" # a person must act on it +_CLEARS = (CLEARS_HOOK, CLEARS_LIFETIME_ONLY, CLEARS_PERSON) + + +@dataclass(frozen=True) +class Kind: + """One kind of platform alert. + + ``prefix`` is the request-id prefix its rows carry; it must fall under a + platform-reserved prefix (`operator_queue_service._RESERVED_ID_PREFIXES`) + so an agent cannot mint a row into it. Kinds may share a prefix (two + conditions under one family) — they then share a lifetime class. ``event`` + kinds have no subject: every observation is its own row. + """ + + name: str + prefix: str + lifetime: Optional[str] + clears: str + budgeted_type: Optional[str] = None + material_keys: Tuple[str, ...] = () + event: bool = False + + def __post_init__(self): + if not re.fullmatch(r"[a-z][a-z0-9_]*", self.name or ""): + raise ValueError(f"kind name must be lower_snake: {self.name!r}") + if not self.prefix or not self.prefix.endswith(("-", "_")): + raise ValueError(f"kind {self.name}: prefix must end in '-' or '_'") + if self.lifetime not in _LIFETIME_CLASSES: + raise ValueError(f"kind {self.name}: unknown lifetime class {self.lifetime!r}") + if self.clears not in _CLEARS: + raise ValueError(f"kind {self.name}: unknown clears {self.clears!r}") + + +# Prefixes whose emitter lives outside this repository (the enterprise +# submodule). The sweep never touches them; the emitter registers its kind +# through `register(...)` from its own package. +EXTERNAL_PREFIXES = frozenset({"role-drift-"}) + +_D, _N = LIFETIME_DEFAULT, LIFETIME_NET + +_BUILTIN_KINDS = ( + # -- the four families #3246 names ------------------------------------ + Kind("subscription_headroom", "sub-headroom-", _D, CLEARS_HOOK, material_keys=("tier",)), + Kind("skills_legacy_adoption", "skills-legacy-adoption-", _D, CLEARS_HOOK), + Kind("base_image_stale", "base-image-stale-", _N, CLEARS_HOOK), + Kind("system_agent_start_failed", "base-image-stale-start-", _N, CLEARS_HOOK), + Kind("circuit_dormant", "cb-dormant-", _N, CLEARS_HOOK), + # -- every other in-repo platform emitter ----------------------------- + Kind("skills_reconcile_refused", "skills-reconcile-", _N, CLEARS_HOOK), + Kind("skills_fleet_reinject", "skills-fleet-reinject-", _D, CLEARS_HOOK), + Kind("sync_failing", "sync-failing-", _N, CLEARS_HOOK), + Kind("sync_diverged", "sync-diverged-", _N, CLEARS_HOOK), + Kind("git_bloat_size", "git-bloat-", _N, CLEARS_HOOK), + Kind("git_bloat_maintenance", "git-bloat-", _N, CLEARS_HOOK), + Kind("db_backup_failure", "db-backup-", _D, CLEARS_HOOK), + Kind("db_backup_stale", "db-backup-", _D, CLEARS_HOOK), + Kind("log_archive_unwritable", "log-archive-", _D, CLEARS_HOOK), + Kind("retention_refused", "retention-guard-", _D, CLEARS_HOOK, material_keys=("reason",)), + Kind("system_seed", "system-seed-", _N, CLEARS_HOOK), + Kind("git_token_scrub_refused", "ent615-git-token-scrub-", _D, CLEARS_HOOK), + Kind("git_token_scrub_unreadable", "ent615-git-token-scrub-unreadable-", _D, CLEARS_HOOK), + Kind("queue_flood", "queue-flood-", _D, CLEARS_HOOK, budgeted_type="queue_flood"), + Kind("alert_budget", "alert-budget-", _D, CLEARS_HOOK), + Kind("skill_not_found", "skill-not-found-", _D, CLEARS_HOOK, budgeted_type="skill_not_found"), + Kind("effect_unguarded", "effect-unguarded-", _D, CLEARS_LIFETIME_ONLY, + budgeted_type="effect_unguarded"), + Kind("validation_failed", "val_", _D, CLEARS_LIFETIME_ONLY), + # -- a person must act: no lifetime ---------------------------------- + Kind("poison", "poison-", None, CLEARS_PERSON), + Kind("portal_inbox_collision", "portal-inbox-collision-", None, CLEARS_PERSON), + Kind("gitignore_untracked", "gitignore-untracked-", None, CLEARS_PERSON, + budgeted_type="gitignore_untracked", event=True), + Kind("workspace_problem_report", "workspace-problem-", None, CLEARS_PERSON, + budgeted_type="workspace_problem_report", event=True), +) + +KINDS: Dict[str, Kind] = {k.name: k for k in _BUILTIN_KINDS} + + +def register(kind: Kind) -> None: + """Register a kind defined outside this module (the enterprise emitters). + + A duplicate name is refused: two emitters silently sharing a kind would + share subjects, and one's clear would end the other's alert. + """ + if kind.name in KINDS: + raise ValueError(f"platform alert kind {kind.name!r} is already registered") + KINDS[kind.name] = kind + + +def _kind(name: str) -> Kind: + try: + return KINDS[name] + except KeyError: + raise KeyError(f"unregistered platform alert kind {name!r}") from None + + +def lifetime_for(kind_name: str) -> Optional[timedelta]: + """The kind's lifetime, measured from its last reading; None = never expires.""" + lifetime = _kind(kind_name).lifetime + if lifetime is None: + return None + if lifetime == LIFETIME_NET: + return timedelta(days=NET_LIFETIME_DAYS) + return timedelta(days=default_lifetime_days()) + + +# --------------------------------------------------------------------------- +# Key belt / subject +# --------------------------------------------------------------------------- + +_KEY_RE = re.compile(r"^[A-Za-z0-9._:-]{1,128}$") + + +def _belt_key(key: Any) -> str: + if isinstance(key, bool) or not isinstance(key, (str, int)): + raise ValueError(f"platform alert key must be a string, got {type(key).__name__}") + text = str(key).strip() + if not text: + raise ValueError("platform alert key is empty") + if _KEY_RE.match(text): + return text + return hashlib.sha256(text.encode("utf-8")).hexdigest()[:16] + + +def subject_for(kind_name: str, key: Any) -> Optional[str]: + """``":"`` with the key belted: id-shaped and ≤128 chars stays, + anything else becomes ``sha256(key)[:16]``. An event kind has no subject + (None); a condition kind without a usable key raises ValueError.""" + kind = _kind(kind_name) + if kind.event: + return None + return f"{kind.name}:{_belt_key(key)}" + + +# --------------------------------------------------------------------------- +# Legacy subject derivation (the upgrade sweep's one source of id parsing) +# --------------------------------------------------------------------------- + +@dataclass(frozen=True) +class LegacyMatch: + """A pre-#3246 row read back: its kind, and its subject when the id (+ one + named context key) identifies the condition. ``subject is None`` means + "this kind, but the row does not say which condition" — never guessed.""" + + kind: str + subject: Optional[str] + + +_TS = r"\d{4}-\d{2}-\d{2}T[0-9:.]+(?:Z|[+-]\d{2}:?\d{2})?" +_DAY = r"\d{4}-\d{2}-\d{2}" + + +def _rx(pattern: str) -> Callable[[str], Optional[str]]: + compiled = re.compile(pattern) + + def parse(rest: str, _ctx: Mapping) -> Optional[str]: + m = compiled.fullmatch(rest) + return m.group(1) if m else None + + return parse + + +def _ctx_str(ctx: Mapping, name: str) -> Optional[str]: + value = ctx.get(name) + return value if isinstance(value, str) and value.strip() else None + + +def _headroom(rest: str, _ctx: Mapping) -> Optional[str]: + # sub-headroom-{safe_sid}-{episode}-{tier} | sub-headroom-fleet-{episode}; + # episode is a day or `unknown-`. Non-greedy sid so `unknown-` is + # read as the episode, not swallowed into a sid. + m = re.fullmatch(rf"(.+?)-(?:unknown-)?{_DAY}-(?:warn|crit)", rest) + if m: + return m.group(1) + if re.fullmatch(rf"fleet-(?:unknown-)?{_DAY}", rest): + return "fleet" + return None + + +def _legacy_adoption(rest: str, ctx: Mapping) -> Optional[str]: + # Steady state carries sha256(raw url)[:12]; the failure branch is + # timestamped and carries only the SCRUBBED url in context. For a URL with + # no embedded credentials the two hashes agree; for one with credentials + # the failure rows land on a second subject — two survivors, never a wrong + # merge. + m = re.fullmatch(r"refused-([0-9a-f]{12})", rest) + if m: + return m.group(1) + url = _ctx_str(ctx, "url") + if url and re.fullmatch(_TS, rest): + return hashlib.sha256(url.strip().encode("utf-8")).hexdigest()[:12] + return None + + +def _effect_unguarded(_rest: str, ctx: Mapping) -> Optional[str]: + effect, reason = _ctx_str(ctx, "effect_type"), _ctx_str(ctx, "reason") + return f"{effect}:{reason}" if effect and reason else None + + +def _alert_budget(rest: str, ctx: Mapping) -> Optional[str]: + return _ctx_str(ctx, "alert_type") or _rx(r".+-([a-z0-9_]+)-b\d+")(rest, ctx) + + +def _context_key(name: str) -> Callable[[str, Mapping], Optional[str]]: + return lambda _rest, ctx: _ctx_str(ctx, name) + + +def _constant(value: str) -> Callable[[str, Mapping], Optional[str]]: + return lambda _rest, _ctx: value + + +def _whole(rest: str, _ctx: Mapping) -> Optional[str]: + return rest or None + + +def _no_subject(_rest: str, _ctx: Mapping) -> Optional[str]: + return None + + +def _db_backup(rest: str, ctx: Mapping) -> Tuple[str, Optional[str]]: + alert_type = ctx.get("alert_type") + if alert_type in ("db_backup_failure", "db_backup_stale"): + return alert_type, _ctx_str(ctx, "backup_dir") + return "db_backup_failure", None + + +# (id prefix, kind, key parser) — matched longest prefix first. A parser +# returns the natural key or None. `db-backup-` picks its kind from context. +_LEGACY_PARSERS: Tuple[Tuple[str, str, Callable], ...] = ( + ("sub-headroom-", "subscription_headroom", _headroom), + ("skills-legacy-adoption-", "skills_legacy_adoption", _legacy_adoption), + ("base-image-stale-start-", "system_agent_start_failed", _rx(r"(.+)-\d+")), + ("base-image-stale-", "base_image_stale", _rx(rf"(.+)-{_TS}")), + ("cb-dormant-", "circuit_dormant", _rx(rf"(.+)-{_TS}")), + ("skills-reconcile-", "skills_reconcile_refused", _rx(r"(.+)-\d+")), + ("skills-fleet-reinject-", "skills_fleet_reinject", _constant("fleet")), + ("sync-failing-", "sync_failing", _rx(rf"(.+)-{_TS}")), + ("sync-diverged-", "sync_diverged", _rx(rf"(.+)-{_TS}")), + # Two conditions (size, maintenance) share this prefix and are told apart + # only in prose: kind known for the lifetime, subject never guessed. + ("git-bloat-", "git_bloat_size", _no_subject), + ("db-backup-", "db_backup_failure", _db_backup), + ("log-archive-", "log_archive_unwritable", _context_key("path")), + ("retention-guard-", "retention_refused", _rx(r"(.+)-\d+")), + ("system-seed-", "system_seed", _whole), + ("ent615-git-token-scrub-unreadable-", "git_token_scrub_unreadable", _rx(rf"(.+)-{_DAY}")), + ("ent615-git-token-scrub-", "git_token_scrub_refused", _rx(rf"(.+)-{_DAY}")), + ("queue-flood-", "queue_flood", _context_key("reason")), + ("alert-budget-", "alert_budget", _alert_budget), + ("skill-not-found-", "skill_not_found", _context_key("command")), + ("effect-unguarded-", "effect_unguarded", _effect_unguarded), + ("val_", "validation_failed", _rx(r"(.+)_\d{14}")), + ("poison-", "poison", _whole), + ("portal-inbox-collision-", "portal_inbox_collision", _whole), + ("gitignore-untracked-", "gitignore_untracked", _no_subject), + ("workspace-problem-", "workspace_problem_report", _no_subject), +) +_LEGACY_PARSERS = tuple(sorted(_LEGACY_PARSERS, key=lambda p: -len(p[0]))) + + +def _as_context(context: Any) -> Dict: + if isinstance(context, dict): + return context + if isinstance(context, (str, bytes)) and context: + try: + parsed = json.loads(context) + except (TypeError, ValueError): + return {} + return parsed if isinstance(parsed, dict) else {} + return {} + + +def derive_legacy_subject(request_id: Any, context: Any) -> Optional[LegacyMatch]: + """Read a pre-#3246 platform row back into (kind, subject). + + None when the prefix is not an in-repo platform kind (an agent's own ask, + a gate row, an enterprise prefix) — the sweep must leave those alone. + """ + if not isinstance(request_id, str) or not request_id: + return None + ctx = _as_context(context) + for prefix, kind_name, parse in _LEGACY_PARSERS: + if not request_id.startswith(prefix) or kind_name not in KINDS: + continue + rest = request_id[len(prefix):] + result = parse(rest, ctx) + if isinstance(result, tuple): + kind_name, key = result + else: + key = result + subject = None + if key is not None: + try: + subject = subject_for(kind_name, key) + except ValueError: + subject = None + return LegacyMatch(kind_name, subject) + return None + + +# --------------------------------------------------------------------------- +# Upgrade sweep planner (pure) +# --------------------------------------------------------------------------- + +@dataclass(frozen=True) +class Stamp: + """One row write the sweep plans. A field left None is not written.""" + + id: Any + subject: Optional[str] = None + last_seen_at: Optional[str] = None + expires_at: Optional[str] = None + context: Optional[Dict] = None + + +@dataclass(frozen=True) +class SweepPlan: + # survivors: subject + last_seen_at + context.seen_count, expires_at when it was NULL + survivor_stamps: Tuple[Stamp, ...] = () + # pending duplicates to end as one batch: cancelled / platform / superseded + ended_ids: Tuple[Any, ...] = () + # known kind, no derivable subject: expires_at only (so it leaves on its own) + lifetime_stamps: Tuple[Stamp, ...] = () + # person-ended rows inside the snooze window: subject only (read by the seam) + snooze_stamps: Tuple[Stamp, ...] = field(default=()) + + def is_empty(self) -> bool: + return not (self.survivor_stamps or self.ended_ids + or self.lifetime_stamps or self.snooze_stamps) + + +def _iso_z(dt: datetime) -> str: + # Same shape as utils.helpers.utc_now_iso (Invariant #16). + return dt.astimezone(timezone.utc).strftime("%Y-%m-%dT%H:%M:%S.%fZ") + + +def _parse_iso(value: Any) -> Optional[datetime]: + if not isinstance(value, str) or not value: + return None + try: + dt = datetime.fromisoformat(value.replace("Z", "+00:00")) + except ValueError: + return None + return dt if dt.tzinfo else dt.replace(tzinfo=timezone.utc) + + +def _age_key(row: Mapping) -> Tuple: + # Newest wins: created_at, then id (the #3130 rule); NULL created_at is oldest. + created = row.get("created_at") + return (created is not None, created or "", str(row.get("id"))) + + +def plan_sweep(rows: Iterable[Mapping], *, now: datetime) -> SweepPlan: + """Plan the one-shot upgrade sweep over fetched queue rows. + + ``rows``: mappings with ``id, agent_name, request_id, status, created_at, + expires_at, context, subject, disposed_by, disposed_at``. Rules: + + * Only rows whose request id derives to an in-repo kind are considered; + unknown and external prefixes are never touched. + * Pending rows are grouped by ``(agent_name, subject)``. A row that already + carries a subject holds the slot (it is the seam's live row); otherwise + the newest by ``created_at`` then ``id`` survives. Every other pending + row of the group is ended. + * Survivors are stamped with ``subject``, ``last_seen_at = created_at`` + (``now`` when NULL), ``context.seen_count = 1`` and — only when + ``expires_at`` is NULL and the kind has a lifetime — ``now + lifetime``. + * Pending rows of a known kind with no derivable subject get the lifetime + stamp only. + * Person-ended rows whose ending falls inside the snooze window get + ``subject`` only, so the seam's snooze can see them. + * Planning over the plan's own result yields an empty plan. + """ + if now.tzinfo is None: + raise ValueError("plan_sweep needs an aware `now`") + snooze_since = now - snooze_window() + + groups: Dict[Tuple[Any, str], list] = {} + lifetime_stamps, snooze_stamps = [], [] + for row in rows: + match = derive_legacy_subject(row.get("request_id"), row.get("context")) + if match is None: + continue + stored_subject = row.get("subject") + status = row.get("status") + if status == "pending": + subject = stored_subject or match.subject + if subject is not None: + groups.setdefault((row.get("agent_name"), subject), []).append(row) + continue + lifetime = lifetime_for(match.kind) + if lifetime is not None and row.get("expires_at") is None: + lifetime_stamps.append(Stamp(id=row.get("id"), expires_at=_iso_z(now + lifetime))) + continue + if (stored_subject is None and match.subject is not None + and row.get("disposed_by") == "person"): + disposed = _parse_iso(row.get("disposed_at")) + if disposed is not None and disposed >= snooze_since: + snooze_stamps.append(Stamp(id=row.get("id"), subject=match.subject)) + + survivor_stamps, ended_ids = [], [] + for (_agent, subject), members in groups.items(): + holders = [r for r in members if r.get("subject")] + survivor = max(holders or members, key=_age_key) + ended_ids.extend(r.get("id") for r in members if r is not survivor) + if survivor.get("subject"): + continue + match = derive_legacy_subject(survivor.get("request_id"), survivor.get("context")) + lifetime = lifetime_for(match.kind) + expires_at = None + if lifetime is not None and survivor.get("expires_at") is None: + expires_at = _iso_z(now + lifetime) + context = dict(_as_context(survivor.get("context"))) + context["seen_count"] = 1 + survivor_stamps.append(Stamp( + id=survivor.get("id"), + subject=subject, + last_seen_at=survivor.get("created_at") or _iso_z(now), + expires_at=expires_at, + context=context, + )) + + return SweepPlan( + survivor_stamps=tuple(survivor_stamps), + ended_ids=tuple(ended_ids), + lifetime_stamps=tuple(lifetime_stamps), + snooze_stamps=tuple(snooze_stamps), + ) diff --git a/tests/registry.json b/tests/registry.json index e8cbb700cb..adcdf1b262 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -1,5 +1,16 @@ { "test_files": [ + { + "file": "unit/test_3246_platform_alerts_leaf.py", + "feature": "#3246", + "added": "2026-10-05", + "categories": [ + "backend", + "operator-queue", + "unit" + ], + "description": "#3246 C1: the platform-alert leaf (services/platform_alerts.py) — loads by path in a bare interpreter (stdlib-only, so both migration tracks can import it); kind registry with lifetime classes (14 d default + env knob, 30 d net, none for the four a person must act on) and the 7-day snooze knob; key belt (id-shaped kept, else sha256[:16]); every legacy id shape mapped to its subject or to 'known kind, no subject', unknown/external prefixes never derived; plan_sweep keeps the newest pending row per (agent, subject), ends the rest, stamps survivors, gives subject-less known kinds a lifetime stamp, stamps person-ended rows inside the snooze window, and is idempotent on its own output. Mutation-checked (survivor rule, sid greediness, snooze window, prefix order). No DB, no Docker." + }, { "file": "unit/test_3237_system_agent_after_setup.py", "feature": "#3237", @@ -4367,7 +4378,7 @@ "payments", "security" ], - "description": "The payer\u2192task binding at the layer that implements it (ent#679 I6): `db/nevermined.py::payer_owns_execution` run against a throwaway SQLite carrying the real `nevermined_payment_log`, because every A2A gate test stubs `db.nevermined_payer_owns_execution` and the one new query on the money path \u2014 the one carrying T5's security property \u2014 never executed in CI. Pins that the payer matches their own settled (and settle_failed) task across the facilitator's unstable checksum casing, that another payer / another agent / an unknown execution are all False, that a SQL-shaped wallet is a bound parameter rather than SQL, and that a missing argument fails closed." + "description": "The payer→task binding at the layer that implements it (ent#679 I6): `db/nevermined.py::payer_owns_execution` run against a throwaway SQLite carrying the real `nevermined_payment_log`, because every A2A gate test stubs `db.nevermined_payer_owns_execution` and the one new query on the money path — the one carrying T5's security property — never executed in CI. Pins that the payer matches their own settled (and settle_failed) task across the facilitator's unstable checksum casing, that another payer / another agent / an unknown execution are all False, that a SQL-shaped wallet is a bound parameter rather than SQL, and that a missing argument fails closed." }, { "file": "unit/test_ent679_paid_turn_service.py", @@ -4424,7 +4435,7 @@ "reliability", "mcp" ], - "description": "The delegation contract (ent#568): what a dispatch receipt means and what to do instead of re-sending, taught as ONE text in the platform prompt (\u00a7Agent Collaboration, spliced from DELEGATION_CONTRACT) and, byte-identical, in src/mcp-server/src/delegation_contract.ts, which the chat_with_agent and chat_with_ descriptions carry. Pins: the contract renders once inside \u00a7Agent Collaboration for every runtime with no unspliced __MARKER__ left; it is runtime-neutral (no mcp__trinity__, no heading); the TS array parses strictly as JSON (exactly one anchored declaration) and equals the Python constant, with guard-the-guard cases proving a comment inside the array raises and a one-character edit is seen; the rule sentence fan_out/send_message repeat is the contract's own; every backticked token is classified and proven real -- each tool exists in a non-test MCP module with each named argument in its zod schema, each status is emitted by a pinned DISPATCH-route file (so ask_operator's `replayed` cannot vouch for it), the events are TASK_COMPLETED_EVENT/TASK_FAILED_EVENT; the Codex orientation lists the tools; the contract stays under its 1,650-char budget and \u00a7Agent Collaboration stays shorter than \u00a7Operator Communication (AC4). The MCP half -- published tools/list descriptions under Claude Code's 2,048-char cap, receipt messages -- is src/mcp-server/src/delegation-contract.test.ts. Mutation-proven (splice dropped / Codex name dropped / TS copy drifted each go red). No Docker, no live backend." + "description": "The delegation contract (ent#568): what a dispatch receipt means and what to do instead of re-sending, taught as ONE text in the platform prompt (§Agent Collaboration, spliced from DELEGATION_CONTRACT) and, byte-identical, in src/mcp-server/src/delegation_contract.ts, which the chat_with_agent and chat_with_ descriptions carry. Pins: the contract renders once inside §Agent Collaboration for every runtime with no unspliced __MARKER__ left; it is runtime-neutral (no mcp__trinity__, no heading); the TS array parses strictly as JSON (exactly one anchored declaration) and equals the Python constant, with guard-the-guard cases proving a comment inside the array raises and a one-character edit is seen; the rule sentence fan_out/send_message repeat is the contract's own; every backticked token is classified and proven real -- each tool exists in a non-test MCP module with each named argument in its zod schema, each status is emitted by a pinned DISPATCH-route file (so ask_operator's `replayed` cannot vouch for it), the events are TASK_COMPLETED_EVENT/TASK_FAILED_EVENT; the Codex orientation lists the tools; the contract stays under its 1,650-char budget and §Agent Collaboration stays shorter than §Operator Communication (AC4). The MCP half -- published tools/list descriptions under Claude Code's 2,048-char cap, receipt messages -- is src/mcp-server/src/delegation-contract.test.ts. Mutation-proven (splice dropped / Codex name dropped / TS copy drifted each go red). No Docker, no live backend." } ] } diff --git a/tests/unit/test_3246_platform_alerts_leaf.py b/tests/unit/test_3246_platform_alerts_leaf.py new file mode 100644 index 0000000000..380490ed2e --- /dev/null +++ b/tests/unit/test_3246_platform_alerts_leaf.py @@ -0,0 +1,451 @@ +"""#3246 C1 — the platform-alert leaf: registry, key belt, legacy subjects, sweep planner. + +A platform alert describes a CONDITION ("this subscription is near its limit"), +but the operator queue stored each reading as a separate MESSAGE, so the pending +set filled with repeats of one condition. The fix keys every platform alert on a +SUBJECT (`kind:key`) the database can see. This file pins the pure half of that: + + * `services/platform_alerts.py` is a stdlib-only leaf — both migration tracks + and the service graph import it, so it must not import them back; + * the registry declares, per kind, its id prefix and lifetime class + (14 d default + env knob, a 30 d net, or none — a person must act); + * the key belt turns any key into an id-shaped subject, deterministically; + * every legacy id shape the in-repo emitters wrote maps to the subject the + seam will use for the same condition — or to "known kind, no subject" when + the id does not identify the condition, never to a guess; + * `plan_sweep` keeps the newest pending row per (agent, subject), ends the + rest, stamps survivors, leaves unknown prefixes alone, and plans nothing on + its own output (idempotent). +""" +import json +import os +import subprocess +import sys +import textwrap +from datetime import datetime, timedelta, timezone + +import pytest + +_BACKEND = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "src", "backend")) +if _BACKEND not in sys.path: + sys.path.insert(0, _BACKEND) + +pytestmark = pytest.mark.unit + +import services.platform_alerts as pa # noqa: E402 + +NOW = datetime(2026, 10, 5, 12, 0, 0, tzinfo=timezone.utc) + + +def _iso(dt): + return dt.strftime("%Y-%m-%dT%H:%M:%S.%fZ") + + +def _row(id, request_id, *, agent="_sub-headroom", created_at=None, status="pending", + context=None, subject=None, expires_at=None, disposed_by=None, + disposed_at=None, priority="high"): + return { + "id": id, + "agent_name": agent, + "request_id": request_id, + "status": status, + "priority": priority, + "created_at": created_at, + "expires_at": expires_at, + "context": context, + "subject": subject, + "disposed_by": disposed_by, + "disposed_at": disposed_at, + } + + +# --------------------------------------------------------------------------- +# The leaf is stdlib-only +# --------------------------------------------------------------------------- + + +class TestImportIsolation: + def test_loads_by_path_in_a_bare_interpreter(self): + """Loaded by file path with NOTHING of the backend on sys.path (the way + `test_schema_parity.py` loads `db/migrations.py`): any non-stdlib import + at module level would raise here.""" + path = os.path.join(_BACKEND, "services", "platform_alerts.py") + code = textwrap.dedent(f""" + import importlib.util, sys + spec = importlib.util.spec_from_file_location("pa_leaf", {path!r}) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + leaked = [m for m in ("database", "db", "services", "services.ask_service", + "services.operator_queue_service", "utils", + "sqlalchemy", "config") if m in sys.modules] + assert not leaked, leaked + assert mod.KINDS, "registry is empty" + print("ok") + """) + out = subprocess.run( + [sys.executable, "-I", "-c", code], capture_output=True, text=True, + cwd="/", timeout=60, + ) + assert out.returncode == 0, out.stderr + assert out.stdout.strip() == "ok" + + +# --------------------------------------------------------------------------- +# Registry +# --------------------------------------------------------------------------- + + +class TestRegistry: + def test_issue_named_kinds_are_registered(self): + for name in ("subscription_headroom", "skills_legacy_adoption", + "base_image_stale", "system_agent_start_failed", "circuit_dormant"): + assert name in pa.KINDS + + def test_lifetime_classes(self, monkeypatch): + monkeypatch.delenv(pa.LIFETIME_ENV, raising=False) + assert pa.lifetime_for("subscription_headroom") == timedelta(days=14) + assert pa.lifetime_for("circuit_dormant") == timedelta(days=pa.NET_LIFETIME_DAYS) == timedelta(days=30) + # The four a person must act on never expire on their own. + for name in ("poison", "gitignore_untracked", "workspace_problem_report", + "portal_inbox_collision"): + assert pa.lifetime_for(name) is None, name + + def test_lifetime_env_knob(self, monkeypatch): + monkeypatch.setenv(pa.LIFETIME_ENV, "3") + assert pa.lifetime_for("subscription_headroom") == timedelta(days=3) + # the 30-day net is fixed, not the knob + assert pa.lifetime_for("circuit_dormant") == timedelta(days=30) + + @pytest.mark.parametrize("bad", ["0", "-2", "abc", "", "1.5"]) + def test_bad_lifetime_env_falls_back_with_warning(self, monkeypatch, caplog, bad): + monkeypatch.setenv(pa.LIFETIME_ENV, bad) + with caplog.at_level("WARNING"): + assert pa.default_lifetime_days() == pa.DEFAULT_LIFETIME_DAYS == 14 + if bad: + assert pa.LIFETIME_ENV in caplog.text + + def test_snooze_window_is_seven_days_with_env_knob(self, monkeypatch, caplog): + monkeypatch.delenv(pa.SNOOZE_ENV, raising=False) + assert pa.snooze_window() == timedelta(days=7) + monkeypatch.setenv(pa.SNOOZE_ENV, "2") + assert pa.snooze_window() == timedelta(days=2) + monkeypatch.setenv(pa.SNOOZE_ENV, "nope") + with caplog.at_level("WARNING"): + assert pa.snooze_window() == timedelta(days=7) + assert pa.SNOOZE_ENV in caplog.text + + def test_kinds_sharing_a_prefix_share_a_lifetime_class(self): + by_prefix = {} + for kind in pa.KINDS.values(): + by_prefix.setdefault(kind.prefix, set()).add(kind.lifetime) + assert all(len(v) == 1 for v in by_prefix.values()), by_prefix + + def test_register_refuses_a_duplicate_and_accepts_a_new_kind(self, monkeypatch): + monkeypatch.setattr(pa, "KINDS", dict(pa.KINDS)) + with pytest.raises(ValueError): + pa.register(pa.Kind(name="circuit_dormant", prefix="cb-dormant-", + lifetime=pa.LIFETIME_NET, clears=pa.CLEARS_HOOK)) + pa.register(pa.Kind(name="role_drift", prefix="role-drift-", + lifetime=pa.LIFETIME_DEFAULT, clears=pa.CLEARS_HOOK)) + assert pa.subject_for("role_drift", "agent-a") == "role_drift:agent-a" + + def test_gate_and_external_prefixes_are_not_kinds(self): + prefixes = {k.prefix for k in pa.KINDS.values()} + assert "gate-" not in prefixes + assert pa.EXTERNAL_PREFIXES == frozenset({"role-drift-"}) + assert not prefixes & pa.EXTERNAL_PREFIXES + + +# --------------------------------------------------------------------------- +# Key belt / subject +# --------------------------------------------------------------------------- + + +class TestSubject: + def test_id_shaped_key_is_kept(self): + assert pa.subject_for("circuit_dormant", "agent-a") == "circuit_dormant:agent-a" + assert pa.subject_for("poison", 42) == "poison:42" + + def test_odd_or_long_key_is_hashed_deterministically(self): + odd = pa.subject_for("skill_not_found", "/do thing now") + assert odd == pa.subject_for("skill_not_found", "/do thing now") + assert odd.startswith("skill_not_found:") and len(odd.split(":", 1)[1]) == 16 + long_key = "a" * 129 + assert len(pa.subject_for("circuit_dormant", long_key).split(":", 1)[1]) == 16 + assert pa.subject_for("circuit_dormant", "a" * 128) == "circuit_dormant:" + "a" * 128 + + @pytest.mark.parametrize("bad", [None, "", " ", True, 1.5, ["x"]]) + def test_missing_key_on_a_condition_kind_is_refused(self, bad): + with pytest.raises(ValueError): + pa.subject_for("circuit_dormant", bad) + + def test_event_kind_has_no_subject(self): + assert pa.subject_for("gitignore_untracked", None) is None + assert pa.subject_for("workspace_problem_report", "anything") is None + + def test_unregistered_kind_raises(self): + with pytest.raises(KeyError): + pa.subject_for("no_such_kind", "x") + + +# --------------------------------------------------------------------------- +# Legacy subject derivation — one row per legacy id shape +# --------------------------------------------------------------------------- + +TS = "2026-09-01T10:30:00.123456Z" + + +@pytest.mark.parametrize("request_id,context,expected", [ + # A1/A2 subscription headroom — per subscription (dashed sid), both tiers, fleet + ("sub-headroom-sub-abc-123-2026-10-09-warn", None, ("subscription_headroom", "sub-abc-123")), + ("sub-headroom-sub-abc-123-2026-10-09-crit", None, ("subscription_headroom", "sub-abc-123")), + ("sub-headroom-s1-unknown-2026-10-01-warn", None, ("subscription_headroom", "s1")), + ("sub-headroom-fleet-2026-10-09", None, ("subscription_headroom", "fleet")), + # A3/A4 system agent — the start-failure shape must not be read as staleness + (f"base-image-stale-trinity-system-{TS}", None, ("base_image_stale", "trinity-system")), + ("base-image-stale-start-trinity-system-987654", None, ("system_agent_start_failed", "trinity-system")), + # A5 reconcile refusal — the orphan count is a reading, not identity + ("skills-reconcile-agent-a-7", None, ("skills_reconcile_refused", "agent-a")), + # A6/A7 legacy adoption — both branches, one subject (T7) + ("skills-legacy-adoption-refused-0123456789ab", None, ("skills_legacy_adoption", "0123456789ab")), + (f"skills-legacy-adoption-{TS}", {"url": " https://example.com/lib.git "}, + ("skills_legacy_adoption", "0d2c2b5e6c5d")), + # A8–A10 per-agent transport / sync + (f"cb-dormant-agent-a-{TS}", None, ("circuit_dormant", "agent-a")), + (f"sync-failing-agent-a-{TS}", None, ("sync_failing", "agent-a")), + (f"sync-diverged-agent-a-{TS}", None, ("sync_diverged", "agent-a")), + # A12 db backup — the condition lives in context + (f"db-backup-{TS}", {"alert_type": "db_backup_failure", "backup_dir": "/data/backups"}, + ("db_backup_failure", "/data/backups")), + (f"db-backup-{TS}", {"alert_type": "db_backup_stale", "backup_dir": "/data/backups"}, + ("db_backup_stale", "/data/backups")), + # A13 archive — keyed by the path in context + ("log-archive-data-archives-2026-09-01", {"path": "/data/archives"}, + ("log_archive_unwritable", "/data/archives")), + # B1–B14 + ("poison-exec-123", None, ("poison", "exec-123")), + ("system-seed-template_missing", None, ("system_seed", "template_missing")), + ("val_exec-9_20260901103000", None, ("validation_failed", "exec-9")), + ("ent615-git-token-scrub-agent-a-2026-09-01", None, ("git_token_scrub_refused", "agent-a")), + ("ent615-git-token-scrub-unreadable-agent-a-2026-09-01", None, ("git_token_scrub_unreadable", "agent-a")), + ("retention-guard-execution_retention_days-30", None, ("retention_refused", "execution_retention_days")), + (f"skills-fleet-reinject-{TS}", None, ("skills_fleet_reinject", "fleet")), + (f"effect-unguarded-agent-a-{TS}", {"effect_type": "email", "reason": "no_execution_id"}, + ("effect_unguarded", "email:no_execution_id")), + (f"skill-not-found-agent-a-{TS}", {"command": "/daily"}, ("skill_not_found", "/daily")), + ("portal-inbox-collision-inbox", None, ("portal_inbox_collision", "inbox")), + (f"queue-flood-agent-a-{TS}", {"held": 3, "reason": "ingestion_cap"}, ("queue_flood", "ingestion_cap")), + ("alert-budget-agent-a-effect_unguarded-b123", {"alert_type": "effect_unguarded"}, + ("alert_budget", "effect_unguarded")), +]) +def test_legacy_subject_per_shape(request_id, context, expected): + kind, key = expected + if kind == "skills_legacy_adoption" and context: + import hashlib + key = hashlib.sha256(context["url"].strip().encode()).hexdigest()[:12] + got = pa.derive_legacy_subject(request_id, context) + assert got == pa.LegacyMatch(kind, pa.subject_for(kind, key)) + + +@pytest.mark.parametrize("request_id,context,kind", [ + # two conditions under one prefix, told apart only in prose → never guessed + (f"git-bloat-agent-a-{TS}", None, "git_bloat_size"), + # a known family whose row lacks the context the subject needs + (f"db-backup-{TS}", None, "db_backup_failure"), + (f"skills-legacy-adoption-{TS}", None, "skills_legacy_adoption"), + (f"skill-not-found-agent-a-{TS}", {}, "skill_not_found"), + # events: each is its own row by design + (f"gitignore-untracked-agent-a-{TS}", None, "gitignore_untracked"), + ("workspace-problem-" + "f" * 32, None, "workspace_problem_report"), + # a malformed id under a known prefix + ("sub-headroom-garbage", None, "subscription_headroom"), +]) +def test_known_kind_without_a_subject(request_id, context, kind): + got = pa.derive_legacy_subject(request_id, context) + assert got is not None and got.subject is None + assert pa.KINDS[got.kind].lifetime == pa.KINDS[kind].lifetime + + +@pytest.mark.parametrize("request_id", [ + "role-drift-agent-a-1", "gate-abc", "gate-note-abc", "my-own-ask-1", "", None, +]) +def test_unknown_or_external_prefix_is_not_derived(request_id): + assert pa.derive_legacy_subject(request_id, None) is None + + +def test_a_longer_prefix_is_always_tried_before_a_shorter_one_it_extends(): + """`base-image-stale-start-` / `ent615-git-token-scrub-unreadable-` must win + over the family prefix they extend, whatever order the table is written in.""" + order = [prefix for prefix, _kind, _parse in pa._LEGACY_PARSERS] + for i, short in enumerate(order): + for longer in order[i + 1:]: + assert not (longer.startswith(short) and longer != short), (short, longer) + + +def test_context_may_arrive_as_json_text(): + ctx = json.dumps({"alert_type": "db_backup_stale", "backup_dir": "/b"}) + assert pa.derive_legacy_subject(f"db-backup-{TS}", ctx).subject == pa.subject_for("db_backup_stale", "/b") + assert pa.derive_legacy_subject(f"db-backup-{TS}", "{not json").subject is None + + +# --------------------------------------------------------------------------- +# plan_sweep +# --------------------------------------------------------------------------- + + +def _ids(stamps): + return sorted(s.id for s in stamps) + + +class TestPlanSweep: + def test_keeps_newest_per_subject_and_ends_the_rest(self, monkeypatch): + monkeypatch.delenv(pa.LIFETIME_ENV, raising=False) + rows = [ + _row("a", "sub-headroom-s1-2026-10-09-warn", created_at="2026-10-01T00:00:00.000000Z"), + _row("b", "sub-headroom-s1-2026-10-09-crit", created_at="2026-10-03T00:00:00.000000Z", + context={"tier": "crit"}), + _row("c", "sub-headroom-s2-2026-10-09-warn", created_at="2026-10-02T00:00:00.000000Z"), + ] + plan = pa.plan_sweep(rows, now=NOW) + assert plan.ended_ids == ("a",) + survivors = {s.id: s for s in plan.survivor_stamps} + assert set(survivors) == {"b", "c"} + b = survivors["b"] + assert b.subject == "subscription_headroom:s1" + assert b.last_seen_at == "2026-10-03T00:00:00.000000Z" + assert b.expires_at == _iso(NOW + timedelta(days=14)) + assert b.context == {"tier": "crit", "seen_count": 1} + + def test_same_subject_on_different_agents_is_two_groups(self): + rows = [ + _row("a", f"cb-dormant-x-{TS}", agent="x", created_at="2026-10-01T00:00:00Z"), + _row("b", f"cb-dormant-x-{TS}", agent="y", created_at="2026-10-02T00:00:00Z"), + ] + plan = pa.plan_sweep(rows, now=NOW) + assert plan.ended_ids == () + assert _ids(plan.survivor_stamps) == ["a", "b"] + + def test_tie_on_created_at_broken_by_id_and_null_sorts_oldest(self): + rows = [ + _row("m", f"cb-dormant-x-{TS}", agent="x", created_at=None), + _row("k", f"cb-dormant-x-{TS}", agent="x", created_at="2026-10-01T00:00:00Z"), + _row("l", f"cb-dormant-x-{TS}", agent="x", created_at="2026-10-01T00:00:00Z"), + ] + plan = pa.plan_sweep(rows, now=NOW) + assert [s.id for s in plan.survivor_stamps] == ["l"] + assert sorted(plan.ended_ids) == ["k", "m"] + + def test_null_created_at_survivor_is_last_seen_now(self): + plan = pa.plan_sweep([_row("m", f"cb-dormant-x-{TS}", agent="x")], now=NOW) + assert plan.survivor_stamps[0].last_seen_at == _iso(NOW) + + def test_existing_expiry_is_kept_and_opt_out_kind_gets_none(self): + rows = [ + _row("a", f"cb-dormant-x-{TS}", agent="x", expires_at="2026-11-01T00:00:00.000000Z"), + _row("p", "poison-exec-1", agent="x"), + ] + stamps = {s.id: s for s in pa.plan_sweep(rows, now=NOW).survivor_stamps} + assert stamps["a"].expires_at is None # None = leave the column as it is + assert stamps["p"].expires_at is None + assert stamps["p"].subject == "poison:exec-1" + + def test_net_kind_gets_thirty_days(self): + stamps = pa.plan_sweep([_row("a", f"cb-dormant-x-{TS}", agent="x")], now=NOW).survivor_stamps + assert stamps[0].expires_at == _iso(NOW + timedelta(days=30)) + + def test_unknown_and_external_prefixes_are_untouched(self): + rows = [ + _row("r1", "role-drift-x-1", agent="x"), + _row("r2", "role-drift-x-2", agent="x"), + _row("g", "gate-abc", agent="x"), + _row("o", "my-own-ask", agent="x"), + ] + plan = pa.plan_sweep(rows, now=NOW) + assert plan.is_empty() + + def test_known_kind_without_subject_gets_a_lifetime_stamp_only(self): + rows = [ + _row("g1", f"git-bloat-x-{TS}", agent="x"), + _row("g2", f"git-bloat-x-2026-09-02T00:00:00.000000Z", agent="x"), + _row("w", f"gitignore-untracked-x-{TS}", agent="x"), # event, no lifetime + _row("g3", f"git-bloat-x-{TS}", agent="x", expires_at="2026-12-01T00:00:00Z"), + ] + plan = pa.plan_sweep(rows, now=NOW) + assert plan.ended_ids == () + assert plan.survivor_stamps == () + assert [(s.id, s.expires_at) for s in plan.lifetime_stamps] == [ + ("g1", _iso(NOW + timedelta(days=30))), + ("g2", _iso(NOW + timedelta(days=30))), + ] + + def test_already_stamped_pending_row_holds_its_subject(self): + """A row the seam already owns keeps the index slot; unstamped + duplicates of its subject end, whatever their age.""" + rows = [ + _row("live", f"cb-dormant-x-{TS}", agent="x", subject="circuit_dormant:x", + created_at="2026-09-01T00:00:00Z"), + _row("old", f"cb-dormant-x-{TS}", agent="x", created_at="2026-10-01T00:00:00Z"), + ] + plan = pa.plan_sweep(rows, now=NOW) + assert plan.ended_ids == ("old",) + assert plan.survivor_stamps == () + + def test_person_ended_row_in_snooze_window_gets_subject_only(self, monkeypatch): + monkeypatch.delenv(pa.SNOOZE_ENV, raising=False) + rows = [ + _row("p1", f"cb-dormant-x-{TS}", agent="x", status="responded", + disposed_by="person", disposed_at=_iso(NOW - timedelta(days=6))), + _row("p2", f"cb-dormant-x-{TS}", agent="x", status="cancelled", + disposed_by="person", disposed_at=_iso(NOW - timedelta(days=8))), + _row("t", f"cb-dormant-x-{TS}", agent="x", status="expired", + disposed_by="timeout", disposed_at=_iso(NOW - timedelta(days=1))), + _row("p3", f"cb-dormant-x-{TS}", agent="x", status="responded", + disposed_by="person", disposed_at="garbage"), + ] + plan = pa.plan_sweep(rows, now=NOW) + assert [(s.id, s.subject) for s in plan.snooze_stamps] == [("p1", "circuit_dormant:x")] + assert plan.ended_ids == () and plan.survivor_stamps == () + + def test_idempotent_on_its_own_output(self): + rows = [ + _row("a", "sub-headroom-s1-2026-10-09-warn", created_at="2026-10-01T00:00:00Z"), + _row("b", "sub-headroom-s1-2026-10-09-crit", created_at="2026-10-03T00:00:00Z"), + _row("g", f"git-bloat-x-{TS}", agent="x"), + _row("p", f"cb-dormant-x-{TS}", agent="x", status="responded", + disposed_by="person", disposed_at=_iso(NOW - timedelta(days=1))), + ] + first = pa.plan_sweep(rows, now=NOW) + applied = _apply(rows, first, now=NOW) + second = pa.plan_sweep(applied, now=NOW + timedelta(minutes=1)) + assert second.is_empty(), second + + def test_context_text_is_parsed_and_bad_context_replaced(self): + rows = [ + _row("a", f"cb-dormant-x-{TS}", agent="x", context=json.dumps({"k": 1})), + _row("b", f"cb-dormant-y-{TS}", agent="y", context="{bad"), + ] + stamps = {s.id: s for s in pa.plan_sweep(rows, now=NOW).survivor_stamps} + assert stamps["a"].context == {"k": 1, "seen_count": 1} + assert stamps["b"].context == {"seen_count": 1} + + def test_naive_now_is_refused(self): + with pytest.raises(ValueError): + pa.plan_sweep([], now=datetime(2026, 10, 5)) + + +def _apply(rows, plan, *, now): + """What C2's migration will do with a plan, over plain dicts.""" + by_id = {r["id"]: dict(r) for r in rows} + for rid in plan.ended_ids: + by_id[rid].update(status="cancelled", disposed_by="platform", disposed_at=_iso(now)) + for s in plan.survivor_stamps: + r = by_id[s.id] + r["subject"], r["last_seen_at"] = s.subject, s.last_seen_at + if s.expires_at is not None: + r["expires_at"] = s.expires_at + r["context"] = s.context + for s in plan.lifetime_stamps: + by_id[s.id]["expires_at"] = s.expires_at + for s in plan.snooze_stamps: + by_id[s.id]["subject"] = s.subject + return list(by_id.values()) From 2eedb0df56b77800e9563d156c7dc0d0e2f2a910 Mon Sep 17 00:00:00 2001 From: Trinity Agent Date: Mon, 5 Oct 2026 12:59:09 -0400 Subject: [PATCH 02/19] feat(operator-queue): subject columns, backlog sweep and one-pending-per-subject index (#3246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C2 of #3246, both schema tracks in one migration, ordered columns → sweep → index: the partial unique index can only be created after the sweep has collapsed the duplicates an installed backlog holds. - `operator_queue.subject TEXT` + `last_seen_at TEXT` (nullable, no backfill beyond the sweep) in schema.py, tables.py and both migration tracks. - `run_platform_alert_sweep(run)` in db/migrations.py — one function both tracks call over their own connection (sqlite3 cursor / SQLAlchemy `text()`), so the survivor rule cannot drift. It hands the fetched rows to the leaf's pure `plan_sweep` (function-local import; neither track pulls the service graph into `init_database()`), stamps the newest pending row per derived subject, ends the rest as ONE batch in the ent#611 vocabulary (`cancelled` / `platform` / `superseded` / NULL email / `batch_id`) with a compare-and-set on `status='pending'`, stamps a lifetime only on known kinds whose subject cannot be derived (never merged on a guess), and stamps `subject` on person-ended rows inside the 7-day snooze window. Agent-raised rows, `gate-` rows and external prefixes never derive. - `uq_operator_queue_pending_subject` (partial: pending + subject) and `idx_operator_queue_agent_subject`, created after the sweep. - Alembic `0090_platform_alert_subjects` chained after the live head `0089_supersede_queue_flood_backlog`. Test (red first at collection — the migration did not exist): tests/unit/test_3246_platform_alert_sweep.py drives the registered entry against the pre-upgrade `schema.py` DDL holding duplicates: survivor and stamps, one batch, untouched rows, snooze stamp, a second run changes nothing, the index exists only after the run and bites on a second pending row, DDL parity with schema.py, both tracks registered. Co-Authored-By: Claude Fable 5.1 --- src/backend/db/migrations.py | 150 +++++++++++ src/backend/db/schema.py | 11 + src/backend/db/tables.py | 14 + .../versions/0090_platform_alert_subjects.py | 58 ++++ tests/unit/test_3246_platform_alert_sweep.py | 248 ++++++++++++++++++ 5 files changed, 481 insertions(+) create mode 100644 src/backend/migrations/versions/0090_platform_alert_subjects.py create mode 100644 tests/unit/test_3246_platform_alert_sweep.py diff --git a/src/backend/db/migrations.py b/src/backend/db/migrations.py index 4cad464811..3b40a5e2d6 100644 --- a/src/backend/db/migrations.py +++ b/src/backend/db/migrations.py @@ -5178,6 +5178,155 @@ def _migrate_supersede_queue_flood_backlog(cursor, conn): conn.commit() +# #3246: the shared sweep — both tracks run this function over their own +# connection (the Alembic revision imports it), so the survivor rule cannot +# drift between them. `run(sql, params, fetch=False)` is the one driver seam: +# sqlite3 and SQLAlchemy `text()` both bind `:name` parameters. +_PLATFORM_ALERT_SWEEP_SELECT = """ +SELECT id, agent_name, request_id, status, created_at, expires_at, context, + subject, disposed_by, disposed_at +FROM operator_queue +WHERE status = 'pending' + OR (disposed_by = 'person' AND subject IS NULL AND disposed_at >= :since) +""" + +_PLATFORM_ALERT_SWEEP_END = """ +UPDATE operator_queue +SET status = 'cancelled', + disposition = 'cancelled', + disposed_at = :now, + disposed_by = 'platform', + disposed_by_email = NULL, + disposition_reason = 'superseded', + batch_id = :batch_id +WHERE status = 'pending' AND id IN ({ids}) +""" + +_SWEEP_CHUNK = 400 # well under every driver's bound-parameter limit + + +def run_platform_alert_sweep(run, *, now=None, batch_id=None) -> dict: + """#3246: collapse the platform-alert backlog to one pending row per subject. + + Reads every pending row plus the person-ended rows inside the snooze + window, hands them to the leaf's pure ``plan_sweep`` (the ONE source of + per-kind id parsing — imported function-locally so neither migration + track pulls the service graph into ``init_database()``), then writes: + + * survivors: ``subject``, ``last_seen_at``, ``context.seen_count = 1`` and + ``expires_at`` only where it was NULL; + * every other pending row of the subject: ended as ONE batch — + ``cancelled`` / ``platform`` / ``superseded`` / NULL email / ``batch_id`` + — the ent#611 vocabulary, compare-and-set on ``status = 'pending'`` so a + row a person ends first is never overwritten; + * known kinds with no derivable subject: ``expires_at`` only (the row + leaves on its own; nothing is merged on a guess); + * person-ended rows inside the snooze window: ``subject`` only. + + Agent-raised rows, ``gate-`` rows and external prefixes never derive and + are never touched. Idempotent: a second run finds the survivors stamped + and plans nothing. Returns the write counts for the boot log. + """ + import json as _json + import uuid as _uuid + from datetime import datetime as _dt, timezone as _tz + + from services.platform_alerts import plan_sweep, snooze_window + + now = now or _dt.now(_tz.utc) + now_iso = now.strftime("%Y-%m-%dT%H:%M:%S.%fZ") + since_iso = (now - snooze_window()).strftime("%Y-%m-%dT%H:%M:%S.%fZ") + cols = ("id", "agent_name", "request_id", "status", "created_at", "expires_at", + "context", "subject", "disposed_by", "disposed_at") + rows = [dict(zip(cols, r)) for r in run(_PLATFORM_ALERT_SWEEP_SELECT, {"since": since_iso}, fetch=True)] + plan = plan_sweep(rows, now=now) + counts = {"survivors": 0, "ended": 0, "lifetime_only": 0, "snoozed": 0} + if plan.is_empty(): + return counts + + for stamp in plan.survivor_stamps: + run( + "UPDATE operator_queue SET subject = :subject, last_seen_at = :last_seen_at, " + "context = :context, expires_at = COALESCE(expires_at, :expires_at) " + "WHERE id = :id AND status = 'pending'", + {"id": stamp.id, "subject": stamp.subject, "last_seen_at": stamp.last_seen_at, + "context": _json.dumps(stamp.context) if stamp.context is not None else None, + "expires_at": stamp.expires_at}, + ) + counts["survivors"] += 1 + + batch = batch_id or _uuid.uuid4().hex + ended = list(plan.ended_ids) + for start in range(0, len(ended), _SWEEP_CHUNK): + chunk = ended[start:start + _SWEEP_CHUNK] + params = {f"e{i}": id_ for i, id_ in enumerate(chunk)} + placeholders = ", ".join(f":{k}" for k in params) + params.update({"now": now_iso, "batch_id": batch}) + run(_PLATFORM_ALERT_SWEEP_END.format(ids=placeholders), params) + counts["ended"] += len(chunk) + + for stamp in plan.lifetime_stamps: + run("UPDATE operator_queue SET expires_at = :expires_at " + "WHERE id = :id AND status = 'pending' AND expires_at IS NULL", + {"id": stamp.id, "expires_at": stamp.expires_at}) + counts["lifetime_only"] += 1 + + for stamp in plan.snooze_stamps: + run("UPDATE operator_queue SET subject = :subject WHERE id = :id AND subject IS NULL", + {"id": stamp.id, "subject": stamp.subject}) + counts["snoozed"] += 1 + return counts + + +# Both tracks create the same two indexes, by name (schema.py declares them too). +PLATFORM_ALERT_INDEX_DDL = ( + "CREATE UNIQUE INDEX IF NOT EXISTS uq_operator_queue_pending_subject " + "ON operator_queue(agent_name, subject) WHERE status = 'pending' AND subject IS NOT NULL", + "CREATE INDEX IF NOT EXISTS idx_operator_queue_agent_subject ON operator_queue(agent_name, subject)", +) + + +def _migrate_platform_alert_subjects(cursor, conn): + """#3246 — platform alerts: one pending row per subject. + + Ordered columns → sweep → index, in ONE migration: the partial unique index + can only be created once the sweep has collapsed the duplicates an + installed backlog holds, so the three steps cannot be split across + migrations without breaking the upgrade of exactly the installs this is + for. Adds `subject TEXT` and `last_seen_at TEXT` (nullable, PRAGMA-guarded), + runs `run_platform_alert_sweep` (the leaf's `plan_sweep` decides; see its + docstring for the rules), then creates `uq_operator_queue_pending_subject` + (partial, pending + subject only) and `idx_operator_queue_agent_subject`. + + No wake and no file write follow: these are platform-minted rows + (`is_platform_minted`), which take no part in either. Idempotent. + + Mirrored by the Alembic revision 0090_platform_alert_subjects. + """ + for column in ("subject", "last_seen_at"): + _safe_add_column( + cursor, + "operator_queue", + column, + f"ALTER TABLE operator_queue ADD COLUMN {column} TEXT", + log_msg=f"Adding {column} to operator_queue for platform alerts (#3246)", + ) + conn.commit() + + def _run(sql, params, fetch=False): + cursor.execute(sql, params) + return cursor.fetchall() if fetch else None + + counts = run_platform_alert_sweep(_run) + conn.commit() + if any(counts.values()): + logger.info("Platform-alert backlog sweep (#3246): %s", counts) + + for ddl in PLATFORM_ALERT_INDEX_DDL: + cursor.execute(ddl) + conn.commit() + + MIGRATIONS = [ ("agent_sharing", _migrate_agent_sharing_table), ("schedule_executions_observability", _migrate_schedule_executions_observability), @@ -5337,4 +5486,5 @@ def _migrate_supersede_queue_flood_backlog(cursor, conn): ("pull_sync", _migrate_pull_sync), ("skill_gate_requests_table", _migrate_skill_gate_requests_table), ("supersede_queue_flood_backlog", _migrate_supersede_queue_flood_backlog), + ("platform_alert_subjects", _migrate_platform_alert_subjects), ] diff --git a/src/backend/db/schema.py b/src/backend/db/schema.py index 68951aa670..726719b0ae 100644 --- a/src/backend/db/schema.py +++ b/src/backend/db/schema.py @@ -1744,6 +1744,8 @@ resolved_to TEXT, proposal TEXT, supersedes_expired TEXT, + subject TEXT, + last_seen_at TEXT, FOREIGN KEY (responded_by_id) REFERENCES users(id) ) """, @@ -2271,6 +2273,15 @@ # platform-minted `id` (uuid) is the global handle; `request_id` carries the # agent's string, so two agents can reuse the same id without collision. "CREATE UNIQUE INDEX IF NOT EXISTS idx_operator_queue_agent_request ON operator_queue(agent_name, request_id)", + # #3246: a platform alert is a condition, not a message — at most ONE + # pending row per (agent, subject). Partial: ended rows and subject-less + # rows (agent-raised asks, gates, not-yet-migrated emitters) stay outside + # it. Created by the migration only AFTER the backlog sweep collapsed + # duplicates, so an install holding them upgrades cleanly. + "CREATE UNIQUE INDEX IF NOT EXISTS uq_operator_queue_pending_subject ON operator_queue(agent_name, subject) WHERE status = 'pending' AND subject IS NOT NULL", + # The seam's snooze lookup reads person-ended rows by subject — a partial + # index over pending rows cannot serve it. + "CREATE INDEX IF NOT EXISTS idx_operator_queue_agent_subject ON operator_queue(agent_name, subject)", # trinity-enterprise#751 — the gate's caps count pending rows per executor # and per requester; the sweep reads by state. "CREATE INDEX IF NOT EXISTS idx_skill_gate_requests_agent_state ON skill_gate_requests(agent_name, state)", diff --git a/src/backend/db/tables.py b/src/backend/db/tables.py index f5a71590f7..14f48eda8b 100644 --- a/src/backend/db/tables.py +++ b/src/backend/db/tables.py @@ -1508,6 +1508,20 @@ def process_bind_param(self, value, dialect): Column("resolved_to", Text), # JSON list of person refs Column("proposal", Text), # JSON — the frozen action Column("supersedes_expired", Text), # the predecessor row's uuid + # #3246: a platform alert is a condition, not a message. `subject` is + # `kind:key`, the dedup key the seam keeps at most one pending row for; + # `last_seen_at` moves on every reading. Both nullable, no backfill beyond + # the upgrade sweep: agent-raised asks and gates never carry a subject. + Column("subject", Text), + Column("last_seen_at", Text), + Index( + "uq_operator_queue_pending_subject", + "agent_name", "subject", + unique=True, + sqlite_where=text("status = 'pending' AND subject IS NOT NULL"), + postgresql_where=text("status = 'pending' AND subject IS NOT NULL"), + ), + Index("idx_operator_queue_agent_subject", "agent_name", "subject"), ) skill_gate_requests = Table( diff --git a/src/backend/migrations/versions/0090_platform_alert_subjects.py b/src/backend/migrations/versions/0090_platform_alert_subjects.py new file mode 100644 index 0000000000..1190497da1 --- /dev/null +++ b/src/backend/migrations/versions/0090_platform_alert_subjects.py @@ -0,0 +1,58 @@ +"""platform_alert_subjects — one pending platform alert per subject (#3246). + +PostgreSQL half of the dual-track pair; the SQLite half is +``db/migrations.py::platform_alert_subjects``. Ordered columns → sweep → +index in ONE revision: the partial unique index can only be created once the +sweep has collapsed the duplicates an installed backlog holds. + +Adds ``subject TEXT`` and ``last_seen_at TEXT`` (nullable, no backfill beyond +the sweep), runs the shared ``run_platform_alert_sweep`` (both tracks call the +same function over their own connection, so the survivor rule cannot drift: +per (agent, subject) the newest pending row — by ``created_at``, then ``id`` — +stays stamped; the rest end ``cancelled`` / ``disposed_by = 'platform'`` / +``superseded`` as one batch; rows whose subject cannot be derived are never +merged on a guess), then creates ``uq_operator_queue_pending_subject`` +(partial: ``status = 'pending' AND subject IS NOT NULL``) and +``idx_operator_queue_agent_subject``. + +Revision ID: 0090_platform_alert_subjects +Revises: 0089_supersede_queue_flood_backlog +""" +from alembic import op +from sqlalchemy import text + + +revision = "0090_platform_alert_subjects" +down_revision = "0089_supersede_queue_flood_backlog" +branch_labels = None +depends_on = None + +_COLUMNS = ("subject", "last_seen_at") +_INDEXES = ("uq_operator_queue_pending_subject", "idx_operator_queue_agent_subject") + + +def upgrade() -> None: + from db.migrations import PLATFORM_ALERT_INDEX_DDL, run_platform_alert_sweep + + for column in _COLUMNS: + op.execute(f"ALTER TABLE operator_queue ADD COLUMN IF NOT EXISTS {column} TEXT") + + bind = op.get_bind() + + def _run(sql, params, fetch=False): + result = bind.execute(text(sql), params) + return result.fetchall() if fetch else None + + run_platform_alert_sweep(_run) + + for ddl in PLATFORM_ALERT_INDEX_DDL: + op.execute(ddl) + + +def downgrade() -> None: + for name in reversed(_INDEXES): + op.execute(f"DROP INDEX IF EXISTS {name}") + for column in reversed(_COLUMNS): + op.execute(f"ALTER TABLE operator_queue DROP COLUMN IF EXISTS {column}") + # The sweep's endings are deliberately not reverted: the inverse is + # "re-open hundreds of duplicate platform alarms", which no rollback wants. diff --git a/tests/unit/test_3246_platform_alert_sweep.py b/tests/unit/test_3246_platform_alert_sweep.py new file mode 100644 index 0000000000..80eb34e359 --- /dev/null +++ b/tests/unit/test_3246_platform_alert_sweep.py @@ -0,0 +1,248 @@ +""" +#3246 — the upgrade sweep: subject columns, the backlog collapse, the index. + +Drives the registered SQLite migration (``platform_alert_subjects``) against +the real ``schema.py`` DDL for ``operator_queue`` with the two new columns +and the new indexes stripped — a pre-upgrade install holding duplicates — +and asserts the ordered contract columns → sweep → index: the newest pending +row per derived subject survives and is stamped, the rest end as ONE batch +authored by the platform in the ent#611 vocabulary, agent-raised / gate / +external rows are untouched, a second run changes nothing, and only after +the sweep does the partial unique index exist and bite. + +The decision function is the leaf's ``plan_sweep`` (test_3246_platform_alerts_leaf.py); +this file proves the migration applies it to a database. +""" +import json +import os +import re +import sqlite3 +import sys +from datetime import datetime, timezone +from pathlib import Path + +import pytest + +_BACKEND = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "src", "backend")) +if _BACKEND not in sys.path: + sys.path.insert(0, _BACKEND) + +from db.migrations import MIGRATIONS, _migrate_platform_alert_subjects # noqa: E402 +from db.schema import INDEXES, TABLES # noqa: E402 + +TS = "2026-10-01T00:00:00.000000Z" +NEW_COLUMNS = ("subject", "last_seen_at") +UNIQUE_INDEX = "uq_operator_queue_pending_subject" +PLAIN_INDEX = "idx_operator_queue_agent_subject" + + +def _pre_upgrade_ddl() -> str: + ddl = TABLES["operator_queue"] + for column in NEW_COLUMNS: + ddl, n = re.subn(rf"^\s*{column} TEXT,?\n", "", ddl, flags=re.MULTILINE) + assert n == 1, f"schema.py must declare {column} TEXT on operator_queue" + return ddl + + +@pytest.fixture +def conn(): + conn = sqlite3.connect(":memory:") + conn.execute(_pre_upgrade_ddl()) + for ddl in INDEXES: + if "operator_queue(" in ddl and UNIQUE_INDEX not in ddl and PLAIN_INDEX not in ddl: + conn.execute(ddl) + cols = {r[1] for r in conn.execute("PRAGMA table_info(operator_queue)")} + assert not cols & set(NEW_COLUMNS) + return conn + + +def _add(conn, id_, agent, rid, created, *, status="pending", context=None, + expires_at=None, disposed_by=None, disposed_at=None, type_="alert"): + conn.execute( + "INSERT INTO operator_queue (id, agent_name, request_id, type, status, priority, title, " + "question, created_at, context, expires_at, disposed_by, disposed_at) " + "VALUES (?, ?, ?, ?, ?, 'high', 't', 'q', ?, ?, ?, ?, ?)", + (id_, agent, rid, type_, status, created, + json.dumps(context) if context is not None else None, expires_at, disposed_by, disposed_at), + ) + + +def _snapshot(conn): + return conn.execute("SELECT * FROM operator_queue ORDER BY id").fetchall() + + +def _run(conn): + _migrate_platform_alert_subjects(conn.cursor(), conn) + + +def _seed_duplicates(conn): + # three readings of one subscription's headroom — warn, warn, crit — plus the fleet one + _add(conn, "h1", "_sub-headroom", "sub-headroom-sub-abc-123-2026-09-28-warn", "2026-09-28T00:00:00.000000Z") + _add(conn, "h2", "_sub-headroom", "sub-headroom-sub-abc-123-2026-09-29-warn", "2026-09-29T00:00:00.000000Z") + _add(conn, "h3", "_sub-headroom", "sub-headroom-sub-abc-123-2026-09-30-crit", "2026-09-30T00:00:00.000000Z", + context={"tier": "crit"}) + _add(conn, "hf", "_sub-headroom", "sub-headroom-fleet-2026-09-30", "2026-09-30T00:00:00.000000Z") + # the same circuit-dormant subject on two agents is two groups + _add(conn, "c1", "x", f"cb-dormant-x-{TS}", "2026-09-20T00:00:00.000000Z") + _add(conn, "c2", "x", f"cb-dormant-x-2026-09-21T00:00:00.000000Z", "2026-09-21T00:00:00.000000Z") + _add(conn, "c3", "y", f"cb-dormant-y-{TS}", "2026-09-21T00:00:00.000000Z") + # opt-out lifetime kind: survives with expires_at NULL + _add(conn, "p1", "x", "poison-exec-1", "2026-09-21T00:00:00.000000Z") + # known kind, no derivable subject: lifetime stamp only + _add(conn, "g1", "x", f"git-bloat-x-{TS}", "2026-09-21T00:00:00.000000Z") + _add(conn, "g2", "x", f"git-bloat-x-2026-09-22T00:00:00.000000Z", "2026-09-22T00:00:00.000000Z") + # never touched: an agent's own ask, a gate row, an external-prefix row, a non-pending row + _add(conn, "own", "x", "my-own-ask", "2026-09-21T00:00:00.000000Z", type_="question") + _add(conn, "gate", "x", "gate-abc", "2026-09-21T00:00:00.000000Z", type_="approval") + _add(conn, "r1", "x", "role-drift-x-1", "2026-09-21T00:00:00.000000Z") + _add(conn, "r2", "x", "role-drift-x-2", "2026-09-22T00:00:00.000000Z") + _add(conn, "done", "x", f"cb-dormant-x-2026-09-01T00:00:00.000000Z", "2026-09-01T00:00:00.000000Z", + status="responded", disposed_by="person", disposed_at="2026-09-02T00:00:00.000000Z") + + +class TestColumnsThenSweep: + def test_columns_are_added_and_the_migration_is_pragma_guarded(self, conn): + _run(conn) + cols = {r[1] for r in conn.execute("PRAGMA table_info(operator_queue)")} + assert set(NEW_COLUMNS) <= cols + _run(conn) # a second run must not re-ALTER + + def test_keeps_the_newest_pending_row_per_subject_and_ends_the_rest_as_one_batch(self, conn): + _seed_duplicates(conn) + _run(conn) + status = dict(conn.execute("SELECT id, status FROM operator_queue").fetchall()) + assert status["h3"] == "pending" and status["h1"] == status["h2"] == "cancelled" + assert status["hf"] == "pending" + assert status["c2"] == "pending" and status["c1"] == "cancelled" + assert status["c3"] == "pending" + assert status["p1"] == "pending" + assert status["g1"] == status["g2"] == "pending" # no subject → never merged on a guess + assert status["own"] == status["gate"] == status["r1"] == status["r2"] == "pending" + assert status["done"] == "responded" + + endings = conn.execute( + "SELECT DISTINCT disposition, disposed_by, disposition_reason, disposed_by_email " + "FROM operator_queue WHERE status='cancelled'").fetchall() + assert endings == [("cancelled", "platform", "superseded", None)] + batches = conn.execute( + "SELECT DISTINCT batch_id FROM operator_queue WHERE status='cancelled'").fetchall() + assert len(batches) == 1 and batches[0][0] + assert all(r[0] for r in conn.execute( + "SELECT disposed_at FROM operator_queue WHERE status='cancelled'")) + + def test_survivors_are_stamped(self, conn): + _seed_duplicates(conn) + _run(conn) + subject, last_seen, expires, context = conn.execute( + "SELECT subject, last_seen_at, expires_at, context FROM operator_queue WHERE id='h3'").fetchone() + assert subject == "subscription_headroom:sub-abc-123" + assert last_seen == "2026-09-30T00:00:00.000000Z" + assert expires and expires.endswith("Z") + assert json.loads(context) == {"tier": "crit", "seen_count": 1} + # the ended duplicates carry no subject: the partial index sees one pending row + assert conn.execute("SELECT subject FROM operator_queue WHERE id='h1'").fetchone()[0] is None + # opt-out kind keeps expires_at NULL; a derived subject still lands + assert conn.execute( + "SELECT subject, expires_at FROM operator_queue WHERE id='p1'").fetchone() == ("poison:exec-1", None) + # known kind without a subject: lifetime stamp only + for gid in ("g1", "g2"): + subject, expires = conn.execute( + "SELECT subject, expires_at FROM operator_queue WHERE id=?", (gid,)).fetchone() + assert subject is None and expires + # untouched rows carry nothing + for rid in ("own", "gate", "r1", "r2"): + assert conn.execute( + "SELECT subject, last_seen_at, expires_at FROM operator_queue WHERE id=?", (rid,) + ).fetchone() == (None, None, None) + + def test_person_ended_row_inside_the_snooze_window_gets_subject_only(self, conn, monkeypatch): + import services.platform_alerts as pa + now = datetime.now(timezone.utc) + recent = now.strftime("%Y-%m-%dT%H:%M:%S.%fZ") + _add(conn, "acked", "x", f"cb-dormant-x-{TS}", TS, status="responded", + disposed_by="person", disposed_at=recent) + _add(conn, "timed", "x", f"cb-dormant-x-2026-09-21T00:00:00.000000Z", TS, status="expired", + disposed_by="timeout", disposed_at=recent) + _add(conn, "old", "x", f"cb-dormant-x-2026-09-22T00:00:00.000000Z", TS, status="responded", + disposed_by="person", disposed_at="2020-01-01T00:00:00.000000Z") + _run(conn) + rows = dict(conn.execute("SELECT id, subject FROM operator_queue").fetchall()) + assert rows == {"acked": "circuit_dormant:x", "timed": None, "old": None} + for rid in ("acked", "timed", "old"): + assert conn.execute( + "SELECT status, last_seen_at, expires_at FROM operator_queue WHERE id=?", (rid,) + ).fetchone()[1:] == (None, None) + + def test_a_second_run_changes_nothing(self, conn): + _seed_duplicates(conn) + _run(conn) + before = _snapshot(conn) + _run(conn) + assert _snapshot(conn) == before + + +class TestIndexAfterSweep: + def test_the_indexes_exist_only_after_the_sweep_collapsed_duplicates(self, conn): + _seed_duplicates(conn) + names = lambda: {r[0] for r in conn.execute( # noqa: E731 + "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='operator_queue'")} + assert UNIQUE_INDEX not in names() + _run(conn) + assert {UNIQUE_INDEX, PLAIN_INDEX} <= names() + + def test_the_partial_unique_index_bites_on_a_second_pending_row(self, conn): + _seed_duplicates(conn) + _run(conn) + with pytest.raises(sqlite3.IntegrityError): + conn.execute( + "INSERT INTO operator_queue (id, agent_name, request_id, type, status, priority, title, " + "question, created_at, subject) VALUES ('dup', '_sub-headroom', 'new', 'alert', 'pending', " + "'high', 't', 'q', ?, 'subscription_headroom:sub-abc-123')", (TS,)) + # ended and subject-less rows are outside the index + conn.execute( + "INSERT INTO operator_queue (id, agent_name, request_id, type, status, priority, title, " + "question, created_at, subject) VALUES ('ended', '_sub-headroom', 'new2', 'alert', 'cancelled', " + "'high', 't', 'q', ?, 'subscription_headroom:sub-abc-123')", (TS,)) + conn.execute( + "INSERT INTO operator_queue (id, agent_name, request_id, type, status, priority, title, " + "question, created_at) VALUES ('nosub', '_sub-headroom', 'new3', 'alert', 'pending', " + "'high', 't', 'q', ?)", (TS,)) + + def test_the_index_ddl_matches_schema_py(self, conn): + _seed_duplicates(conn) + _run(conn) + live = dict(conn.execute( + "SELECT name, sql FROM sqlite_master WHERE type='index' AND name IN (?, ?)", + (UNIQUE_INDEX, PLAIN_INDEX)).fetchall()) + declared = {} + for ddl in INDEXES: + for name in (UNIQUE_INDEX, PLAIN_INDEX): + if f" {name} " in ddl: + declared[name] = ddl + assert set(declared) == {UNIQUE_INDEX, PLAIN_INDEX} + norm = lambda s: " ".join(s.replace("IF NOT EXISTS ", "").split()) # noqa: E731 + for name in (UNIQUE_INDEX, PLAIN_INDEX): + assert norm(live[name]) == norm(declared[name]) + + +class TestBothTracks: + def test_registered_on_both_tracks_after_the_flood_sweep(self): + names = [name for name, _ in MIGRATIONS] + assert names.index("platform_alert_subjects") > names.index("supersede_queue_flood_backlog") + versions = Path(_BACKEND) / "migrations" / "versions" + files = [p for p in versions.glob("*_platform_alert_subjects.py")] + assert len(files) == 1 + text = files[0].read_text() + assert 'down_revision = "0089_supersede_queue_flood_backlog"' in text + assert "plan_sweep" not in text or "run_platform_alert_sweep" in text + + def test_tables_py_declares_columns_and_both_indexes(self): + from db.tables import operator_queue + assert set(NEW_COLUMNS) <= set(operator_queue.c.keys()) + idx = {i.name: i for i in operator_queue.indexes} + assert idx[UNIQUE_INDEX].unique is True + assert str(idx[UNIQUE_INDEX].dialect_options["sqlite"]["where"]) == \ + "status = 'pending' AND subject IS NOT NULL" + assert str(idx[UNIQUE_INDEX].dialect_options["postgresql"]["where"]) == \ + "status = 'pending' AND subject IS NOT NULL" + assert PLAIN_INDEX in idx and idx[PLAIN_INDEX].unique is False From 0d5490380f1e69f3237b9558e6d9ef9cfa4da319 Mon Sep 17 00:00:00 2001 From: Trinity Agent Date: Mon, 5 Oct 2026 13:03:59 -0400 Subject: [PATCH 03/19] feat(operator-queue): locked platform-alert accessors and the platform ending (#3246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C3 of #3246 — the DB seam the platform-alert service (C4) will stand on. db/operator_queue.py: - `create_platform_item(agent, item, *, subject, max_pending_for_type)`: find → touch → count → insert in ONE `_lock_agent_for_create` transaction. A reading of a subject with a pending row updates it in place by a compare-and-set on `status='pending'` (title / question / priority / context with `seen_count`+1 / `last_seen_at` / `expires_at`); a lost CAS falls through to a fresh row, so a row a person ended is never overwritten. The find runs BEFORE the #1677 per-type count, so an update is never refused at budget. An `IntegrityError` from the partial unique index (a lock that failed open) re-finds and touches the winner. Returns `{"outcome": created|updated|refused_at_budget, "row", "changed"}`; `changed` is false on a bare repeat reading so nothing broadcasts. - `find_pending_by_subject`, `find_person_ended_by_subject` (the seam's snooze read, newest person ending at or after `since`). - `end_items_by_platform(ids, *, reason, batch_id)` — the `bulk_cancel_items` shape with `disposed_by='platform'`, NULL email, re-selected by batch id so only CAS-won rows come back. - `mark_expired`'s per-id CAS gains `expires_at < now`: a row refreshed between the candidate select and the CAS keeps its new deadline. - `_insert_values` takes keyword-only `subject` / `last_seen_at`; `_row_to_item` / `_SELECT_COLS` carry both. services/ask_service.py: `clear_platform(ids, *, reason, batch_id=None)` mirroring `expire()` — no Actor, `PLATFORM_ENDING_REASONS = (condition_cleared, superseded)` enforced, one `platform_cleared` audit row, one thin `operator_queue_cancelled` trigger per agent, observers get only the rows this call won. database.py facade for the four accessors. `test_ent329_operator_resume.py` G2 list gains the new CAS accessor. #3130 / #3220: `_own_pending_conds`, the flood guard and `create_bounded_alert_outcome` are untouched; their suites stay green. Test (red first — the accessors did not exist): tests/unit/test_3246_platform_alerts_db.py on the unit island's real SQLite: two readings → one row, bare repeat → unchanged, person-ended row never overwritten, budget refuses only with nothing to update, the index refuses a second pending row, `mark_expired` keeps a refreshed row and still expires an overdue one, the platform ending skips a person-ended row and hands observers only CAS-won rows. Co-Authored-By: Claude Fable 5.1 --- src/backend/database.py | 18 ++ src/backend/db/operator_queue.py | 212 +++++++++++++++++++ src/backend/services/ask_service.py | 52 +++++ tests/unit/test_3246_platform_alerts_db.py | 225 +++++++++++++++++++++ tests/unit/test_ent329_operator_resume.py | 6 +- 5 files changed, 512 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_3246_platform_alerts_db.py diff --git a/src/backend/database.py b/src/backend/database.py index e3ae2796d1..6f6857384f 100644 --- a/src/backend/database.py +++ b/src/backend/database.py @@ -3901,6 +3901,24 @@ def mark_operator_queue_acknowledged(self, agent_name, item_id): # #1631: agent-scoped — item_id is the agent's request_id, not the uuid. return self._operator_queue_ops.mark_acknowledged(agent_name, item_id) + # #3246 — platform alerts: one pending row per (agent, subject) + def find_pending_operator_queue_by_subject(self, agent_name, subject): + return self._operator_queue_ops.find_pending_by_subject(agent_name, subject) + + def find_person_ended_operator_queue_by_subject(self, agent_name, subject, since): + return self._operator_queue_ops.find_person_ended_by_subject(agent_name, subject, since) + + def create_platform_operator_queue_item(self, agent_name, item, *, subject, + max_pending_for_type=None): + # Returns {"outcome": created|updated|refused_at_budget, "row", "changed"}. + return self._operator_queue_ops.create_platform_item( + agent_name, item, subject=subject, max_pending_for_type=max_pending_for_type, + ) + + def end_operator_queue_items_by_platform(self, ids, *, reason, batch_id=None): + # Returns {"batch_id", "rows"} — the rows THIS call ended (CAS-won). + return self._operator_queue_ops.end_items_by_platform(ids, reason=reason, batch_id=batch_id) + def mark_operator_queue_expired(self): # trinity-enterprise#611: returns the rows this sweep ended (was a count). return self._operator_queue_ops.mark_expired() diff --git a/src/backend/db/operator_queue.py b/src/backend/db/operator_queue.py index e7fad0ce54..3f489572fc 100644 --- a/src/backend/db/operator_queue.py +++ b/src/backend/db/operator_queue.py @@ -19,6 +19,7 @@ from datetime import datetime from sqlalchemy import select, update, func, and_, or_, case, delete +from sqlalchemy.exc import IntegrityError from .engine import get_engine, make_insert from .tables import operator_queue @@ -172,6 +173,9 @@ def _row_to_item(row) -> Dict: "resolved_to": json.loads(row["resolved_to"]) if row["resolved_to"] else None, "proposal": json.loads(row["proposal"]) if row["proposal"] else None, "supersedes_expired": row["supersedes_expired"], + # #3246 — the platform alert's condition key and its latest reading + "subject": row["subject"], + "last_seen_at": row["last_seen_at"], } # Columns selected for a full queue-item record, in the canonical order. @@ -217,6 +221,8 @@ def _row_to_item(row) -> Dict: operator_queue.c.resolved_to, operator_queue.c.proposal, operator_queue.c.supersedes_expired, + operator_queue.c.subject, # #3246 — platform alerts: kind:key + operator_queue.c.last_seen_at, ) def create_item( @@ -391,6 +397,8 @@ def _insert_values( *, channel: Optional[str], raised_by: Optional[str], + subject: Optional[str] = None, + last_seen_at: Optional[str] = None, ) -> Tuple[str, Dict]: """The DB-sink belts and the column values of a new row, shared by every create so the belts cannot drift between the file and native paths.""" @@ -477,9 +485,208 @@ def _insert_values( # it does not decide it, and it must never derive it from `context` # (which is agent-authored). addressed_to_email=item.get("addressed_to_email"), + # #3246: platform-owned, keyword-only — never read from `item`. + subject=subject, + last_seen_at=last_seen_at, ) return request_id, values + # ------------------------------------------------------------------ + # #3246 — platform alerts: one pending row per (agent, subject) + # ------------------------------------------------------------------ + _RENDERED_FIELDS = ("title", "question", "priority") + + def find_pending_by_subject(self, agent_name: str, subject: str) -> Optional[Dict]: + """The one pending row for `(agent, subject)`, or None (#3246).""" + with get_engine().connect() as conn: + row = self._find_pending_by_subject(conn, agent_name, subject) + return self._row_to_item(row) if row else None + + def _find_pending_by_subject(self, conn, agent_name: str, subject: str): + return conn.execute( + select(*self._SELECT_COLS).where(and_( + operator_queue.c.agent_name == agent_name, + operator_queue.c.subject == subject, + operator_queue.c.status == "pending", + )) + ).mappings().first() + + def find_person_ended_by_subject( + self, agent_name: str, subject: str, since: str, + ) -> Optional[Dict]: + """The newest row for `(agent, subject)` a PERSON ended at or after + `since` (ISO-Z text, Invariant #16), any terminal status — the seam's + snooze read (#3246). Served by `idx_operator_queue_agent_subject`.""" + stmt = ( + select(*self._SELECT_COLS) + .where(and_( + operator_queue.c.agent_name == agent_name, + operator_queue.c.subject == subject, + operator_queue.c.status != "pending", + operator_queue.c.disposed_by == "person", + operator_queue.c.disposed_at.isnot(None), + operator_queue.c.disposed_at >= since, + )) + .order_by(operator_queue.c.disposed_at.desc(), operator_queue.c.id.desc()) + .limit(1) + ) + with get_engine().connect() as conn: + row = conn.execute(stmt).mappings().first() + return self._row_to_item(row) if row else None + + def create_platform_item( + self, + agent_name: str, + item: Dict, + *, + subject: Optional[str], + max_pending_for_type: Optional[int] = None, + ) -> Dict: + """Record a platform reading: find → touch → count → insert, in ONE + locked transaction (#3246). + + `subject` is the condition key (`kind:key`); None is an event, which is + always a new row. With a subject, the pending row for `(agent, subject)` + is updated in place — title / question / priority / context / + `last_seen_at` / `expires_at`, `context.seen_count` + 1 — by a + compare-and-set on `status = 'pending'`: a row a person ended first is + never overwritten (the lock covers creates, not `respond_to_item`), and + a lost CAS falls through to a fresh row. The find runs BEFORE the + #1677 per-type count, so a would-be update is never refused at budget; + `max_pending_for_type` is the cap for a budgeted kind (None: unbudgeted). + + Returns `{"outcome": "created" | "updated" | "refused_at_budget", + "row", "changed"}`; `changed` is whether a RENDERED field (title / + question / priority / context minus `seen_count`) moved, so a bare + repeat reading sends no broadcast. The partial unique index + `uq_operator_queue_pending_subject` is the backstop for a lock that + failed open: its `IntegrityError` is caught and the call re-finds and + touches the row that won. + """ + now = utc_now_iso() + for attempt in (1, 2): + try: + with get_engine().begin() as conn: + self._lock_agent_for_create(conn, agent_name) + if subject is not None: + current = self._find_pending_by_subject(conn, agent_name, subject) + if current is not None: + touched = self._touch(conn, current, item, now=now) + if touched is not None: + return touched + if attempt == 2: + # The index said a pending row existed and it is gone + # (ended between the two transactions): fall through. + pass + if max_pending_for_type is not None: + pending = conn.execute( + select(func.count()).where(and_( + *_own_pending_conds(agent_name), + operator_queue.c.type == item.get("type", "question"), + )) + ).scalar() or 0 + if pending >= max_pending_for_type: + return {"outcome": "refused_at_budget", "row": None, "changed": False} + context = item.get("context") + context = dict(context) if isinstance(context, dict) else {} + if subject is not None: + context["seen_count"] = 1 + values_item = {**item, "context": context or None} + request_id, values = self._insert_values( + agent_name, values_item, channel=None, raised_by=None, + subject=subject, last_seen_at=now, + ) + conn.execute( + make_insert(operator_queue).values(**values).on_conflict_do_nothing( + index_elements=["agent_name", "request_id"]) + ) + row = conn.execute(select(*self._SELECT_COLS).where(and_( + operator_queue.c.agent_name == agent_name, + operator_queue.c.request_id == request_id, + ))).mappings().first() + return {"outcome": "created", "row": self._row_to_item(row), "changed": True} + except IntegrityError: + if subject is None or attempt == 2: + raise + # the partial unique index won the race the lock did not cover: + # a pending row for the subject now exists — re-find and touch it. + continue + + def _touch(self, conn, current, item: Dict, *, now: str) -> Optional[Dict]: + """Update the pending row `current` in place; None when the CAS lost.""" + old_context = json.loads(current["context"]) if current["context"] else {} + if not isinstance(old_context, dict): + old_context = {} + new_context = item.get("context") + new_context = dict(new_context) if isinstance(new_context, dict) else {} + seen = old_context.get("seen_count") + new_context["seen_count"] = (seen if isinstance(seen, int) and seen > 0 else 1) + 1 + changed = any( + (item.get(f) if item.get(f) is not None else current[f]) != current[f] + for f in self._RENDERED_FIELDS + ) or {k: v for k, v in old_context.items() if k != "seen_count"} != {k: v for k, v in new_context.items() if k != "seen_count"} + context_json = json.dumps(new_context) + if len(context_json.encode("utf-8")) > _DB_BELT_CONTEXT_MAX_BYTES: + raise ValueError(f"operator-queue 'context' exceeds {_DB_BELT_CONTEXT_MAX_BYTES} bytes") + result = conn.execute( + update(operator_queue) + .where(and_(operator_queue.c.id == current["id"], operator_queue.c.status == "pending")) + .values( + title=item.get("title") or current["title"], + question=item.get("question") or current["question"], + priority=item.get("priority") or current["priority"], + context=context_json, + last_seen_at=now, + expires_at=_iso_z_deadline(item.get("expires_at")), + ) + ) + if not result.rowcount: + return None + row = conn.execute( + select(*self._SELECT_COLS).where(operator_queue.c.id == current["id"]) + ).mappings().first() + return {"outcome": "updated", "row": self._row_to_item(row), "changed": bool(changed)} + + def end_items_by_platform( + self, ids: List[str], *, reason: str, batch_id: Optional[str] = None, + ) -> Dict: + """End the listed pending rows as the PLATFORM (#3246): the + `bulk_cancel_items` shape — one compare-and-set UPDATE on + `status = 'pending'` stamping the ent#611 ledger with + `disposed_by = 'platform'`, no email — re-selected by `batch_id`, so + the rows returned are exactly the ones THIS call ended; a row a person + ended first is skipped, never re-ended. Returns `{"batch_id", "rows"}`; + `batch_id` is None when nothing was ended.""" + empty = {"batch_id": None, "rows": []} + ids = list(dict.fromkeys(ids or ())) + if not ids: + return empty + batch_id = batch_id or uuid.uuid4().hex + now = utc_now_iso() + with get_engine().begin() as conn: + result = conn.execute( + update(operator_queue) + .where(and_(operator_queue.c.status == "pending", operator_queue.c.id.in_(ids))) + .values( + status="cancelled", + disposition="cancelled", + disposed_at=now, + disposed_by="platform", + disposed_by_email=None, + disposition_reason=reason, + batch_id=batch_id, + ) + ) + if result.rowcount == 0: + return empty + rows = conn.execute( + select(*self._SELECT_COLS).where(and_( + operator_queue.c.id.in_(ids), + operator_queue.c.batch_id == batch_id, + )) + ).mappings().all() + return {"batch_id": batch_id, "rows": [self._row_to_item(r) for r in rows]} + def get_item(self, item_id: str) -> Optional[Dict]: """Get a single queue item by ID.""" stmt = select(*self._SELECT_COLS).where(operator_queue.c.id == item_id) @@ -1218,6 +1425,11 @@ def mark_expired(self) -> List[Dict]: and_( operator_queue.c.id == item_id, operator_queue.c.status == "pending", + # #3246: a platform alert refreshed between the + # candidate select and this CAS moved its deadline; + # it is not expired, and the refresh wins. + operator_queue.c.expires_at.isnot(None), + operator_queue.c.expires_at < now, ) ) .values( diff --git a/src/backend/services/ask_service.py b/src/backend/services/ask_service.py index c3a49a22fd..763d25d746 100644 --- a/src/backend/services/ask_service.py +++ b/src/backend/services/ask_service.py @@ -328,6 +328,58 @@ def expire() -> Ending: return _ended(EndingEvent(EXPIRED, tuple(rows), None), audit, None) +# #3246: the platform ends its own alerts for exactly these reasons — DATA on +# the ledger (`disposition_reason`), rendered by name on the card. A person's +# cancel reason is free text; the platform's is a closed vocabulary. +CONDITION_CLEARED = "condition_cleared" +SUPERSEDED = "superseded" +PLATFORM_ENDING_REASONS = (CONDITION_CLEARED, SUPERSEDED) + + +def clear_platform(ids: Iterable[str], *, reason: str, batch_id: Optional[str] = None) -> Ending: + """The PLATFORM ended its own alerts: the condition cleared, or a newer + reading superseded them (#3246). Mirrors `expire`: no Actor, the ending is + `cancelled` / `disposed_by = 'platform'` / NULL email, and only the rows + this call won the compare-and-set for are returned and handed to the + observers — a row a person ended first is skipped, never re-ended. One + audit row (`platform_cleared`) and one thin `operator_queue_cancelled` + trigger per agent; listeners refetch through the access-controlled list. + """ + if reason not in PLATFORM_ENDING_REASONS: + raise ValueError(f"platform ending reason must be one of {PLATFORM_ENDING_REASONS}, got {reason!r}") + ids = list(dict.fromkeys(ids)) + out = db.end_operator_queue_items_by_platform(ids, reason=reason, batch_id=batch_id) + rows, batch_id = out["rows"], out["batch_id"] + if not rows: + return Ending(rows=[], batch_id=None) + audit = [{ + "event_action": "platform_cleared", + "source": "system", + "target_type": "operator_queue", + "details": { + "batch_id": batch_id, + "reason": reason, + "cancelled": len(rows), + "skipped": len(ids) - len(rows), + "ids": [r["id"] for r in rows], + "agent_names": sorted({r["agent_name"] for r in rows}), + }, + }] + agents = list(dict.fromkeys(r["agent_name"] for r in rows)) + triggers = [_broadcast_payload({"type": "operator_queue_cancelled", + "data": {"agent_name": agent, "batch_id": batch_id}}) + for agent in agents] + for extra in triggers[1:]: + try: + operator_resume_service.spawn_on_loop(lambda extra=extra: _announce([], extra)) + except Exception: # noqa: BLE001 — the ending is committed; it must stand + logger.warning("[AskService] could not schedule a platform-clear trigger", exc_info=True) + return _ended( + EndingEvent(CANCELLED, tuple(rows), None, reason=reason, batch_id=batch_id), + audit, triggers[0], batch_id=batch_id, + ) + + # --------------------------------------------------------------------------- # The one way an ask begins (PR B): an agent — or, later, a gate — raises it # --------------------------------------------------------------------------- diff --git a/tests/unit/test_3246_platform_alerts_db.py b/tests/unit/test_3246_platform_alerts_db.py new file mode 100644 index 0000000000..36de08b8e9 --- /dev/null +++ b/tests/unit/test_3246_platform_alerts_db.py @@ -0,0 +1,225 @@ +""" +#3246 C3 — the locked platform-alert accessors and the platform ending. + +Against the unit island's real SQLite (`init_database()` schema, the #3130 +`real_db` shape). What is proved here, each at the layer it lives in: + +* `create_platform_item` is find → touch → count → insert in ONE locked + transaction: two readings of a subject are one pending row, the second + updates in place (`changed` says whether a rendered field moved), a + budgeted kind is refused only when there is nothing to update, and the + partial unique index can never surface as an error. +* The touch is a compare-and-set on `status = 'pending'`: a person's ending + is never overwritten; the next reading opens a fresh row. +* `mark_expired` no longer expires a row refreshed between its candidate + select and its per-id compare-and-set (`expires_at < now` joins the CAS). +* `end_items_by_platform` / `ask_service.clear_platform` end only the rows + they win, in the ent#611 vocabulary authored by the platform, and the + observers receive exactly those rows. +""" +import os +import sys +import uuid +from datetime import datetime, timedelta, timezone + +import pytest + +_BACKEND = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "src", "backend")) +if _BACKEND not in sys.path: + sys.path.insert(0, _BACKEND) + +pytestmark = pytest.mark.unit + + +def _iso(dt): + return dt.astimezone(timezone.utc).strftime("%Y-%m-%dT%H:%M:%S.%fZ") + + +def _item(rid, *, title="t", question="q", priority="high", context=None, expires_in=14, type_="alert"): + exp = _iso(datetime.now(timezone.utc) + timedelta(days=expires_in)) if expires_in else None + return {"id": rid, "type": type_, "status": "pending", "priority": priority, + "title": title, "question": question, "context": context or {}, "expires_at": exp} + + +@pytest.fixture +def real_db(): + from database import db + return db + + +@pytest.fixture +def agent(): + return f"agent-3246-{uuid.uuid4().hex[:6]}" + + +@pytest.fixture +def subject(): + return f"circuit_dormant:{uuid.uuid4().hex[:6]}" + + +class TestCreatePlatformItem: + def test_two_readings_are_one_pending_row_updated_in_place(self, real_db, agent, subject): + first = real_db.create_platform_operator_queue_item( + agent, _item("cb-1", context={"k": 1}), subject=subject) + assert first["outcome"] == "created" and first["changed"] is True + row = first["row"] + assert row["subject"] == subject and row["last_seen_at"] and row["context"]["seen_count"] == 1 + + second = real_db.create_platform_operator_queue_item( + agent, _item("cb-2", title="t2", priority="critical", context={"k": 2}), subject=subject) + assert second["outcome"] == "updated" and second["changed"] is True + assert second["row"]["id"] == row["id"] + assert second["row"]["request_id"] == "cb-1" # the row keeps its first id + assert second["row"]["title"] == "t2" and second["row"]["priority"] == "critical" + assert second["row"]["context"] == {"k": 2, "seen_count": 2} + assert second["row"]["last_seen_at"] >= row["last_seen_at"] + pending = real_db.list_operator_queue_items(agent_name=agent, status="pending") + assert len(pending) == 1 + + def test_a_bare_repeat_reading_is_not_a_change(self, real_db, agent, subject): + real_db.create_platform_operator_queue_item(agent, _item("cb-1", context={"k": 1}), subject=subject) + again = real_db.create_platform_operator_queue_item(agent, _item("cb-2", context={"k": 1}), subject=subject) + assert again["outcome"] == "updated" and again["changed"] is False + assert again["row"]["context"]["seen_count"] == 2 + + def test_find_pending_by_subject(self, real_db, agent, subject): + assert real_db.find_pending_operator_queue_by_subject(agent, subject) is None + real_db.create_platform_operator_queue_item(agent, _item("cb-1"), subject=subject) + found = real_db.find_pending_operator_queue_by_subject(agent, subject) + assert found and found["request_id"] == "cb-1" + assert real_db.find_pending_operator_queue_by_subject("someone-else", subject) is None + + def test_subject_none_always_inserts(self, real_db, agent): + a = real_db.create_platform_operator_queue_item(agent, _item("ev-1"), subject=None) + b = real_db.create_platform_operator_queue_item(agent, _item("ev-2"), subject=None) + assert a["outcome"] == b["outcome"] == "created" and a["row"]["id"] != b["row"]["id"] + assert a["row"]["subject"] is None + + def test_update_never_overwrites_a_row_a_person_ended(self, real_db, agent, subject): + first = real_db.create_platform_operator_queue_item(agent, _item("cb-1"), subject=subject) + real_db.respond_to_operator_queue_item(first["row"]["id"], "ack", "got it", None, "p@example.com") + ended = real_db.get_operator_queue_item(first["row"]["id"]) + assert ended["status"] == "responded" and ended["disposed_by"] == "person" + + second = real_db.create_platform_operator_queue_item(agent, _item("cb-2", title="t2"), subject=subject) + assert second["outcome"] == "created" and second["row"]["id"] != first["row"]["id"] + still = real_db.get_operator_queue_item(first["row"]["id"]) + assert still["status"] == "responded" and still["title"] == "t" # the person's ending stands + + def test_find_person_ended_by_subject(self, real_db, agent, subject): + first = real_db.create_platform_operator_queue_item(agent, _item("cb-1"), subject=subject) + since = _iso(datetime.now(timezone.utc) - timedelta(days=7)) + assert real_db.find_person_ended_operator_queue_by_subject(agent, subject, since) is None + real_db.respond_to_operator_queue_item(first["row"]["id"], "ack", None, None, "p@example.com") + found = real_db.find_person_ended_operator_queue_by_subject(agent, subject, since) + assert found and found["id"] == first["row"]["id"] + future = _iso(datetime.now(timezone.utc) + timedelta(minutes=1)) + assert real_db.find_person_ended_operator_queue_by_subject(agent, subject, future) is None + + def test_budget_refuses_only_when_there_is_nothing_to_update(self, real_db, agent): + for i in range(2): + out = real_db.create_platform_operator_queue_item( + agent, _item(f"b-{i}", type_="skill_not_found"), subject=f"skill_not_found:{i}", + max_pending_for_type=2) + assert out["outcome"] == "created" + refused = real_db.create_platform_operator_queue_item( + agent, _item("b-9", type_="skill_not_found"), subject="skill_not_found:9", max_pending_for_type=2) + assert refused == {"outcome": "refused_at_budget", "row": None, "changed": False} + # a reading of an EXISTING subject is an update, charged no budget + upd = real_db.create_platform_operator_queue_item( + agent, _item("b-x", type_="skill_not_found", title="t2"), subject="skill_not_found:0", + max_pending_for_type=2) + assert upd["outcome"] == "updated" + + def test_a_second_pending_row_for_a_subject_is_impossible(self, real_db, agent, subject): + import sqlalchemy.exc + from db.engine import get_engine, make_insert + from db.tables import operator_queue + real_db.create_platform_operator_queue_item(agent, _item("cb-1"), subject=subject) + with pytest.raises(sqlalchemy.exc.IntegrityError): + with get_engine().begin() as conn: + conn.execute(make_insert(operator_queue).values( + id=uuid.uuid4().hex, agent_name=agent, request_id="raw", type="alert", + status="pending", priority="high", title="t", question="q", + created_at=_iso(datetime.now(timezone.utc)), subject=subject)) + + +class TestMarkExpired: + def test_a_refresh_between_select_and_cas_keeps_the_row(self, real_db, agent, subject, monkeypatch): + from db import operator_queue as oq + past = _iso(datetime.now(timezone.utc) - timedelta(minutes=5)) + item = _item("cb-1"); item["expires_at"] = past + created = real_db.create_platform_operator_queue_item(agent, item, subject=subject) + row_id = created["row"]["id"] + + real_select = oq.select + state = {"refreshed": False} + + def _refreshing_select(*cols, **kw): + # after the candidate select has been built, refresh the row before the CAS runs + stmt = real_select(*cols, **kw) + if not state["refreshed"] and cols and getattr(cols[0], "name", None) == "id": + state["refreshed"] = True + real_db.create_platform_operator_queue_item(agent, _item("cb-2"), subject=subject) + return stmt + + monkeypatch.setattr(oq, "select", _refreshing_select) + ended_ids = {r["id"] for r in real_db.mark_operator_queue_expired()} + assert state["refreshed"] + assert row_id not in ended_ids + assert real_db.get_operator_queue_item(row_id)["status"] == "pending" + + def test_a_row_past_its_deadline_still_expires(self, real_db, agent, subject): + item = _item("cb-1"); item["expires_at"] = _iso(datetime.now(timezone.utc) - timedelta(minutes=5)) + row_id = real_db.create_platform_operator_queue_item(agent, item, subject=subject)["row"]["id"] + assert row_id in {r["id"] for r in real_db.mark_operator_queue_expired()} + assert real_db.get_operator_queue_item(row_id)["disposed_by"] == "timeout" + + +class TestPlatformEnding: + def test_end_items_by_platform_wins_only_pending_rows(self, real_db, agent, subject): + a = real_db.create_platform_operator_queue_item(agent, _item("cb-1"), subject=subject)["row"] + b = real_db.create_platform_operator_queue_item(agent, _item("cb-2"), subject=subject + "b")["row"] + real_db.respond_to_operator_queue_item(b["id"], "ack", None, None, "p@example.com") + out = real_db.end_operator_queue_items_by_platform([a["id"], b["id"], "nope"], reason="condition_cleared") + assert [r["id"] for r in out["rows"]] == [a["id"]] and out["batch_id"] + ended = real_db.get_operator_queue_item(a["id"]) + assert (ended["status"], ended["disposition"], ended["disposed_by"], ended["disposed_by_email"], + ended["disposition_reason"], ended["batch_id"]) == \ + ("cancelled", "cancelled", "platform", None, "condition_cleared", out["batch_id"]) + assert real_db.get_operator_queue_item(b["id"])["disposed_by"] == "person" + assert real_db.end_operator_queue_items_by_platform([], reason="superseded") == {"batch_id": None, "rows": []} + + def test_clear_platform_mirrors_expire(self, real_db, agent, subject, monkeypatch): + from services import ask_service + a = real_db.create_platform_operator_queue_item(agent, _item("cb-1"), subject=subject)["row"] + b = real_db.create_platform_operator_queue_item(agent, _item("cb-2"), subject=subject + "b")["row"] + real_db.respond_to_operator_queue_item(b["id"], "ack", None, None, "p@example.com") + + seen, scheduled = [], [] + ask_service.register_ending_observer(seen.append) + monkeypatch.setattr(ask_service.operator_resume_service, "spawn_on_loop", scheduled.append) + try: + ending = ask_service.clear_platform([a["id"], b["id"]], reason="condition_cleared") + finally: + ask_service._observers.remove(seen.append) + assert [r["id"] for r in ending.rows] == [a["id"]] and ending.batch_id and ending.observers_ok + assert len(seen) == 1 + event = seen[0] + assert event.disposition == "cancelled" and event.actor_email is None + assert event.reason == "condition_cleared" and event.batch_id == ending.batch_id + assert [r["id"] for r in event.rows] == [a["id"]] + assert len(scheduled) == 1 # one announcement: audit + thin per-agent trigger + + def test_clear_platform_refuses_a_reason_outside_the_vocabulary(self, real_db): + from services import ask_service + assert set(ask_service.PLATFORM_ENDING_REASONS) == {"condition_cleared", "superseded"} + with pytest.raises(ValueError): + ask_service.clear_platform(["x"], reason="because") + + def test_clear_platform_with_nothing_won_announces_nothing(self, real_db, monkeypatch): + from services import ask_service + scheduled = [] + monkeypatch.setattr(ask_service.operator_resume_service, "spawn_on_loop", scheduled.append) + ending = ask_service.clear_platform(["nope"], reason="superseded") + assert ending.rows == [] and ending.batch_id is None and scheduled == [] diff --git a/tests/unit/test_ent329_operator_resume.py b/tests/unit/test_ent329_operator_resume.py index bf7622cc98..1de859b77d 100644 --- a/tests/unit/test_ent329_operator_resume.py +++ b/tests/unit/test_ent329_operator_resume.py @@ -412,7 +412,11 @@ def test_trigger_counts_as_autonomous(): _SINK = "services/ask_service.py" # The set-based CAS writers: their RETURN is the set of rows this call ended # (bulk: the sweep's own batch_id re-select; expiry: per-id CAS winners). -_SET_CAS_ACCESSORS = ("bulk_cancel_operator_queue_items(", "mark_operator_queue_expired(") +_SET_CAS_ACCESSORS = ( + "bulk_cancel_operator_queue_items(", + "mark_operator_queue_expired(", + "end_operator_queue_items_by_platform(", # #3246 — the platform ending +) def _dispatch_call_sites(spawner: str = "spawn_resume_dispatch(") -> list[pathlib.Path]: From 7055064ed3c4c47b2af270e5baf5f02bbdb3e0f4 Mon Sep 17 00:00:00 2001 From: Trinity Agent Date: Mon, 5 Oct 2026 13:04:30 -0400 Subject: [PATCH 04/19] test(operator-queue): register the #3246 C2/C3 unit files (#3246) Co-Authored-By: Claude Fable 5.1 --- tests/registry.json | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/registry.json b/tests/registry.json index adcdf1b262..7b71eaf4bc 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -1,5 +1,28 @@ { "test_files": [ + { + "file": "unit/test_3246_platform_alert_sweep.py", + "feature": "#3246", + "added": "2026-10-05", + "categories": [ + "backend", + "operator-queue", + "migrations", + "unit" + ], + "description": "#3246 C2: the upgrade sweep, driven as the registered SQLite migration (`platform_alert_subjects`) against the real schema.py operator_queue DDL with the two new columns and indexes stripped (a pre-upgrade install holding duplicates). Ordered columns → sweep → index: `subject` / `last_seen_at` added PRAGMA-guarded; the newest pending row per derived subject survives stamped (subject, last_seen_at, context.seen_count=1, expires_at only where NULL — opt-out kinds keep NULL) and the rest end as ONE batch (cancelled / platform / superseded / NULL email / one batch_id); known kinds without a derivable subject get a lifetime stamp only (never merged on a guess); person-ended rows inside the 7-day snooze window get subject only; agent-raised, gate- and external-prefix rows untouched; a second run changes nothing; the partial unique index exists only after the run, bites on a second pending row, and matches schema.py's DDL; both tracks registered after the #3130 flood sweep, tables.py declares columns and both indexes. No Docker." + }, + { + "file": "unit/test_3246_platform_alerts_db.py", + "feature": "#3246", + "added": "2026-10-05", + "categories": [ + "backend", + "operator-queue", + "unit" + ], + "description": "#3246 C3: the locked platform-alert accessors and the platform ending, on the unit island's real SQLite. `create_platform_item` is find → touch → count → insert in one locked transaction: two readings of a subject are one pending row updated in place (seen_count+1, rendered fields, last_seen_at), a bare repeat is `changed=False`, a budgeted kind is refused only when there is nothing to update, `subject=None` always inserts, and a person-ended row is never overwritten (the next reading opens a fresh row); the partial unique index refuses a second pending row. `mark_expired` keeps a row refreshed between its candidate select and its per-id CAS and still expires an overdue one. `end_items_by_platform` / `ask_service.clear_platform` end only CAS-won rows in the ent#611 vocabulary authored by the platform (no actor), refuse a reason outside `PLATFORM_ENDING_REASONS`, announce once per call, and hand observers exactly the rows won. No Docker." + }, { "file": "unit/test_3246_platform_alerts_leaf.py", "feature": "#3246", From 86bf259d158cbb668ba29b03a76768fcf6a34127 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 13:30:22 -0400 Subject: [PATCH 05/19] feat(operator-queue): platform alert seam, reserved prefixes and the emitter ratchet (#3246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C4. `platform_alerts.observe / clear / reconcile` on top of C3's locked accessors: one pending row per (agent, subject), updated in place; the kind's lifetime as `expires_at`; the T5 rule (after a person ended a subject's row, nothing is filed for 7 days unless the priority rose or a material key changed); never raises. Imports stay function-local, so the module is still a stdlib-only leaf at import time. `skills-reconcile-`, `skills-fleet-reinject-`, `retention-guard-` and `ent615-git-token-scrub-` become reserved prefixes. Guards (test_3246_platform_alerts_registry.py): two-way prefix parity, the EMITTERS_NOT_YET_ON_SEAM ratchet (every platform emitter not yet on the seam, `create_bounded_alert_outcome` included, each naming its follow-up), the clear-site check, and the caller guard on the #3246 accessors. Also places C3's `subject` / `last_seen_at` on the machine side of the ent#715 row allowlist (not person data); test_ent715 re-pinned — it was red since C3. Co-Authored-By: Claude Opus 5.5 --- src/backend/routers/operator_queue.py | 1 + .../services/operator_queue_service.py | 8 + src/backend/services/platform_alerts.py | 154 +++++++++ tests/registry.json | 22 ++ .../test_3246_platform_alerts_registry.py | 297 ++++++++++++++++++ tests/unit/test_3246_platform_alerts_seam.py | 170 ++++++++++ tests/unit/test_ent715_queue_person_fields.py | 1 + 7 files changed, 653 insertions(+) create mode 100644 tests/unit/test_3246_platform_alerts_registry.py create mode 100644 tests/unit/test_3246_platform_alerts_seam.py diff --git a/src/backend/routers/operator_queue.py b/src/backend/routers/operator_queue.py index 5d809c1c3d..8889ccc77c 100644 --- a/src/backend/routers/operator_queue.py +++ b/src/backend/routers/operator_queue.py @@ -95,6 +95,7 @@ def _assert_agent_accessible(agent_name: str, accessible: Optional[Set[str]]) -> "disposition", "disposed_at", "disposed_by", "disposition_reason", "batch_id", "raised_by", "channel", "to_role", "proposal", "supersedes_expired", "aging", "aged_since", + "subject", "last_seen_at", # #3246: a platform alert's condition key + latest reading ) diff --git a/src/backend/services/operator_queue_service.py b/src/backend/services/operator_queue_service.py index 18dcf985ed..eb77503d51 100644 --- a/src/backend/services/operator_queue_service.py +++ b/src/backend/services/operator_queue_service.py @@ -230,6 +230,14 @@ # `~/.trinity/operator-queue.json`. Correct: the sweep alarm is a platform # alarm ABOUT the agent, not a loop the agent opened and is waiting on. "gitignore-untracked-", + # #3246: four platform families that were never reserved. Reserving them + # also makes `is_platform_minted` true for their rows, which keeps them out + # of the agent's own file and its answer wake — intended: each is an alarm + # ABOUT the agent (or the install), not a loop the agent opened. + "skills-reconcile-", # skill_service reconcile refusal (ent#236) + "skills-fleet-reinject-", # skills_sync_service fleet re-inject failures (ent#236) + "retention-guard-", # retention_guard prune refusal (#1644) + "ent615-git-token-scrub-", # git_service/token_scrub refusal + unreadable (ent#615) ROLE_DRIFT_ALERT_PREFIX, # role-assignment drift (trinity-enterprise#500) — # the role file lives in the AGENT'S OWN workspace, so # an unreserved prefix would let it pre-create the id diff --git a/src/backend/services/platform_alerts.py b/src/backend/services/platform_alerts.py index 9cb923076f..d275c06052 100644 --- a/src/backend/services/platform_alerts.py +++ b/src/backend/services/platform_alerts.py @@ -519,3 +519,157 @@ def plan_sweep(rows: Iterable[Mapping], *, now: datetime) -> SweepPlan: lifetime_stamps=tuple(lifetime_stamps), snooze_stamps=tuple(snooze_stamps), ) + + +# --------------------------------------------------------------------------- +# The seam: observe / clear / reconcile (#3246 C4) +# +# Every import below is function-local: this module stays a stdlib-only leaf +# at import time (the migration tracks import `plan_sweep` from it, and +# `operator_queue_service` calls back into it — a module-level import of the +# service graph would close the cycle E1 names). +# --------------------------------------------------------------------------- + +OBSERVED_CREATED = "created" +OBSERVED_UPDATED = "updated" +OBSERVED_SNOOZED = "snoozed" +OBSERVED_REFUSED_UNREGISTERED = "refused_unregistered" +OBSERVED_REFUSED_AT_BUDGET = "refused_at_budget" +OBSERVED_FAILED = "failed" + +_PRIORITY_RANK = {"low": 0, "medium": 1, "high": 2, "critical": 3} + + +def _snoozed_by(kind: Kind, ended: Optional[Mapping], priority: str, context: Mapping) -> bool: + """A person ended this subject's row inside the snooze window, and the new + reading is neither a priority increase nor a material change.""" + if not ended: + return False + if _PRIORITY_RANK.get(priority, 0) > _PRIORITY_RANK.get(ended.get("priority"), 0): + return False + old = _as_context(ended.get("context")) + return all(context.get(k) == old.get(k) for k in kind.material_keys) + + +def _spawn(factory: Callable[[], Any]) -> None: + try: + from services import operator_resume_service + operator_resume_service.spawn_on_loop(factory) + except Exception: # noqa: BLE001 — the row is committed; a lost trigger is not fatal + logger.warning("[platform-alerts] could not schedule a follow-up", exc_info=True) + + +async def _broadcast_sync() -> None: + """Thin trigger (#918: no payload); listeners refetch the access-controlled list.""" + from services import operator_queue_service as oqs + manager = oqs._websocket_manager + if manager: + await manager.broadcast(json.dumps({"type": "operator_queue_sync", "data": {}})) + + +def observe( + agent_name: str, + kind: str, + key: Any = None, + *, + title: str, + question: str, + priority: str = "high", + context: Optional[Mapping] = None, +) -> str: + """Report a new reading of a platform condition (#3246). Never raises. + + The subject's pending row is updated in place (one row per subject, the + latest reading); with no pending row, a reading a person ended inside the + snooze window (`OPERATOR_PLATFORM_ALERT_SNOOZE_DAYS`, default 7) files + nothing unless its priority rose or a `material_keys` value changed; else + a fresh row is filed with the kind's lifetime as `expires_at`, measured + from this reading. Returns one of the ``OBSERVED_*`` outcomes. + """ + try: + k = KINDS.get(kind) + if k is None: + logger.error("[platform-alerts] refusing an alert of unregistered kind %r for %s", + kind, agent_name) + return OBSERVED_REFUSED_UNREGISTERED + from database import db + from utils.helpers import utc_now_iso + + subject = subject_for(kind, key) + ctx = dict(context) if isinstance(context, Mapping) else {} + now = datetime.now(timezone.utc) + if subject is not None and db.find_pending_operator_queue_by_subject(agent_name, subject) is None: + since = _iso_z(now - snooze_window()) + ended = db.find_person_ended_operator_queue_by_subject(agent_name, subject, since) + if _snoozed_by(k, ended, priority, ctx): + return OBSERVED_SNOOZED + lifetime = lifetime_for(kind) + stem = _belt_key(key) + "-" if subject is not None else "" + item = { + "id": f"{k.prefix}{stem}{utc_now_iso()}", + "type": k.budgeted_type or "alert", + "status": "pending", + "priority": priority, + "title": title, + "question": question, + "context": ctx, + "created_at": utc_now_iso(), + "expires_at": _iso_z(now + lifetime) if lifetime is not None else None, + } + cap = None + if k.budgeted_type: + from services import operator_queue_service as oqs + cap = oqs.OPERATOR_ALERT_MAX_PENDING_PER_TYPE + out = db.create_platform_operator_queue_item( + agent_name, item, subject=subject, max_pending_for_type=cap) + if out["outcome"] == OBSERVED_REFUSED_AT_BUDGET: + from services import operator_queue_service as oqs + _spawn(lambda: oqs._maybe_emit_alert_budget_episode( + agent_name, k.budgeted_type, ctx.get("triggered_by"))) + return OBSERVED_REFUSED_AT_BUDGET + if out.get("changed"): + _spawn(_broadcast_sync) + return out["outcome"] + except Exception: # noqa: BLE001 — an alert must never break its emitter + logger.error("[platform-alerts] observe failed for %s/%s", agent_name, kind, exc_info=True) + return OBSERVED_FAILED + + +def _end(rows: Iterable[Mapping], reason: Optional[str]) -> int: + ids = [r["id"] for r in rows if r] + if not ids: + return 0 + from services import ask_service + return len(ask_service.clear_platform(ids, reason=reason or ask_service.CONDITION_CLEARED).rows) + + +def clear(agent_name: str, kind: str, key: Any = None, *, reason: Optional[str] = None) -> int: + """The condition behind `(agent, kind, key)` cleared: the platform ends its + pending row (ent#611 ledger, `disposed_by = 'platform'`). A row a person + ended first is untouched. Returns the number ended; never raises.""" + try: + subject = subject_for(kind, key) + if subject is None: + return 0 + from database import db + return _end([db.find_pending_operator_queue_by_subject(agent_name, subject)], reason) + except Exception: # noqa: BLE001 + logger.error("[platform-alerts] clear failed for %s/%s", agent_name, kind, exc_info=True) + return 0 + + +def reconcile(agent_name: str, kind: str, live_keys: Iterable[Any]) -> int: + """End every pending row of `kind` for `agent_name` whose key is not in + `live_keys` — for an emitter that knows the full current set rather than + each clear. Returns the number ended; never raises.""" + try: + _kind(kind) + live = {subject_for(kind, key) for key in live_keys} + from database import db + rows = db.list_operator_queue_items(agent_name=agent_name, status="pending", limit=1000) + stale = [r for r in rows + if (r.get("subject") or "").startswith(f"{kind}:") and r["subject"] not in live] + return _end(stale, None) + except Exception: # noqa: BLE001 + logger.error("[platform-alerts] reconcile failed for %s/%s", agent_name, kind, exc_info=True) + return 0 diff --git a/tests/registry.json b/tests/registry.json index 7b71eaf4bc..c62b2a645c 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -1,5 +1,27 @@ { "test_files": [ + { + "file": "unit/test_3246_platform_alerts_seam.py", + "feature": "#3246", + "added": "2026-10-05", + "categories": [ + "backend", + "operator-queue", + "unit" + ], + "description": "#3246 C4: platform_alerts.observe / clear / reconcile at the seam on the unit island's real SQLite — two readings are one row updated in place (fields, seen_count, last_seen_at, expires_at), the kind's lifetime is the deadline (30 d net, 14 d default, none for person kinds) and a real mark_expired ends it, rows wear their kind prefix and are platform-minted, an unregistered kind is refused and a DB fault returns FAILED without raising; clear ends the pending row as the platform (condition_cleared) and is a no-op otherwise; reconcile ends only subjects no longer live; T5 snooze: after a person's ending the same reading files nothing for 7 days, a priority increase or material change files a fresh row, a platform ending never snoozes. No Docker." + }, + { + "file": "unit/test_3246_platform_alerts_registry.py", + "feature": "#3246", + "added": "2026-10-05", + "categories": [ + "backend", + "operator-queue", + "unit" + ], + "description": "#3246 C4: the seam's static guards — two-way prefix parity between the kind registry and _RESERVED_ID_PREFIXES (the four newly reserved families are platform-minted); the EMITTERS_NOT_YET_ON_SEAM ratchet (every direct or #1677-budgeted platform emitter listed with its follow-up, no stale entries, prefixes registered); the clear-site check (every hook kind on the seam has a platform_alerts.clear/reconcile call naming it, or a named NO_CLEAR_SITE reason); the caller guard (only the seam calls the #3246 accessors). AST, no DB." + }, { "file": "unit/test_3246_platform_alert_sweep.py", "feature": "#3246", diff --git a/tests/unit/test_3246_platform_alerts_registry.py b/tests/unit/test_3246_platform_alerts_registry.py new file mode 100644 index 0000000000..c6a4cb5adb --- /dev/null +++ b/tests/unit/test_3246_platform_alerts_registry.py @@ -0,0 +1,297 @@ +""" +#3246 C4 — the platform-alert seam's static guards. + +* Two-way prefix parity: every registered kind's prefix is platform-reserved + (no agent can mint a row into it), and every reserved prefix is either a + registered kind's, an external (enterprise) emitter's, or the gate-ask + prefix (an ask, not an alert — #3247's territory). +* The ratchet: every platform emitter that still writes operator-queue rows + directly (or through the #1677 budget helper) is listed, with the follow-up + that moves it. It may only shrink: an emitter moved onto the seam leaves it + in the same commit, and a new direct emitter cannot land unlisted. +* The clear-site check: a `clears="hook"` kind that is on the seam has a + `clear(...)` / `reconcile(...)` call naming it — or a named reason why the + code has no place that knows the condition cleared. +* The caller guard: only the seam writes and ends platform rows through the + #3246 accessors. + +AST-shaped (the test_1677 idiom): a mention in a comment is never a call site. +Related flow: docs/memory/feature-flows/operating-room.md +""" +from __future__ import annotations + +import ast +import os +import sys +from pathlib import Path + +import pytest + +_BACKEND = Path(__file__).resolve().parents[2] / "src" / "backend" +if str(_BACKEND) not in sys.path: + sys.path.insert(0, str(_BACKEND)) + +pytestmark = pytest.mark.unit + +_EXCLUDE_DIRS = ("enterprise/",) + +_PR2 = "follow-up PR 2 of #3246 (the remaining emitters)" + +# --------------------------------------------------------------------------- +# The ratchet — every platform emitter NOT yet on the seam. Shrinks per PR; +# empty when PR 2 lands. Key: (path under src/backend, enclosing qualname); +# value: (the id prefix it writes, the follow-up that moves it). +# --------------------------------------------------------------------------- +EMITTERS_NOT_YET_ON_SEAM = { + ("services/subscription_headroom_alerts.py", "_emit"): ("sub-headroom-", "#3246 C5′"), + ("services/skill_service.py", "SkillService._record_adoption_failure"): + ("skills-legacy-adoption-", "#3246 C5′"), + ("services/system_agent_service.py", "SystemAgentService._emit_base_image_stale_alert"): + ("base-image-stale-", "#3246 C5′"), + ("services/system_agent_service.py", "SystemAgentService._emit_start_failed_alert"): + ("base-image-stale-start-", "#3246 C5′"), + ("services/agent_client/circuit.py", "_emit_dormant_alert"): ("cb-dormant-", "#3246 C5′"), + # -- PR 2 --------------------------------------------------------------- + ("services/skill_service.py", "SkillService._announce_reconcile_refusal"): + ("skills-reconcile-", _PR2), + ("services/skills_sync_service.py", "SkillsLibrarySyncService._announce_fleet_failures"): + ("skills-fleet-reinject-", _PR2), + ("services/sync_health_service.py", "SyncHealthService._emit_sync_failing_alert"): + ("sync-failing-", _PR2), + ("services/sync_health_service.py", "SyncHealthService._emit_sync_diverged_alert"): + ("sync-diverged-", _PR2), + ("services/sync_health_service.py", "SyncHealthService._emit_git_bloat_alert"): + ("git-bloat-", _PR2), + ("services/db_backup_service.py", "DBBackupService._emit_alarm"): ("db-backup-", _PR2), + ("services/archive_storage.py", "_alarm_unwritable_archive_dir"): ("log-archive-", _PR2), + ("services/retention_guard.py", "announce_refusal"): ("retention-guard-", _PR2), + ("services/system_seed_service.py", "SystemSeedService._notify_operator"): ("system-seed-", _PR2), + ("services/git_service/token_scrub.py", "_alarm_git_token_scrub_refused"): + ("ent615-git-token-scrub-", _PR2), + ("services/git_service/token_scrub.py", "_alarm_git_token_scrub_unreadable"): + ("ent615-git-token-scrub-unreadable-", _PR2), + ("services/lease_reaper_service.py", "_create_park_item"): ("poison-", _PR2), + ("services/validation_service.py", "ValidationService._notify_operator_on_failure"): + ("val_", _PR2), + ("client_portal/service.py", "_alert_collided_inbox"): ("portal-inbox-collision-", _PR2), + ("services/operator_queue_service.py", "_maybe_emit_alert_budget_episode"): + ("alert-budget-", _PR2), + # The #1677 budget helper stays as it is in PR 1; the budgeted emitters + # below call it and move onto the seam (which composes the budget) in PR 2. + ("services/operator_queue_service.py", "create_bounded_alert_outcome"): (None, _PR2), + ("services/operator_queue_service.py", "OperatorQueueSyncService._maybe_emit_flood_alert"): + ("queue-flood-", _PR2), + ("services/idempotency_service.py", "_on_unguarded_effect"): ("effect-unguarded-", _PR2), + ("services/task_execution_service.py", "_alert_skill_not_found"): ("skill-not-found-", _PR2), + ("services/git_service/gitignore_sweep.py", "_emit_gitignore_untracked_alert"): + ("gitignore-untracked-", _PR2), + ("client_portal/service.py", "raise_problem_report"): ("workspace-problem-", _PR2), +} + +# Writers that are not platform emitters: the facades, the agent-authored +# seams, and the bool view of the budget helper. +_NOT_EMITTERS = { + ("database.py", "DatabaseManager.create_operator_queue_item"), + ("database.py", "DatabaseManager.create_operator_queue_item_with_outcome"), + ("database.py", "DatabaseManager.create_native_operator_queue_item"), + ("services/ask_service.py", "raise_ask"), + ("services/operator_queue_service.py", "OperatorQueueSyncService._sync_agent"), + ("services/operator_queue_service.py", "create_bounded_alert"), +} + +# A hook kind on the seam whose emitter has no place that knows the condition +# cleared: kind -> the reason, so "reviewed" is told apart from "forgotten". +NO_CLEAR_SITE: dict = {} + +_DIRECT = ("create_operator_queue_item", "create_operator_queue_item_with_outcome", + "create_native_operator_queue_item") +_OPS = ("create_item", "create_item_with_outcome", "create_native_item") +_BOUNDED = ("create_bounded_alert", "create_bounded_alert_outcome") +# The #3246 accessors: only the seam (and their own facade / sink) may call them. +_SEAM_ONLY = { + "create_platform_operator_queue_item": {"services/platform_alerts.py"}, + "end_operator_queue_items_by_platform": {"services/ask_service.py"}, + "clear_platform": {"services/platform_alerts.py"}, + "create_platform_item": {"database.py"}, + "end_items_by_platform": {"database.py"}, +} + + +def _callee(call: ast.Call): + f = call.func + if isinstance(f, ast.Name): + return f.id, [] + if isinstance(f, ast.Attribute): + names, node = [], f.value + while isinstance(node, ast.Attribute): + names.append(node.attr) + node = node.value + if isinstance(node, ast.Name): + names.append(node.id) + return f.attr, names + return None, [] + + +def _walk(rel, text): + """(qualname, call) for every call in a module, qualname = enclosing defs.""" + try: + tree = ast.parse(text) + except SyntaxError: + return [("", None)] + out = [] + + def visit(node, stack): + for child in ast.iter_child_nodes(node): + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + visit(child, stack + [child.name]) + else: + if isinstance(child, ast.Call): + out.append((".".join(stack) or "", child)) + visit(child, stack) + + visit(tree, []) + return out + + +def _backend_files(token): + for path in sorted(_BACKEND.rglob("*.py")): + rel = path.relative_to(_BACKEND).as_posix() + if any(rel.startswith(d) for d in _EXCLUDE_DIRS): + continue + text = path.read_text(encoding="utf-8") + if any(t in text for t in token): + yield rel, text + + +def _emitter_sites(): + sites = set() + for rel, text in _backend_files(("operator_queue", "create_bounded_alert")): + for qual, call in _walk(rel, text): + if call is None: + sites.add((rel, qual)) + continue + name, receivers = _callee(call) + if name in _DIRECT or name in _BOUNDED or ( + name in _OPS and {"_operator_queue_ops", "OperatorQueueOperations"} & set(receivers)): + sites.add((rel, qual)) + return sites - _NOT_EMITTERS + + +def _seam_kind_calls(): + """{kind literal} for every `clear(` / `reconcile(` call outside the seam.""" + kinds = set() + for rel, text in _backend_files(("platform_alerts",)): + if rel == "services/platform_alerts.py": + continue + for _qual, call in _walk(rel, text): + if call is None: + continue + name, receivers = _callee(call) + if name not in ("clear", "reconcile") or "platform_alerts" not in receivers: + continue + arg = call.args[1] if len(call.args) > 1 else next( + (k.value for k in call.keywords if k.arg == "kind"), None) + if isinstance(arg, ast.Constant) and isinstance(arg.value, str): + kinds.add(arg.value) + return kinds + + +# --------------------------------------------------------------------------- +# Prefix parity +# --------------------------------------------------------------------------- + +def test_every_kind_prefix_is_reserved(): + from services import operator_queue_service as oqs + from services import platform_alerts as pa + reserved = oqs._RESERVED_ID_PREFIXES + loose = {k.name: k.prefix for k in pa.KINDS.values() + if not any(k.prefix.startswith(p) for p in reserved)} + assert not loose, f"kind prefixes an agent could author: {loose}" + + +def test_every_reserved_prefix_is_a_known_platform_family(): + from services import operator_queue_service as oqs + from services import platform_alerts as pa + known = {k.prefix for k in pa.KINDS.values()} | set(pa.EXTERNAL_PREFIXES) | {oqs.GATE_ASK_ID_PREFIX} + stray = [p for p in oqs._RESERVED_ID_PREFIXES if p not in known] + assert not stray, f"reserved prefixes with no registered kind: {stray}" + + +@pytest.mark.parametrize("prefix", ["skills-reconcile-", "skills-fleet-reinject-", + "retention-guard-", "ent615-git-token-scrub-"]) +def test_the_four_unreserved_families_are_now_platform_minted(prefix): + from services import operator_queue_service as oqs + assert oqs.is_platform_minted({"request_id": f"{prefix}x-1"}) + + +# --------------------------------------------------------------------------- +# The ratchet +# --------------------------------------------------------------------------- + +def test_every_direct_platform_emitter_is_on_the_ratchet(): + unlisted = _emitter_sites() - set(EMITTERS_NOT_YET_ON_SEAM) + assert not unlisted, ( + f"platform emitters writing rows outside the #3246 seam: {sorted(unlisted)} — " + "report through services.platform_alerts.observe/clear") + + +def test_no_ratchet_entry_is_stale(): + stale = set(EMITTERS_NOT_YET_ON_SEAM) - _emitter_sites() + assert not stale, f"moved onto the seam — remove from the ratchet: {sorted(stale)}" + + +def test_ratchet_prefixes_are_registered_kinds(): + from services import platform_alerts as pa + prefixes = {k.prefix for k in pa.KINDS.values()} + bad = {e: p for e, (p, _f) in EMITTERS_NOT_YET_ON_SEAM.items() if p is not None and p not in prefixes} + assert not bad, bad + + +# --------------------------------------------------------------------------- +# Clear sites +# --------------------------------------------------------------------------- + +def _hook_kinds_on_seam(): + from services import platform_alerts as pa + waiting = {p for (p, _f) in EMITTERS_NOT_YET_ON_SEAM.values()} + return {k.name for k in pa.KINDS.values() + if k.clears == pa.CLEARS_HOOK and k.prefix not in waiting} + + +def test_every_hook_kind_on_the_seam_has_a_clear_site(): + missing = _hook_kinds_on_seam() - _seam_kind_calls() - set(NO_CLEAR_SITE) + assert not missing, ( + f"hook kinds on the seam with no platform_alerts.clear/reconcile call: {sorted(missing)} " + "— call it where the code knows the condition cleared, or name why in NO_CLEAR_SITE") + + +def test_no_clear_site_exemptions_are_live(): + stale = set(NO_CLEAR_SITE) & _seam_kind_calls() + assert not stale, f"exempted kinds that now have a clear site: {sorted(stale)}" + + +def test_clear_site_scanner_reads_positional_and_keyword_kind(): + calls = [c for _q, c in _walk("x.py", ( + "from services import platform_alerts\n" + "def f():\n" + " platform_alerts.clear('a', 'circuit_dormant', 'a')\n" + " platform_alerts.reconcile('a', kind='subscription_headroom', live_keys=[])\n" + " other.clear('a', 'base_image_stale')\n"))] + names = [(_callee(c)[0], _callee(c)[1]) for c in calls] + assert ("clear", ["platform_alerts"]) in names and ("clear", ["other"]) in names + + +# --------------------------------------------------------------------------- +# The caller guard +# --------------------------------------------------------------------------- + +def test_only_the_seam_calls_the_platform_accessors(): + offenders = [] + for rel, text in _backend_files(tuple(_SEAM_ONLY)): + for qual, call in _walk(rel, text): + if call is None: + continue + name, _r = _callee(call) + if name in _SEAM_ONLY and rel not in _SEAM_ONLY[name]: + offenders.append((rel, qual, name, call.lineno)) + assert not offenders, f"platform-alert accessors called outside the seam: {offenders}" diff --git a/tests/unit/test_3246_platform_alerts_seam.py b/tests/unit/test_3246_platform_alerts_seam.py new file mode 100644 index 0000000000..939ded3f84 --- /dev/null +++ b/tests/unit/test_3246_platform_alerts_seam.py @@ -0,0 +1,170 @@ +""" +#3246 C4 — `platform_alerts.observe / clear / reconcile`, driven at the seam +against the unit island's real SQLite. + +* Two readings of one subject are one pending row: the second updates it in + place (fields, `context.seen_count`, `last_seen_at`, `expires_at`). +* The kind's lifetime becomes `expires_at`; a person-only kind never expires; + a real `mark_expired` ends a row whose lifetime ran out. +* `clear` ends the pending row as the PLATFORM and never a person's ending. +* T5: after a person ended the subject's row, the same reading files nothing + for 7 days; a priority increase or a material change files a fresh row; + a person's ending older than the window does not snooze. +* `observe` never raises and refuses an unregistered kind. +""" +import os +import sys +import uuid +from datetime import datetime, timedelta, timezone + +import pytest + +_BACKEND = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "src", "backend")) +if _BACKEND not in sys.path: + sys.path.insert(0, _BACKEND) + +pytestmark = pytest.mark.unit + + +@pytest.fixture +def db(): + from database import db + return db + + +@pytest.fixture +def pa(monkeypatch): + from services import platform_alerts, operator_resume_service + monkeypatch.setattr(operator_resume_service, "spawn_on_loop", lambda factory: None) + return platform_alerts + + +@pytest.fixture +def agent(): + return f"agent-3246s-{uuid.uuid4().hex[:6]}" + + +def _pending(db, agent): + return db.list_operator_queue_items(agent_name=agent, status="pending") + + +def _parse(ts): + return datetime.fromisoformat(ts.replace("Z", "+00:00")) + + +def _person_end(db, row): + db.respond_to_operator_queue_item(row["id"], "ack", "seen", None, "person@example.com") + + +class TestObserve: + def test_two_readings_are_one_row_updated_in_place(self, db, pa, agent): + assert pa.observe(agent, "circuit_dormant", agent, title="t1", question="q1", + context={"failures": 3}) == pa.OBSERVED_CREATED + first = _pending(db, agent)[0] + assert pa.observe(agent, "circuit_dormant", agent, title="t2", question="q2", + priority="critical", context={"failures": 9}) == pa.OBSERVED_UPDATED + rows = _pending(db, agent) + assert len(rows) == 1 and rows[0]["id"] == first["id"] + row = rows[0] + assert row["subject"] == f"circuit_dormant:{agent}" + assert row["title"] == "t2" and row["priority"] == "critical" + assert row["context"] == {"failures": 9, "seen_count": 2} + assert row["last_seen_at"] >= first["last_seen_at"] + assert row["expires_at"] >= first["expires_at"] + + def test_the_kind_lifetime_is_the_deadline(self, db, pa, agent): + pa.observe(agent, "circuit_dormant", agent, title="t", question="q") + pa.observe(agent, "subscription_headroom", "sub1", title="t", question="q", + context={"tier": "warn"}) + pa.observe(agent, "poison", "exec-1", title="t", question="q") + rows = {r["subject"].split(":")[0]: r for r in _pending(db, agent)} + now = datetime.now(timezone.utc) + net = _parse(rows["circuit_dormant"]["expires_at"]) - now + default = _parse(rows["subscription_headroom"]["expires_at"]) - now + assert timedelta(days=29) < net <= timedelta(days=30) + assert timedelta(days=13) < default <= timedelta(days=14) + assert rows["poison"]["expires_at"] is None + + def test_rows_wear_their_kind_prefix_and_are_platform_minted(self, db, pa, agent): + from services import operator_queue_service as oqs + pa.observe(agent, "base_image_stale", "trinity-system", title="t", question="q") + row = _pending(db, agent)[0] + assert row["request_id"].startswith("base-image-stale-trinity-system-") + assert oqs.is_platform_minted(row) + + def test_unregistered_kind_is_refused_and_nothing_raises(self, db, pa, agent, monkeypatch): + assert pa.observe(agent, "no_such_kind", "x", title="t", question="q") == \ + pa.OBSERVED_REFUSED_UNREGISTERED + monkeypatch.setattr(db, "create_platform_operator_queue_item", + lambda *a, **k: (_ for _ in ()).throw(RuntimeError("db down"))) + assert pa.observe(agent, "circuit_dormant", agent, title="t", question="q") == pa.OBSERVED_FAILED + assert _pending(db, agent) == [] + + def test_lifetime_ends_the_row_through_mark_expired(self, db, pa, agent, monkeypatch): + monkeypatch.setattr(pa, "lifetime_for", lambda kind: timedelta(seconds=-1)) + pa.observe(agent, "circuit_dormant", agent, title="t", question="q") + db.mark_operator_queue_expired() + assert _pending(db, agent) == [] + + +class TestClear: + def test_clear_ends_the_pending_row_as_the_platform(self, db, pa, agent): + pa.observe(agent, "circuit_dormant", agent, title="t", question="q") + row = _pending(db, agent)[0] + assert pa.clear(agent, "circuit_dormant", agent) == 1 + ended = db.get_operator_queue_item(row["id"]) + assert ended["status"] == "cancelled" + assert ended["disposed_by"] == "platform" + assert ended["disposition_reason"] == "condition_cleared" + assert pa.clear(agent, "circuit_dormant", agent) == 0 + + def test_clear_with_nothing_pending_is_a_noop(self, pa, agent): + assert pa.clear(agent, "circuit_dormant", agent) == 0 + + def test_reconcile_ends_only_the_subjects_no_longer_live(self, db, pa, agent): + for sid in ("a", "b"): + pa.observe(agent, "subscription_headroom", sid, title="t", question="q", + context={"tier": "warn"}) + pa.observe(agent, "circuit_dormant", agent, title="t", question="q") + assert pa.reconcile(agent, "subscription_headroom", ["a"]) == 1 + assert sorted(r["subject"] for r in _pending(db, agent)) == [ + f"circuit_dormant:{agent}", "subscription_headroom:a"] + + +class TestSnooze: + @pytest.fixture + def ended(self, db, pa, agent): + """A headroom warning a person acknowledged.""" + pa.observe(agent, "subscription_headroom", "sub1", title="75%", question="q", + priority="low", context={"tier": "warn"}) + row = _pending(db, agent)[0] + _person_end(db, row) + assert _pending(db, agent) == [] + return row + + def test_the_same_reading_files_nothing(self, db, pa, agent, ended): + assert pa.observe(agent, "subscription_headroom", "sub1", title="80%", question="q", + priority="low", context={"tier": "warn"}) == pa.OBSERVED_SNOOZED + assert _pending(db, agent) == [] + + def test_a_priority_increase_files_a_fresh_row(self, db, pa, agent, ended): + assert pa.observe(agent, "subscription_headroom", "sub1", title="95%", question="q", + priority="high", context={"tier": "warn"}) == pa.OBSERVED_CREATED + + def test_a_material_change_files_a_fresh_row(self, db, pa, agent, ended): + assert pa.observe(agent, "subscription_headroom", "sub1", title="92%", question="q", + priority="low", context={"tier": "crit"}) == pa.OBSERVED_CREATED + + def test_an_ending_older_than_seven_days_does_not_snooze(self, db, pa, agent, ended, monkeypatch): + monkeypatch.setattr(pa, "snooze_window", lambda: timedelta(seconds=0)) + assert pa.observe(agent, "subscription_headroom", "sub1", title="80%", question="q", + priority="low", context={"tier": "warn"}) == pa.OBSERVED_CREATED + + def test_the_window_is_seven_days(self, pa, monkeypatch): + monkeypatch.delenv(pa.SNOOZE_ENV, raising=False) + assert pa.snooze_window() == timedelta(days=7) + + def test_a_platform_ending_does_not_snooze(self, db, pa, agent): + pa.observe(agent, "circuit_dormant", agent, title="t", question="q") + pa.clear(agent, "circuit_dormant", agent) + assert pa.observe(agent, "circuit_dormant", agent, title="t", question="q") == pa.OBSERVED_CREATED diff --git a/tests/unit/test_ent715_queue_person_fields.py b/tests/unit/test_ent715_queue_person_fields.py index 56d8d3d97a..f16b149af3 100644 --- a/tests/unit/test_ent715_queue_person_fields.py +++ b/tests/unit/test_ent715_queue_person_fields.py @@ -61,6 +61,7 @@ "disposition", "disposed_at", "disposed_by", "disposition_reason", "batch_id", "raised_by", "channel", "to_role", "proposal", "supersedes_expired", "aging", "aged_since", + "subject", "last_seen_at", }) AGENT = "agent-715-reads" From c1eebd85de03235923737c9fa18261787db1ad1e Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 13:32:02 -0400 Subject: [PATCH 06/19] feat(operator-queue): agent circuit breaker reports through the platform alert seam (#3246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dormant transition reports through `platform_alerts.observe` (kind `circuit_dormant`, keyed by agent, 30-day net) and the two places the code already knows the circuit closed — `record_success` recovering from dormant, and the admin `reset_circuit` — end the row through `platform_alerts.clear`. Leaves the #1677 allowlist and the ratchet. Co-Authored-By: Claude Opus 5.5 --- src/backend/services/agent_client/circuit.py | 56 +++++++------ tests/registry.json | 11 +++ .../unit/test_1677_operator_alert_emitters.py | 2 - .../unit/test_3246_platform_alert_emitters.py | 84 +++++++++++++++++++ .../test_3246_platform_alerts_registry.py | 1 - 5 files changed, 126 insertions(+), 28 deletions(-) create mode 100644 tests/unit/test_3246_platform_alert_emitters.py diff --git a/src/backend/services/agent_client/circuit.py b/src/backend/services/agent_client/circuit.py index 8f89ddb12a..5cbe1bf761 100644 --- a/src/backend/services/agent_client/circuit.py +++ b/src/backend/services/agent_client/circuit.py @@ -209,55 +209,57 @@ def is_circuit_failure(exc: BaseException) -> bool: def _emit_dormant_alert(agent_name: str) -> None: - """Insert a circuit_breaker_dormant operator-queue entry. + """Report the circuit_breaker_dormant condition through the platform + alert seam (#3246): one pending row per agent, ended by the platform when + the circuit closes again (`_clear_dormant_alert`). Called from CircuitState.record_failure on the closed/open → dormant transition. The Lua atomicity of _RECORD_FAILURE_LUA guarantees exactly - one worker observes the transition, so this fires at most once per - distinct dormant entry — no de-dupe layer required. + one worker observes the transition; the seam keeps it one row anyway. """ try: - from database import db - from utils.helpers import utc_now_iso - now = utc_now_iso() - # Use the generic 'alert' type so the existing Operating Room UI - # (QueueCard.vue / QueueItemDetail.vue branch on 'approval|question|alert') - # renders an Acknowledge control. The narrower discriminator goes in - # context.alert_type for callers that want to filter — same pattern - # the existing `sync_failing` work should adopt when its UI is touched. - item = { - "id": f"cb-dormant-{agent_name}-{now}", - "agent_name": agent_name, - "type": "alert", - "status": "pending", - "priority": "high", - "title": "Agent circuit breaker DORMANT", - "question": ( + from services import platform_alerts + # The seam files type 'alert', so the Operating Room UI (QueueCard.vue / + # QueueItemDetail.vue branch on 'approval|question|alert') renders an + # Acknowledge control. The narrower discriminator goes in + # context.alert_type for callers that want to filter. + outcome = platform_alerts.observe( + agent_name, "circuit_dormant", agent_name, + priority="high", + title="Agent circuit breaker DORMANT", + question=( f"{agent_name}'s circuit breaker entered DORMANT after " f"{CIRCUIT_DORMANT_AFTER_OPEN_PROBES} consecutive failed probes. " f"Scheduled tasks fast-fail until the agent recovers via the " f"~{int(CIRCUIT_DORMANT_COOLDOWN_SECONDS / 60)} min cooldown " f"probe or an admin reset." ), - "context": { + context={ "agent_name": agent_name, "alert_type": "circuit_breaker_dormant", "transition": "dormant", "dormant_after_open_probes": CIRCUIT_DORMANT_AFTER_OPEN_PROBES, "dormant_cooldown_seconds": CIRCUIT_DORMANT_COOLDOWN_SECONDS, }, - "created_at": now, - } - db.create_operator_queue_item(agent_name, item) + ) logger.warning( - "[CB] circuit_breaker_dormant operator-queue entry emitted for %s", - agent_name, + "[CB] circuit_breaker_dormant operator-queue entry for %s: %s", + agent_name, outcome, ) except Exception: # Don't let alert-delivery failure mask the breaker transition. logger.exception("[CB] failed to emit dormant alert for %s", agent_name) +def _clear_dormant_alert(agent_name: str) -> None: + """The circuit closed: the platform ends the dormant alert (#3246).""" + try: + from services import platform_alerts + platform_alerts.clear(agent_name, "circuit_dormant", agent_name) + except Exception: + logger.exception("[CB] failed to clear dormant alert for %s", agent_name) + + class CircuitState: """Per-agent circuit breaker, Redis-backed (#631). @@ -362,6 +364,8 @@ def record_success(self) -> None: "Circuit CLOSED for agent %s (recovered from %s)", self.agent_name, prior, ) + if prior == "dormant": + _clear_dormant_alert(self.agent_name) except Exception as e: logger.warning("Circuit record_success swallowed (%s)", e) _reset_circuit_redis_client() @@ -486,5 +490,7 @@ def reset_circuit(agent_name: str) -> None: logger.info("Circuit reset to CLOSED for %s", agent_name) except Exception as e: logger.warning("reset_circuit(%s) swallowed: %s", agent_name, e) + return + _clear_dormant_alert(agent_name) diff --git a/tests/registry.json b/tests/registry.json index c62b2a645c..b0712afc46 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -1,5 +1,16 @@ { "test_files": [ + { + "file": "unit/test_3246_platform_alert_emitters.py", + "feature": "#3246", + "added": "2026-10-05", + "categories": [ + "backend", + "operator-queue", + "unit" + ], + "description": "#3246 C5′: the four named emitter families report through the platform alert seam on the unit island's real SQLite — a repeated reading is one pending row updated in place, and the place the code knows the condition cleared ends it as the platform (circuit breaker: recovery from dormant and the admin reset). No Docker, no Redis (the Lua scripts are stubbed)." + }, { "file": "unit/test_3246_platform_alerts_seam.py", "feature": "#3246", diff --git a/tests/unit/test_1677_operator_alert_emitters.py b/tests/unit/test_1677_operator_alert_emitters.py index 2f24e5dccf..4b4cd68551 100644 --- a/tests/unit/test_1677_operator_alert_emitters.py +++ b/tests/unit/test_1677_operator_alert_emitters.py @@ -71,8 +71,6 @@ "NOT platform-only — bounded instead by the #1632 depth cap, enforced ATOMICALLY " "per agent inside create_native_item, and by the SAME per-agent + fleet rate " "buckets as the file seam; every field validated at the call", - ("services/agent_client/circuit.py", "_emit_dormant_alert"): # 1028: package split - "platform-only: edge-triggered after consecutive failed CB probes (cb-dormant)", ("services/archive_storage.py", "_alarm_unwritable_archive_dir"): "platform-only: raised from probe_archive_writability() at the start of an " "archival RUN (daily maintenance cadence), and the id is bucketed per path " diff --git a/tests/unit/test_3246_platform_alert_emitters.py b/tests/unit/test_3246_platform_alert_emitters.py new file mode 100644 index 0000000000..335684afae --- /dev/null +++ b/tests/unit/test_3246_platform_alert_emitters.py @@ -0,0 +1,84 @@ +""" +#3246 C5′ — the four named emitter families report through the platform +alert seam, against the unit island's real SQLite: a repeated reading is one +pending row updated in place, and the place the code knows the condition +cleared ends that row as the platform. +""" +import os +import sys +import uuid + +import pytest + +_BACKEND = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "src", "backend")) +if _BACKEND not in sys.path: + sys.path.insert(0, _BACKEND) + +pytestmark = pytest.mark.unit + + +@pytest.fixture +def db(monkeypatch): + from database import db + from services import operator_resume_service + monkeypatch.setattr(operator_resume_service, "spawn_on_loop", lambda factory: None) + return db + + +def _pending(db, agent): + return db.list_operator_queue_items(agent_name=agent, status="pending") + + +def _ended(db, row): + return db.get_operator_queue_item(row["id"]) + + +class TestCircuitDormant: + @pytest.fixture + def agent(self): + return f"agent-3246cb-{uuid.uuid4().hex[:6]}" + + def test_repeat_dormant_entries_are_one_row(self, db, agent): + from services.agent_client import circuit + circuit._emit_dormant_alert(agent) + circuit._emit_dormant_alert(agent) + rows = _pending(db, agent) + assert len(rows) == 1 + row = rows[0] + assert row["request_id"].startswith(f"cb-dormant-{agent}-") + assert row["type"] == "alert" and row["priority"] == "high" + assert row["context"]["alert_type"] == "circuit_breaker_dormant" + assert row["context"]["seen_count"] == 2 + assert row["expires_at"] is not None # the 30-day net + + def test_recovering_from_dormant_ends_the_row_as_the_platform(self, db, agent, monkeypatch): + from services.agent_client import circuit + circuit._emit_dormant_alert(agent) + row = _pending(db, agent)[0] + monkeypatch.setattr(circuit, "_ensure_scripts", + lambda client: (None, None, lambda **kw: "dormant")) + circuit.CircuitState(agent, redis_client=object()).record_success() + assert _pending(db, agent) == [] + assert _ended(db, row)["disposed_by"] == "platform" + assert _ended(db, row)["disposition_reason"] == "condition_cleared" + + def test_recovering_from_open_leaves_nothing_to_clear(self, db, agent, monkeypatch): + from services.agent_client import circuit + calls = [] + monkeypatch.setattr(circuit, "_clear_dormant_alert", calls.append) + monkeypatch.setattr(circuit, "_ensure_scripts", + lambda client: (None, None, lambda **kw: "open")) + circuit.CircuitState(agent, redis_client=object()).record_success() + assert calls == [] + + def test_admin_reset_ends_the_row(self, db, agent, monkeypatch): + from services.agent_client import circuit + + class _Redis: + def delete(self, *keys): + return len(keys) + + circuit._emit_dormant_alert(agent) + monkeypatch.setattr(circuit, "_get_circuit_redis", lambda: _Redis()) + circuit.reset_circuit(agent) + assert _pending(db, agent) == [] diff --git a/tests/unit/test_3246_platform_alerts_registry.py b/tests/unit/test_3246_platform_alerts_registry.py index c6a4cb5adb..ad63891abf 100644 --- a/tests/unit/test_3246_platform_alerts_registry.py +++ b/tests/unit/test_3246_platform_alerts_registry.py @@ -50,7 +50,6 @@ ("base-image-stale-", "#3246 C5′"), ("services/system_agent_service.py", "SystemAgentService._emit_start_failed_alert"): ("base-image-stale-start-", "#3246 C5′"), - ("services/agent_client/circuit.py", "_emit_dormant_alert"): ("cb-dormant-", "#3246 C5′"), # -- PR 2 --------------------------------------------------------------- ("services/skill_service.py", "SkillService._announce_reconcile_refusal"): ("skills-reconcile-", _PR2), From 4f5219503c4400620886bf736a30df87a816fcd9 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 13:36:01 -0400 Subject: [PATCH 07/19] feat(operator-queue): system agent stale base image reports through the platform alert seam (#3246) Both #1816 alarms report through `platform_alerts.observe`, keyed by the system agent: `base_image_stale` (high) and `system_agent_start_failed` (critical), 30-day net. A newer reading updates the one pending row, so the N-workers-N-rows boot and the per-bucket start-failure rows collapse to one row each. Cleared where the code already knows: a running boot that finds the image current, and a start that adopted the rebuilt image (`image_drift`), end `base_image_stale`; a successful start (delegated or the pre-flight plain start) ends `system_agent_start_failed`. The per-process staleness cooldown stays as emission spacing. `START_FAILED_ALERT_BUCKET_SECONDS` no longer shapes the id (only its own constant test reads it). Five test_1816 tests re-pinned from the direct DB write to the seam call; the cross-process dedup test now proves one pending row on the real DB. Co-Authored-By: Claude Opus 5.5 --- src/backend/services/system_agent_service.py | 72 ++++++++++--------- .../unit/test_1677_operator_alert_emitters.py | 5 -- tests/unit/test_1816_system_agent_adoption.py | 63 ++++++++-------- .../unit/test_3246_platform_alert_emitters.py | 29 ++++++++ .../test_3246_platform_alerts_registry.py | 4 -- 5 files changed, 100 insertions(+), 73 deletions(-) diff --git a/src/backend/services/system_agent_service.py b/src/backend/services/system_agent_service.py index 9d593f04d2..daa8f5328a 100644 --- a/src/backend/services/system_agent_service.py +++ b/src/backend/services/system_agent_service.py @@ -216,6 +216,7 @@ async def ensure_deployed(self) -> dict: else: result["message"] = "System agent already running" logger.info("System agent already running on a current base image") + self._clear_alert("base_image_stale") return result # #1816 — STOPPED: the cold boundary. Delegate to the shared @@ -262,6 +263,7 @@ async def ensure_deployed(self) -> dict: "(pre-flight failed — see the preceding warning)" ) logger.info("System agent started (adoption skipped by pre-flight)") + self._clear_alert("system_agent_start_failed") return result except Exception as e: result["action"] = "start_failed" @@ -285,6 +287,9 @@ async def ensure_deployed(self) -> dict: "System agent started (recreated=%s, reason=%s)", result["recreated"], result["recreate_reason"], ) + self._clear_alert("system_agent_start_failed") + if start_result.get("recreate_reason") == "image_drift": + self._clear_alert("base_image_stale") return result except Exception as e: result["action"] = "start_failed" @@ -625,14 +630,15 @@ def _emit_base_image_stale_alert(self) -> None: SystemAgentService._last_base_image_alert_at = now ts = utc_now_iso() - item = { - "id": f"{BASE_IMAGE_STALE_ALERT_PREFIX}{SYSTEM_AGENT_NAME}-{ts}", - "agent_name": SYSTEM_AGENT_NAME, - "type": "alert", - "status": "pending", - "priority": "high", - "title": "System agent is running a stale base image", - "question": ( + # #3246: through the platform alert seam — one pending row for the + # system agent's image, ended by `_clear_alert` once a boot finds the + # image current or a start adopts the rebuilt one. + from services import platform_alerts + outcome = platform_alerts.observe( + SYSTEM_AGENT_NAME, "base_image_stale", SYSTEM_AGENT_NAME, + priority="high", + title="System agent is running a stale base image", + question=( f"{SYSTEM_AGENT_NAME} is still running the base image it was " f"created from, and a newer trinity-agent-base has been built. " f"It is never recreated while running. Restart it " @@ -640,14 +646,19 @@ def _emit_base_image_stale_alert(self) -> None: f"NOTE: adoption replaces the container, so anything installed " f"outside /home/developer is lost." ), - "context": {"agent_name": SYSTEM_AGENT_NAME, "detected_at": ts}, - "created_at": ts, - } - try: - db.create_operator_queue_item(SYSTEM_AGENT_NAME, item) - logger.warning("base-image staleness alarm emitted for %s", SYSTEM_AGENT_NAME) - except Exception: - logger.exception("failed to emit base-image staleness alarm") + context={"agent_name": SYSTEM_AGENT_NAME, "detected_at": ts}, + ) + logger.warning("base-image staleness alarm for %s: %s", SYSTEM_AGENT_NAME, outcome) + + @staticmethod + def _clear_alert(kind: str) -> None: + """#3246: the condition behind a system-agent alert cleared — the + platform ends its pending row. Never raises.""" + from services import platform_alerts + if kind == "base_image_stale": + platform_alerts.clear(SYSTEM_AGENT_NAME, "base_image_stale", SYSTEM_AGENT_NAME) + elif kind == "system_agent_start_failed": + platform_alerts.clear(SYSTEM_AGENT_NAME, "system_agent_start_failed", SYSTEM_AGENT_NAME) def _emit_start_failed_alert(self, reason: str) -> None: """#1816 R1: the platform may now have NO system agent, and nobody is @@ -666,26 +677,21 @@ def _emit_start_failed_alert(self, reason: str) -> None: `summary_or_error`).""" ts = utc_now_iso() reason = sanitize_text(reason) - bucket = int(time.time()) // START_FAILED_ALERT_BUCKET_SECONDS - item = { - "id": f"{BASE_IMAGE_STALE_ALERT_PREFIX}start-{SYSTEM_AGENT_NAME}-{bucket}", - "agent_name": SYSTEM_AGENT_NAME, - "type": "alert", - "status": "pending", - "priority": "critical", - "title": "System agent could not be started", - "question": ( + # #3246: through the platform alert seam — one pending row per system + # agent (a newer failure updates it in place), ended by `_clear_alert` + # when a start succeeds. + from services import platform_alerts + outcome = platform_alerts.observe( + SYSTEM_AGENT_NAME, "system_agent_start_failed", SYSTEM_AGENT_NAME, + priority="critical", + title="System agent could not be started", + question=( f"{SYSTEM_AGENT_NAME} failed to start: {reason}. The platform " f"orchestrator may be unavailable." ), - "context": {"agent_name": SYSTEM_AGENT_NAME, "detected_at": ts}, - "created_at": ts, - } - try: - db.create_operator_queue_item(SYSTEM_AGENT_NAME, item) - logger.warning("system-agent start-failure alarm emitted: %s", reason) - except Exception: - logger.exception("failed to emit system-agent start-failure alarm") + context={"agent_name": SYSTEM_AGENT_NAME, "detected_at": ts}, + ) + logger.warning("system-agent start-failure alarm (%s): %s", outcome, reason) def _set_system_scope(self, key_id: str): """Update MCP key to have system scope (bypasses permissions).""" diff --git a/tests/unit/test_1677_operator_alert_emitters.py b/tests/unit/test_1677_operator_alert_emitters.py index 4b4cd68551..6b7529545e 100644 --- a/tests/unit/test_1677_operator_alert_emitters.py +++ b/tests/unit/test_1677_operator_alert_emitters.py @@ -131,11 +131,6 @@ "platform-only: deterministic per-episode id (sync-diverged-{agent}-" "{diverged_since}), and an episode must last > 24 h before it can raise " "one, so at most one per agent per day (trinity-enterprise#706)", - ("services/system_agent_service.py", - "SystemAgentService._emit_base_image_stale_alert"): - "platform-only: per-process cooldown, hosted on trinity-system (#1816)", - ("services/system_agent_service.py", "SystemAgentService._emit_start_failed_alert"): - "platform-only: time-bucketed idempotent id, hosted on trinity-system (#1816)", ("services/system_seed_service.py", "SystemSeedService._notify_operator"): "platform-only: deterministic id ⇒ ≤1 open row per seed kind (ent#124)", ("services/validation_service.py", "ValidationService._notify_operator_on_failure"): diff --git a/tests/unit/test_1816_system_agent_adoption.py b/tests/unit/test_1816_system_agent_adoption.py index 7ce9de6af6..7aad437840 100644 --- a/tests/unit/test_1816_system_agent_adoption.py +++ b/tests/unit/test_1816_system_agent_adoption.py @@ -228,17 +228,26 @@ def test_running_and_current_is_the_quiet_path(sas, monkeypatch): assert emitted == [] +def _record_observations(monkeypatch): + """#3246: the alarms report through `platform_alerts.observe`; record each + reading as the row it would file (agent_name / kind / priority / text).""" + from services import platform_alerts + created = [] + + def observe(agent_name, kind, key=None, **kw): + created.append({"agent_name": agent_name, "kind": kind, "key": key, **kw}) + return platform_alerts.OBSERVED_CREATED + + monkeypatch.setattr(platform_alerts, "observe", observe) + return created + + def test_stale_alarm_fires_once_then_is_suppressed_by_cooldown(sas, monkeypatch): """'running + stale' persists across every backend restart until an operator acts, and a restart resets the in-memory edge — so without a cooldown a restart loop files one queue item per boot.""" monkeypatch.setattr(sas, "check_base_image_state", AsyncMock(return_value="drift")) - created = [] - fake_db = MagicMock() - fake_db.create_operator_queue_item.side_effect = lambda name, item: created.append( - item - ) - monkeypatch.setattr(sas, "db", fake_db) + created = _record_observations(monkeypatch) svc = sas.SystemAgentService() _run(svc.ensure_deployed()) @@ -246,22 +255,16 @@ def test_stale_alarm_fires_once_then_is_suppressed_by_cooldown(sas, monkeypatch) assert len(created) == 1, "the second cycle must be suppressed by the cooldown" item = created[0] - assert item["id"].startswith(sas.BASE_IMAGE_STALE_ALERT_PREFIX) + assert item["kind"] == "base_image_stale" and item["key"] == "trinity-system" assert item["priority"] == "high" assert item["agent_name"] == "trinity-system" - assert item["status"] == "pending" def test_stale_alarm_payload_carries_no_image_identifiers(sas, monkeypatch): """canary G-04's lesson: this row is durable and operator-visible, so it carries identifiers and instructions — never image ids or digests.""" monkeypatch.setattr(sas, "check_base_image_state", AsyncMock(return_value="drift")) - created = [] - fake_db = MagicMock() - fake_db.create_operator_queue_item.side_effect = lambda name, item: created.append( - item - ) - monkeypatch.setattr(sas, "db", fake_db) + created = _record_observations(monkeypatch) _run(sas.SystemAgentService().ensure_deployed()) @@ -330,12 +333,7 @@ def test_delegated_start_failure_alarms_and_does_not_raise(sas, monkeypatch): monkeypatch.setattr( sas, "start_agent_internal", AsyncMock(side_effect=RuntimeError("no network")) ) - created = [] - fake_db = MagicMock() - fake_db.create_operator_queue_item.side_effect = lambda name, item: created.append( - item - ) - monkeypatch.setattr(sas, "db", fake_db) + created = _record_observations(monkeypatch) result = _run(sas.SystemAgentService().ensure_deployed()) @@ -1337,8 +1335,7 @@ def test_start_failure_alarm_sanitizes_the_interpolated_reason(sas, monkeypatch) """`reason` is an arbitrary exception string and lands in `operator_queue.question` — durable, operator-visible state. Canary G-04's rule: the emit chokepoint sanitizes.""" - created = [] - monkeypatch.setattr(sas.db, "create_operator_queue_item", lambda agent, item: created.append(item)) + created = _record_observations(monkeypatch) sas.SystemAgentService()._emit_start_failed_alert( "docker refused: token=sk-ant-api03-DEADBEEFDEADBEEFDEADBEEFDEADBEEF" @@ -1379,21 +1376,25 @@ def test_the_destructive_system_agent_endpoints_are_human_only(): def test_start_failure_alarm_is_deduped_across_processes(sas, monkeypatch): """A per-process cursor cannot bound this alarm: a crash-looping backend - re-emits from a FRESH process every time. The bucketed id collapses the - burst in the DB via create_item's (agent_name, request_id) conflict target. + re-emits from a FRESH process every time. #3246: the platform alert seam + keys it by subject, so every process's reading lands on the ONE pending + row (the bucketed id it replaced only collapsed a burst within a bucket). """ - created = [] - monkeypatch.setattr(sas.db, "create_operator_queue_item", lambda agent, item: created.append(item)) + from database import db as real_db + from services import operator_resume_service + monkeypatch.setattr(operator_resume_service, "spawn_on_loop", lambda factory: None) + for row in real_db.list_operator_queue_items(agent_name="trinity-system", status="pending"): + real_db.end_operator_queue_items_by_platform([row["id"]], reason="superseded") # Two emissions from two distinct service instances — the cross-process case. sas.SystemAgentService()._emit_start_failed_alert("network missing") sas.SystemAgentService()._emit_start_failed_alert("network missing") - assert len(created) == 2, "both still attempt the write — the DB dedupes" - assert created[0]["id"] == created[1]["id"], ( - "same bucket must yield the same request_id so on_conflict_do_nothing collapses them" - ) - assert not created[0]["id"].endswith("Z"), "a raw timestamp would defeat the dedup" + rows = [r for r in real_db.list_operator_queue_items(agent_name="trinity-system", status="pending") + if r["subject"] == "system_agent_start_failed:trinity-system"] + assert len(rows) == 1, "both readings land on one pending row" + assert rows[0]["context"]["seen_count"] == 2 + assert rows[0]["request_id"].startswith(sas.BASE_IMAGE_STALE_ALERT_PREFIX + "start-") def test_start_failure_alarm_bucket_re_arms(): diff --git a/tests/unit/test_3246_platform_alert_emitters.py b/tests/unit/test_3246_platform_alert_emitters.py index 335684afae..4f5416f9a9 100644 --- a/tests/unit/test_3246_platform_alert_emitters.py +++ b/tests/unit/test_3246_platform_alert_emitters.py @@ -82,3 +82,32 @@ def delete(self, *keys): monkeypatch.setattr(circuit, "_get_circuit_redis", lambda: _Redis()) circuit.reset_circuit(agent) assert _pending(db, agent) == [] + + +class TestSystemAgent: + AGENT = "trinity-system" + + def _rows(self, db, kind): + return [r for r in _pending(db, self.AGENT) if r["subject"] == f"{kind}:{self.AGENT}"] + + def test_stale_image_is_one_row_ended_when_the_image_is_current(self, db, monkeypatch): + from services import system_agent_service as sas + monkeypatch.setattr(sas.SystemAgentService, "_last_base_image_alert_at", None) + sas.SystemAgentService._clear_alert("base_image_stale") + sas.SystemAgentService()._emit_base_image_stale_alert() + rows = self._rows(db, "base_image_stale") + assert len(rows) == 1 and rows[0]["request_id"].startswith("base-image-stale-trinity-system-") + sas.SystemAgentService._clear_alert("base_image_stale") + assert self._rows(db, "base_image_stale") == [] + assert _ended(db, rows[0])["disposed_by"] == "platform" + + def test_start_failure_is_one_row_ended_when_a_start_succeeds(self, db): + from services import system_agent_service as sas + sas.SystemAgentService._clear_alert("system_agent_start_failed") + sas.SystemAgentService()._emit_start_failed_alert("network missing") + sas.SystemAgentService()._emit_start_failed_alert("still missing") + rows = self._rows(db, "system_agent_start_failed") + assert len(rows) == 1 and "still missing" in rows[0]["question"] + assert rows[0]["priority"] == "critical" + sas.SystemAgentService._clear_alert("system_agent_start_failed") + assert self._rows(db, "system_agent_start_failed") == [] diff --git a/tests/unit/test_3246_platform_alerts_registry.py b/tests/unit/test_3246_platform_alerts_registry.py index ad63891abf..fafd6c33c8 100644 --- a/tests/unit/test_3246_platform_alerts_registry.py +++ b/tests/unit/test_3246_platform_alerts_registry.py @@ -46,10 +46,6 @@ ("services/subscription_headroom_alerts.py", "_emit"): ("sub-headroom-", "#3246 C5′"), ("services/skill_service.py", "SkillService._record_adoption_failure"): ("skills-legacy-adoption-", "#3246 C5′"), - ("services/system_agent_service.py", "SystemAgentService._emit_base_image_stale_alert"): - ("base-image-stale-", "#3246 C5′"), - ("services/system_agent_service.py", "SystemAgentService._emit_start_failed_alert"): - ("base-image-stale-start-", "#3246 C5′"), # -- PR 2 --------------------------------------------------------------- ("services/skill_service.py", "SkillService._announce_reconcile_refusal"): ("skills-reconcile-", _PR2), From 19ef54a711df56030fdc27425600d0841d7c7460 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 13:53:58 -0400 Subject: [PATCH 08/19] feat(operator-queue): subscription headroom reports through the platform alert seam (#3246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both ent#434 alerts report through `platform_alerts.observe` on the `_sub-headroom` host: a subscription keys on the same cleaned id the legacy `sub-headroom-{sid}-…` ids carried (`subject_key`), so a row the upgrade sweep re-subjected matches a new reading; the fleet-wide alert keys on `fleet`. A later reading updates the one pending row in place and a critical reading replaces the warning (`tier` is the kind's material key), which retires the documented "a 75% row still reads 75% at 92%" residual. The reset day moves from the id to the row's context (`window`). The evaluation pass had no place that named a recovery, so it now ends with a reconcile (`clear_recovered`): only a subscription MEASURED back under its threshold (`HAS_HEADROOM`) loses its row — an unassessable member keeps it — and the fleet row survives until some member is measured with room. Off the #1677 allowlist and the ratchet. ent#434's id-identity tests are re-pinned to the subject; test_433_headroom_history asserted nothing about the id shape and needed no change. Co-Authored-By: Claude Opus 5.5 --- .../services/subscription_headroom_alerts.py | 119 ++++++++---------- .../services/subscription_recovery_service.py | 13 ++ .../unit/test_1677_operator_alert_emitters.py | 8 -- .../unit/test_3246_platform_alert_emitters.py | 59 +++++++++ .../test_3246_platform_alerts_registry.py | 1 - tests/unit/test_ent434_headroom_alerts.py | 87 ++++++------- 6 files changed, 160 insertions(+), 127 deletions(-) diff --git a/src/backend/services/subscription_headroom_alerts.py b/src/backend/services/subscription_headroom_alerts.py index fce3653f8d..3a4ce0c520 100644 --- a/src/backend/services/subscription_headroom_alerts.py +++ b/src/backend/services/subscription_headroom_alerts.py @@ -23,30 +23,19 @@ 1. A hysteresis floor is dead code — utilization does not fall inside a window, so the only real re-arm is the reset. -2. `resets_at` therefore IS the window's identity, and putting it in the alert - id makes the id the entire state machine: - - sub-headroom-{sid}-{reset-day}-{tier} - - `db.create_operator_queue_item` maps `item["id"]` onto `request_id`, which - is `UNIQUE(agent_name, request_id)` with ON CONFLICT DO NOTHING. So the - same window re-emits into the same row (no duplicate), a reset mints a new - id (re-armed), and the escalation carries its own tier suffix. Cross-worker - and cross-restart dedup fall out for free, with no lock and no memo. - -The id is quantised to the DAY rather than the exact instant. Under the -measured fixed-window semantics that is exactly one episode per window; if a -different provider plan ever did behave as rolling, it degrades to at most one -alert per day instead of one per probe. The cheap belt is worth more than the -precision. - -## Residual, stated rather than hidden - -`create_item` has no UPDATE path, so a warning row keeps the number it was -raised with — a 75% alert still reads 75% when the subscription later sits at -92%. Same residual `retention_guard` documents for its own alarm. The -escalation is a SEPARATE id with a self-contained body, so the newer figure -does arrive; it just arrives as a second item rather than an edit. +2. `resets_at` therefore IS the window's identity. It travels on the row's + context as `window` (quantised to the day — a provider that did behave as + rolling degrades to one window per day). + +## One row per subscription (#3246) + +Each reading is reported through `services.platform_alerts.observe` with the +subject `subscription_headroom:` (or `:fleet`), so a subscription +has at most one pending row: a later reading updates it in place, a critical +reading replaces the warning (`tier` is the material key the snooze compares), +and the evaluation pass ends the row (`clear_recovered`) once the subscription +is measured back under its threshold — which, under the fixed window, is the +reset. Cross-worker dedup is the seam's partial unique index on the subject. """ import logging @@ -64,7 +53,6 @@ SATURATED, UNASSESSABLE, ) -from utils.helpers import utc_now_iso logger = logging.getLogger(__name__) @@ -239,13 +227,14 @@ def episode_key(resets_at: Optional[str], *, now: Optional[datetime] = None) -> return f"unknown-{current.date().isoformat()}" -def alert_id(subscription_id: str, episode: str, tier: str) -> str: - safe_sid = _SID_SAFE.sub("-", str(subscription_id))[:64] - return f"{ALARM_ID_PREFIX}{safe_sid}-{episode}-{tier}" +# #3246: the subject key of the fleet-wide alert. +FLEET_KEY = "fleet" -def fleet_alert_id(episode: str) -> str: - return f"{ALARM_ID_PREFIX}fleet-{episode}" +def subject_key(subscription_id: str) -> str: + """The cleaned subscription id the legacy `sub-headroom-{sid}-…` ids + carried, so a row the upgrade sweep re-subjected matches a new reading.""" + return _SID_SAFE.sub("-", str(subscription_id))[:64] def decide_tier( @@ -331,42 +320,33 @@ def _fmt_reset(resets_at: Optional[str]) -> str: return dt.strftime("%Y-%m-%d %H:%M UTC") if dt else "reset unknown" -def _emit(item_id: str, *, title: str, question: str, priority: str, +def _emit(key: str, *, title: str, question: str, priority: str, context: Dict[str, Any]) -> bool: - """One platform create on the sentinel host. Never raises. - - Platform-only by construction: an agent cannot drive the VOLUME. The - cadence is the sweep's, the id is deterministic per (subscription, window, - tier) so a re-emit is an on-conflict no-op, and the per-cycle cap bounds a - whole fleet crossing at once. That is what makes it a direct create with an - allowlist entry in `tests/unit/test_1677_operator_alert_emitters.py` rather - than a `create_bounded_alert` caller. - - Note the precise claim: agent-chosen *names* do reach the body (an agent may - spawn children and name them), but they arrive sanitized, are capped at five - per alert, and already appear on every operator surface. It is the volume, - not the absence of agent-derived text, that justifies the exemption. + """Report one reading through the platform alert seam (#3246). Never raises. + + One pending row per subject — a subscription's cleaned id, or `fleet` — + on the sentinel host. A newer reading updates that row in place, so a + critical reading replaces the warning (`tier` is the kind's material key) + and the figure on the row is always the latest. The row is ended by the + platform when the evaluation pass finds the subscription back under its + threshold (`clear_recovered`). Returns True when a row was filed or + updated. """ - item = { - "id": item_id, - "type": "alert", - "status": "pending", - "priority": priority, - "title": title, - "question": question, - "context": context, - "created_at": utc_now_iso(), - # Must stay None: `mark_operator_queue_expired` flips any pending row - # past `expires_at` to expired fleet-wide every 5s. - "expires_at": None, - } - try: - db.create_operator_queue_item(ALARM_AGENT_NAME, item) - logger.warning("[headroom-alert] %s", title) - return True - except Exception: # noqa: BLE001 - logger.exception("[headroom-alert] failed to emit %s", item_id) - return False + from services import platform_alerts + outcome = platform_alerts.observe( + ALARM_AGENT_NAME, "subscription_headroom", key, + title=title, question=question, priority=priority, context=context, + ) + logger.warning("[headroom-alert] %s (%s)", title, outcome) + return outcome in (platform_alerts.OBSERVED_CREATED, platform_alerts.OBSERVED_UPDATED) + + +def clear_recovered(live_keys: Sequence[str]) -> int: + """#3246: end every pending headroom row whose subject is not in + `live_keys` — the subscriptions (and `fleet`) the evaluation pass could + not show to be back under the threshold. Never raises.""" + from services import platform_alerts + return platform_alerts.reconcile(ALARM_AGENT_NAME, "subscription_headroom", live_keys) def emit_subscription_alert( @@ -383,9 +363,8 @@ def emit_subscription_alert( ) -> bool: """One subscription crossed its weekly threshold. - The body is self-contained. A warning row cannot be edited later (see the - module docstring), so the escalation must stand on its own rather than - assume the operator reads it beside the earlier item. + The body is self-contained: the escalation replaces the warning on the + subscription's one row (#3246), so it must stand on its own. """ label = subscription_name or subscription_id episode = episode_key(resets_at, now=now) @@ -421,7 +400,7 @@ def emit_subscription_alert( lines.append("No agents are currently assigned to this subscription.") return _emit( - alert_id(subscription_id, episode, tier), + subject_key(subscription_id), title=headline, question="\n".join(lines), priority=priority_for(tier, projected_end), @@ -433,6 +412,7 @@ def emit_subscription_alert( "projected_end_pct": projected_end, "threshold_pct": threshold_pct, "resets_at": resets_at, + "window": episode, "agent_count": len(agents), "on_pace_to_exhaust": on_pace, }, @@ -470,7 +450,7 @@ def emit_fleet_alert( "There is no subscription with meaningful headroom left to move agents onto.", ] return _emit( - fleet_alert_id(episode), + FLEET_KEY, title=f"All {len(saturated)} subscriptions are near their weekly limit", priority="high", question="\n".join(lines), @@ -479,5 +459,6 @@ def emit_fleet_alert( "subscription_count": len(saturated), "threshold_pct": threshold_pct, "earliest_reset_at": earliest_reset, + "window": episode, }, ) diff --git a/src/backend/services/subscription_recovery_service.py b/src/backend/services/subscription_recovery_service.py index 5a44bd067f..ae4fb2ea40 100644 --- a/src/backend/services/subscription_recovery_service.py +++ b/src/backend/services/subscription_recovery_service.py @@ -404,6 +404,19 @@ async def _evaluate_alerts( } verdict = alerts.fleet_verdict(classifications) + # #3246: the evaluation pass is the one place that knows a subscription + # is back under its threshold, so it ends the rows it no longer backs. + # Only a MEASURED `HAS_HEADROOM` clears — an unassessable member keeps + # its row (no evidence is not recovery, the ent#100 rule) — and the + # fleet row survives until some member is measured with room. + live_keys = [ + alerts.subject_key(sid) for sid, c in classifications.items() + if c != alerts.HAS_HEADROOM + ] + if not verdict["has_headroom_ids"]: + live_keys.append(alerts.FLEET_KEY) + await asyncio.to_thread(alerts.clear_recovered, live_keys) + # The fleet alert names every saturated subscription, so emitting the # individual ones beside it would be N+1 operator items for one event. if verdict["saturated"]: diff --git a/tests/unit/test_1677_operator_alert_emitters.py b/tests/unit/test_1677_operator_alert_emitters.py index 6b7529545e..d018b5cfb8 100644 --- a/tests/unit/test_1677_operator_alert_emitters.py +++ b/tests/unit/test_1677_operator_alert_emitters.py @@ -97,14 +97,6 @@ "create — must never be throttled (the reason a db-sink bound was rejected)", ("services/operator_queue_service.py", "OperatorQueueSyncService._sync_agent"): "the #1632-capped agent-file ingestion seam itself (depth+rate+clamp)", - ("services/subscription_headroom_alerts.py", "_emit"): - "platform-only: edge-triggered per weekly window via a deterministic " - "id (sub-headroom-{sid}-{reset-day}-{tier}) whose ON CONFLICT DO NOTHING " - "makes a re-emit a no-op; volume is bound by the sweep cadence and a " - "per-cycle cap. Agent-CHOSEN names do reach the body (an agent may " - "spawn children and name them) but arrive sanitized and capped at five " - "per alert — it is the VOLUME an agent cannot drive, which is what this " - "exemption rests on (ent#434)", ("services/operator_queue_service.py", "create_bounded_alert_outcome"): "the #1677 budget helper's own admit-path create (the seam itself)", ("services/operator_queue_service.py", "_maybe_emit_alert_budget_episode"): diff --git a/tests/unit/test_3246_platform_alert_emitters.py b/tests/unit/test_3246_platform_alert_emitters.py index 4f5416f9a9..ec25e23e9a 100644 --- a/tests/unit/test_3246_platform_alert_emitters.py +++ b/tests/unit/test_3246_platform_alert_emitters.py @@ -111,3 +111,62 @@ def test_start_failure_is_one_row_ended_when_a_start_succeeds(self, db): assert rows[0]["priority"] == "critical" sas.SystemAgentService._clear_alert("system_agent_start_failed") assert self._rows(db, "system_agent_start_failed") == [] + + +class TestSubscriptionHeadroom: + AGENT = "_sub-headroom" + + @pytest.fixture + def sid(self): + return f"sub/3246-{uuid.uuid4().hex[:6]}" + + def _rows(self, db, key): + return [r for r in _pending(db, self.AGENT) + if r["subject"] == f"subscription_headroom:{key}"] + + def _alert(self, sid, tier, util): + from services import subscription_headroom_alerts as a + return a.emit_subscription_alert( + subscription_id=sid, subscription_name="one", tier=tier, + utilization_pct=util, projected_end=None, + resets_at="2026-10-09T00:00:00Z", threshold_pct=75, agents=[]) + + def test_a_critical_reading_replaces_the_warning_on_one_row(self, db, sid): + from services import subscription_headroom_alerts as a + key = a.subject_key(sid) + assert self._alert(sid, "warn", 78.0) is True + assert self._alert(sid, "warn", 84.0) is True + assert self._alert(sid, "crit", 93.0) is True + rows = self._rows(db, key) + assert len(rows) == 1 + assert rows[0]["request_id"].startswith(f"sub-headroom-{key}-") + assert "93%" in rows[0]["title"] and rows[0]["context"]["tier"] == "crit" + + def test_the_evaluation_pass_ends_a_row_it_no_longer_backs(self, db, sid): + from services import subscription_headroom_alerts as a + other = f"{sid}-x" + self._alert(sid, "warn", 80.0) + self._alert(other, "warn", 80.0) + a.clear_recovered([a.subject_key(other), a.FLEET_KEY]) + assert self._rows(db, a.subject_key(sid)) == [] + assert len(self._rows(db, a.subject_key(other))) == 1 + ended = [r for r in db.list_operator_queue_items(agent_name=self.AGENT, limit=1000) + if r["subject"] == f"subscription_headroom:{a.subject_key(sid)}"] + assert ended and ended[0]["disposed_by"] == "platform" + + def test_the_sweep_keeps_an_unassessable_row_and_clears_a_measured_recovery( + self, db, sid, monkeypatch): + """Only a MEASURED `HAS_HEADROOM` clears — no evidence is not recovery.""" + import asyncio + from services import subscription_headroom_alerts as a + from services import subscription_recovery_service as svc + recovered, unknown = sid, f"{sid}-u" + self._alert(recovered, "warn", 80.0) + self._alert(unknown, "warn", 80.0) + subs = [type("S", (), {"id": recovered, "name": "r"})(), + type("S", (), {"id": unknown, "name": "u"})()] + results = [{"sid": recovered, "classification": a.HAS_HEADROOM, "reading": None}, + {"sid": unknown, "classification": None, "reading": None}] + asyncio.run(svc.SubscriptionRecoveryService()._evaluate_alerts(subs, results, 75)) + assert self._rows(db, a.subject_key(recovered)) == [] + assert len(self._rows(db, a.subject_key(unknown))) == 1 diff --git a/tests/unit/test_3246_platform_alerts_registry.py b/tests/unit/test_3246_platform_alerts_registry.py index fafd6c33c8..a6e0799121 100644 --- a/tests/unit/test_3246_platform_alerts_registry.py +++ b/tests/unit/test_3246_platform_alerts_registry.py @@ -43,7 +43,6 @@ # value: (the id prefix it writes, the follow-up that moves it). # --------------------------------------------------------------------------- EMITTERS_NOT_YET_ON_SEAM = { - ("services/subscription_headroom_alerts.py", "_emit"): ("sub-headroom-", "#3246 C5′"), ("services/skill_service.py", "SkillService._record_adoption_failure"): ("skills-legacy-adoption-", "#3246 C5′"), # -- PR 2 --------------------------------------------------------------- diff --git a/tests/unit/test_ent434_headroom_alerts.py b/tests/unit/test_ent434_headroom_alerts.py index d3b549be96..f9ade82fef 100644 --- a/tests/unit/test_ent434_headroom_alerts.py +++ b/tests/unit/test_ent434_headroom_alerts.py @@ -251,43 +251,40 @@ def test_unknown_projection_is_not_treated_as_an_emergency(self): # ============================================================================= class TestEpisodeIdentity: - - def test_same_window_same_id_so_a_re_emit_is_an_on_conflict_noop(self): - """Three cycles at 80/81/82 inside one window produce ONE id, so the - sink's ON CONFLICT DO NOTHING makes it one row. This is the whole - edge-trigger, with no durable memo.""" - from services.subscription_headroom_alerts import alert_id, episode_key - resets = "2026-09-01T00:00:00Z" - ids = {alert_id("s1", episode_key(resets, now=NOW), "warn") for _ in range(3)} - assert len(ids) == 1 - - def test_window_reset_mints_a_new_id_so_the_alert_re_arms(self): - from services.subscription_headroom_alerts import alert_id, episode_key - first = alert_id("s1", episode_key("2026-09-01T00:00:00Z", now=NOW), "warn") - after = alert_id("s1", episode_key("2026-09-08T00:00:00Z", now=NOW), "warn") - assert first != after - - def test_escalation_carries_its_own_id(self): - from services.subscription_headroom_alerts import alert_id, episode_key - ep = episode_key("2026-09-01T00:00:00Z", now=NOW) - assert alert_id("s1", ep, "warn") != alert_id("s1", ep, "crit") - - def test_a_moving_reset_degrades_to_one_id_per_day_not_one_per_probe(self): + """#3246 re-pin: the id is no longer the state machine — the subject is. + One subscription is one subject across readings, tiers and windows; the + window travels on the context.""" + + def test_every_reading_of_one_subscription_shares_one_subject(self): + from services import platform_alerts as pa + from services.subscription_headroom_alerts import subject_key + subjects = {pa.subject_for("subscription_headroom", subject_key("s1")) for _ in range(3)} + assert subjects == {"subscription_headroom:s1"} + + def test_the_subject_is_the_cleaned_sid_the_legacy_ids_carried(self): + """So a row the upgrade sweep re-subjected matches a new reading.""" + from services import platform_alerts as pa + from services.subscription_headroom_alerts import subject_key + legacy = pa.derive_legacy_subject("sub-headroom-ab-c-2026-09-01-crit", {}) + assert legacy.subject == pa.subject_for("subscription_headroom", subject_key("ab/c")) + + def test_tier_is_the_material_key(self): + from services import platform_alerts as pa + assert pa.KINDS["subscription_headroom"].material_keys == ("tier",) + + def test_a_moving_reset_degrades_to_one_window_per_day_not_one_per_probe(self): """The belt against the measurement being wrong elsewhere. If some provider plan did behave as a rolling window, quantising to the day - bounds the blast radius at one alert per day instead of one per - probe.""" + bounds the window label at one per day instead of one per probe.""" from services.subscription_headroom_alerts import episode_key a = episode_key("2026-09-01T00:00:00Z", now=NOW) b = episode_key("2026-09-01T18:30:00Z", now=NOW) assert a == b - def test_id_carries_the_reserved_prefix(self): - from services.subscription_headroom_alerts import ( - ALARM_ID_PREFIX, alert_id, fleet_alert_id, - ) - assert alert_id("s1", "2026-09-01", "warn").startswith(ALARM_ID_PREFIX) - assert fleet_alert_id("2026-09-01").startswith(ALARM_ID_PREFIX) + def test_the_kind_carries_the_reserved_prefix(self): + from services import platform_alerts as pa + from services.subscription_headroom_alerts import ALARM_ID_PREFIX + assert pa.KINDS["subscription_headroom"].prefix == ALARM_ID_PREFIX # ============================================================================= @@ -486,27 +483,19 @@ def test_alarm_host_is_uncreatable_as_a_real_agent(self): from services.subscription_headroom_alerts import ALARM_AGENT_NAME assert sanitize_agent_name(ALARM_AGENT_NAME) != ALARM_AGENT_NAME - def test_emitter_is_on_the_1677_platform_only_allowlist(self): - """A new direct `create_operator_queue_item` call site reds that guard - until it is classified. This asserts the classification exists rather - than re-running the guard.""" - src = (_REPO / "tests" / "unit" - / "test_1677_operator_alert_emitters.py").read_text() - assert "services/subscription_headroom_alerts.py" in src - - def test_alerts_emit_never_sets_expires_at(self): - """`mark_operator_queue_expired` flips any pending row past - `expires_at` to expired fleet-wide every 5s.""" + def test_emitter_reports_through_the_platform_alert_seam(self): + """#3246: no direct `create_operator_queue_item` left in the module — + every reading goes through `platform_alerts.observe`, so it is off the + #1677 allowlist and the seam sets the lifetime (`expires_at`).""" src = (_BACKEND / "services" / "subscription_headroom_alerts.py").read_text() tree = ast.parse(src) - found = False - for node in ast.walk(tree): - if isinstance(node, ast.Dict): - for k, v in zip(node.keys, node.values): - if isinstance(k, ast.Constant) and k.value == "expires_at": - found = True - assert isinstance(v, ast.Constant) and v.value is None - assert found, "the emitted item must set expires_at explicitly" + calls = {n.func.attr for n in ast.walk(tree) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute)} + assert "observe" in calls and "reconcile" in calls + assert "create_operator_queue_item" not in calls + guard = (_REPO / "tests" / "unit" + / "test_1677_operator_alert_emitters.py").read_text() + assert "services/subscription_headroom_alerts.py" not in guard # ============================================================================= From bdd77a3e97570e0d02afc37df7bb872a03877eab Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 14:08:06 -0400 Subject: [PATCH 09/19] feat(operator-queue): legacy skills-library adoption reports through the platform alert seam (#3246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_record_adoption_failure` reports through `platform_alerts.observe` on the `_skills-sync` host with ONE subject per URL (`sha256(raw url)[:12]`), shared by the steady-state refusal and both failure branches. A repeat — any branch — updates the one pending row (latest message and priority, seen count) rather than filing another, so one sync never files a second row (#2744's guarantee, now stated on the subject). Severity per branch is unchanged (steady state low/info, failures high/error); the URL echo stays scrubbed. Cleared where the code knows: adoption success and a URL that already names a source end the URL's row (`_clear_adoption_alert`); every adoption pass reconciles against the current setting (`_reconcile_adoption_alerts`), so a removed or changed `skills_library_url` ends the rows it no longer backs. Off the #1677 allowlist and the ratchet, which now holds only PR-2 entries. test_2744 re-pinned from request_id to subject (test 4 now asserts the failure branches keep `high` on the URL's one subject); three ent#346 tests re-pinned from the direct DB write to the seam call. Co-Authored-By: Claude Opus 5.5 --- src/backend/services/skill_service.py | 135 ++++++++++-------- .../unit/test_1677_operator_alert_emitters.py | 7 - ..._2744_skills_adoption_alert_idempotency.py | 91 +++++++----- .../unit/test_3246_platform_alert_emitters.py | 46 ++++++ .../test_3246_platform_alerts_registry.py | 2 - .../test_ent346_skills_source_injection.py | 13 +- 6 files changed, 182 insertions(+), 112 deletions(-) diff --git a/src/backend/services/skill_service.py b/src/backend/services/skill_service.py index c56b397af5..91ff2e7618 100644 --- a/src/backend/services/skill_service.py +++ b/src/backend/services/skill_service.py @@ -797,6 +797,9 @@ def _adopt_legacy_clone(self) -> Optional[str]: url = get_skills_library_url() except Exception: # noqa: BLE001 return None + # #3246: a refusal row is live only while the setting still names its + # URL — a removed or changed setting ends the rows it no longer backs. + self._reconcile_adoption_alerts(url) if not url: return None @@ -855,6 +858,7 @@ def _adopt_legacy_clone(self) -> Optional[str]: existing = [s for s in db.list_skill_sources() if _same_skills_repo(s.url, normalized_url)] if existing: + self._clear_adoption_alert(url) return existing[0].id if db.count_skill_sources() > 0: @@ -916,9 +920,38 @@ def _adopt_legacy_clone(self) -> Optional[str]: source.id, e, ) + self._clear_adoption_alert(url) logger.info("migrated skills_library_url into skill source %s", source.id) return source.id + # #3246: the legacy adoption refusal is one platform condition per URL. + + @staticmethod + def _adoption_alert_key(url: Any) -> str: + """`sha256(url)[:12]` over the RAW value — never the URL itself (a raw + URL is a conflict key that could carry a PAT). Raises on a non-str + setting value; every caller is inside a try.""" + return hashlib.sha256(url.strip().encode()).hexdigest()[:12] + + def _clear_adoption_alert(self, url: Any) -> None: + """Adoption succeeded (or the URL already names a source): end the + URL's refusal row as the platform. Never raises.""" + from services import platform_alerts + try: + platform_alerts.clear(RECONCILE_ALARM_AGENT_NAME, "skills_legacy_adoption", + self._adoption_alert_key(url)) + except Exception as e: # noqa: BLE001 + logger.warning(f"could not clear skills adoption alarm: {e}") + + def _reconcile_adoption_alerts(self, url: Any) -> None: + """End every refusal row whose URL is no longer the setting. Never raises.""" + from services import platform_alerts + try: + live = [self._adoption_alert_key(url)] if url else [] + platform_alerts.reconcile(RECONCILE_ALARM_AGENT_NAME, "skills_legacy_adoption", live) + except Exception as e: # noqa: BLE001 + logger.warning(f"could not reconcile skills adoption alarms: {e}") + # ------------------------------------------------------------------------- # Durable sync status (ent#236) # ------------------------------------------------------------------------- @@ -2511,43 +2544,32 @@ def _record_adoption_failure( silently is how the refusal gets rediscovered as "skills stopped working". - A NEW CALL SITE MUST CHOOSE `steady_state` EXPLICITLY. The default is - the unbounded timestamped/`high` shape, which is correct only for a - genuine transient failure — nothing enforces that choice (the #1677 AST - guard scans call sites of `create_operator_queue_item`, not of this - method), so the docstring is the enforcement. + A NEW CALL SITE MUST CHOOSE `steady_state` EXPLICITLY — it picks the + severity of the reading. - Two classes of refusal share this emitter (#2744): + Two classes of refusal share this emitter (#2744), and since #3246 one + subject — `skills_legacy_adoption:` on the + `_skills-sync` host — so a URL has at most one pending row: * A genuine FAILURE — the URL failed validation, or adoption threw. - `priority: "high"` and a TIMESTAMPED id, so a repeat is visible AS a - repeat rather than deduped away by `create_item`'s ON CONFLICT DO - NOTHING. Repeat-visible here is a PRODUCT DECISION, not a claim that - these branches are bounded: a permanently invalid `skills_library_url` - re-enters the validation branch on every sync just as forever. + `priority: "high"`, `logger.error`. A repeat is visible AS a repeat + through the row's seen count and last-seen time, not as a new row. * The STEADY STATE — "this install already has sources, so it is past migration": the designed resting state of every migrated install whose - legacy key lingers, not a failure. `priority: "low"`, `logger.info`, - and a STABLE id derived from the refused URL, so the `(agent_name, - request_id)` ON CONFLICT collapses every repeat to ONE row and an - operator's dismissal sticks. The family prefix is registered in - `operator_queue_service._RESERVED_ID_PREFIXES`, because a stable id - derived from an admin-visible URL is guessable and an agent could - otherwise pre-create it and silence the alarm (the #1632 C2 class); - that tuple also drives `is_platform_minted`. - - The id is the sha256 of the URL, never the URL itself: `request_id` is a - conflict key, not a display field, and a raw URL would break the - id-shape (`^[A-Za-z0-9._:-]+$`) the reserved-prefix machinery assumes. - - **The bound this emitter's #1677 platform-only classification rests on** - is: the only input is the `skills_library_url` setting, which - `routers/settings.py` blocks on the generic settings PUT + legacy key lingers, not a failure. `priority: "low"`, `logger.info`. + A person's dismissal holds through the seam's snooze window. + + The kind's id prefix stays in + `operator_queue_service._RESERVED_ID_PREFIXES`, so an agent cannot + pre-create one of these rows (the #1632 C2 class); that tuple also + drives `is_platform_minted`. The key is the sha256 of the URL, never + the URL itself: a raw URL is not id-shaped and could carry a PAT. + + The volume bound: the only input is the `skills_library_url` setting, + which `routers/settings.py` blocks on the generic settings PUT (`LEGACY_SKILLS_LIBRARY_KEYS`, 422) and no other writer reaches — so no - agent can drive the volume. It is NOT "admin-driven cadence": ent#236's - auto-sync calls the same `sync_library()` unattended on a 300s-86400s - timer. Keep this comment and the `_ALLOWED_CALLERS` justification in - `tests/unit/test_1677_operator_alert_emitters.py` in step. + agent can drive it. ent#236's auto-sync calls the same `sync_library()` + unattended on a 300s-86400s timer, which is why one row per URL matters. The echoed URL is SCRUBBED (#2744). Naming what was refused is the whole point of the alarm, but `EmbeddedCredentialError` is a `ValueError` @@ -2564,38 +2586,25 @@ def _record_adoption_failure( else: logger.error(f"[ent#346] {message} (url={safe_url})") try: - from utils.helpers import utc_now_iso - - context = { - "alert_type": "skills_legacy_adoption_refused", - "url": safe_url, - } - if steady_state: - # Computed INSIDE the try: `url.strip().encode()` raises - # AttributeError on a non-str setting value, and out here that - # degrades to a warning and no alarm — the fail-soft guarantee. - # Hoisting it above the try turns a decorative alarm into a - # raiser. - request_id = ( - "skills-legacy-adoption-refused-" - f"{hashlib.sha256(url.strip().encode()).hexdigest()[:12]}" - ) - else: - # Timestamped, so a repeated failure is visible as repeated - # rather than silently deduped by the ON CONFLICT DO NOTHING - # in `create_item`. - request_id = f"skills-legacy-adoption-{utc_now_iso()}" - - db.create_operator_queue_item( - RECONCILE_ALARM_AGENT_NAME, - { - "id": request_id, - "type": "alert", - "priority": "low" if steady_state else "high", - "title": "Legacy skills-library adoption refused", - "question": message, - "context": context, - "expires_at": None, + from services import platform_alerts + + # #3246: one platform condition per URL — `sha256(raw url)[:12]` + # (computed INSIDE the try: a non-str setting value raises here and + # degrades to a warning, the fail-soft guarantee). Both branches + # share the subject, so a repeat — steady-state or a genuine + # failure — updates the one pending row (seen count, latest + # message and priority) instead of filing a second, and the row is + # ended by the platform when adoption succeeds or the setting is + # removed (`_clear_adoption_alert` / `_reconcile_adoption_alerts`). + platform_alerts.observe( + RECONCILE_ALARM_AGENT_NAME, "skills_legacy_adoption", + self._adoption_alert_key(url), + priority="low" if steady_state else "high", + title="Legacy skills-library adoption refused", + question=message, + context={ + "alert_type": "skills_legacy_adoption_refused", + "url": safe_url, }, ) except Exception as e: # noqa: BLE001 — the alarm is decorative diff --git a/tests/unit/test_1677_operator_alert_emitters.py b/tests/unit/test_1677_operator_alert_emitters.py index d018b5cfb8..1a49fb2a0b 100644 --- a/tests/unit/test_1677_operator_alert_emitters.py +++ b/tests/unit/test_1677_operator_alert_emitters.py @@ -105,13 +105,6 @@ "platform-only: edge-gated (fresh) + idempotent id (#1644)", ("services/skill_service.py", "SkillService._announce_reconcile_refusal"): "platform-only: assignment-driven, quasi-idempotent id (ent#236)", - ("services/skill_service.py", "SkillService._record_adoption_failure"): - "platform-only: the only input is `skills_library_url`, blocked on the " - "generic settings PUT by routers/settings.py LEGACY_SKILLS_LIBRARY_KEYS " - "(ent#346), so no agent can drive volume; the steady-state branch is " - "additionally idempotent by a URL-derived id ⇒ ≤1 row per refused URL. " - "NOT admin-driven — ent#236's auto-sync calls the same sync_library() " - "unattended on a 300s-86400s timer (#2744)", ("services/skills_sync_service.py", "SkillsLibrarySyncService._announce_fleet_failures"): "platform-only: leader-locked, one per sync run (ent#236)", diff --git a/tests/unit/test_2744_skills_adoption_alert_idempotency.py b/tests/unit/test_2744_skills_adoption_alert_idempotency.py index f67ce5a90a..d4fec7b568 100644 --- a/tests/unit/test_2744_skills_adoption_alert_idempotency.py +++ b/tests/unit/test_2744_skills_adoption_alert_idempotency.py @@ -31,6 +31,14 @@ `(agent_name, request_id)`), covered by its own tests. What the service controls — and what this file asserts — is that the id is **stable and URL-derived**. +**#3246 re-pin.** The emitter now reports through `platform_alerts.observe` +with ONE subject per URL (`skills_legacy_adoption:`) shared +by all three call sites, so the guarantee is stated on the subject: one sync — +or any number of them — never files a second row for one URL. Test 4 now pins +that the actionable branches keep `high` on that same subject; the repeat is +visible as the row's seen count. The seam's own dedup (one pending row per +subject) is covered by `test_3246_platform_alerts_*`. + Lives in its own file rather than appended to `test_ent346_skills_source_injection.py` because #2763 is being implemented in the same function and wants that file. """ @@ -78,19 +86,19 @@ PAT = "ghp_AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH0000" PAT_URL = f"https://{PAT}@github.com/acme/private-skills" -STABLE_PREFIX = "skills-legacy-adoption-refused-" _ID_RE = re.compile(r"^[A-Za-z0-9._:-]+$") _DB_BELT_ID_MAX = 512 -def expected_stable_id(url: str) -> str: +def expected_subject(url: str) -> str: """The contract, spelled out rather than re-derived from the source. - Asserting "the two ids differ" alone cannot distinguish an id derived from - the URL from one derived from the clock — the timestamp satisfies it on the - unfixed tree. Only the exact form pins AC 3. + #3246: the identity moved from the `request_id` to the platform alert + subject — one per URL, `sha256(url)[:12]`, shared by both branches. + Asserting "the two subjects differ" alone cannot distinguish one derived + from the URL from one derived from the clock; only the exact form pins AC 3. """ - return f"{STABLE_PREFIX}{hashlib.sha256(url.strip().encode()).hexdigest()[:12]}" + return f"skills_legacy_adoption:{hashlib.sha256(url.strip().encode()).hexdigest()[:12]}" # --------------------------------------------------------------------------- @@ -140,9 +148,17 @@ def _boom(): monkeypatch.setattr( svc_mod.db, "create_skill_source", lambda **kw: created.append(kw) ) - monkeypatch.setattr( - svc_mod.db, "create_operator_queue_item", lambda agent, item: items.append(item) - ) + # #3246: the emitter reports through the platform alert seam; capture each + # reading as the item the seam would file or update. + from services import platform_alerts + + def _observe(agent, kind, key=None, **kw): + items.append({"agent": agent, "subject": platform_alerts.subject_for(kind, key), **kw}) + return platform_alerts.OBSERVED_CREATED + + monkeypatch.setattr(platform_alerts, "observe", _observe) + monkeypatch.setattr(platform_alerts, "clear", lambda *a, **kw: 0) + monkeypatch.setattr(platform_alerts, "reconcile", lambda *a, **kw: 0) return items, created @@ -213,11 +229,11 @@ def test_n_syncs_in_the_terminal_state_file_exactly_one_item(monkeypatch): assert created == [], "the terminal branch must never create a source" assert len(items) == 5, "the refusal must stay observable on every sync" _assert_terminal_branch(items[0]) - assert len({i["id"] for i in items}) == 1, ( - "five syncs minted five distinct request_ids — the ON CONFLICT dedup " - "cannot collapse them, which is the unbounded growth this fixes" + assert len({i["subject"] for i in items}) == 1, ( + "five syncs reported five distinct subjects — the seam cannot collapse " + "them onto one row, which is the unbounded growth this fixes" ) - assert items[0]["id"] == expected_stable_id(TERMINAL_URL) + assert items[0]["subject"] == expected_subject(TERMINAL_URL) # --------------------------------------------------------------------------- @@ -240,10 +256,10 @@ def test_a_different_refused_url_gets_its_own_item(monkeypatch): service._adopt_legacy_clone() # the SAME url twice assert items, "the refusal must be observable" _assert_terminal_branch(items[0]) - assert {i["id"] for i in items} == {expected_stable_id(url)}, ( - f"the id for {url} is not the sha256 of that URL" + assert {i["subject"] for i in items} == {expected_subject(url)}, ( + f"the subject for {url} is not the sha256 of that URL" ) - ids[url] = items[0]["id"] + ids[url] = items[0]["subject"] assert ids[TERMINAL_URL] != ids[OTHER_URL], ( "two different refused URLs collapsed onto one id — a second refused " @@ -290,14 +306,12 @@ def test_the_terminal_refusal_is_not_high_priority(monkeypatch, caplog): ("adoption exception", TERMINAL_URL, True, "adoption failed"), ], ) -def test_the_actionable_branches_keep_high_and_repeat_visible_ids( +def test_the_actionable_branches_keep_high_on_the_urls_one_subject( monkeypatch, case, url, list_raises, marker ): - """A repeat here carries information: something is still broken. - - This is the anti-regression half of the fix. It is GREEN on the unfixed - tree by design — it goes red only if the `low` / stable-id treatment is - applied to all three call sites instead of the one the issue names. + """A repeat here carries information: something is still broken — so the + failure branches keep `high`. Since #3246 the repeat shows as the one + row's seen count rather than as a second row. """ _increment_clock(monkeypatch) items, _ = _wire(monkeypatch, url, count=0, list_raises=list_raises) @@ -306,17 +320,16 @@ def test_the_actionable_branches_keep_high_and_repeat_visible_ids( service._adopt_legacy_clone() service._adopt_legacy_clone() - assert len(items) == 2, f"{case}: expected one alarm per attempt" + assert len(items) == 2, f"{case}: expected one reading per attempt" for item in items: assert marker in item["question"], f"{case}: drove the wrong branch" assert item["priority"] == "high", f"{case}: lost its severity" - assert not item["id"].startswith(STABLE_PREFIX), ( - f"{case}: took the steady-state id — a repeat of a genuine failure " - "is no longer visible as a repeat (AC 4)" - ) - assert items[0]["id"] != items[1]["id"], ( - f"{case}: two attempts share one request_id, so the second is deduped " - "away by ON CONFLICT DO NOTHING" + # #3246: the failure branches share the URL's one subject with the steady + # state, so one sync never files a second row (#2744's guarantee); the + # repeat stays visible as the row's seen count, not as a new row. + assert {i["subject"] for i in items} == {expected_subject(url)}, ( + f"{case}: two attempts reported two subjects — each would file its own " + "row, which is the per-sync growth #2744 removed" ) @@ -341,20 +354,24 @@ def test_the_stable_id_is_reserved_and_id_shaped(monkeypatch): service = _service() service._adopt_legacy_clone() + from services import platform_alerts + assert items _assert_terminal_branch(items[0]) - item_id = items[0]["id"] + subject = items[0]["subject"] + prefix = platform_alerts.KINDS["skills_legacy_adoption"].prefix - assert any(item_id.startswith(p) for p in oqs._RESERVED_ID_PREFIXES), ( - f"{item_id!r} uses no reserved prefix — an agent can pre-create it and " + assert any(prefix.startswith(p) for p in oqs._RESERVED_ID_PREFIXES), ( + f"{prefix!r} is no reserved prefix — an agent can pre-create the row and " "suppress the platform's own alarm (#1632 C2)" ) - digest = item_id[len(STABLE_PREFIX):] + digest = subject.split(":", 1)[1] assert re.fullmatch(r"[0-9a-f]{12}", digest), ( - "the id segment is not a 12-char sha256 prefix" + "the subject key is not a 12-char sha256 prefix" ) - assert TERMINAL_URL not in item_id, "the raw URL must never be the conflict key" - assert _ID_RE.match(item_id) and len(item_id) <= _DB_BELT_ID_MAX + assert TERMINAL_URL not in subject, "the raw URL must never be the conflict key" + request_id = f"{prefix}{digest}-2026-09-14T00:00:00.000000Z" + assert _ID_RE.match(request_id) and len(request_id) <= _DB_BELT_ID_MAX # --------------------------------------------------------------------------- diff --git a/tests/unit/test_3246_platform_alert_emitters.py b/tests/unit/test_3246_platform_alert_emitters.py index ec25e23e9a..3ca5dd008f 100644 --- a/tests/unit/test_3246_platform_alert_emitters.py +++ b/tests/unit/test_3246_platform_alert_emitters.py @@ -170,3 +170,49 @@ def test_the_sweep_keeps_an_unassessable_row_and_clears_a_measured_recovery( asyncio.run(svc.SubscriptionRecoveryService()._evaluate_alerts(subs, results, 75)) assert self._rows(db, a.subject_key(recovered)) == [] assert len(self._rows(db, a.subject_key(unknown))) == 1 + + +class TestSkillsLegacyAdoption: + AGENT = "_skills-sync" + + @pytest.fixture + def svc(self): + from services.skill_service import SkillService + return SkillService.__new__(SkillService) + + @pytest.fixture + def url(self): + return f"https://github.com/acme/skills-{uuid.uuid4().hex[:6]}" + + def _rows(self, db, svc, url): + subject = f"skills_legacy_adoption:{svc._adoption_alert_key(url)}" + return [r for r in _pending(db, self.AGENT) if r["subject"] == subject] + + def test_repeated_syncs_and_a_failure_are_one_row(self, db, svc, url): + """#2744's guarantee on the real DB: no sync files a second row.""" + for _ in range(3): + svc._record_adoption_failure(url, "already has sources", steady_state=True) + svc._record_adoption_failure(url, "adoption failed: boom") + rows = self._rows(db, svc, url) + assert len(rows) == 1 + assert rows[0]["priority"] == "high" and "boom" in rows[0]["question"] + + def test_removing_the_setting_ends_the_row_as_the_platform(self, db, svc, url): + svc._record_adoption_failure(url, "already has sources", steady_state=True) + [row] = self._rows(db, svc, url) + svc._reconcile_adoption_alerts(None) + assert self._rows(db, svc, url) == [] + assert _ended(db, row)["disposed_by"] == "platform" + + def test_a_changed_setting_ends_only_the_old_urls_row(self, db, svc, url): + new = url + "-new" + svc._record_adoption_failure(url, "already has sources", steady_state=True) + svc._record_adoption_failure(new, "already has sources", steady_state=True) + svc._reconcile_adoption_alerts(new) + assert self._rows(db, svc, url) == [] + assert len(self._rows(db, svc, new)) == 1 + + def test_adoption_success_ends_the_row(self, db, svc, url): + svc._record_adoption_failure(url, "adoption failed: boom") + svc._clear_adoption_alert(url) + assert self._rows(db, svc, url) == [] diff --git a/tests/unit/test_3246_platform_alerts_registry.py b/tests/unit/test_3246_platform_alerts_registry.py index a6e0799121..d4bc0365c4 100644 --- a/tests/unit/test_3246_platform_alerts_registry.py +++ b/tests/unit/test_3246_platform_alerts_registry.py @@ -43,8 +43,6 @@ # value: (the id prefix it writes, the follow-up that moves it). # --------------------------------------------------------------------------- EMITTERS_NOT_YET_ON_SEAM = { - ("services/skill_service.py", "SkillService._record_adoption_failure"): - ("skills-legacy-adoption-", "#3246 C5′"), # -- PR 2 --------------------------------------------------------------- ("services/skill_service.py", "SkillService._announce_reconcile_refusal"): ("skills-reconcile-", _PR2), diff --git a/tests/unit/test_ent346_skills_source_injection.py b/tests/unit/test_ent346_skills_source_injection.py index 215efc4aca..5770406c87 100644 --- a/tests/unit/test_ent346_skills_source_injection.py +++ b/tests/unit/test_ent346_skills_source_injection.py @@ -214,8 +214,11 @@ def test_adoption_is_refused_once_the_install_has_sources(monkeypatch): monkeypatch.setattr(svc_mod.db, "list_skill_sources", lambda: []) monkeypatch.setattr(svc_mod.db, "count_skill_sources", lambda: 3) # configured alarms = [] + # #3246: the refusal reports through the platform alert seam. + from services import platform_alerts monkeypatch.setattr( - svc_mod.db, "create_operator_queue_item", lambda agent, item: alarms.append(item) + platform_alerts, "observe", + lambda agent, kind, key=None, **kw: alarms.append({"type": "alert", **kw}), ) service = svc_mod.SkillService.__new__(svc_mod.SkillService) @@ -277,8 +280,11 @@ def test_a_refusal_raises_an_operator_alarm_not_just_a_log_line(monkeypatch): monkeypatch.setattr(svc_mod, "get_skills_library_branch", lambda: "main", raising=False) monkeypatch.setattr(svc_mod.db, "list_skill_sources", lambda: []) monkeypatch.setattr(svc_mod.db, "count_skill_sources", lambda: 0) + # #3246: the refusal reports through the platform alert seam. + from services import platform_alerts monkeypatch.setattr( - svc_mod.db, "create_operator_queue_item", lambda agent, item: alarms.append(item) + platform_alerts, "observe", + lambda agent, kind, key=None, **kw: alarms.append({"type": "alert", **kw}), ) service = svc_mod.SkillService.__new__(svc_mod.SkillService) @@ -302,7 +308,8 @@ def test_the_alarm_never_blocks_a_sync(monkeypatch): def boom(*a, **k): raise RuntimeError("operator queue down") - monkeypatch.setattr(svc_mod.db, "create_operator_queue_item", boom) + from services import platform_alerts + monkeypatch.setattr(platform_alerts, "observe", boom) service = svc_mod.SkillService.__new__(svc_mod.SkillService) service.library_root = Path(tempfile.mkdtemp()) From 1f49c7406573cf35666c67ccebe6e6923153beb1 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Mon, 5 Oct 2026 14:33:12 -0400 Subject: [PATCH 10/19] feat(operator-queue): platform-ended alerts and the seen count on the queue surfaces (#3246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A row the platform ended (disposed_by='platform', reason condition_cleared | superseded) now reads "Ended by the platform — " on ResolvedCard, QueueItemDetail, /m and the Workspace ask views (one rule: utils/operatorQueue.js::queueEnding/queueEndingText) — never a person's answer, never a timeout, never the raw reason token as an operator note. An expired alert reads "nobody acted on it". client_portal/asks/service.py::_ending_of returns "platform" for such a row instead of falling through to "operator". A pending alert seen more than once shows "seen N times · last seen " (queueSeenLine) on QueueCard, QueueItemDetail and /m; the Workspace asks panel never renders platform alerts, so it gets no line. Canary _PLATFORM_ALARM_SENTINELS gains _skills-sync (L-03 would otherwise read the seam's rows as orphans). Stale "person | timeout" strings corrected in dependencies.py, mcp-server types.ts and the get_my_ask description. Tests: platformAlertSurfaces.spec.js (mounted; 12/15 red with the util change reverted), test_3246_platform_alert_surfaces.py (4/7 red with the backend change reverted). Co-Authored-By: Claude Opus 5.5 --- src/backend/canary/snapshot.py | 7 +- src/backend/client_portal/asks/models.py | 2 +- src/backend/client_portal/asks/service.py | 6 +- src/backend/dependencies.py | 5 +- .../src/components/operator/QueueCard.vue | 7 +- .../components/operator/QueueItemDetail.vue | 23 +- .../src/components/operator/ResolvedCard.vue | 5 +- src/frontend/src/utils/operatorQueue.js | 60 +++++- src/frontend/src/views/MobileAdmin.vue | 4 +- .../tests/unit/platformAlertSurfaces.spec.js | 198 ++++++++++++++++++ src/mcp-server/src/tools/operator_queue.ts | 2 +- src/mcp-server/src/types.ts | 4 +- .../unit/test_3246_platform_alert_surfaces.py | 81 +++++++ 13 files changed, 383 insertions(+), 21 deletions(-) create mode 100644 src/frontend/tests/unit/platformAlertSurfaces.spec.js create mode 100644 tests/unit/test_3246_platform_alert_surfaces.py diff --git a/src/backend/canary/snapshot.py b/src/backend/canary/snapshot.py index f642a5dbe6..4d7c35daf8 100644 --- a/src/backend/canary/snapshot.py +++ b/src/backend/canary/snapshot.py @@ -153,8 +153,13 @@ _LOG_ARCHIVE_AGENT = "_log-archive" # ent#434: the weekly-headroom alarm host. _SUB_HEADROOM_AGENT = "_sub-headroom" +# #3246: the skills-library alarm host (mirrors +# services.skill_service.RECONCILE_ALARM_AGENT_NAME) — its rows now arrive +# through the platform-alert seam and would otherwise read as L-03 orphans. +_SKILLS_SYNC_AGENT = "_skills-sync" _PLATFORM_ALARM_SENTINELS = (_RETENTION_GUARD_AGENT, _DB_BACKUP_AGENT, - _LOG_ARCHIVE_AGENT, _SUB_HEADROOM_AGENT) + _LOG_ARCHIVE_AGENT, _SUB_HEADROOM_AGENT, + _SKILLS_SYNC_AGENT) _SENTINEL_SQL_LIST = ", ".join(f"'{name}'" for name in _PLATFORM_ALARM_SENTINELS) ORPHAN_SCAN_TABLES = [ diff --git a/src/backend/client_portal/asks/models.py b/src/backend/client_portal/asks/models.py index d8e958b838..5225d0bf96 100644 --- a/src/backend/client_portal/asks/models.py +++ b/src/backend/client_portal/asks/models.py @@ -35,7 +35,7 @@ class WorkspaceAsk(BaseModel): # (trinity-enterprise#611); the ANSWER response projects the row it recorded. status: str # How the ask ended, COARSE on purpose (trinity-enterprise#611): `ended_by` - # is `you` | `operator` | `timeout`, never an email, and the operator's + # is `you` | `operator` | `platform` (#3246) | `timeout`, never an email, and the operator's # cancel reason never crosses. `ended_at` is when it ended — None when the # platform does not know (a row that ended before the ledger), never the # time the ask was filed. diff --git a/src/backend/client_portal/asks/service.py b/src/backend/client_portal/asks/service.py index c4de024584..abbd3bd5cb 100644 --- a/src/backend/client_portal/asks/service.py +++ b/src/backend/client_portal/asks/service.py @@ -104,7 +104,9 @@ def _ending_of(item: dict, viewer_email: Optional[str]) -> tuple: """`(ended_at, ended_by)` for a client — COARSE on purpose. `ended_by` is `you` (the viewer answered), `operator` (another person - answered or cancelled) or `timeout`; never an email and never the cancel + answered or cancelled), `platform` (the platform ended the row itself — + `disposed_by = 'platform'`, #3246: never a person's answer, never a + timeout) or `timeout`; never an email and never the cancel reason (both are the operator's, not the client's). `ended_at` is the ledger's time, or a legacy answer's time — never `created_at`, which is when the ask was filed, not when it ended. @@ -114,6 +116,8 @@ def _ending_of(item: dict, viewer_email: Optional[str]) -> tuple: return None, None if status == "expired": return item.get("disposed_at"), "timeout" + if item.get("disposed_by") == "platform": + return item.get("disposed_at"), "platform" by = item.get("disposed_by_email") or (item.get("responded_by_email") if status == "answered" else None) who = "you" if by and viewer_email and by.lower() == viewer_email.lower() else "operator" at = item.get("disposed_at") or (item.get("responded_at") if status == "answered" else None) diff --git a/src/backend/dependencies.py b/src/backend/dependencies.py index 5fd2b8af19..178feb611d 100644 --- a/src/backend/dependencies.py +++ b/src/backend/dependencies.py @@ -1162,8 +1162,9 @@ def is_interactive_principal(current_user: User) -> bool: def is_person_principal(current_user: User) -> bool: """Is this caller a PERSON — the only author an ask's ending may record? - The endings ledger records `disposed_by` as an enum of two, `person` or - `timeout` (trinity-enterprise#611). An agent-scoped key resolves to its OWNER + The endings ledger records `disposed_by` as `person`, `timeout` or — for a + platform alert the platform itself ended (#3130/#3246) — `platform`; only + `person` is a principal (trinity-enterprise#611). An agent-scoped key resolves to its OWNER carrying the owner's role, so before this an agent could answer or cancel any ask its owner could reach — its own approval included — and the row recorded the owner. Recording that as `person` would make the ledger lie. diff --git a/src/frontend/src/components/operator/QueueCard.vue b/src/frontend/src/components/operator/QueueCard.vue index 99f38e5288..710c278b2a 100644 --- a/src/frontend/src/components/operator/QueueCard.vue +++ b/src/frontend/src/components/operator/QueueCard.vue @@ -30,6 +30,10 @@

+ +

{{ seenLine }}

@@ -218,7 +222,7 @@ import AskMarkdown from './AskMarkdown.vue' import { useOperatorQueueStore } from '../../stores/operatorQueue' import { useAgentsStore } from '../../stores/agents' import { agentNameTooltip } from '../../utils/agentName' -import { queueTypeLabel, queueResponseKind, queueSyncBadge, queueReaskBadges } from '../../utils/operatorQueue' +import { queueTypeLabel, queueResponseKind, queueSyncBadge, queueReaskBadges, queueSeenLine } from '../../utils/operatorQueue' import AgentAvatar from '../AgentAvatar.vue' import BaseBadge from '../base/BaseBadge.vue' import InlineError from '../InlineError.vue' @@ -240,6 +244,7 @@ const agentsStore = useAgentsStore() const responseKind = computed(() => queueResponseKind(props.item)) // #2915: sync/delivery/aging badge and the refused-response notice. const syncBadge = computed(() => queueSyncBadge(props.item)) +const seenLine = computed(() => queueSeenLine(props.item)) // #3246 const reaskBadges = computed(() => queueReaskBadges(props.item, store.items)) // trinity-enterprise#611 const diverged = computed(() => store.divergedItemId === props.item.id) const notAddressee = computed(() => store.notAddresseeItemId === props.item.id) diff --git a/src/frontend/src/components/operator/QueueItemDetail.vue b/src/frontend/src/components/operator/QueueItemDetail.vue index 2d1b3c4330..7972717a56 100644 --- a/src/frontend/src/components/operator/QueueItemDetail.vue +++ b/src/frontend/src/components/operator/QueueItemDetail.vue @@ -40,6 +40,8 @@

{{ item.title }}

+ +

{{ seenLine }}

@@ -98,11 +100,18 @@

Response

-

{{ item.response }}

-

{{ item.response_text }}

-

- by {{ item.responded_by_email }} · {{ formatDate(item.responded_at) }} + +

+ {{ endingText }}

+
@@ -195,7 +204,7 @@ import { renderMarkdown } from '../../utils/markdown' import { useOperatorQueueStore } from '../../stores/operatorQueue' import { useAgentsStore } from '../../stores/agents' import { agentNameTooltip } from '../../utils/agentName' -import { queueResponseKind, queueTypeLabel } from '../../utils/operatorQueue' +import { queueResponseKind, queueTypeLabel, queueEnding, queueEndingText, queueSeenLine } from '../../utils/operatorQueue' const store = useOperatorQueueStore() const agentsStore = useAgentsStore() @@ -209,6 +218,10 @@ const item = computed(() => store.selectedItem) // closed from the queue at all — and a budgeted alert type whose items // cannot be closed jams its own pending cap permanently. const responseKind = computed(() => queueResponseKind(item.value)) +const ending = computed(() => queueEnding(item.value)) +const endedByPlatform = computed(() => ending.value?.who === 'the platform') +const endingText = computed(() => queueEndingText(ending.value)) +const seenLine = computed(() => queueSeenLine(item.value)) const selectedOption = ref(null) const responseText = ref('') diff --git a/src/frontend/src/components/operator/ResolvedCard.vue b/src/frontend/src/components/operator/ResolvedCard.vue index f0be831fed..3f14ffc3f3 100644 --- a/src/frontend/src/components/operator/ResolvedCard.vue +++ b/src/frontend/src/components/operator/ResolvedCard.vue @@ -105,7 +105,10 @@ const isTerminalWithoutResponse = computed(() => ) const ending = computed(() => queueEnding(props.item)) // trinity-enterprise#611 const endingText = computed(() => queueEndingText(ending.value)) -const note = computed(() => props.item.response_text || props.item.disposition_reason || '') +// #3246: a platform ending's reason is a token the ending text already words +// (`queueEndingText`), never printed raw as though an operator wrote it. +const note = computed(() => props.item.response_text + || (ending.value && ending.value.who === 'the platform' ? '' : props.item.disposition_reason) || '') const endedAtAbsolute = computed(() => (ending.value?.when ? formatLocalDateTime(ending.value.when) : '')) // An answer's "who" rides beside its time; a cancel / expiry names it in its label. const endingMeta = computed(() => { diff --git a/src/frontend/src/utils/operatorQueue.js b/src/frontend/src/utils/operatorQueue.js index 2c4bcaa140..8b7b38d027 100644 --- a/src/frontend/src/utils/operatorQueue.js +++ b/src/frontend/src/utils/operatorQueue.js @@ -334,8 +334,15 @@ export const QUEUE_RESPONSE_NOT_ADDRESSEE = * Workspace projection's own `status` (`answered`) reads the same way. * - `who` — the person, for the Operating Room (`disposed_by_email`, or a * legacy answer's `responded_by_email`); the Workspace projection's coarse - * `ended_by` (`you` / `the operator`); `timeout` for an expiry; `null` when - * the platform does not know. + * `ended_by` (`you` / `the operator`); `timeout` for an expiry; `the + * platform` for a row the platform itself ended (#3246: `disposed_by = + * 'platform'`, or the projection's `ended_by = 'platform'`); `null` when the + * platform does not know. + * - `reason` — #3246: the platform's ending in words (`condition_cleared`, + * `superseded`); `null` for every other ending. A platform ending is never a + * person's answer and never a timeout. + * - `alert` — the row is an alert: its expiry reads "nobody acted on it", + * because nobody was asked anything. * - `when` — the ledger's `disposed_at`, the projection's `ended_at`, or a * legacy answer's `responded_at`. NEVER `created_at`: that is when the ask * was filed, and showing it as the ending time is the defect this replaces. @@ -347,6 +354,13 @@ export const ENDING_LABELS = Object.freeze({ expired: 'Expired', }) +/** #3246 — why the platform ended a row, in words. An unknown reason token + * falls back to nothing rather than being printed raw. */ +export const PLATFORM_ENDING_REASONS = Object.freeze({ + condition_cleared: 'the condition cleared', + superseded: 'superseded by a newer reading', +}) + function endingKind(item) { const d = item.disposition if (d === 'answered' || d === 'dismissed' || d === 'cancelled' || d === 'expired') return d @@ -362,8 +376,12 @@ export function queueEnding(item) { if (!kind) return null const when = item.disposed_at || item.ended_at || (kind === 'answered' ? item.responded_at : null) || null let who = null + let reason = null if (kind === 'expired') { who = 'timeout' + } else if (item.disposed_by === 'platform' || item.ended_by === 'platform') { + who = 'the platform' + reason = PLATFORM_ENDING_REASONS[item.disposition_reason] || null } else if (item.ended_by === 'you') { who = 'you' } else if (item.ended_by === 'operator') { @@ -371,17 +389,49 @@ export function queueEnding(item) { } else { who = item.disposed_by_email || (kind === 'answered' ? item.responded_by_email : null) || null } - return { kind, label: ENDING_LABELS[kind], who, when } + return { kind, label: ENDING_LABELS[kind], who, when, reason, alert: item.type === 'alert' } } /** The ending in words: "Cancelled by op@…", "Answered by you", - * "Expired — nobody answered in time", or the bare label when nobody is known. */ + * "Ended by the platform — the condition cleared", "Expired — nobody answered + * in time" ("nobody acted on it" for an alert), or the bare label when nobody + * is known. */ export function queueEndingText(ending) { if (!ending) return '' - if (ending.kind === 'expired') return `${ending.label} — nobody answered in time` + if (ending.kind === 'expired') { + return `${ending.label} — ${ending.alert ? 'nobody acted on it' : 'nobody answered in time'}` + } + if (ending.who === 'the platform') { + return ending.reason ? `Ended by the platform — ${ending.reason}` : 'Ended by the platform' + } return ending.who ? `${ending.label} by ${ending.who}` : ending.label } +function relativeAgo(iso, now) { + const then = new Date(iso).getTime() + if (Number.isNaN(then)) return null + const diffMin = Math.floor((now - then) / 60000) + if (diffMin < 1) return 'just now' + if (diffMin < 60) return `${diffMin}m ago` + const diffHr = Math.floor(diffMin / 60) + if (diffHr < 24) return `${diffHr}h ago` + return `${Math.floor(diffHr / 24)}d ago` +} + +/** + * #3246 — a pending platform alert is ONE row per condition, updated in place by + * every new reading, so a row seen many times must not read as a fresh alert. + * "seen N times · last seen 5m ago" when `context.seen_count > 1`; `''` for an + * ended row, a row seen once, and anything that is not an alert. + */ +export function queueSeenLine(item, now = Date.now()) { + if (!item || item.status !== 'pending' || item.type !== 'alert') return '' + const n = Number(item.context && item.context.seen_count) + if (!Number.isInteger(n) || n < 2) return '' + const ago = item.last_seen_at ? relativeAgo(item.last_seen_at, now) : null + return ago ? `seen ${n} times · last seen ${ago}` : `seen ${n} times` +} + /** What the resolved feed sorts by (#627 AC6): when the item ended; a legacy * answer's time; else — the only timestamp such a row has — when it was filed. */ export function queueEndingSortTime(item) { diff --git a/src/frontend/src/views/MobileAdmin.vue b/src/frontend/src/views/MobileAdmin.vue index 32f70c5282..cd1a67baef 100644 --- a/src/frontend/src/views/MobileAdmin.vue +++ b/src/frontend/src/views/MobileAdmin.vue @@ -258,6 +258,8 @@
{{ queueTypeLabel(item.type) }}

+ +

{{ queueSeenLine(item) }}