From b63131e863be38ba1274d1077543aaf9320500b3 Mon Sep 17 00:00:00 2001 From: tegwick Date: Fri, 4 Sep 2026 12:57:12 +0200 Subject: [PATCH] feat: reconcile granted ops run closes Assistant: codex Assistant-Model: gpt-5.6-sol Assistant-Session: 01a06bfe-2a55-7ed3-bacd-879977b099bf --- docs/ops-run-queue.md | 41 +++- ...add_repository_grant_and_close_identity.py | 42 ++++ src/activity_core/activities.py | 23 ++ src/activity_core/definition_parser.py | 17 ++ src/activity_core/glas_evidence.py | 138 ++++++++++++ src/activity_core/models.py | 4 + src/activity_core/ops_run_queue.py | 201 +++++++++++++++--- src/activity_core/ops_runs_api.py | 58 +++-- src/activity_core/orm.py | 7 + src/activity_core/repository_grant.py | 133 ++++++++++++ src/activity_core/rules/actions.py | 5 + src/activity_core/run_artifacts.py | 8 + tests/rules/test_actions.py | 29 +++ tests/test_glas_evidence.py | 59 +++++ tests/test_issue_sink.py | 53 +++++ tests/test_ops_run_queue.py | 194 +++++++++++++++++ tests/test_ops_runs_api.py | 25 +++ tests/test_repository_grant.py | 82 +++++++ tests/test_run_artifacts.py | 2 + tests/test_sync_activity_definitions.py | 36 ++++ ...7-repository-grant-close-reconciliation.md | 126 +++++++++++ 21 files changed, 1240 insertions(+), 43 deletions(-) create mode 100644 migrations/versions/0010_add_repository_grant_and_close_identity.py create mode 100644 src/activity_core/repository_grant.py create mode 100644 tests/test_repository_grant.py create mode 100644 workplans/ACTIVITY-WP-0037-repository-grant-close-reconciliation.md diff --git a/docs/ops-run-queue.md b/docs/ops-run-queue.md index 55d7810..6b123c8 100644 --- a/docs/ops-run-queue.md +++ b/docs/ops-run-queue.md @@ -29,7 +29,9 @@ workplan task file. Not an issue-core or Forgejo ticket. | `approach_hint` | text nullable | **Legacy** definition-matching hint (ACT-ADR-006) | | `harness_profile_ref` | text nullable | Authoritative execution selector, pinned `@` | | `execution_refs` | jsonb | Attribution refs carried through, not authored here | +| `repository_grant` | jsonb nullable | Separately typed repository mutation authority; never inferred or stored in `execution_refs` | | `result` | JSONB | Completion metadata | +| `close_intent_digest` | text nullable | Digest of the accepted normalized terminal close intent; absent for legacy terminal and reopened rows | | `created_at` / `updated_at` | timestamptz | | ## API (actcore-api) @@ -62,6 +64,19 @@ workplan task file. Not an issue-core or Forgejo ticket. - Stale claims (`state=claimed` and `lease_until <= now()`) are reopened before select. - Heartbeat, complete, and fail lock the row and require an active lease (`lease_until > now()`). An expired worker cannot revive or close its claim. +- A first terminal close returns `close_disposition: applied`. If its response + is lost, an exact normalized repeat by the same claim owner returns HTTP 200 + with `close_disposition: reconciled`; it does not require the cleared lease + and does not mutate the row again. +- Refusals are machine-distinct. A missing row is HTTP 404 `not_found`; + wrong-owner, expired-lease, non-claimed-state, mismatched repository evidence, + and different terminal intents are HTTP 409 with codes `wrong_owner`, + `expired_lease`, `state_conflict`, `evidence_conflict`, and + `terminal_conflict` respectively. A pre-migration terminal row has no close + digest and therefore fails closed as `terminal_conflict`. +- A failure accepted with `reopen: true` below the attempt ceiling is not + terminal: it clears owner/lease/close identity and returns to `open`. A repeat + cannot be treated as terminal reconciliation. ### Complete / fail body @@ -112,6 +127,16 @@ prompts, messages, provider responses, credential fields, unknown nested blobs, and undeclared refs are never stored. Read projections normalize historic rows again before returning them. +Repository transaction evidence is independently allowlisted under +`result.repository_transaction`: bounded transaction/correlation identifiers, +baseline digests, repository grant and acceptance-policy identifiers, accepted +commit/path evidence, and external-metrics identity. Raw grant patterns, +provider/tool payloads, and unknown fields are dropped. If an `ops_run` carries +a repository grant, completion additionally requires the result's grant id, +acceptance-policy id, and positive acceptance evidence to match the queued +grant. Failure may omit transaction evidence when setup never began; supplied +grant evidence must match. + ## Emit path On `emit_tasks` (when `OPS_RUN_QUEUE_ENABLED` is truthy, **default true**): @@ -210,7 +235,8 @@ resolver rather than at emission. That residual gap is accepted and recorded in ACT-ADR-006; closing it needs a scoped glas-harness API, not a local catalogue. Task-emitting rules declare the selector and optional attribution refs on the -action; instructions use the same fields at instruction level: +action; instructions use the same fields at instruction level. Repository +authority is a separate, static declaration and is never template-rendered: ```yaml action: @@ -220,6 +246,11 @@ action: execution_refs: correlation_id: context.request.correlation_id goal_refs: [context.request.goal_ref] + repository_grant: + version: "1" + allowed_paths: [docs/, README.md] + commit_count: {min: 1, max: 1} + publish: false ``` File sync validates every declared profile structurally and rejects malformed @@ -228,6 +259,14 @@ batch before opening the database or IssueSink. A profile policy failure is a non-retryable activity error; no earlier item in that batch is emitted. Only the allowlisted attribution keys are carried to the queue. +Repository-grant v1 requires exactly `version`, `allowed_paths`, +`commit_count`, and `publish`. It accepts 1–100 unique, repository-relative +POSIX path patterns, commit bounds `1 <= min <= max <= 32`, and only +`publish: false`; absolute/traversal/backslash/`.git` grants, ambiguous scalar +types, unknown fields, and publication are rejected at definition sync and +again across the full emission batch. The admitted mapping is copied unchanged +to the queue claim/read response and never merged into `execution_refs`. + `ACTIVITY_CORE_REQUIRE_HARNESS_PROFILE=true` makes a missing profile ref an error both during file sync and emission. Deterministic report-only instructions are exempt because they create no execution request. The flag diff --git a/migrations/versions/0010_add_repository_grant_and_close_identity.py b/migrations/versions/0010_add_repository_grant_and_close_identity.py new file mode 100644 index 0000000..14bb8bf --- /dev/null +++ b/migrations/versions/0010_add_repository_grant_and_close_identity.py @@ -0,0 +1,42 @@ +"""add repository grant and terminal close identity + +Revision ID: 0010 +Revises: 0009 +Create Date: 2026-09-04 + +ACTIVITY-WP-0037 — authoritative repository-grant carriage and exact terminal +close reconciliation. Both columns are additive so legacy and open rows remain +valid; pre-migration terminal rows deliberately cannot claim exact-repeat +reconciliation because they have no accepted intent digest. +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +revision: str = "0010" +down_revision: str | Sequence[str] | None = "0009" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.add_column( + "ops_runs", + sa.Column( + "repository_grant", + postgresql.JSONB(astext_type=sa.Text()), + nullable=True, + ), + ) + op.add_column( + "ops_runs", + sa.Column("close_intent_digest", sa.Text(), nullable=True), + ) + + +def downgrade() -> None: + op.drop_column("ops_runs", "close_intent_digest") + op.drop_column("ops_runs", "repository_grant") diff --git a/src/activity_core/activities.py b/src/activity_core/activities.py index 7aef4b3..0db2127 100644 --- a/src/activity_core/activities.py +++ b/src/activity_core/activities.py @@ -536,6 +536,11 @@ async def evaluate_instructions(payload: dict) -> dict: "approach_hint": instruction.approach_hint, "harness_profile_ref": instruction.harness_profile_ref, "execution_refs": instruction.execution_refs, + "repository_grant": ( + instruction.repository_grant.payload() + if instruction.repository_grant is not None + else None + ), }) return {"task_specs": task_specs, "reports": reports} @@ -579,6 +584,10 @@ async def emit_tasks(payload: dict) -> list[str]: normalise_execution_refs, resolve_execution_selector, ) + from activity_core.repository_grant import ( + RepositoryGrantError, + validate_repository_grant, + ) validated_specs: list[dict] = [] for index, raw_spec in enumerate(task_specs_raw): @@ -604,6 +613,19 @@ async def emit_tasks(payload: dict) -> list[str]: spec_dict["execution_refs"] = normalise_execution_refs( spec_dict.get("execution_refs") ) + raw_grant = spec_dict.get("repository_grant") + if raw_grant is not None: + try: + spec_dict["repository_grant"] = validate_repository_grant(raw_grant) + except RepositoryGrantError as exc: + source = ( + f"{spec_dict.get('source_type', 'rule')}:" + f"{spec_dict.get('source_id', '')}" + ) + raise ApplicationError( + f"repository grant refused for {source}: {exc}", + non_retryable=True, + ) from exc validated_specs.append(spec_dict) sink = get_issue_sink() @@ -640,6 +662,7 @@ async def emit_tasks(payload: dict) -> list[str]: approach_hint=spec_dict.get("approach_hint"), harness_profile_ref=spec_dict.get("harness_profile_ref"), execution_refs=spec_dict.get("execution_refs"), + repository_grant=spec_dict.get("repository_grant"), ) if ops_id is not None: activity.logger.info( diff --git a/src/activity_core/definition_parser.py b/src/activity_core/definition_parser.py index a49b9cb..0a7a1b8 100644 --- a/src/activity_core/definition_parser.py +++ b/src/activity_core/definition_parser.py @@ -23,6 +23,10 @@ from activity_core.glas_profile import ( require_harness_profile, validate_profile_ref, ) +from activity_core.repository_grant import ( + RepositoryGrantError, + validate_repository_grant, +) class ParseError(Exception): @@ -122,6 +126,19 @@ def _validate_execution_declarations( f"{location} execution_refs must be a YAML mapping", ) + repository_grant = declaration.get("repository_grant") + if repository_grant is not None: + try: + declaration["repository_grant"] = validate_repository_grant( + repository_grant + ) + except RepositoryGrantError as exc: + raise ParseError( + file, + None, + f"{location} has invalid repository_grant: {exc}", + ) from exc + def _normalise_review_advisory( instructions: list[dict[str, Any]], file: Path diff --git a/src/activity_core/glas_evidence.py b/src/activity_core/glas_evidence.py index 0d0f012..9bfec4b 100644 --- a/src/activity_core/glas_evidence.py +++ b/src/activity_core/glas_evidence.py @@ -156,6 +156,137 @@ def _normalise_artifact_urls(raw: Any) -> list[dict[str, str]]: return urls +def _normalise_repository_baseline(raw: Any) -> dict[str, Any]: + if not isinstance(raw, dict): + return {} + result: dict[str, Any] = {} + for key in ( + "repo_id", + "repo_name", + "head", + "branch", + "status_digest", + "index_diff_digest", + "worktree_diff_digest", + "upstream_ref", + "upstream_oid", + "remote_refs_digest", + "protected_git_metadata_digest", + ): + value = raw.get(key) + if isinstance(value, str): + result[key] = value[:500] + elif value is None and key in {"branch", "upstream_ref", "upstream_oid"}: + result[key] = None + for key in ("detached", "clean"): + value = raw.get(key) + if isinstance(value, bool): + result[key] = value + for key in ("dirty_entries", "remote_ref_count"): + value = raw.get(key) + if isinstance(value, int) and not isinstance(value, bool) and value >= 0: + result[key] = min(value, 1_000_000) + return result + + +def _normalise_repository_grant_evidence(raw: Any) -> dict[str, Any]: + if not isinstance(raw, dict): + return {} + result: dict[str, Any] = {} + for key in ( + "grant_id", + "acceptance_policy_id", + "version", + "allowed_paths_digest", + ): + value = raw.get(key) + if isinstance(value, str): + result[key] = value[:500] + for key in ("allowed_path_count", "min_commits", "max_commits"): + value = raw.get(key) + if isinstance(value, int) and not isinstance(value, bool) and value >= 0: + result[key] = min(value, 1_000_000) + publish = raw.get("publish") + if isinstance(publish, bool): + result["publish"] = publish + return result + + +def _normalise_repository_acceptance(raw: Any) -> dict[str, Any]: + if not isinstance(raw, dict): + return {} + result: dict[str, Any] = {} + for key in ( + "policy_id", + "head", + "branch", + "changed_paths_digest", + ): + value = raw.get(key) + if isinstance(value, str): + result[key] = value[:500] + elif value is None and key == "branch": + result[key] = None + for key in ( + "accepted", + "changed_paths_truncated", + "clean_post_state", + "remote_refs_unchanged", + "protected_git_metadata_unchanged", + ): + value = raw.get(key) + if isinstance(value, bool): + result[key] = value + for key in ("commit_count", "changed_path_count"): + value = raw.get(key) + if isinstance(value, int) and not isinstance(value, bool) and value >= 0: + result[key] = min(value, 1_000_000) + for key, limit in (("commits", 32), ("changed_paths", 100)): + value = raw.get(key) + if isinstance(value, list): + result[key] = [ + item[:500] for item in value[:limit] if isinstance(item, str) + ] + return result + + +def _normalise_repository_metrics(raw: Any) -> dict[str, Any]: + if not isinstance(raw, dict): + return {} + result: dict[str, Any] = {} + for key in ("storage", "session_id"): + value = raw.get(key) + if isinstance(value, str): + result[key] = value[:500] + projection_ready = raw.get("projection_ready") + if isinstance(projection_ready, bool): + result["projection_ready"] = projection_ready + return result + + +def normalise_repository_transaction(raw: Any) -> dict[str, Any]: + """Allowlist bounded rein repository transaction/acceptance evidence.""" + if not isinstance(raw, dict): + return {} + + result: dict[str, Any] = {} + for key in ("transaction_id", "correlation_id"): + value = raw.get(key) + if isinstance(value, str): + result[key] = value[:500] + + for key, normalizer in ( + ("baseline", _normalise_repository_baseline), + ("repository_grant", _normalise_repository_grant_evidence), + ("acceptance", _normalise_repository_acceptance), + ("metrics", _normalise_repository_metrics), + ): + value = normalizer(raw.get(key)) + if value: + result[key] = value + return result + + def normalise_ops_result(raw: Any) -> dict[str, Any]: """Allowlist legacy completion facts plus normalized Glas evidence.""" if not isinstance(raw, dict): @@ -177,4 +308,11 @@ def normalise_ops_result(raw: Any) -> dict[str, Any]: evidence = normalise_execution_evidence(raw_evidence) if evidence: result["execution_evidence"] = evidence + + raw_transaction = raw.get("repository_transaction") + if not isinstance(raw_transaction, dict): + raw_transaction = raw.get("transaction") + transaction = normalise_repository_transaction(raw_transaction) + if transaction: + result["repository_transaction"] = transaction return result diff --git a/src/activity_core/models.py b/src/activity_core/models.py index 6d8a617..c79ed5d 100644 --- a/src/activity_core/models.py +++ b/src/activity_core/models.py @@ -11,6 +11,8 @@ from uuid import UUID from pydantic import BaseModel, Field, model_validator +from activity_core.repository_grant import RepositoryGrant + # ── EventEnvelope (T40) ─────────────────────────────────────────────────────── @@ -102,6 +104,7 @@ class ActionDef(BaseModel): approach_hint: str | None = Field(default=None) harness_profile_ref: str | None = Field(default=None) execution_refs: dict[str, Any] = Field(default_factory=dict) + repository_grant: RepositoryGrant | None = Field(default=None) class RuleDef(BaseModel): @@ -152,6 +155,7 @@ class InstructionDef(BaseModel): approach_hint: str | None = Field(default=None) harness_profile_ref: str | None = Field(default=None) execution_refs: dict[str, Any] = Field(default_factory=dict) + repository_grant: RepositoryGrant | None = Field(default=None) @model_validator(mode="before") @classmethod diff --git a/src/activity_core/ops_run_queue.py b/src/activity_core/ops_run_queue.py index 8b65c97..a3a3832 100644 --- a/src/activity_core/ops_run_queue.py +++ b/src/activity_core/ops_run_queue.py @@ -2,8 +2,11 @@ from __future__ import annotations +import hashlib +import json import os import uuid +from dataclasses import dataclass from datetime import datetime, timedelta, timezone from typing import Any @@ -11,18 +14,47 @@ from sqlalchemy import Select, and_, func, select, update from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.ext.asyncio import AsyncSession +from activity_core.glas_evidence import normalise_ops_result from activity_core.glas_profile import ( normalise_execution_refs, resolve_execution_selector, ) -from activity_core.glas_evidence import normalise_ops_result from activity_core.orm import OpsRun +from activity_core.repository_grant import ( + RepositoryGrant, + validate_repository_grant, +) from activity_core.rules.models import TaskSpec OPS_RUN_STATES = frozenset( {"open", "claimed", "succeeded", "failed", "expired"} ) +CLOSE_DISPOSITIONS = frozenset( + { + "applied", + "reconciled", + "not_found", + "wrong_owner", + "expired_lease", + "state_conflict", + "terminal_conflict", + "evidence_conflict", + } +) + + +@dataclass(frozen=True) +class CloseOpsRunOutcome: + """Explicit result of a row-locked terminal/reopen mutation decision.""" + + disposition: str + row: OpsRun | None + + @property + def accepted(self) -> bool: + return self.disposition in {"applied", "reconciled"} + def ops_run_queue_enabled() -> bool: raw = (os.environ.get("OPS_RUN_QUEUE_ENABLED") or "true").strip().lower() @@ -113,7 +145,11 @@ def ops_run_to_dict(row: OpsRun) -> dict[str, Any]: "approach_hint": row.approach_hint, "harness_profile_ref": row.harness_profile_ref, "execution_refs": dict(row.execution_refs or {}), + "repository_grant": ( + dict(row.repository_grant) if row.repository_grant is not None else None + ), "result": normalise_ops_result(row.result), + "close_intent_digest": row.close_intent_digest, "created_at": row.created_at.isoformat() if row.created_at else None, "updated_at": row.updated_at.isoformat() if row.updated_at else None, } @@ -126,6 +162,7 @@ async def create_ops_run_from_spec( approach_hint: str | None = None, harness_profile_ref: str | None = None, execution_refs: dict | None = None, + repository_grant: dict | None = None, ) -> uuid.UUID | None: """Insert ops_run if queue enabled; return id or None if disabled/duplicate.""" if not ops_run_queue_enabled(): @@ -163,7 +200,13 @@ async def create_ops_run_from_spec( approach_hint=hint, harness_profile_ref=profile_ref, execution_refs=normalise_execution_refs(execution_refs), + repository_grant=( + validate_repository_grant(repository_grant) + if repository_grant is not None + else None + ), result={}, + close_intent_digest=None, created_at=now, updated_at=now, ) @@ -287,19 +330,14 @@ async def complete_ops_run( worker_id: str, result: dict[str, Any] | None = None, ) -> OpsRun | None: - row = await session.get(OpsRun, run_id, with_for_update=True) - if row is None: - return None - if row.state != "claimed" or row.claim_owner != worker_id: - return None - now = _utcnow() - if not _has_active_lease(row, now=now): - return None - row.state = "succeeded" - row.lease_until = None - row.result = normalise_ops_result(result) - row.updated_at = now - return row + outcome = await close_ops_run( + session, + run_id, + worker_id=worker_id, + action="complete", + result=result, + ) + return outcome.row if outcome.accepted else None async def fail_ops_run( @@ -311,27 +349,142 @@ async def fail_ops_run( reopen: bool = False, result: dict[str, Any] | None = None, ) -> OpsRun | None: + outcome = await close_ops_run( + session, + run_id, + worker_id=worker_id, + action="fail", + error=error, + reopen=reopen, + result=result, + ) + return outcome.row if outcome.accepted else None + + +async def close_ops_run( + session: AsyncSession, + run_id: uuid.UUID, + *, + worker_id: str, + action: str, + error: str = "", + reopen: bool = False, + result: dict[str, Any] | None = None, +) -> CloseOpsRunOutcome: + """Apply or reconcile one completion/failure under a row lock. + + A first close still requires the authenticated owner and an active lease. + Once terminal, only an exact repeat of the persisted close intent by that + same owner is accepted, making a lost HTTP response safely reconcilable. + """ + if action not in {"complete", "fail"}: + raise ValueError("action must be 'complete' or 'fail'") + row = await session.get(OpsRun, run_id, with_for_update=True) if row is None: - return None - if row.state != "claimed" or row.claim_owner != worker_id: - return None + return CloseOpsRunOutcome("not_found", None) + + payload = normalise_ops_result(result) + if action == "fail" and error: + payload["error"] = error[:2000] + intent_digest = _close_intent_digest( + action=action, + result=payload, + reopen=reopen, + ) + terminal_state = "succeeded" if action == "complete" else "failed" + + if row.state in {"succeeded", "failed"}: + if row.claim_owner != worker_id: + return CloseOpsRunOutcome("wrong_owner", row) + if not _granted_close_evidence_matches(row, action=action, result=payload): + return CloseOpsRunOutcome("evidence_conflict", row) + if ( + row.state == terminal_state + and row.close_intent_digest == intent_digest + and normalise_ops_result(row.result) == payload + ): + return CloseOpsRunOutcome("reconciled", row) + return CloseOpsRunOutcome("terminal_conflict", row) + + if row.state != "claimed": + return CloseOpsRunOutcome("state_conflict", row) + if row.claim_owner != worker_id: + return CloseOpsRunOutcome("wrong_owner", row) + now = _utcnow() if not _has_active_lease(row, now=now): - return None - payload = normalise_ops_result(result) - if error: - payload["error"] = error[:2000] + return CloseOpsRunOutcome("expired_lease", row) + if not _granted_close_evidence_matches(row, action=action, result=payload): + return CloseOpsRunOutcome("evidence_conflict", row) + row.result = payload row.updated_at = now - if reopen and int(row.attempt or 0) < max_attempts(): + if action == "fail" and reopen and int(row.attempt or 0) < max_attempts(): row.state = "open" row.claim_owner = None row.lease_until = None + row.close_intent_digest = None else: - row.state = "failed" + row.state = terminal_state row.lease_until = None - return row + row.close_intent_digest = intent_digest + return CloseOpsRunOutcome("applied", row) + + +def _close_intent_digest( + *, + action: str, + result: dict[str, Any], + reopen: bool, +) -> str: + payload = json.dumps( + {"action": action, "reopen": reopen, "result": result}, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ) + return hashlib.sha256(payload.encode("utf-8")).hexdigest() + + +def _granted_close_evidence_matches( + row: OpsRun, + *, + action: str, + result: dict[str, Any], +) -> bool: + """Bind granted close evidence to the authority stored on the queue row.""" + raw_grant = row.repository_grant + if raw_grant is None: + return True + try: + grant = RepositoryGrant.model_validate(raw_grant) + except (TypeError, ValueError): + return False + + transaction = result.get("repository_transaction") + if not isinstance(transaction, dict): + return action == "fail" + + transaction_id = transaction.get("transaction_id") + grant_evidence = transaction.get("repository_grant") + if not isinstance(transaction_id, str) or not transaction_id: + return False + if not isinstance(grant_evidence, dict): + return False + if grant_evidence.get("grant_id") != grant.grant_id: + return False + if grant_evidence.get("acceptance_policy_id") != grant.acceptance_policy_id: + return False + + if action == "fail": + return True + acceptance = transaction.get("acceptance") + return bool( + isinstance(acceptance, dict) + and acceptance.get("accepted") is True + and acceptance.get("policy_id") == grant.acceptance_policy_id + ) async def list_ops_runs( diff --git a/src/activity_core/ops_runs_api.py b/src/activity_core/ops_runs_api.py index ba401fa..649a20f 100644 --- a/src/activity_core/ops_runs_api.py +++ b/src/activity_core/ops_runs_api.py @@ -5,10 +5,11 @@ from __future__ import annotations import hmac import os import uuid +from collections.abc import Callable from datetime import datetime -from typing import Any, Callable +from typing import Any -from fastapi import APIRouter, Depends, Header, HTTPException, Query, Request +from fastapi import APIRouter, Header, HTTPException, Query, Request from pydantic import BaseModel, Field from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker @@ -20,10 +21,10 @@ from activity_core.ops_auth import ( require_operator, ) from activity_core.ops_run_queue import ( + CloseOpsRunOutcome, claim_ops_runs, - complete_ops_run, + close_ops_run, default_lease_seconds, - fail_ops_run, heartbeat_ops_run, list_ops_runs, ops_run_counts, @@ -186,6 +187,35 @@ class FailBody(BaseModel): result: dict[str, Any] | None = None +_CLOSE_MESSAGES = { + "not_found": "ops_run not found", + "wrong_owner": "ops_run is not owned by this worker", + "expired_lease": "ops_run lease is missing or expired", + "state_conflict": "ops_run is not actively claimed", + "terminal_conflict": "terminal ops_run has a different close intent", + "evidence_conflict": "repository evidence does not match the queued grant", +} + + +def _close_response(outcome: CloseOpsRunOutcome) -> dict[str, Any]: + """Return an applied/reconciled row or a machine-distinct refusal.""" + if outcome.accepted and outcome.row is not None: + response = ops_run_to_dict(outcome.row) + response["close_disposition"] = outcome.disposition + return response + + code = outcome.disposition + status_code = 404 if code == "not_found" else 409 + raise HTTPException( + status_code=status_code, + detail={ + "code": code, + "message": _CLOSE_MESSAGES.get(code, "ops_run close refused"), + "state": getattr(outcome.row, "state", None), + }, + ) + + @router.get("") async def get_ops_runs( request: Request, @@ -329,18 +359,14 @@ async def post_complete( Session = _db() async with Session() as session: async with session.begin(): - row = await complete_ops_run( + outcome = await close_ops_run( session, run_id, worker_id=worker_id, + action="complete", result=body.result, ) - if row is None: - raise HTTPException( - status_code=409, - detail="not actively leased by this worker or not found", - ) - return ops_run_to_dict(row) + return _close_response(outcome) @router.post("/{run_id}/fail") @@ -360,20 +386,16 @@ async def post_fail( Session = _db() async with Session() as session: async with session.begin(): - row = await fail_ops_run( + outcome = await close_ops_run( session, run_id, worker_id=worker_id, + action="fail", error=body.error, reopen=body.reopen, result=body.result, ) - if row is None: - raise HTTPException( - status_code=409, - detail="not actively leased by this worker or not found", - ) - return ops_run_to_dict(row) + return _close_response(outcome) @router.post("/expire-leases") diff --git a/src/activity_core/orm.py b/src/activity_core/orm.py index 878f235..b33f429 100644 --- a/src/activity_core/orm.py +++ b/src/activity_core/orm.py @@ -156,7 +156,14 @@ class OpsRun(Base): # Attribution refs carried through from workforce/leadership vocabulary; # activity-core does not author or interpret them. execution_refs: Mapped[dict] = mapped_column(JSONB, nullable=False, default=dict) + # Explicit repository mutation authority. This is not attribution and must + # remain separate from execution_refs. + repository_grant: Mapped[dict | None] = mapped_column(JSONB, nullable=True) result: Mapped[dict] = mapped_column(JSONB, nullable=False, default=dict) + # Canonical digest of the accepted terminal close request. It enables a + # response-lost caller to reconcile an exact repeat without weakening the + # active-lease requirement for a first mutation. + close_intent_digest: Mapped[str | None] = mapped_column(Text, nullable=True) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), nullable=False, server_default=func.now() ) diff --git a/src/activity_core/repository_grant.py b/src/activity_core/repository_grant.py new file mode 100644 index 0000000..5106e6b --- /dev/null +++ b/src/activity_core/repository_grant.py @@ -0,0 +1,133 @@ +"""Typed repository mutation authority carried by Activity Core. + +The v1 schema mirrors the accepted rein-aharness repository-grant contract, +but Activity Core owns admission and unchanged queue carriage at its boundary. +It deliberately does not infer grants from prose, labels, profiles, or +organizational attribution references. +""" + +from __future__ import annotations + +import hashlib +import json +from typing import Any, Literal + +from pydantic import ( + BaseModel, + ConfigDict, + Field, + StrictBool, + StrictInt, + field_validator, + model_validator, +) + + +class RepositoryGrantError(ValueError): + """A repository grant is malformed, unsafe, or unsupported.""" + + +class CommitCount(BaseModel): + """Allowed number of descendant local commits for repository-grant v1.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + min: StrictInt = Field(ge=1, le=32) + max: StrictInt = Field(ge=1, le=32) + + @model_validator(mode="after") + def _ordered_bounds(self) -> CommitCount: + if self.min > self.max: + raise ValueError("commit bounds must satisfy 1 <= min <= max <= 32") + return self + + +class RepositoryGrant(BaseModel): + """Versioned, local-only repository authority envelope.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + version: Literal["1"] + allowed_paths: list[str] = Field(min_length=1, max_length=100) + commit_count: CommitCount + publish: StrictBool + + @field_validator("allowed_paths") + @classmethod + def _validate_allowed_paths(cls, value: list[str]) -> list[str]: + if len(set(value)) != len(value): + raise ValueError("allowed_paths must not contain duplicates") + for pattern in value: + _validate_path_pattern(pattern) + return value + + @field_validator("publish") + @classmethod + def _local_only(cls, value: bool) -> bool: + if value: + raise ValueError( + "version 1 does not grant publication; publish must be false" + ) + return value + + def payload(self) -> dict[str, Any]: + """Return the admitted field without semantic rewriting or inference.""" + return self.model_dump(mode="python") + + @property + def grant_id(self) -> str: + """Match rein-aharness's canonical, path-order-independent grant id.""" + return _digest( + { + "allowed_paths": sorted(self.allowed_paths), + "commit_count": { + "max": self.commit_count.max, + "min": self.commit_count.min, + }, + "publish": self.publish, + "version": self.version, + } + )[:32] + + @property + def acceptance_policy_id(self) -> str: + """Match the rein-aharness v1 acceptance-policy identity.""" + return _digest( + { + "allowed_paths": sorted(self.allowed_paths), + "max_commits": self.commit_count.max, + "max_evidence_paths": 100, + "min_commits": self.commit_count.min, + } + )[:32] + + +def validate_repository_grant(value: Any) -> dict[str, Any]: + """Validate and return an unchanged-shape repository-grant v1 mapping.""" + try: + return RepositoryGrant.model_validate(value).payload() + except (TypeError, ValueError) as exc: + raise RepositoryGrantError(str(exc)) from exc + + +def _validate_path_pattern(pattern: str) -> None: + if not pattern or len(pattern) > 256: + raise ValueError( + "path patterns must be non-empty strings of at most 256 characters" + ) + normalized = pattern.rstrip("/") + parts = tuple(normalized.split("/")) + if ( + pattern.startswith("/") + or "\\" in pattern + or "\0" in pattern + or any(part in {"", ".", ".."} for part in parts) + ): + raise ValueError(f"path pattern must be repository-relative: {pattern!r}") + if parts and parts[0] == ".git": + raise ValueError("path patterns cannot grant protected .git metadata") + + +def _digest(value: dict[str, Any]) -> str: + payload = json.dumps(value, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(payload.encode("utf-8")).hexdigest() diff --git a/src/activity_core/rules/actions.py b/src/activity_core/rules/actions.py index 7292f14..75fb2e4 100644 --- a/src/activity_core/rules/actions.py +++ b/src/activity_core/rules/actions.py @@ -7,6 +7,7 @@ activity_core.* module outside rules/. from __future__ import annotations import re +from copy import deepcopy from dataclasses import asdict from typing import Any @@ -89,6 +90,10 @@ def _task_spec_for_rule(rule: dict, event: Any, context: dict) -> dict: result["execution_refs"] = _render_execution_refs( action.get("execution_refs"), event, context ) + if "repository_grant" in action: + # Authority is static definition data. It must never be templated from + # event/context values or inferred from another declaration field. + result["repository_grant"] = deepcopy(action.get("repository_grant")) result["condition"] = rule.get("condition", "") return result diff --git a/src/activity_core/run_artifacts.py b/src/activity_core/run_artifacts.py index cbf55d1..cdaa721 100644 --- a/src/activity_core/run_artifacts.py +++ b/src/activity_core/run_artifacts.py @@ -173,6 +173,9 @@ def _ops_summary(row: OpsRun) -> dict[str, Any]: } if execution_evidence: compact_result["execution_evidence"] = execution_evidence + repository_transaction = dict(result.get("repository_transaction") or {}) + if repository_transaction: + compact_result["repository_transaction"] = repository_transaction return { "id": str(row.id), "state": row.state, @@ -185,7 +188,12 @@ def _ops_summary(row: OpsRun) -> dict[str, Any]: "approach_hint": row.approach_hint, "harness_profile_ref": row.harness_profile_ref, "execution_refs": dict(row.execution_refs or {}), + "repository_grant": ( + dict(row.repository_grant) if row.repository_grant is not None else None + ), "execution_evidence": execution_evidence, + "repository_transaction": repository_transaction, + "close_intent_digest": row.close_intent_digest, "created_at": row.created_at.isoformat() if row.created_at else None, "updated_at": row.updated_at.isoformat() if row.updated_at else None, "result": compact_result, diff --git a/tests/rules/test_actions.py b/tests/rules/test_actions.py index fea4bfb..1c2c4ef 100644 --- a/tests/rules/test_actions.py +++ b/tests/rules/test_actions.py @@ -86,6 +86,35 @@ def test_action_carries_declared_profile_and_renders_attribution_refs() -> None: } +def test_action_carries_repository_grant_without_rendering_authority() -> None: + grant = { + "version": "1", + "allowed_paths": ["docs/{context.request.scope}", "README.md"], + "commit_count": {"min": 1, "max": 1}, + "publish": False, + } + rules = [ + { + "id": "granted-task", + "condition": "", + "action": { + "task_template": "Run controlled task", + "target_repo": "activity-core", + "repository_grant": grant, + }, + } + ] + + specs = expand_rule_actions( + rules, + _Event(), + {"request": {"scope": "must-not-be-authority"}}, + ) + + assert specs[0]["repository_grant"] == grant + assert specs[0]["repository_grant"] is not grant + + def test_for_each_binds_each_list_item_before_condition_and_action_rendering() -> None: rules = [ { diff --git a/tests/test_glas_evidence.py b/tests/test_glas_evidence.py index 7c64efb..3c52146 100644 --- a/tests/test_glas_evidence.py +++ b/tests/test_glas_evidence.py @@ -7,6 +7,7 @@ import json from activity_core.glas_evidence import ( normalise_execution_evidence, normalise_ops_result, + normalise_repository_transaction, ) @@ -113,3 +114,61 @@ def test_keeps_legacy_artifact_fields_but_not_unknown_blobs() -> None: {"kind": "report", "label": "Report", "url": "https://example.test/a"} ] assert "messages" not in result + + +def test_preserves_bounded_repository_transaction_identity() -> None: + transaction = normalise_repository_transaction( + { + "transaction_id": "tx-1", + "correlation_id": "run-1", + "baseline": {"repo_id": "repo-1", "head": "base", "secret": "drop"}, + "repository_grant": { + "grant_id": "grant-1", + "acceptance_policy_id": "policy-1", + "version": "1", + "allowed_path_count": 2, + "allowed_paths_digest": "digest", + "min_commits": 1, + "max_commits": 1, + "publish": False, + "allowed_paths": ["must-not-persist"], + }, + "acceptance": { + "policy_id": "policy-1", + "accepted": True, + "head": "result", + "commit_count": 1, + "commits": ["result"], + "changed_path_count": 1, + "changed_paths": ["docs/result.md"] * 101, + "changed_paths_digest": "paths-digest", + "clean_post_state": True, + "provider_blob": "drop", + }, + "metrics": { + "storage": "external", + "session_id": "tx-1", + "projection_ready": True, + }, + "tool_output": "drop", + } + ) + + assert transaction["transaction_id"] == "tx-1" + assert transaction["repository_grant"]["grant_id"] == "grant-1" + assert transaction["acceptance"]["accepted"] is True + assert len(transaction["acceptance"]["changed_paths"]) == 100 + assert transaction["metrics"]["session_id"] == "tx-1" + assert "allowed_paths" not in transaction["repository_grant"] + assert "tool_output" not in transaction + assert "secret" not in transaction["baseline"] + + +def test_ops_result_accepts_repository_transaction_alias() -> None: + result = normalise_ops_result( + {"transaction": {"transaction_id": "tx-alias", "unknown": "drop"}} + ) + + assert result == { + "repository_transaction": {"transaction_id": "tx-alias"} + } diff --git a/tests/test_issue_sink.py b/tests/test_issue_sink.py index 202fa30..272e8e3 100644 --- a/tests/test_issue_sink.py +++ b/tests/test_issue_sink.py @@ -242,6 +242,47 @@ async def test_emit_tasks_refuses_absent_profile_in_strict_mode(monkeypatch) -> ) +@pytest.mark.asyncio +async def test_emit_tasks_refuses_invalid_grant_before_any_side_effect( + monkeypatch, +) -> None: + from temporalio.exceptions import ApplicationError + + monkeypatch.setattr( + activities, + "get_issue_sink", + lambda: pytest.fail("IssueSink must not be opened after grant refusal"), + ) + monkeypatch.setattr( + activities, + "_get_session_factory", + lambda: pytest.fail("DB must not be opened after grant refusal"), + ) + + with pytest.raises(ApplicationError, match="repository grant refused") as exc_info: + await activities.emit_tasks( + { + "activity_id": "00000000-0000-0000-0000-000000000001", + "triggering_event_id": "event-1", + "task_specs": [ + { + "title": "Must not emit", + "source_type": "rule", + "source_id": "unsafe-grant", + "repository_grant": { + "version": "1", + "allowed_paths": ["../escape"], + "commit_count": {"min": 1, "max": 1}, + "publish": False, + }, + } + ], + } + ) + + assert exc_info.value.non_retryable is True + + @pytest.mark.asyncio async def test_emit_tasks_passes_unknown_versioned_profile_to_queue(monkeypatch) -> None: captured: dict[str, Any] = {} @@ -297,6 +338,12 @@ async def test_emit_tasks_passes_unknown_versioned_profile_to_queue(monkeypatch) "correlation_id": "corr-9", "api_key": "must-be-dropped", }, + "repository_grant": { + "version": "1", + "allowed_paths": ["docs/", "README.md"], + "commit_count": {"min": 1, "max": 1}, + "publish": False, + }, } ], } @@ -306,3 +353,9 @@ async def test_emit_tasks_passes_unknown_versioned_profile_to_queue(monkeypatch) assert captured["harness_profile_ref"] == "harness.unknown-locally@9.9.9" assert captured["approach_hint"] == "legacy-only" assert captured["execution_refs"] == {"correlation_id": "corr-9"} + assert captured["repository_grant"] == { + "version": "1", + "allowed_paths": ["docs/", "README.md"], + "commit_count": {"min": 1, "max": 1}, + "publish": False, + } diff --git a/tests/test_ops_run_queue.py b/tests/test_ops_run_queue.py index 04d3140..3860645 100644 --- a/tests/test_ops_run_queue.py +++ b/tests/test_ops_run_queue.py @@ -118,7 +118,11 @@ def test_ops_run_to_dict_shape() -> None: row.source_id = "emit" row.triggering_event_id = "e1" row.approach_hint = None + row.harness_profile_ref = None + row.execution_refs = {} + row.repository_grant = None row.result = {} + row.close_intent_digest = None row.created_at = now row.updated_at = now d = ops_run_to_dict(row) @@ -167,6 +171,33 @@ async def test_create_ops_run_inserts(monkeypatch: pytest.MonkeyPatch) -> None: session.execute.assert_awaited() +@pytest.mark.asyncio +async def test_create_ops_run_carries_repository_grant_separately( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from activity_core.ops_run_queue import create_ops_run_from_spec + + monkeypatch.setenv("OPS_RUN_QUEUE_ENABLED", "true") + result = MagicMock() + result.scalar_one_or_none.return_value = uuid.uuid4() + session = AsyncMock() + session.execute = AsyncMock(return_value=result) + spec = TaskSpec( + title="bounded", + target_repo="activity-core", + activity_definition_id=str(uuid.uuid4()), + source_id="emit-bounded", + triggering_event_id="manual-1", + ) + grant = _repository_grant() + + await create_ops_run_from_spec(session, spec, repository_grant=grant) + + statement = session.execute.await_args.args[0] + assert statement.compile().params["repository_grant"] == grant + assert statement.compile().params["execution_refs"] == {} + + @pytest.mark.asyncio async def test_claim_and_complete_roundtrip(monkeypatch: pytest.MonkeyPatch) -> None: """In-memory style: claim filters labels and complete transitions state.""" @@ -181,6 +212,8 @@ async def test_claim_and_complete_roundtrip(monkeypatch: pytest.MonkeyPatch) -> open_run.claim_owner = None open_run.lease_until = None open_run.result = {} + open_run.repository_grant = None + open_run.close_intent_digest = None session = AsyncMock() @@ -234,6 +267,8 @@ async def test_complete_persists_only_normalized_glas_evidence() -> None: row.claim_owner = "worker-1" row.lease_until = datetime.now(timezone.utc) + timedelta(minutes=1) row.result = {} + row.repository_grant = None + row.close_intent_digest = None session = AsyncMock() session.get = AsyncMock(return_value=row) @@ -279,6 +314,8 @@ async def test_fail_reopen_under_max_attempts(monkeypatch: pytest.MonkeyPatch) - row.lease_until = datetime.now(timezone.utc) + timedelta(minutes=1) row.attempt = 1 row.result = {} + row.repository_grant = None + row.close_intent_digest = None session = AsyncMock() session.get = AsyncMock(return_value=row) @@ -302,6 +339,8 @@ async def test_fail_permanent_at_max_attempts(monkeypatch: pytest.MonkeyPatch) - row.lease_until = datetime.now(timezone.utc) + timedelta(minutes=1) row.attempt = 3 row.result = {} + row.repository_grant = None + row.close_intent_digest = None session = AsyncMock() session.get = AsyncMock(return_value=row) @@ -322,6 +361,8 @@ async def test_fail_persists_redacted_failure_evidence() -> None: row.lease_until = datetime.now(timezone.utc) + timedelta(minutes=1) row.attempt = 1 row.result = {} + row.repository_grant = None + row.close_intent_digest = None session = AsyncMock() session.get = AsyncMock(return_value=row) @@ -390,6 +431,8 @@ async def test_complete_rejects_expired_lease_without_mutation() -> None: row.claim_owner = "worker-1" row.lease_until = now - timedelta(microseconds=1) row.result = {"before": True} + row.repository_grant = None + row.close_intent_digest = None session = AsyncMock() session.get = AsyncMock(return_value=row) @@ -416,6 +459,8 @@ async def test_fail_rejects_wrong_owner_with_active_lease() -> None: row.claim_owner = "worker-1" row.lease_until = now + timedelta(minutes=1) row.result = {"before": True} + row.repository_grant = None + row.close_intent_digest = None session = AsyncMock() session.get = AsyncMock(return_value=row) @@ -432,6 +477,155 @@ async def test_fail_rejects_wrong_owner_with_active_lease() -> None: assert row.result == {"before": True} +@pytest.mark.asyncio +async def test_terminal_complete_exact_repeat_reconciles_and_conflict_is_distinct() -> None: + from activity_core import ops_run_queue as oq + + row = MagicMock() + row.state = "claimed" + row.claim_owner = "worker-1" + row.lease_until = datetime.now(timezone.utc) + timedelta(minutes=1) + row.result = {} + row.repository_grant = None + row.close_intent_digest = None + session = AsyncMock() + session.get = AsyncMock(return_value=row) + + applied = await oq.close_ops_run( + session, + uuid.uuid4(), + worker_id="worker-1", + action="complete", + result={"ok": True, "path": "docs/result.md"}, + ) + digest = row.close_intent_digest + reconciled = await oq.close_ops_run( + session, + uuid.uuid4(), + worker_id="worker-1", + action="complete", + result={"path": "docs/result.md", "ok": True}, + ) + conflict = await oq.close_ops_run( + session, + uuid.uuid4(), + worker_id="worker-1", + action="complete", + result={"ok": True, "path": "docs/different.md"}, + ) + wrong_owner = await oq.close_ops_run( + session, + uuid.uuid4(), + worker_id="worker-2", + action="complete", + result={"ok": True, "path": "docs/result.md"}, + ) + + assert applied.disposition == "applied" + assert reconciled.disposition == "reconciled" + assert conflict.disposition == "terminal_conflict" + assert wrong_owner.disposition == "wrong_owner" + assert row.state == "succeeded" + assert row.lease_until is None + assert row.close_intent_digest == digest + + +@pytest.mark.asyncio +async def test_close_reports_expired_lease_without_mutation() -> None: + from activity_core import ops_run_queue as oq + + now = datetime(2026, 9, 4, 10, 0, tzinfo=timezone.utc) + row = MagicMock() + row.state = "claimed" + row.claim_owner = "worker-1" + row.lease_until = now + row.result = {"before": True} + row.repository_grant = None + row.close_intent_digest = None + session = AsyncMock() + session.get = AsyncMock(return_value=row) + + with patch.object(oq, "_utcnow", return_value=now): + outcome = await oq.close_ops_run( + session, + uuid.uuid4(), + worker_id="worker-1", + action="complete", + result={"ok": True}, + ) + + assert outcome.disposition == "expired_lease" + assert row.result == {"before": True} + assert row.close_intent_digest is None + + +def _repository_grant() -> dict: + return { + "version": "1", + "allowed_paths": ["docs/", "README.md"], + "commit_count": {"min": 1, "max": 1}, + "publish": False, + } + + +def _accepted_repository_result() -> dict: + from activity_core.repository_grant import RepositoryGrant + + grant = RepositoryGrant.model_validate(_repository_grant()) + return { + "ok": True, + "repository_transaction": { + "transaction_id": "tx-1", + "repository_grant": { + "grant_id": grant.grant_id, + "acceptance_policy_id": grant.acceptance_policy_id, + "version": "1", + }, + "acceptance": { + "accepted": True, + "policy_id": grant.acceptance_policy_id, + "head": "deadbeef", + }, + }, + } + + +@pytest.mark.asyncio +async def test_granted_complete_requires_matching_acceptance_identity() -> None: + from activity_core import ops_run_queue as oq + + row = MagicMock() + row.state = "claimed" + row.claim_owner = "worker-1" + row.lease_until = datetime.now(timezone.utc) + timedelta(minutes=1) + row.result = {} + row.repository_grant = _repository_grant() + row.close_intent_digest = None + session = AsyncMock() + session.get = AsyncMock(return_value=row) + + mismatch = _accepted_repository_result() + mismatch["repository_transaction"]["repository_grant"]["grant_id"] = "other" + refused = await oq.close_ops_run( + session, + uuid.uuid4(), + worker_id="worker-1", + action="complete", + result=mismatch, + ) + applied = await oq.close_ops_run( + session, + uuid.uuid4(), + worker_id="worker-1", + action="complete", + result=_accepted_repository_result(), + ) + + assert refused.disposition == "evidence_conflict" + assert applied.disposition == "applied" + assert row.result["repository_transaction"]["transaction_id"] == "tx-1" + + def test_label_filter_any_vs_all() -> None: """Document labels_mode semantics used by claim_ops_runs.""" row_labels = {"automated", "research-brief"} diff --git a/tests/test_ops_runs_api.py b/tests/test_ops_runs_api.py index 68de209..4d4290f 100644 --- a/tests/test_ops_runs_api.py +++ b/tests/test_ops_runs_api.py @@ -8,7 +8,9 @@ import pytest from fastapi import FastAPI, HTTPException from fastapi.testclient import TestClient +from activity_core.ops_run_queue import CloseOpsRunOutcome from activity_core.ops_runs_api import ( + _close_response, bind_worker_id, require_worker, require_worker_or_operator, @@ -140,3 +142,26 @@ def test_read_auth_no_longer_defaults_open(monkeypatch: pytest.MonkeyPatch) -> N require_worker_or_operator(_request()) assert exc.value.status_code == 503 + + +def test_close_response_distinguishes_reconciled_and_refusal_codes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + row = MagicMock() + monkeypatch.setattr( + "activity_core.ops_runs_api.ops_run_to_dict", + lambda value: {"id": "run-1", "state": value.state}, + ) + row.state = "succeeded" + + response = _close_response(CloseOpsRunOutcome("reconciled", row)) + assert response == { + "id": "run-1", + "state": "succeeded", + "close_disposition": "reconciled", + } + + with pytest.raises(HTTPException) as exc: + _close_response(CloseOpsRunOutcome("expired_lease", row)) + assert exc.value.status_code == 409 + assert exc.value.detail["code"] == "expired_lease" diff --git a/tests/test_repository_grant.py b/tests/test_repository_grant.py new file mode 100644 index 0000000..c10ba58 --- /dev/null +++ b/tests/test_repository_grant.py @@ -0,0 +1,82 @@ +"""Repository-grant v1 validation and identity compatibility.""" + +from __future__ import annotations + +import pytest + +from activity_core.models import ActionDef, InstructionDef +from activity_core.orm import OpsRun +from activity_core.repository_grant import ( + RepositoryGrant, + RepositoryGrantError, + validate_repository_grant, +) + + +def _grant(**changes: object) -> dict: + value = { + "version": "1", + "allowed_paths": ["docs/", "README.md"], + "commit_count": {"min": 1, "max": 1}, + "publish": False, + } + value.update(changes) + return value + + +def test_repository_grant_preserves_payload_and_has_stable_identity() -> None: + raw = _grant() + grant = RepositoryGrant.model_validate(raw) + reordered = RepositoryGrant.model_validate( + _grant(allowed_paths=["README.md", "docs/"]) + ) + + assert grant.payload() == raw + assert grant.payload() is not raw + assert grant.grant_id == reordered.grant_id + assert grant.acceptance_policy_id == reordered.acceptance_policy_id + assert grant.grant_id == "af2e7c8275c9ba8c8f78485067f9608e" + assert grant.acceptance_policy_id == "2ed31282d721f4466bc9ea722067d9d0" + + +def test_action_and_instruction_parse_repository_grant_as_typed_authority() -> None: + action = ActionDef(task_template="Do work", repository_grant=_grant()) + instruction = InstructionDef( + id="do-work", + trusted_fields=[], + model="model", + prompt="Do work", + output_schema="schema.json", + repository_grant=_grant(), + ) + + assert isinstance(action.repository_grant, RepositoryGrant) + assert isinstance(instruction.repository_grant, RepositoryGrant) + + +@pytest.mark.parametrize( + "value", + [ + _grant(extra="not-allowed"), + _grant(version=1), + _grant(allowed_paths=[]), + _grant(allowed_paths=["docs/", "docs/"]), + _grant(allowed_paths=["../escape"]), + _grant(allowed_paths=[".git/config"]), + _grant(commit_count={"min": True, "max": 1}), + _grant(commit_count={"min": 2, "max": 1}), + _grant(commit_count={"min": 1, "max": 33}), + _grant(publish=True), + _grant(publish=0), + ], +) +def test_repository_grant_rejects_ambiguous_or_unsafe_values(value: dict) -> None: + with pytest.raises(RepositoryGrantError): + validate_repository_grant(value) + + +def test_ops_run_schema_has_separate_grant_and_close_identity_columns() -> None: + columns = OpsRun.__table__.columns + + assert columns["repository_grant"].nullable is True + assert columns["close_intent_digest"].nullable is True diff --git a/tests/test_run_artifacts.py b/tests/test_run_artifacts.py index 89f0c8e..fa952e5 100644 --- a/tests/test_run_artifacts.py +++ b/tests/test_run_artifacts.py @@ -117,6 +117,8 @@ def test_ops_summary_exposes_compact_execution_constellation() -> None: row.approach_hint = "legacy-only" row.harness_profile_ref = "harness.agent-dev@1.0.0" row.execution_refs = {"goal_refs": ["goal:42@1"]} + row.repository_grant = None + row.close_intent_digest = "close-digest" row.created_at = now row.updated_at = now row.result = { diff --git a/tests/test_sync_activity_definitions.py b/tests/test_sync_activity_definitions.py index 4384901..ec8d1b1 100644 --- a/tests/test_sync_activity_definitions.py +++ b/tests/test_sync_activity_definitions.py @@ -145,6 +145,42 @@ def test_definition_parse_rejects_a_malformed_profile(tmp_path) -> None: parse_file(tmp_path / "profiled.md") +def test_definition_parse_validates_and_preserves_repository_grant(tmp_path) -> None: + _write_profiled_definition( + tmp_path, + """ repository_grant: + version: "1" + allowed_paths: [docs/, README.md] + commit_count: {min: 1, max: 1} + publish: false +""", + ) + + definition = parse_file(tmp_path / "profiled.md") + + assert definition.rules[0]["action"]["repository_grant"] == { + "version": "1", + "allowed_paths": ["docs/", "README.md"], + "commit_count": {"min": 1, "max": 1}, + "publish": False, + } + + +def test_definition_parse_rejects_unsafe_repository_grant(tmp_path) -> None: + _write_profiled_definition( + tmp_path, + """ repository_grant: + version: "1" + allowed_paths: [../escape] + commit_count: {min: 1, max: 1} + publish: false +""", + ) + + with pytest.raises(ParseError, match="invalid repository_grant"): + parse_file(tmp_path / "profiled.md") + + def test_definition_parse_requires_rule_profile_in_strict_mode( tmp_path, monkeypatch, diff --git a/workplans/ACTIVITY-WP-0037-repository-grant-close-reconciliation.md b/workplans/ACTIVITY-WP-0037-repository-grant-close-reconciliation.md new file mode 100644 index 0000000..248fab0 --- /dev/null +++ b/workplans/ACTIVITY-WP-0037-repository-grant-close-reconciliation.md @@ -0,0 +1,126 @@ +--- +id: ACTIVITY-WP-0037 +type: workplan +title: "Carry repository grants and reconcile terminal queue closes" +domain: infotech +repo: activity-core +status: active +owner: codex +topic_slug: activity-core +priority: high +created: "2026-09-04" +updated: "2026-09-04" +related: + - ACT-ADR-005 + - ACT-ADR-006 + - ACTIVITY-WP-0032 + - ACTIVITY-WP-0036 + - REINAH-WP-0003 +--- + +# Carry Repository Grants and Reconcile Terminal Queue Closes + +## Origin + +Rein-aharness enabled its versioned local repository-grant validator and durable +close outbox on 2026-09-04, then reported two Activity Core contract gaps: + +1. queued/profiled work has no separately typed authoritative repository grant; +2. a response-lost terminal close retry receives the same undifferentiated 409 + as a wrong-owner, expired-lease, or conflicting-result mutation, while the + result allowlist drops repository transaction and acceptance identity. + +These are Activity Core responsibilities at the definition/emission and durable +queue boundaries. The grant must not be inferred from task prose, labels, +`execution_refs`, a profile, or repository defaults. + +## Review and specify the cross-repository contract + +```task +id: ACTIVITY-WP-0037-T01 +status: done +priority: high +``` + +Review rein-aharness repository-grant v1, its close-outbox identity, and exact +ADR-002 revision `36e1096`. Define compatibility, rejection, evidence, and +repeat-close semantics before changing the queue. + +Decision: preserve the four-field v1 grant unchanged in a dedicated JSON +column after strict structural and path-safety validation. A first accepted +terminal close returns `applied`; an identical repeat by the same claim owner +returns `reconciled`. Missing rows, wrong owners, expired leases, non-terminal +states, and different terminal intents receive distinct refusal codes. Legacy +grant-absent rows remain valid during migration. + +## Carry the authoritative repository grant + +```task +id: ACTIVITY-WP-0037-T02 +status: done +priority: high +``` + +Add a typed repository-grant v1 model to rule actions and instructions, reject +unsupported or unsafe grants during definition parsing and emission preflight, +and carry the exact field through `ops_runs` and claim/read projections. Add an +additive migration; do not place the grant in attribution-only +`execution_refs` or synthesize it from another field. + +Completed 2026-09-04. `RepositoryGrant` and `CommitCount` enforce the exact v1 +shape, strict scalar types, local-only publication, bounded commit counts, and +safe repository-relative patterns. Definition sync and complete-batch emission +preflight both reject invalid grants. Rule actions copy static grant data +without rendering it; instructions carry the typed payload. Migration `0010` +adds nullable `ops_runs.repository_grant`, and claim/read/run projections keep +it separate from `execution_refs`. Cross-implementation identity tests match +rein-aharness grant id `af2e7c8275c9ba8c8f78485067f9608e` and acceptance +policy id `2ed31282d721f4466bc9ea722067d9d0` for the shared fixture. + +## Preserve evidence and reconcile terminal closes + +```task +id: ACTIVITY-WP-0037-T03 +status: done +priority: high +``` + +Allowlist bounded repository transaction, grant, acceptance, and external +metrics identities in queue results. Persist a digest of the accepted terminal +close intent. Under the row lock, accept an identical repeat by the original +claim owner as reconciled without requiring a now-cleared lease, while refusing +different outcomes/actions/flags, wrong owners, expired active claims, and +unknown rows with distinct API codes. Reopened failures are not terminal and +must not be mistaken for reconciled delivery. + +Completed 2026-09-04. The result allowlist now retains bounded +`repository_transaction` baseline, grant, acceptance, and external-metrics +evidence while dropping raw grant patterns and unknown blobs. Migration `0010` +adds `close_intent_digest`. The row-locked close decision returns `applied` for +the first accepted mutation and `reconciled` for an exact normalized terminal +repeat by the same owner. It reports distinct `not_found`, `wrong_owner`, +`expired_lease`, `state_conflict`, `evidence_conflict`, and +`terminal_conflict` refusals. Granted completion additionally proves that the +reported grant and acceptance-policy identities match the queued authority and +that acceptance is positive. Reopened failures clear close identity and remain +non-terminal. + +## Verify and hand off + +```task +id: ACTIVITY-WP-0037-T04 +status: progress +priority: high +``` + +Run focused and full tests, migration/static checks, update the queue contract, +sync State Hub, and return the exact revision/schema/repeat semantics to +rein-aharness. Production rollout and live outbox activation remain separate +until both repositories consume and deploy the reviewed contract. + +Source verification 2026-09-04: 493 tests passed with one live integration test +skipped; critical Python lint, compilation, and whitespace checks passed. +Alembic reports one head (`0010`). A throwaway PostgreSQL 16 database passed +full upgrade, `0010` downgrade to `0009`, column-removal inspection, re-upgrade, +and type/nullability inspection; it was then removed. Remaining work is the +exact-revision State Hub handoff and consistency sync.