From 4707a85a5fa3f4f2b1ce9f69ece770db570f4ade Mon Sep 17 00:00:00 2001 From: hhyykk <4408344+hhyykk@users.noreply.github.com> Date: Sun, 1 Nov 2026 19:03:11 +0800 Subject: [PATCH 01/11] feat(issue-fix): admit maintainer corrections by provider event with stable Todo ids Extend the maintainer-correction input to v1 with a required source_event (issue_comment/review/review_comment id, author, timestamps) that LoopX verifies against body-free PR comment and review metadata. v0 inputs stay accepted as host_asserted. Admission runs deterministic gates in order (event observed, revision not stale, not self/bot author, maintainer association, PR not terminal) and records every rejection in the pr-lifecycle ledger as a context_only entry with its reason instead of dropping it silently. The correction Todo is written under a stable operation id derived from repo, PR, event id and updated_at (or the correction fingerprint for v0), so replays yield already_exists even after the Todo is done, distinct events with identical wording yield distinct Todos, and a newer revision supersedes the earlier entry. The ledger row gains maintainer_corrections entries written with todo_id null before the Todo write and filled after; each execute run re-gates and retries pending accepted entries, refusing a stopped Goal with goal_not_active. Legacy maintainer_correction fields keep the latest accepted correction for older readers. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com> --- .../issue-fix-maintainer-correction-smoke.py | 13 +- loopx/capabilities/issue_fix/README.md | 55 +- loopx/capabilities/issue_fix/cli.py | 293 +++++++- loopx/capabilities/issue_fix/github_public.py | 75 ++ loopx/capabilities/issue_fix/pr_lifecycle.py | 667 +++++++++++++++++- loopx/domain_packs/issue_fix.py | 80 ++- ...sue_fix_maintainer_correction_admission.py | 569 +++++++++++++++ 7 files changed, 1693 insertions(+), 59 deletions(-) create mode 100644 tests/capabilities/test_issue_fix_maintainer_correction_admission.py diff --git a/examples/issue-fix-maintainer-correction-smoke.py b/examples/issue-fix-maintainer-correction-smoke.py index 19a53ab67b..5a17f1db6f 100644 --- a/examples/issue-fix-maintainer-correction-smoke.py +++ b/examples/issue-fix-maintainer-correction-smoke.py @@ -143,14 +143,11 @@ def check_corrections(provider: str) -> None: assert first["todo_write"]["claimed_by"] == AGENT_ID, first assert first["todo_write"]["path_recorded"] is False, first - replay = run_cli(registry, common) - assert replay["ok"] is True, replay - assert replay["todo_write"]["write_performed"] is False, replay - assert replay["todo_write"]["already_exists"] is True, replay - ledger_write = replay["domain_state_projection"]["write_result"] - assert ledger_write["status"] == "unchanged", replay - state_text = (project / "ACTIVE_GOAL_STATE.md").read_text(encoding="utf-8") - assert state_text.count("action_kind=issue_fix_maintainer_correction_patch") == 1 + admission = first["maintainer_correction_admission"] + assert admission["admission_basis"] == "host_asserted", first + assert admission["disposition"] == "accepted", first + # Replay idempotence, stable operation ids and admission gates are + # covered by tests/capabilities/test_issue_fix_maintainer_correction_admission.py. ambiguity = correction_file( root, diff --git a/loopx/capabilities/issue_fix/README.md b/loopx/capabilities/issue_fix/README.md index bf1f593af4..019a5badcd 100644 --- a/loopx/capabilities/issue_fix/README.md +++ b/loopx/capabilities/issue_fix/README.md @@ -400,19 +400,51 @@ or structured no-follow-up; the agent must not stop silently in monitor-only state. When a review or public maintainer comment contains a concrete correction, the -host compacts it into `issue_fix_maintainer_correction_input_v0` and passes it +host compacts it into `issue_fix_maintainer_correction_input_v1` and passes it to `pr-lifecycle`. The compact input keeps only the correction kind, a public -source reference, a bounded summary, and one of: verification plus PR update -path, a concrete ambiguity question, or missing authority scopes. It never -copies the raw review/comment body. - -With `--execute-transition`, an `actionable_patch` creates exactly one +source reference, a bounded summary, one of: verification plus PR update +path, a concrete ambiguity question, or missing authority scopes, and a +`source_event` naming the provider event that carried the correction: +`{kind: issue_comment|review|review_comment, id, html_url, author_login, +author_association, created_at, updated_at}`. It never copies the raw +review/comment body. The older `issue_fix_maintainer_correction_input_v0` +shape without `source_event` is still accepted and recorded as +`admission_basis: host_asserted`; a v1 input is `provider_verified` because +LoopX matches the event id and `updated_at` against the PR's body-free +comment and review metadata (from the same `--metadata-json` fixture keys +`comments`, `reviews`, `reviewComments`, or fetched with `gh api` under +`--fetch-metadata`; reviews use `submitted_at` as their revision). + +Every correction passes deterministic admission gates before any Todo write, +and every rejection is recorded in the ledger row as a `context_only` entry +with its `reason` instead of being dropped: `source_event_not_observed`, +`source_event_revision_mismatch`, `stale_revision` (an older `updated_at` for +an already admitted event), `self_or_bot_author` (login ending in `[bot]`; +no configured LoopX actor login exists yet), `author_not_maintainer` +(association outside COLLABORATOR/MEMBER/OWNER) and `terminal_pr_state`. +Immediately before the Todo write the gates run again against the ledger as it +stands, and a stopped Goal refuses with `goal_not_active`. + +With `--execute-transition`, an accepted `actionable_patch` creates exactly one `issue_fix_maintainer_correction_patch` todo claimed by the registered agent. `semantic_ambiguity` and `missing_authority` create a concrete user gate that -blocks that same agent. `unchanged` creates no todo. The normalized correction -fingerprint and deterministic todo text make retries idempotent, while terminal -merged/closed state still takes precedence over late feedback. - +blocks that same agent. `unchanged` creates no todo. The Todo is written under +a stable operation id derived from the repository, PR, event id and +`updated_at` (or the correction fingerprint for v0), so an exact replay yields +`already_exists` even after the Todo is done, two different comments with the +same wording yield two Todos, and a newer revision of the same comment yields a +new Todo while the earlier entry is marked `superseded`. The ledger row keeps +these entries in `maintainer_corrections` +(`operation_id`, `source_event`, `correction_fingerprint`, `admission_basis`, +`disposition`, `reason`, `todo_id`, `recorded_at`, `superseded_by`, +`todo_intent`); the accepted entry is written with `todo_id: null` before the +Todo write and filled afterwards, and any later run retries a still-pending +accepted entry under its operation id. Setting +`LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO=1` aborts between those two +writes and exists only to exercise that recovery. The legacy +`maintainer_correction` fields keep the latest accepted correction for older +readers. Terminal merged/closed state still takes precedence over late +feedback. When a connected lifecycle writeback observes `MERGED`, LoopX also appends one repository-qualified, public-safe, idempotent `pr_merge` rollout event. This is the event consumed by todo resume projection. A dependent todo such as @@ -1129,7 +1161,8 @@ loopx issue-fix pr-lifecycle \ The correction source is provider-neutral: any public HTTPS or repo-relative reference may be used, while the current PR monitor remains the authoritative lifecycle source. Exact retries neither add another successor nor rewrite the -same lifecycle row. +same lifecycle row; `maintainer_correction_admission` in the output names the +operation id, admission basis, disposition and reason for this input. ## Status And Output View diff --git a/loopx/capabilities/issue_fix/cli.py b/loopx/capabilities/issue_fix/cli.py index 69c9caa772..711f8a67b9 100644 --- a/loopx/capabilities/issue_fix/cli.py +++ b/loopx/capabilities/issue_fix/cli.py @@ -1,18 +1,22 @@ from __future__ import annotations import argparse +import os from collections.abc import Callable from pathlib import Path from typing import Any from ...agent_registry import load_goal_from_registry from ...boundary_authority import checkpointed_boundary_authority_summary +from ...control_plane.goals.activation import goal_is_stopped from ...control_plane.runtime.time import now_utc_iso from ...domain_packs.issue_fix import ( default_issue_fix_candidate_preflight_ledger_path, default_issue_fix_domain_state_ledger_path, default_issue_fix_feasibility_ledger_path, default_issue_fix_repository_snapshot_ledger_path, + issue_fix_maintainer_correction_entries, + read_issue_fix_pr_lifecycle_ledger_row, retain_issue_fix_repository_snapshot_jsonl, upsert_issue_fix_candidate_preflight_ledger_jsonl, upsert_issue_fix_feasibility_ledger_jsonl, @@ -60,6 +64,7 @@ from .metadata_preview import normalise_github_issue_reference from .pr_lifecycle import ( build_issue_fix_pr_lifecycle_monitor_packet, + recheck_issue_fix_maintainer_correction_effect_gates, render_issue_fix_pr_lifecycle_monitor_markdown, validate_issue_fix_pr_lifecycle_monitor_packet, ) @@ -105,9 +110,210 @@ FormatSelector = Callable[..., str] AddFormat = Callable[[argparse.ArgumentParser], None] +# Set to "1" to abort between the accepted ledger entry write and its Todo +# write. Exists only to prove the next run reconciles the pending entry. +MAINTAINER_CORRECTION_FAULT_ENV = "LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO" -def _comparable_delivery_evidence( - value: dict[str, Any] | None, + +def _goal_stopped_for_correction(registry_path: Path | None, goal_id: str | None) -> bool: + if registry_path is None or not goal_id: + return False + try: + return goal_is_stopped(load_goal_from_registry(registry_path, goal_id)) + except ValueError: + # An unreadable activation record cannot prove the Goal is active. + return True + + +def _write_maintainer_correction_todo( + *, + registry_path: Path, + goal_id: str, + claimed_by: str, + project: Path, + operation_id: str, + intent: dict[str, Any], +) -> dict[str, Any]: + role = str(intent.get("role") or "agent") + return add_goal_todo( + registry_path=registry_path, + goal_id=goal_id, + role=role, + text=str(intent.get("text") or ""), + task_class=str(intent.get("task_class") or "advancement_task"), + action_kind=str(intent.get("action_kind") or ""), + required_write_scopes=( + list(intent.get("required_write_scopes") or []) if role == "agent" else None + ), + claimed_by=claimed_by if role == "agent" else None, + agent_id=claimed_by if role == "user" else None, + project=project, + operation_id=operation_id, + ) + + +def _maintainer_correction_todo_write_receipt( + todo_write: dict[str, Any], *, operation_id: str +) -> dict[str, Any]: + todo_changed = bool( + todo_write.get("added") + or todo_write.get("metadata_updated") + or todo_write.get("status_changed") + ) + created_todo = todo_write.get("todo") + created_todo = created_todo if isinstance(created_todo, dict) else {} + return { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": todo_changed, + "already_exists": bool(todo_write.get("already_exists")), + "todo_id": todo_write.get("todo_id"), + "operation_id": operation_id, + "role": todo_write.get("role"), + "claimed_by": created_todo.get("claimed_by"), + "blocks_agent": created_todo.get("blocks_agent"), + "path_recorded": False, + } + + +def _settle_maintainer_correction_entries( + *, + payload: dict[str, Any], + ledger_path: Path, + registry_path: Path, + goal_id: str, + claimed_by: str, + project: Path, +) -> dict[str, Any]: + """Write Todos for accepted ledger entries that still lack a todo_id. + + Each accepted entry stores its own `todo_intent`, so an entry left pending + by an interrupted run is retried under its original operation id on any + later run. Every candidate is re-gated against the ledger as it stands + now, the same observation, and the Goal activation state. + """ + + observation = payload.get("observation") + observation = observation if isinstance(observation, dict) else {} + admission = payload.get("maintainer_correction_admission") + admission = admission if isinstance(admission, dict) else {} + current_operation_id = str(admission.get("operation_id") or "") + goal_stopped = _goal_stopped_for_correction(registry_path, goal_id) + repo = str(observation.get("repo") or "") + pr_ref = str(observation.get("pr_ref") or "") + + row = read_issue_fix_pr_lifecycle_ledger_row(ledger_path, repo=repo, pr_ref=pr_ref) + ledger_entries = issue_fix_maintainer_correction_entries(row) + settlements: list[dict[str, Any]] = [] + current_todo_write: dict[str, Any] | None = None + changed_entries: list[dict[str, Any]] = [] + for entry in ledger_entries: + operation_id = str(entry.get("operation_id") or "") + is_current = operation_id == current_operation_id + intent = entry.get("todo_intent") + if ( + entry.get("disposition") != "accepted" + or not isinstance(intent, dict) + or (entry.get("todo_id") and not is_current) + ): + continue + refusal = recheck_issue_fix_maintainer_correction_effect_gates( + entry, + observation=observation, + ledger_entries=ledger_entries, + goal_stopped=goal_stopped, + ) + if refusal is not None and not entry.get("todo_id"): + changed_entries.append( + {**entry, "disposition": "context_only", "reason": refusal, + "effect_recheck": "refused"} + ) + settlements.append( + {"operation_id": operation_id, "status": "refused", "reason": refusal} + ) + if is_current: + current_todo_write = { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": False, + "already_exists": False, + "skip_reason": refusal, + "operation_id": operation_id, + "path_recorded": False, + } + continue + if refusal is not None: + # The Todo already exists for this revision; a replay only reads it + # back and leaves its lifecycle to the Todo owner. + settlements.append( + {"operation_id": operation_id, "status": "settled_earlier", "reason": refusal} + ) + if is_current: + current_todo_write = { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": False, + "already_exists": True, + "todo_id": entry.get("todo_id"), + "operation_id": operation_id, + "skip_reason": refusal, + "path_recorded": False, + } + continue + if os.environ.get(MAINTAINER_CORRECTION_FAULT_ENV) == "1": + raise RuntimeError( + "maintainer correction Todo write aborted by " + f"{MAINTAINER_CORRECTION_FAULT_ENV}" + ) + todo_write = _write_maintainer_correction_todo( + registry_path=registry_path, + goal_id=goal_id, + claimed_by=claimed_by, + project=project, + operation_id=operation_id, + intent=intent, + ) + receipt = _maintainer_correction_todo_write_receipt( + todo_write, operation_id=operation_id + ) + if entry.get("todo_id") != receipt["todo_id"]: + changed_entries.append({**entry, "todo_id": receipt["todo_id"]}) + settlements.append( + { + "operation_id": operation_id, + "status": "written" if receipt["write_performed"] else "already_exists", + "todo_id": receipt["todo_id"], + } + ) + if is_current: + current_todo_write = receipt + if current_todo_write is None and current_operation_id: + current_todo_write = { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": False, + "already_exists": False, + "skip_reason": ( + str(admission.get("reason") or admission.get("disposition") or "") + if admission.get("disposition") != "accepted" + else "unchanged_monitor_quiet" + ), + "operation_id": current_operation_id, + "path_recorded": False, + } + if changed_entries: + payload["maintainer_corrections"] = changed_entries + upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) + row = read_issue_fix_pr_lifecycle_ledger_row( + ledger_path, repo=repo, pr_ref=pr_ref + ) + payload["maintainer_corrections"] = issue_fix_maintainer_correction_entries(row) + return { + "schema_version": "issue_fix_maintainer_correction_settlement_v0", + "settlements": settlements, + "goal_stopped": goal_stopped, + "todo_write": current_todo_write, + "path_recorded": False, + } + + +def _comparable_delivery_evidence( value: dict[str, Any] | None, ) -> dict[str, Any] | None: if value is None: return None @@ -1259,6 +1465,32 @@ def handle_issue_fix_command( raise ValueError("--execute-transition requires --goal-id") if args.execute_transition and not args.claimed_by: raise ValueError("--execute-transition requires --claimed-by") + should_write_domain_state = bool( + not args.no_write_domain_state and (args.goal_id or args.ledger_path) + ) + ledger_path: Path | None = None + prior_maintainer_corrections = None + if should_write_domain_state: + ledger_path = ( + Path(args.ledger_path).expanduser() + if args.ledger_path + else default_issue_fix_domain_state_ledger_path( + project=args.project, + goal_id=args.goal_id, + ) + ) + if args.maintainer_correction_json: + prior_reference = normalise_github_issue_reference( + repo=args.repo, issue_ref=args.pr_ref, url=args.url + ) + prior_row = read_issue_fix_pr_lifecycle_ledger_row( + ledger_path, + repo=str(prior_reference["repo"]), + pr_ref=str(prior_reference["issue_ref"]), + ) + prior_maintainer_corrections = ( + issue_fix_maintainer_correction_entries(prior_row) + ) payload = build_issue_fix_pr_lifecycle_monitor_packet( repo=args.repo, pr_ref=args.pr_ref, @@ -1274,20 +1506,10 @@ def handle_issue_fix_command( if args.maintainer_correction_json else None ), + prior_maintainer_corrections=prior_maintainer_corrections, generated_at=generated_at, ) - should_write_domain_state = bool( - not args.no_write_domain_state and (args.goal_id or args.ledger_path) - ) - if should_write_domain_state: - ledger_path = ( - Path(args.ledger_path).expanduser() - if args.ledger_path - else default_issue_fix_domain_state_ledger_path( - project=args.project, - goal_id=args.goal_id, - ) - ) + if should_write_domain_state and ledger_path is not None: upsert_issue_fix_pr_lifecycle_ledger_jsonl( ledger_path, payload, @@ -1331,7 +1553,48 @@ def handle_issue_fix_command( grouped_monitor_writeback.get("write_performed") ) decision = str(transition.get("decision") or "") - if decision in {"runnable_successor", "user_gate"}: + correction_admission = payload.get("maintainer_correction_admission") + settlement: dict[str, Any] | None = None + if ledger_path is not None: + settlement = _settle_maintainer_correction_entries( + payload=payload, + ledger_path=ledger_path, + registry_path=registry_path, + goal_id=args.goal_id, + claimed_by=args.claimed_by, + project=Path(args.project).expanduser(), + ) + if settlement.get("settlements") or isinstance( + correction_admission, dict + ): + payload["maintainer_correction_settlement"] = settlement + if isinstance(correction_admission, dict) and settlement is not None: + settled_write = settlement.get("todo_write") + todo_write_receipt = ( + settled_write if isinstance(settled_write, dict) else {} + ) + todo_changed = bool(todo_write_receipt.get("write_performed")) + if not todo_write_receipt: + todo_write_receipt = { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": False, + "already_exists": False, + "skip_reason": "terminal_or_no_followup", + "path_recorded": False, + } + if ( + not todo_changed + and "skip_reason" in todo_write_receipt + and decision == "monitor_continuation" + and correction_admission.get("disposition") == "accepted" + ): + todo_write_receipt["skip_reason"] = "unchanged_monitor_quiet" + payload["todo_write"] = todo_write_receipt + payload["todo_write_performed"] = todo_changed + writeback_contract = payload.get("writeback_contract") + if isinstance(writeback_contract, dict): + writeback_contract["todo_write_performed"] = bool(todo_changed) + elif decision in {"runnable_successor", "user_gate"}: role = str(transition.get("role") or "agent") todo_write = add_goal_todo( registry_path=registry_path, diff --git a/loopx/capabilities/issue_fix/github_public.py b/loopx/capabilities/issue_fix/github_public.py index 2e4ba8a005..1cdbd0b01d 100644 --- a/loopx/capabilities/issue_fix/github_public.py +++ b/loopx/capabilities/issue_fix/github_public.py @@ -333,6 +333,81 @@ def _fetch_issue_comment_metadata( return {"comments": comments} +def _fetch_body_free_event_list( + gh: str, endpoint: str, jq_filter: str, *, timeout_seconds: float +) -> list[Any]: + result = subprocess.run( + [ + gh, + "api", + "-H", + "Accept: application/vnd.github+json", + "--paginate", + endpoint, + "--jq", + jq_filter, + ], + check=False, + text=True, encoding="utf-8", errors="replace", + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=timeout_seconds, + ) + if result.returncode != 0: + raise RuntimeError("gh pull request event metadata request failed") + # --paginate with --jq emits one JSON array per page. + events: list[Any] = [] + for line in result.stdout.splitlines(): + if not line.strip(): + continue + page = json.loads(line) + if not isinstance(page, list): + raise RuntimeError("pull request event metadata must be a JSON array") + events.extend(page) + return events + + +def fetch_github_pull_request_event_metadata( + *, owner: str, repo: str, number: int, timeout_seconds: float +) -> dict[str, Any]: + """Fetch PR issue comments, reviews and review comments without bodies. + + Each item keeps only the provider id, author login, author association, + timestamps and html URL so a host-asserted correction can be matched to + the event that carried it. + """ + + gh = shutil.which("gh") + if not gh: + raise RuntimeError("pull request event metadata fetch requires GitHub CLI `gh`") + comment_filter = ( + "[.[] | {id: .id, author: .user.login, " + "author_association: .author_association, created_at: .created_at, " + "updated_at: .updated_at, url: .html_url}]" + ) + review_filter = ( + "[.[] | {id: .id, author: .user.login, " + "author_association: .author_association, submitted_at: .submitted_at, " + "url: .html_url}]" + ) + base = f"repos/{owner}/{repo}" + return { + "comments": _fetch_body_free_event_list( + gh, f"{base}/issues/{number}/comments?per_page=100", comment_filter, + timeout_seconds=timeout_seconds, + ), + "reviews": _fetch_body_free_event_list( + gh, f"{base}/pulls/{number}/reviews?per_page=100", review_filter, + timeout_seconds=timeout_seconds, + ), + "reviewComments": _fetch_body_free_event_list( + gh, f"{base}/pulls/{number}/comments?per_page=100", comment_filter, + timeout_seconds=timeout_seconds, + ), + "body_captured": False, + } + + def _parse_github_timestamp(value: Any) -> datetime | None: if not value: return None diff --git a/loopx/capabilities/issue_fix/pr_lifecycle.py b/loopx/capabilities/issue_fix/pr_lifecycle.py index 037afc6a6b..7f8e569012 100644 --- a/loopx/capabilities/issue_fix/pr_lifecycle.py +++ b/loopx/capabilities/issue_fix/pr_lifecycle.py @@ -13,6 +13,10 @@ from ...control_plane.runtime.public_safety import public_safe_compact_text +from .github_public import ( + MAINTAINER_ASSOCIATIONS, + fetch_github_pull_request_event_metadata, +) from .metadata_preview import ( normalise_github_issue_link_reference, normalise_github_issue_reference, @@ -26,6 +30,20 @@ ISSUE_FIX_MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSION = ( "issue_fix_maintainer_correction_input_v0" ) +ISSUE_FIX_MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSION_V1 = ( + "issue_fix_maintainer_correction_input_v1" +) +MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSIONS = { + ISSUE_FIX_MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSION, + ISSUE_FIX_MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSION_V1, +} +ISSUE_FIX_MAINTAINER_CORRECTION_ADMISSION_SCHEMA_VERSION = ( + "issue_fix_maintainer_correction_admission_v0" +) +ISSUE_FIX_MAINTAINER_CORRECTION_LEDGER_ENTRY_SCHEMA_VERSION = ( + "issue_fix_maintainer_correction_ledger_entry_v0" +) +MAINTAINER_CORRECTION_OPERATION_ID_PREFIX = "issue-fix-correction:" MAINTAINER_CORRECTION_KINDS = { "actionable_patch", "semantic_ambiguity", @@ -33,6 +51,36 @@ "unchanged", } MAINTAINER_CORRECTION_SOURCE_KINDS = {"review", "maintainer_comment"} +MAINTAINER_CORRECTION_SOURCE_EVENT_KINDS = { + "issue_comment", + "review", + "review_comment", +} +MAINTAINER_CORRECTION_SOURCE_EVENT_FIELDS = { + "kind", + "id", + "html_url", + "author_login", + "author_association", + "created_at", + "updated_at", +} +MAINTAINER_CORRECTION_ADMISSION_BASES = {"host_asserted", "provider_verified"} +MAINTAINER_CORRECTION_DISPOSITIONS = {"accepted", "context_only", "superseded"} +MAINTAINER_CORRECTION_REJECTION_REASONS = { + "source_event_not_observed", + "source_event_revision_mismatch", + "stale_revision", + "self_or_bot_author", + "author_not_maintainer", + "terminal_pr_state", + "goal_not_active", +} +# GitHub Apps post as `[bot]`; without a configured LoopX actor login this +# suffix is the only self/bot signal available from body-free event metadata. +BOT_LOGIN_SUFFIX = "[bot]" +_GITHUB_LOGIN_PATTERN = re.compile(r"[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})(?:\[bot\])?") +_PROVIDER_EVENT_ID_PATTERN = re.compile(r"[0-9]{1,20}") WRITE_SCOPES = {"write", "publish", "external_review_request"} MAINTAINER_CORRECTION_INPUT_FIELDS = { "schema_version", @@ -44,8 +92,8 @@ "pr_update_path", "user_question", "missing_authority_scopes", + "source_event", } - TERMINAL_PR_STATES = {"MERGED", "CLOSED"} FAILING_CHECK_STATES = { "ACTION_REQUIRED", @@ -230,6 +278,73 @@ def _public_source_reference(value: Any) -> str: return path.as_posix() +def _normalise_provider_event_id(value: Any) -> str: + if isinstance(value, bool): + raise ValueError("maintainer correction source_event id must be a provider id") + text = str(value if value is not None else "").strip() + if not _PROVIDER_EVENT_ID_PATTERN.fullmatch(text): + raise ValueError("maintainer correction source_event id must be a numeric provider id") + return text + + +def _normalise_author_login(value: Any, *, field: str) -> str: + text = str(value or "").strip() + if not _GITHUB_LOGIN_PATTERN.fullmatch(text): + raise ValueError(f"maintainer correction {field} must be a public provider login") + return text + + +def _normalise_event_timestamp(value: Any, *, field: str) -> str: + text = str(value or "").strip() + if _timestamp(text) is None: + raise ValueError( + f"maintainer correction {field} must be a timezone-aware ISO timestamp" + ) + return text[:40] + + +def normalise_issue_fix_maintainer_correction_source_event( + value: Any, +) -> dict[str, Any]: + """Normalise the provider event that carried a maintainer correction. + + Only identity and revision metadata survive; no body field is accepted. + """ + + if not isinstance(value, Mapping): + raise ValueError("maintainer correction source_event must be an object") + unknown_fields = set(value) - MAINTAINER_CORRECTION_SOURCE_EVENT_FIELDS + if unknown_fields: + raise ValueError( + "maintainer correction source_event contains unsupported fields: " + + ", ".join(sorted(str(field) for field in unknown_fields)) + ) + kind = str(value.get("kind") or "").strip() + if kind not in MAINTAINER_CORRECTION_SOURCE_EVENT_KINDS: + raise ValueError( + "maintainer correction source_event kind must be issue_comment, review, or review_comment" + ) + html_url = _public_source_reference(value.get("html_url")) + if "://" not in html_url: + raise ValueError("maintainer correction source_event html_url must be a public https URL") + return { + "kind": kind, + "id": _normalise_provider_event_id(value.get("id")), + "html_url": html_url, + "author_login": _normalise_author_login( + value.get("author_login"), field="source_event author_login" + ), + "author_association": _upper_label(value.get("author_association"), "NONE"), + "created_at": _normalise_event_timestamp( + value.get("created_at"), field="source_event created_at" + ), + "updated_at": _normalise_event_timestamp( + value.get("updated_at"), field="source_event updated_at" + ), + "body_captured": False, + } + + def normalise_issue_fix_maintainer_correction_input( value: Mapping[str, Any], ) -> dict[str, Any]: @@ -239,13 +354,31 @@ def normalise_issue_fix_maintainer_correction_input( "maintainer correction input contains unsupported fields: " + ", ".join(sorted(str(field) for field in unknown_fields)) ) - if ( - value.get("schema_version") - != ISSUE_FIX_MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSION - ): + schema_version = value.get("schema_version") + if schema_version not in MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSIONS: raise ValueError( - "maintainer correction schema_version must be issue_fix_maintainer_correction_input_v0" + "maintainer correction schema_version must be " + "issue_fix_maintainer_correction_input_v0 or issue_fix_maintainer_correction_input_v1" + ) + provider_verified = ( + schema_version == ISSUE_FIX_MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSION_V1 + ) + if provider_verified: + if "source_event" not in value: + raise ValueError( + "issue_fix_maintainer_correction_input_v1 requires source_event" + ) + source_event: dict[str, Any] | None = ( + normalise_issue_fix_maintainer_correction_source_event( + value.get("source_event") + ) ) + else: + if "source_event" in value: + raise ValueError( + "source_event requires schema_version issue_fix_maintainer_correction_input_v1" + ) + source_event = None correction_kind = str(value.get("correction_kind") or "").strip() if correction_kind not in MAINTAINER_CORRECTION_KINDS: raise ValueError( @@ -285,7 +418,11 @@ def normalise_issue_fix_maintainer_correction_input( missing_authority_scopes.append(scope) if not missing_authority_scopes: raise ValueError("missing_authority requires at least one authority scope") - normalized = { + # The fingerprint covers the compacted correction content under the v0 + # field set, so existing v0 ledger rows keep their recorded fingerprints and + # a v1 event carrying identical content hashes the same; event identity + # lives in the operation id, not here. + content = { "schema_version": ISSUE_FIX_MAINTAINER_CORRECTION_INPUT_SCHEMA_VERSION, "correction_kind": correction_kind, "source_kind": source_kind, @@ -300,12 +437,366 @@ def normalise_issue_fix_maintainer_correction_input( "raw_provider_payload_captured": False, "local_paths_captured": False, } - normalized["correction_fingerprint"] = hashlib.sha256( - json.dumps(normalized, sort_keys=True, separators=(",", ":")).encode("utf-8") + fingerprint = hashlib.sha256( + json.dumps(content, sort_keys=True, separators=(",", ":")).encode("utf-8") ).hexdigest()[:16] + normalized = { + **content, + "schema_version": str(schema_version), + "source_event": source_event, + "admission_basis": "provider_verified" if provider_verified else "host_asserted", + } + normalized["correction_fingerprint"] = fingerprint return normalized +def issue_fix_maintainer_correction_operation_id( + correction: Mapping[str, Any], *, repo: str, pr_ref: str +) -> str: + """Return the stable Todo operation id for one correction revision.""" + + source_event = correction.get("source_event") + if isinstance(source_event, Mapping): + identity = [ + repo, + pr_ref, + str(source_event.get("kind")), + str(source_event.get("id")), + str(source_event.get("updated_at")), + ] + else: + identity = [repo, pr_ref, str(correction.get("correction_fingerprint"))] + digest = hashlib.sha256( + json.dumps(identity, separators=(",", ":")).encode("utf-8") + ).hexdigest()[:24] + return f"{MAINTAINER_CORRECTION_OPERATION_ID_PREFIX}{digest}" + + +def _provider_event_entries(value: Any) -> list[Mapping[str, Any]]: + if isinstance(value, Sequence) and not isinstance(value, (str, bytes)): + return [item for item in value if isinstance(item, Mapping)] + return [] + + +def normalise_issue_fix_pr_source_events( + provider_payload: Mapping[str, Any] | None, +) -> dict[str, Any]: + """Project body-free PR comment/review metadata into verifiable events. + + Reviews expose ``submitted_at`` instead of ``updated_at``; that timestamp + stands in as the review revision because GitHub does not re-time edits of a + review summary through the list endpoint. + """ + + payload = provider_payload if isinstance(provider_payload, Mapping) else {} + groups = ( + ("issue_comment", payload.get("comments") or payload.get("issueComments")), + ("review", payload.get("reviews")), + ("review_comment", payload.get("reviewComments") or payload.get("review_comments")), + ) + available = any( + isinstance(raw, Sequence) and not isinstance(raw, (str, bytes)) + for _kind, raw in groups + ) + events: list[dict[str, Any]] = [] + for kind, raw in groups: + for item in _provider_event_entries(raw): + raw_id = item.get("id") + if isinstance(raw_id, bool) or raw_id is None: + continue + event_id = str(raw_id).strip() + if not _PROVIDER_EVENT_ID_PATTERN.fullmatch(event_id): + continue + author = item.get("author") or item.get("user") + if isinstance(author, Mapping): + author = author.get("login") + updated_at = ( + item.get("updated_at") + or item.get("updatedAt") + or item.get("submitted_at") + or item.get("submittedAt") + ) + events.append( + { + "kind": kind, + "id": event_id, + "author_login": str(author or "").strip()[:60] or None, + "author_association": _upper_label( + item.get("author_association") + or item.get("authorAssociation"), + "NONE", + ), + "created_at": str( + item.get("created_at") or item.get("createdAt") or "" + )[:40] + or None, + "updated_at": str(updated_at or "")[:40] or None, + "body_captured": False, + } + ) + return { + "status": "available" if available else "unavailable", + "events": events, + "body_captured": False, + } + + +def _same_instant(left: Any, right: Any) -> bool: + left_time = _timestamp(left) + right_time = _timestamp(right) + return left_time is not None and right_time is not None and left_time == right_time + + +def _is_bot_or_self_login(login: Any) -> bool: + text = str(login or "").strip().lower() + return text.endswith(BOT_LOGIN_SUFFIX) + + +def _prior_entries(value: Any) -> list[dict[str, Any]]: + if not isinstance(value, Sequence) or isinstance(value, (str, bytes)): + return [] + return [dict(item) for item in value if isinstance(item, Mapping)] + + +def _latest_accepted_revision( + prior_entries: Sequence[Mapping[str, Any]], *, source_event: Mapping[str, Any] +) -> dict[str, Any] | None: + latest: dict[str, Any] | None = None + for entry in prior_entries: + event = entry.get("source_event") + if not isinstance(event, Mapping): + continue + if event.get("kind") != source_event.get("kind") or str( + event.get("id") + ) != str(source_event.get("id")): + continue + if entry.get("disposition") not in {"accepted", "superseded"}: + continue + candidate_time = _timestamp(event.get("updated_at")) + if candidate_time is None: + continue + latest_time = ( + _timestamp(latest["source_event"].get("updated_at")) if latest else None + ) + if latest_time is None or candidate_time > latest_time: + latest = dict(entry) + return latest + + +def evaluate_issue_fix_maintainer_correction_admission( + correction: Mapping[str, Any], + *, + observation: Mapping[str, Any], + source_events: Mapping[str, Any] | None, + prior_entries: Sequence[Mapping[str, Any]] | None, + prior_ledger_read: bool, + recorded_at: str | None, +) -> dict[str, Any]: + """Run the deterministic admission gates for one maintainer correction. + + Returns the admission record plus the ledger entries this run touches: + the entry for this correction and any earlier revision it supersedes. + """ + + repo = str(observation.get("repo") or "") + pr_ref = str(observation.get("pr_ref") or "") + operation_id = issue_fix_maintainer_correction_operation_id( + correction, repo=repo, pr_ref=pr_ref + ) + source_event = correction.get("source_event") + admission_basis = str(correction.get("admission_basis") or "host_asserted") + prior = _prior_entries(prior_entries) + existing_entry = next( + (entry for entry in prior if entry.get("operation_id") == operation_id), None + ) + gates: list[dict[str, Any]] = [] + reason: str | None = None + verified_event: dict[str, Any] | None = None + provider_events_status = ( + str(source_events.get("status") or "unavailable") + if isinstance(source_events, Mapping) + else "unavailable" + ) + + def record(name: str, passed: bool, failure_reason: str | None = None) -> None: + nonlocal reason + gates.append({"gate": name, "passed": passed}) + if not passed and reason is None: + reason = failure_reason + + if isinstance(source_event, Mapping): + observed = [ + event + for event in ( + source_events.get("events") if isinstance(source_events, Mapping) else [] + ) + if isinstance(event, Mapping) + and event.get("kind") == source_event.get("kind") + and str(event.get("id")) == str(source_event.get("id")) + ] + if not observed: + record("source_event_observed", False, "source_event_not_observed") + elif not any( + _same_instant(event.get("updated_at"), source_event.get("updated_at")) + for event in observed + ): + record("source_event_observed", False, "source_event_revision_mismatch") + else: + record("source_event_observed", True) + provider_event = observed[0] + verified_event = { + **dict(source_event), + "author_login": str( + provider_event.get("author_login") or source_event.get("author_login") + ), + "author_association": _upper_label( + provider_event.get("author_association"), + str(source_event.get("author_association") or "NONE"), + ), + "created_at": str( + provider_event.get("created_at") or source_event.get("created_at") + ), + } + effective_event = verified_event or dict(source_event) + latest_prior = _latest_accepted_revision(prior, source_event=source_event) + prior_time = ( + _timestamp(latest_prior["source_event"].get("updated_at")) + if latest_prior + else None + ) + incoming_time = _timestamp(source_event.get("updated_at")) + revision_relation = "first" + if prior_time is not None and incoming_time is not None: + if incoming_time < prior_time: + revision_relation = "stale" + elif incoming_time == prior_time: + revision_relation = "replay" + else: + revision_relation = "newer" + record("revision_not_stale", revision_relation != "stale", "stale_revision") + record( + "not_self_or_bot_author", + not _is_bot_or_self_login(effective_event.get("author_login")), + "self_or_bot_author", + ) + record( + "author_is_maintainer", + _upper_label(effective_event.get("author_association"), "NONE") + in MAINTAINER_ASSOCIATIONS, + "author_not_maintainer", + ) + else: + effective_event = None + latest_prior = None + revision_relation = "replay" if existing_entry else "first" + record( + "pr_state_not_terminal", + str(observation.get("state") or "") not in TERMINAL_PR_STATES, + "terminal_pr_state", + ) + + accepted = reason is None + same_revision = existing_entry is not None and revision_relation == "replay" + replay = bool( + same_revision + and existing_entry is not None + and existing_entry.get("disposition") in {"accepted", "superseded"} + ) + if replay and existing_entry is not None: + entry = dict(existing_entry) + if entry.get("disposition") == "accepted" and not accepted: + # A later poll can still see terminal PR state for an earlier + # accepted revision; keep the record and mark why it may no longer + # produce work. Its Todo, if any, is left to the Todo lifecycle. + entry["effect_recheck_reason"] = reason + else: + # A previously rejected revision may be re-admitted once its gate + # input changes (for example provider events became observable); + # an unchanged rejection keeps its original recording time. + entry = { + "schema_version": ISSUE_FIX_MAINTAINER_CORRECTION_LEDGER_ENTRY_SCHEMA_VERSION, + "operation_id": operation_id, + "source_event": effective_event, + "correction_fingerprint": correction.get("correction_fingerprint"), + "correction_kind": correction.get("correction_kind"), + "admission_basis": admission_basis, + "disposition": "accepted" if accepted else "context_only", + "reason": reason, + "todo_id": None, + "recorded_at": ( + existing_entry.get("recorded_at") or recorded_at + if same_revision and existing_entry is not None and not accepted + else recorded_at + ), + "superseded_by": None, + "body_captured": False, + } + touched_entries = [entry] + superseded_operation_id = None + if ( + accepted + and revision_relation == "newer" + and latest_prior is not None + and latest_prior.get("operation_id") != operation_id + ): + superseded_operation_id = str(latest_prior.get("operation_id")) + touched_entries.append( + { + **latest_prior, + "disposition": "superseded", + "superseded_by": operation_id, + } + ) + return { + "schema_version": ISSUE_FIX_MAINTAINER_CORRECTION_ADMISSION_SCHEMA_VERSION, + "operation_id": operation_id, + "admission_basis": admission_basis, + "disposition": entry.get("disposition"), + "reason": entry.get("reason") if not accepted else None, + "gates": gates, + "revision_relation": revision_relation, + "replay": replay, + "supersedes_operation_id": superseded_operation_id, + "provider_events_status": provider_events_status, + "prior_ledger_read": bool(prior_ledger_read), + "source_event": effective_event, + "entries": touched_entries, + "body_captured": False, + } + + +def recheck_issue_fix_maintainer_correction_effect_gates( + entry: Mapping[str, Any], + *, + observation: Mapping[str, Any], + ledger_entries: Sequence[Mapping[str, Any]] | None, + goal_stopped: bool, +) -> str | None: + """Re-run the effect-time gates for an accepted entry just before its Todo write.""" + + source_event = entry.get("source_event") + if isinstance(source_event, Mapping): + latest = _latest_accepted_revision( + _prior_entries(ledger_entries), source_event=source_event + ) + if latest is not None and latest.get("operation_id") != entry.get("operation_id"): + latest_time = _timestamp(latest["source_event"].get("updated_at")) + own_time = _timestamp(source_event.get("updated_at")) + if latest_time is not None and own_time is not None and own_time < latest_time: + return "stale_revision" + if _is_bot_or_self_login(source_event.get("author_login")): + return "self_or_bot_author" + if ( + _upper_label(source_event.get("author_association"), "NONE") + not in MAINTAINER_ASSOCIATIONS + ): + return "author_not_maintainer" + if str(observation.get("state") or "") in TERMINAL_PR_STATES: + return "terminal_pr_state" + if goal_stopped: + return "goal_not_active" + return None + def _maintainer_correction_transition( correction: Mapping[str, Any], *, @@ -314,6 +805,15 @@ def _maintainer_correction_transition( correction_kind = str(correction.get("correction_kind")) source_ref = str(correction.get("source_ref")) summary = str(correction.get("summary")) + source_event = correction.get("source_event") + # Todo creation dedupes identical active text, so distinct provider events + # or revisions with the same wording must name the event they came from. + event_suffix = ( + f" Source event: {source_event.get('kind')} {source_event.get('id')}" + f" updated {source_event.get('updated_at')}." + if isinstance(source_event, Mapping) + else "" + ) if correction_kind == "actionable_patch": verification_plan = str(correction.get("verification_plan")) pr_update_path = str(correction.get("pr_update_path")) @@ -327,6 +827,7 @@ def _maintainer_correction_transition( text=( f"[P0] Apply maintainer correction for {pr_permalink} from {source_ref}: " f"{summary} Verify: {verification_plan} Update PR via: {pr_update_path}" + f"{event_suffix}" ), ) | { "terminal_state_precedence": False, @@ -344,7 +845,7 @@ def _maintainer_correction_transition( depends_on=["issue_fix_pr_lifecycle_monitor"], text=( f"[P0] Clarify maintainer correction for {pr_permalink} from {source_ref}: " - f"{correction.get('user_question')} Context: {summary}" + f"{correction.get('user_question')} Context: {summary}{event_suffix}" ), ) | {"terminal_state_precedence": False, "material_change": True} if correction_kind == "missing_authority": @@ -361,7 +862,7 @@ def _maintainer_correction_transition( depends_on=["issue_fix_pr_lifecycle_monitor"], text=( f"[P0] Grant or decline [{scopes}] authority for maintainer correction " - f"{source_ref} on {pr_permalink}: {summary}" + f"{source_ref} on {pr_permalink}: {summary}{event_suffix}" ), ) | { "terminal_state_precedence": False, @@ -978,6 +1479,26 @@ def validate_github_pr_provider_identity( return provider_reference +def _fetch_pr_source_events( + reference: Mapping[str, Any], *, timeout_seconds: int +) -> dict[str, Any]: + repo = str(reference.get("repo") or "") + number = reference.get("number") + if "/" not in repo or not isinstance(number, int): + return {"status": "unavailable", "events": [], "body_captured": False} + owner, repo_name = repo.split("/", 1) + try: + payload = fetch_github_pull_request_event_metadata( + owner=owner, + repo=repo_name, + number=number, + timeout_seconds=timeout_seconds, + ) + except (RuntimeError, ValueError, OSError, subprocess.TimeoutExpired): + return {"status": "unavailable", "events": [], "body_captured": False} + return normalise_issue_fix_pr_source_events(payload) + + def build_issue_fix_pr_lifecycle_monitor_packet( *, repo: str = "public_repo_fixture", @@ -988,6 +1509,7 @@ def build_issue_fix_pr_lifecycle_monitor_packet( fetch_metadata: bool = False, fetch_timeout_seconds: int = 10, maintainer_correction_input: Mapping[str, Any] | None = None, + prior_maintainer_corrections: Sequence[Mapping[str, Any]] | None = None, generated_at: str | None = "2026-06-23T00:00:00Z", ) -> dict[str, Any]: reference = normalise_github_issue_reference(repo=repo, issue_ref=pr_ref, url=url) @@ -1046,18 +1568,52 @@ def build_issue_fix_pr_lifecycle_monitor_packet( if maintainer_correction_input is not None else None ) - if ( - maintainer_correction is not None - and observation["state"] not in TERMINAL_PR_STATES - ): - transition = _maintainer_correction_transition( - maintainer_correction, - pr_permalink=str( - observation.get("permalink") - or reference.get("permalink") - or observation["pr_ref"] - ), + maintainer_correction_admission: dict[str, Any] | None = None + source_events: dict[str, Any] | None = None + if maintainer_correction is not None: + if isinstance(maintainer_correction.get("source_event"), Mapping): + if fetch_metadata: + source_events = _fetch_pr_source_events( + reference, timeout_seconds=fetch_timeout_seconds + ) + else: + source_events = normalise_issue_fix_pr_source_events(payload) + maintainer_correction_admission = ( + evaluate_issue_fix_maintainer_correction_admission( + maintainer_correction, + observation=observation, + source_events=source_events, + prior_entries=prior_maintainer_corrections, + prior_ledger_read=prior_maintainer_corrections is not None, + recorded_at=generated_at, + ) ) + if ( + maintainer_correction_admission["disposition"] == "accepted" + and observation["state"] not in TERMINAL_PR_STATES + ): + transition = _maintainer_correction_transition( + maintainer_correction, + pr_permalink=str( + observation.get("permalink") + or reference.get("permalink") + or observation["pr_ref"] + ), + ) + if transition["decision"] in {"runnable_successor", "user_gate"}: + # The entry carries its own Todo intent so a later run can + # finish an interrupted write without the original input. + maintainer_correction_admission["entries"][0]["todo_intent"] = { + "role": transition["role"], + "text": transition["text"], + "task_class": transition["task_class"], + "action_kind": transition["action_kind"], + "required_write_scopes": ( + list(transition.get("required_write_scopes") or []) + if transition["role"] == "agent" + else None + ), + } packet: dict[str, Any] = { "ok": True, "schema_version": ISSUE_FIX_PR_LIFECYCLE_MONITOR_SCHEMA_VERSION, @@ -1071,8 +1627,7 @@ def build_issue_fix_pr_lifecycle_monitor_packet( if maintainer_correction is not None else None ), - "transition": transition, - "grouped_monitor_projection": _grouped_monitor_projection( + "transition": transition, "grouped_monitor_projection": _grouped_monitor_projection( observation, transition ), "first_screen": { @@ -1124,6 +1679,16 @@ def build_issue_fix_pr_lifecycle_monitor_packet( first_push_ci = _first_push_ci_evidence(observation, observed_at=generated_at) if first_push_ci is not None: packet["first_push_ci"] = first_push_ci + if maintainer_correction_admission is not None: + entries = maintainer_correction_admission.pop("entries") + packet["maintainer_correction_admission"] = maintainer_correction_admission + packet["maintainer_corrections"] = entries + if source_events is not None: + packet["source_events_observed"] = { + "status": source_events.get("status"), + "event_count": len(source_events.get("events") or []), + "body_captured": False, + } validation = validate_issue_fix_pr_lifecycle_monitor_packet(packet) packet["ok"] = bool(validation["ok"]) packet["validation"] = validation @@ -1229,6 +1794,47 @@ def validate_issue_fix_pr_lifecycle_monitor_packet( ) if state in TERMINAL_PR_STATES and decision != "no_followup": errors.append("terminal PR state must supersede maintainer correction") + admission = packet.get("maintainer_correction_admission") + if correction is not None and admission is None: + errors.append("maintainer_correction requires maintainer_correction_admission") + if admission is not None: + if not isinstance(admission, Mapping): + errors.append("maintainer_correction_admission must be an object") + else: + if admission.get("body_captured") is not False: + errors.append("maintainer_correction_admission body_captured must be false") + if admission.get("admission_basis") not in MAINTAINER_CORRECTION_ADMISSION_BASES: + errors.append("maintainer_correction_admission admission_basis is invalid") + if admission.get("disposition") not in MAINTAINER_CORRECTION_DISPOSITIONS: + errors.append("maintainer_correction_admission disposition is invalid") + if admission.get("disposition") == "context_only" and ( + admission.get("reason") not in MAINTAINER_CORRECTION_REJECTION_REASONS + ): + errors.append("context_only maintainer correction must record a reason") + if not str(admission.get("operation_id") or "").startswith( + MAINTAINER_CORRECTION_OPERATION_ID_PREFIX + ): + errors.append("maintainer_correction_admission operation_id is invalid") + entries = packet.get("maintainer_corrections") + if entries is not None: + if not isinstance(entries, list): + errors.append("maintainer_corrections must be a list") + else: + for entry in entries: + if not isinstance(entry, Mapping): + errors.append("maintainer_corrections entries must be objects") + continue + if entry.get("body_captured") is not False: + errors.append("maintainer_corrections entry body_captured must be false") + if entry.get("disposition") not in MAINTAINER_CORRECTION_DISPOSITIONS: + errors.append("maintainer_corrections entry disposition is invalid") + event = entry.get("source_event") + if event is not None and ( + not isinstance(event, Mapping) + or event.get("body_captured") is not False + or any(str(key).lower() in {"body", "body_text", "body_html"} for key in event) + ): + errors.append("maintainer_corrections source_event must stay body-free") contract = packet.get("writeback_contract") if not isinstance(contract, Mapping): errors.append("writeback_contract is required") @@ -1369,6 +1975,19 @@ def render_issue_fix_pr_lifecycle_monitor_markdown(payload: dict[str, Any]) -> s f"- correction_fingerprint: `{correction.get('correction_fingerprint')}`", ] ) + admission = payload.get("maintainer_correction_admission") + if isinstance(admission, Mapping): + lines.extend( + [ + "", + "## Maintainer Correction Admission", + "", + f"- admission_basis: `{admission.get('admission_basis')}`", + f"- disposition: `{admission.get('disposition')}`", + f"- reason: `{admission.get('reason')}`", + f"- operation_id: `{admission.get('operation_id')}`", + ] + ) domain_state = payload.get("domain_state_projection") if isinstance(domain_state, Mapping): lines.extend( diff --git a/loopx/domain_packs/issue_fix.py b/loopx/domain_packs/issue_fix.py index 193e4b640a..7ccae497ac 100644 --- a/loopx/domain_packs/issue_fix.py +++ b/loopx/domain_packs/issue_fix.py @@ -240,6 +240,59 @@ def promote_issue_fix_feasibility_ledger_jsonl( } +def issue_fix_maintainer_correction_entries(row: dict[str, Any] | None) -> list[dict[str, Any]]: + """Return the row's maintainer-correction entries; legacy rows read as empty.""" + + entries = row.get("maintainer_corrections") if isinstance(row, dict) else None + if not isinstance(entries, list): + return [] + return [dict(entry) for entry in entries if isinstance(entry, dict)] + + +def merge_issue_fix_maintainer_correction_entries( + existing: list[dict[str, Any]], incoming: list[dict[str, Any]] +) -> list[dict[str, Any]]: + """Union entries by operation_id; incoming entries replace same-id existing ones.""" + + merged: dict[str, dict[str, Any]] = {} + for entry in [*existing, *incoming]: + operation_id = str(entry.get("operation_id") or "") + if not operation_id: + continue + merged[operation_id] = dict(entry) + return list(merged.values()) + + +def read_issue_fix_pr_lifecycle_ledger_row( + ledger_path: str | Path, *, repo: str, pr_ref: str +) -> dict[str, Any] | None: + """Return the current ledger row for one PR, or None when absent.""" + + path = Path(ledger_path).expanduser() + if not path.exists(): + return None + key = {"repo": repo, "pr_ref": pr_ref} + found: dict[str, Any] | None = None + for line in path.read_text(encoding="utf-8").split("\n"): + if not line.strip(): + continue + try: + row = json.loads(line) + except json.JSONDecodeError: + continue + if not isinstance(row, dict): + continue + row_key = row.get("domain_state_key") + if row_key is None: + try: + row_key = issue_fix_pr_lifecycle_ledger_key(row) + except ValueError: + continue + if row_key == key: + found = row + return found + + def issue_fix_pr_lifecycle_ledger_key(payload: dict[str, Any]) -> dict[str, Any]: observation = payload.get("observation") if not isinstance(observation, dict): @@ -297,6 +350,8 @@ def unchanged_quiet_observation( and existing_correction_fingerprint == incoming_correction_fingerprint and existing_receipts == incoming_receipts and existing_queue == incoming_queue + and issue_fix_maintainer_correction_entries(existing) + == issue_fix_maintainer_correction_entries(incoming) ) def preserve_compact_lifecycle_evidence( @@ -321,8 +376,31 @@ def preserve_compact_lifecycle_evidence( first_push_ci = existing.get("first_push_ci") if "first_push_ci" not in merged and isinstance(first_push_ci, dict): merged = {**merged, "first_push_ci": dict(first_push_ci)} + existing_entries = issue_fix_maintainer_correction_entries(existing) + if existing_entries or "maintainer_corrections" in merged: + merged = { + **merged, + "maintainer_corrections": merge_issue_fix_maintainer_correction_entries( + existing_entries, issue_fix_maintainer_correction_entries(merged) + ), + } + # Older readers take `maintainer_correction` as the latest accepted + # correction; a rejected or absent correction must not blank it. + admission = merged.get("maintainer_correction_admission") + incoming_accepted = ( + isinstance(admission, dict) and admission.get("disposition") == "accepted" + ) + if not incoming_accepted and isinstance( + existing.get("maintainer_correction"), dict + ): + merged = { + **merged, + "maintainer_correction": dict(existing["maintainer_correction"]), + "maintainer_correction_fingerprint": existing.get( + "maintainer_correction_fingerprint" + ), + } return merged - return _upsert_issue_fix_payload( ledger_path, payload, diff --git a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py new file mode 100644 index 0000000000..cd5eda65f9 --- /dev/null +++ b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py @@ -0,0 +1,569 @@ +"""Maintainer-correction admission through the real ``pr-lifecycle`` CLI. + +Each case runs ``loopx issue-fix pr-lifecycle --execute-transition`` as a +subprocess against a disposable File or SQLite registry, then reads the +canonical Todo projection and the ``pr-lifecycle.jsonl`` ledger directly. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from loopx.control_plane.testing.canary_harness import import_fixture_authority +from loopx.domain_packs.issue_fix import ( + default_issue_fix_domain_state_ledger_path, + read_issue_fix_pr_lifecycle_ledger_row, +) +from loopx.todos import complete_goal_todo, list_goal_todos + +ROOT = Path(__file__).resolve().parents[2] +GOAL_ID = "issue-fix-correction-admission" +AGENT_ID = "issue-fix-agent" +PR_URL = "https://github.com/example-org/example-repo/pull/3121" +REPO = "example-org/example-repo" +PR_REF = "pull_3121" +PATCH_ACTION = "issue_fix_maintainer_correction_patch" +CORRECTION_ACTIONS = { + PATCH_ACTION, + "clarify_issue_fix_maintainer_correction", + "grant_issue_fix_maintainer_correction_authority", +} +MAINTAINER = {"login": "maintainer-a", "association": "MEMBER"} + + +def _event( + event_id: str, + *, + kind: str = "issue_comment", + login: str = MAINTAINER["login"], + association: str = MAINTAINER["association"], + created_at: str = "2026-06-20T10:00:00Z", + updated_at: str = "2026-06-20T10:00:00Z", +) -> dict[str, object]: + return { + "kind": kind, + "id": event_id, + "html_url": f"https://github.com/example-org/example-repo/pull/3121#issuecomment-{event_id}", + "author_login": login, + "author_association": association, + "created_at": created_at, + "updated_at": updated_at, + } + + +def _fixture_event(event: dict[str, object]) -> dict[str, object]: + """Body-free provider metadata row for one comment or review.""" + + row = { + "id": int(str(event["id"])), + "user": {"login": event["author_login"]}, + "author_association": event["author_association"], + "created_at": event["created_at"], + "html_url": event["html_url"], + } + if event["kind"] == "review": + row["submitted_at"] = event["updated_at"] + else: + row["updated_at"] = event["updated_at"] + return row + + +def _metadata( + events: list[dict[str, object]], *, state: str = "OPEN", merged_at: str | None = None +) -> dict[str, object]: + comments = [_fixture_event(e) for e in events if e["kind"] == "issue_comment"] + reviews = [_fixture_event(e) for e in events if e["kind"] == "review"] + review_comments = [_fixture_event(e) for e in events if e["kind"] == "review_comment"] + payload: dict[str, object] = { + "state": state, + "reviewDecision": "REVIEW_REQUIRED", + "mergeStateStatus": "CLEAN", + "statusCheckRollup": [{"name": "focused", "conclusion": "SUCCESS"}], + "comments": comments, + "reviews": reviews, + "reviewComments": review_comments, + } + if merged_at: + payload["mergedAt"] = merged_at + return payload + + +def _correction_v1(source_event: dict[str, object], **overrides: object) -> dict[str, object]: + payload: dict[str, object] = { + "schema_version": "issue_fix_maintainer_correction_input_v1", + "correction_kind": "actionable_patch", + "source_kind": "maintainer_comment", + "source_ref": str(source_event["html_url"]), + "summary": "Keep the compatibility branch and add the missing regression assertion.", + "verification_plan": "Run the focused compatibility test and the module smoke.", + "pr_update_path": "Commit the bounded patch, push the PR branch, and post the validation summary.", + "source_event": source_event, + } + payload.update(overrides) + return payload + + +def _correction_v0(**overrides: object) -> dict[str, object]: + payload: dict[str, object] = { + "schema_version": "issue_fix_maintainer_correction_input_v0", + "correction_kind": "actionable_patch", + "source_kind": "review", + "source_ref": "https://code.example.org/reviews/42", + "summary": "Keep the compatibility branch and add the missing regression assertion.", + "verification_plan": "Run the focused compatibility test and the module smoke.", + "pr_update_path": "Commit the bounded patch, push the PR branch, and post the validation summary.", + } + payload.update(overrides) + return payload + + +class Harness: + def __init__(self, root: Path, provider: str, *, goal_overrides: dict | None = None): + self.root = root + self.project = root / "project" + self.project.mkdir() + state_file = self.project / "ACTIVE_GOAL_STATE.md" + state_file.write_text( + "# Correction Admission\n\n" + "## User Todo / Owner Review Reading Queue\n\n" + "## Agent Todo\n", + encoding="utf-8", + ) + self.registry = root / "registry.json" + goal = { + "id": GOAL_ID, + "repo": str(self.project), + "state_file": str(state_file), + "coordination": { + "agent_model": "peer_v1", + "registered_agents": [AGENT_ID], + }, + } + goal.update(goal_overrides or {}) + self.registry.write_text( + json.dumps({"common_runtime_root": str(root / "runtime"), "goals": [goal]}), + encoding="utf-8", + ) + import_fixture_authority(self.registry, GOAL_ID, provider=provider) + self._counter = 0 + + def _write_json(self, name: str, payload: dict[str, object]) -> Path: + self._counter += 1 + path = self.root / f"{self._counter:03d}-{name}.json" + path.write_text(json.dumps(payload), encoding="utf-8") + return path + + def run( + self, + *, + metadata: dict[str, object], + correction: dict[str, object] | None, + env: dict[str, str] | None = None, + check: bool = True, + ) -> subprocess.CompletedProcess[str]: + args = [ + sys.executable, "-m", "loopx.cli", + "--registry", str(self.registry), "--format", "json", + "issue-fix", "pr-lifecycle", + "--url", PR_URL, + "--metadata-json", str(self._write_json("metadata", metadata)), + "--goal-id", GOAL_ID, + "--project", str(self.project), + "--claimed-by", AGENT_ID, + "--execute-transition", + ] + if correction is not None: + args += ["--maintainer-correction-json", str(self._write_json("correction", correction))] + result = subprocess.run( + args, cwd=ROOT, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + env={**os.environ, **(env or {})}, + ) + if check: + assert result.returncode == 0, result.stderr + return result + + def run_json(self, **kwargs: object) -> dict[str, object]: + result = self.run(**kwargs) # type: ignore[arg-type] + payload = json.loads(result.stdout) + assert payload["ok"] is True, payload + return payload + + def correction_todos(self) -> list[dict[str, object]]: + native = list_goal_todos(registry_path=self.registry, goal_id=GOAL_ID) + assert native["authority_read"]["decision_read_from_provider"] is True, native + return [t for t in native["todos"] if t.get("action_kind") in CORRECTION_ACTIONS] + + def ledger_row(self) -> dict[str, object]: + ledger = default_issue_fix_domain_state_ledger_path(project=self.project, goal_id=GOAL_ID) + row = read_issue_fix_pr_lifecycle_ledger_row(ledger, repo=REPO, pr_ref=PR_REF) + assert row is not None + assert row.get("maintainer_correction_body_captured") is False, row + assert "body" not in json.dumps(row.get("maintainer_corrections")).lower().replace( + "body_captured", "" + ) + return row + + def entries(self) -> list[dict[str, object]]: + return list(self.ledger_row().get("maintainer_corrections") or []) + + def mark_done(self, todo_id: str) -> None: + result = complete_goal_todo( + registry_path=self.registry, goal_id=GOAL_ID, todo_id=todo_id, + role="agent", agent_id=AGENT_ID, claimed_by=AGENT_ID, + evidence="patch landed in fixture", no_followup=True, + ) + assert result.get("status") == "done", result + + +@pytest.fixture(params=["file", "sqlite"]) +def harness(tmp_path: Path, request: pytest.FixtureRequest) -> Harness: + return Harness(tmp_path, request.param) + + +def _correction_write(payload: dict[str, object]) -> dict[str, object]: + """The correction Todo receipt, independent of grouped-monitor writes.""" + + receipt = payload["maintainer_correction_settlement"]["todo_write"] + assert receipt["schema_version"] == "issue_fix_maintainer_correction_todo_write_v0" + return receipt + + +def _assert_no_correction_todo(payload: dict[str, object], h: Harness, reason: str) -> None: + admission = payload["maintainer_correction_admission"] + assert admission["disposition"] == "context_only", admission + assert admission["reason"] == reason, admission + assert admission["admission_basis"] == "provider_verified" + assert _correction_write(payload)["write_performed"] is False, _correction_write(payload) + assert h.correction_todos() == [] + entries = h.entries() + assert len(entries) == 1, entries + assert entries[0]["disposition"] == "context_only" + assert entries[0]["reason"] == reason + assert entries[0]["todo_id"] is None + + +def test_t01_same_event_replayed_yields_one_todo_even_after_done(harness: Harness) -> None: + event = _event("1001") + metadata = _metadata([event]) + first = harness.run_json(metadata=metadata, correction=_correction_v1(event)) + assert first["maintainer_correction_admission"]["disposition"] == "accepted" + assert first["maintainer_correction_admission"]["admission_basis"] == "provider_verified" + assert _correction_write(first)["write_performed"] is True, _correction_write(first) + assert _correction_write(first)["claimed_by"] == AGENT_ID + operation_id = first["maintainer_correction_admission"]["operation_id"] + assert operation_id.startswith("issue-fix-correction:") + todo_id = _correction_write(first)["todo_id"] + + for _ in range(9): + replay = harness.run_json(metadata=metadata, correction=_correction_v1(event)) + assert _correction_write(replay)["write_performed"] is False, _correction_write(replay) + assert _correction_write(replay)["already_exists"] is True, _correction_write(replay) + assert _correction_write(replay)["todo_id"] == todo_id + assert replay["maintainer_correction_admission"]["replay"] is True + assert replay["domain_state_projection"]["write_result"]["status"] == "unchanged" + + todos = harness.correction_todos() + assert len(todos) == 1, todos + assert todos[0]["todo_id"] == todo_id + entries = harness.entries() + assert len(entries) == 1, entries + assert entries[0]["operation_id"] == operation_id + assert entries[0]["todo_id"] == todo_id + assert entries[0]["disposition"] == "accepted" + assert entries[0]["source_event"]["id"] == "1001" + + harness.mark_done(todo_id) + after_done = harness.run_json(metadata=metadata, correction=_correction_v1(event)) + assert _correction_write(after_done)["write_performed"] is False, _correction_write(after_done) + assert _correction_write(after_done)["already_exists"] is True + todos = harness.correction_todos() + assert len(todos) == 1, todos + assert todos[0]["status"] == "done" + assert len(harness.entries()) == 1 + + +def test_t02_distinct_events_with_identical_text_yield_two_todos(harness: Harness) -> None: + first_event = _event("2001") + second_event = _event("2002", created_at="2026-06-20T11:00:00Z", updated_at="2026-06-20T11:00:00Z") + metadata = _metadata([first_event, second_event]) + shared_ref = "https://github.com/example-org/example-repo/pull/3121" + first = harness.run_json( + metadata=metadata, correction=_correction_v1(first_event, source_ref=shared_ref) + ) + second = harness.run_json( + metadata=metadata, correction=_correction_v1(second_event, source_ref=shared_ref) + ) + assert first["maintainer_correction"]["summary"] == second["maintainer_correction"]["summary"] + assert first["transition"]["text"] != second["transition"]["text"] + assert "issue_comment 2001" in first["transition"]["text"] + assert "issue_comment 2002" in second["transition"]["text"] + assert _correction_write(first)["write_performed"] is True + assert _correction_write(second)["write_performed"] is True + assert _correction_write(first)["todo_id"] != _correction_write(second)["todo_id"] + assert first["maintainer_correction_fingerprint"] == second["maintainer_correction_fingerprint"] + assert len(harness.correction_todos()) == 2 + entries = harness.entries() + assert len(entries) == 2 + assert {e["source_event"]["id"] for e in entries} == {"2001", "2002"} + assert all(e["disposition"] == "accepted" and e["todo_id"] for e in entries) + + +def test_t03_older_revision_is_stale_and_newer_supersedes(harness: Harness) -> None: + older = _event("3001", updated_at="2026-06-20T10:00:00Z") + newer = _event("3001", updated_at="2026-06-20T12:00:00Z") + newest = _event("3001", updated_at="2026-06-20T13:00:00Z") + + adopted = harness.run_json(metadata=_metadata([newer]), correction=_correction_v1(newer)) + assert adopted["maintainer_correction_admission"]["disposition"] == "accepted" + adopted_operation = adopted["maintainer_correction_admission"]["operation_id"] + adopted_todo = _correction_write(adopted)["todo_id"] + + stale = harness.run_json(metadata=_metadata([older]), correction=_correction_v1(older)) + admission = stale["maintainer_correction_admission"] + assert admission["disposition"] == "context_only" + assert admission["reason"] == "stale_revision" + assert admission["revision_relation"] == "stale" + assert _correction_write(stale)["write_performed"] is False + assert _correction_write(stale)["skip_reason"] == "stale_revision" + entries = {e["operation_id"]: e for e in harness.entries()} + assert len(entries) == 2 + assert entries[adopted_operation]["disposition"] == "accepted" + assert entries[adopted_operation]["todo_id"] == adopted_todo + assert entries[adopted_operation]["superseded_by"] is None + todos = harness.correction_todos() + assert [t["todo_id"] for t in todos] == [adopted_todo] + + superseding = harness.run_json(metadata=_metadata([newest]), correction=_correction_v1(newest)) + admission = superseding["maintainer_correction_admission"] + assert admission["disposition"] == "accepted" + assert admission["revision_relation"] == "newer" + assert admission["supersedes_operation_id"] == adopted_operation + assert _correction_write(superseding)["write_performed"] is True + assert _correction_write(superseding)["todo_id"] != adopted_todo + entries = {e["operation_id"]: e for e in harness.entries()} + assert entries[adopted_operation]["disposition"] == "superseded" + assert entries[adopted_operation]["superseded_by"] == admission["operation_id"] + assert entries[admission["operation_id"]]["disposition"] == "accepted" + assert len(harness.correction_todos()) == 2 + + +def test_t04_bot_author_is_context_only(harness: Harness) -> None: + event = _event("4001", login="loopx-automation[bot]", association="MEMBER") + payload = harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + _assert_no_correction_todo(payload, harness, "self_or_bot_author") + + +@pytest.mark.parametrize("association", ["NONE", "CONTRIBUTOR"]) +def test_t05_non_maintainer_association_is_context_only(harness: Harness, association: str) -> None: + event = _event("5001", association=association) + payload = harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + _assert_no_correction_todo(payload, harness, "author_not_maintainer") + + +def test_t05_provider_association_wins_over_host_assertion(harness: Harness) -> None: + event = _event("5002", association="CONTRIBUTOR") + asserted = {**event, "author_association": "OWNER"} + payload = harness.run_json(metadata=_metadata([event]), correction=_correction_v1(asserted)) + _assert_no_correction_todo(payload, harness, "author_not_maintainer") + + +def test_t05_unobserved_event_is_context_only(harness: Harness) -> None: + observed = _event("5101") + claimed = _event("5102") + payload = harness.run_json(metadata=_metadata([observed]), correction=_correction_v1(claimed)) + _assert_no_correction_todo(payload, harness, "source_event_not_observed") + assert payload["maintainer_correction_admission"]["provider_events_status"] == "available" + + +def test_t05_revision_mismatch_is_context_only(harness: Harness) -> None: + observed = _event("5201", updated_at="2026-06-20T12:00:00Z") + claimed = _event("5201", updated_at="2026-06-20T10:00:00Z") + payload = harness.run_json(metadata=_metadata([observed]), correction=_correction_v1(claimed)) + _assert_no_correction_todo(payload, harness, "source_event_revision_mismatch") + + +def test_t05_missing_event_metadata_is_context_only(harness: Harness) -> None: + event = _event("5301") + metadata = _metadata([event]) + for key in ("comments", "reviews", "reviewComments"): + metadata.pop(key) + payload = harness.run_json(metadata=metadata, correction=_correction_v1(event)) + _assert_no_correction_todo(payload, harness, "source_event_not_observed") + assert payload["maintainer_correction_admission"]["provider_events_status"] == "unavailable" + + +@pytest.mark.parametrize("provider", ["file", "sqlite"]) +def test_t06_stopped_goal_refuses_at_effect_time(tmp_path: Path, provider: str) -> None: + h = Harness(tmp_path, provider, goal_overrides={"activation_state": "stopped"}) + event = _event("6001") + payload = h.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + admission = payload["maintainer_correction_admission"] + # Packet-time gates pass; only the effect-time Goal read refuses. + assert admission["disposition"] == "accepted", admission + settlement = payload["maintainer_correction_settlement"] + assert settlement["goal_stopped"] is True + assert settlement["settlements"] == [ + {"operation_id": admission["operation_id"], "status": "refused", "reason": "goal_not_active"} + ] + assert _correction_write(payload)["write_performed"] is False + assert _correction_write(payload)["skip_reason"] == "goal_not_active" + assert h.correction_todos() == [] + entries = h.entries() + assert len(entries) == 1 + assert entries[0]["disposition"] == "context_only" + assert entries[0]["reason"] == "goal_not_active" + assert entries[0]["effect_recheck"] == "refused" + assert entries[0]["todo_id"] is None + + +def test_t06_terminal_state_refuses_pending_entry_at_effect_time(harness: Harness) -> None: + event = _event("6101") + open_metadata = _metadata([event]) + faulted = harness.run( + metadata=open_metadata, correction=_correction_v1(event), + env={"LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO": "1"}, check=False, + ) + assert faulted.returncode != 0 + pending = harness.entries() + assert pending[0]["disposition"] == "accepted" and pending[0]["todo_id"] is None + merged = _metadata([event], state="MERGED", merged_at="2026-06-21T00:00:00Z") + payload = harness.run_json(metadata=merged, correction=None) + assert payload["transition"]["decision"] == "no_followup" + settlement = payload["maintainer_correction_settlement"] + assert settlement["settlements"][0]["reason"] == "terminal_pr_state" + assert harness.correction_todos() == [] + entries = harness.entries() + assert entries[0]["disposition"] == "context_only" + assert entries[0]["reason"] == "terminal_pr_state" + + +def test_t11_crash_between_ledger_entry_and_todo_write_is_reconciled(harness: Harness) -> None: + event = _event("11001") + metadata = _metadata([event]) + faulted = harness.run( + metadata=metadata, correction=_correction_v1(event), + env={"LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO": "1"}, check=False, + ) + assert faulted.returncode != 0, faulted.stdout + assert "LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO" in faulted.stderr + faulted.stdout + assert harness.correction_todos() == [] + pending = harness.entries() + assert len(pending) == 1 + assert pending[0]["disposition"] == "accepted" + assert pending[0]["todo_id"] is None + assert pending[0]["todo_intent"]["action_kind"] == PATCH_ACTION + operation_id = pending[0]["operation_id"] + + # A plain poll without the correction input finishes the pending write. + poll = harness.run_json(metadata=metadata, correction=None) + settlement = poll["maintainer_correction_settlement"] + assert settlement["settlements"] == [ + {"operation_id": operation_id, "status": "written", + "todo_id": settlement["settlements"][0]["todo_id"]} + ] + todos = harness.correction_todos() + assert len(todos) == 1 + entries = harness.entries() + assert len(entries) == 1 + assert entries[0]["todo_id"] == todos[0]["todo_id"] + assert entries[0]["operation_id"] == operation_id + + # Re-sending the same correction afterwards is a replay, not a second Todo. + replay = harness.run_json(metadata=metadata, correction=_correction_v1(event)) + assert _correction_write(replay)["already_exists"] is True + assert _correction_write(replay)["write_performed"] is False + assert len(harness.correction_todos()) == 1 + + +def test_t19_merged_pr_records_late_correction_as_context_only(harness: Harness) -> None: + event = _event("19001") + merged = _metadata([event], state="MERGED", merged_at="2026-06-21T00:00:00Z") + payload = harness.run_json(metadata=merged, correction=_correction_v1(event)) + assert payload["transition"]["decision"] == "no_followup" + assert payload["transition"]["terminal_state_precedence"] is True + _assert_no_correction_todo(payload, harness, "terminal_pr_state") + assert _correction_write(payload)["skip_reason"] == "terminal_pr_state" + row = harness.ledger_row() + assert row["maintainer_correction_admission"]["reason"] == "terminal_pr_state" + + +def test_v0_input_replays_through_stable_operation_id(harness: Harness) -> None: + """Formerly the smoke's replay assertion: host-asserted v0 stays idempotent.""" + + metadata = _metadata([]) + first = harness.run_json(metadata=metadata, correction=_correction_v0()) + admission = first["maintainer_correction_admission"] + assert admission["admission_basis"] == "host_asserted" + assert admission["disposition"] == "accepted" + assert first["transition"]["action_kind"] == PATCH_ACTION + assert _correction_write(first)["write_performed"] is True + assert _correction_write(first)["claimed_by"] == AGENT_ID + assert _correction_write(first)["path_recorded"] is False + + replay = harness.run_json(metadata=metadata, correction=_correction_v0()) + assert _correction_write(replay)["write_performed"] is False + assert _correction_write(replay)["already_exists"] is True + assert replay["domain_state_projection"]["write_result"]["status"] == "unchanged" + state_text = (harness.project / "ACTIVE_GOAL_STATE.md").read_text(encoding="utf-8") + assert state_text.count(f"action_kind={PATCH_ACTION}") == 1 + assert len(harness.correction_todos()) == 1 + entries = harness.entries() + assert len(entries) == 1 + assert entries[0]["source_event"] is None + assert entries[0]["admission_basis"] == "host_asserted" + assert entries[0]["todo_id"] == _correction_write(first)["todo_id"] + + harness.mark_done(_correction_write(first)["todo_id"]) + after_done = harness.run_json(metadata=metadata, correction=_correction_v0()) + assert _correction_write(after_done)["already_exists"] is True + assert len(harness.correction_todos()) == 1 + + +def test_v0_terminal_state_is_recorded_not_dropped(harness: Harness) -> None: + merged = _metadata([], state="MERGED", merged_at="2026-06-21T00:00:00Z") + payload = harness.run_json(metadata=merged, correction=_correction_v0()) + admission = payload["maintainer_correction_admission"] + assert admission["disposition"] == "context_only" + assert admission["reason"] == "terminal_pr_state" + assert admission["admission_basis"] == "host_asserted" + entries = harness.entries() + assert len(entries) == 1 and entries[0]["reason"] == "terminal_pr_state" + assert harness.correction_todos() == [] + + +def test_v1_input_rejects_body_fields_and_v0_rejects_source_event() -> None: + from loopx.capabilities.issue_fix.pr_lifecycle import ( + normalise_issue_fix_maintainer_correction_input, + ) + + event = _event("9001") + with pytest.raises(ValueError): + normalise_issue_fix_maintainer_correction_input( + _correction_v1({**event, "body": "raw body must not enter"}) + ) + with pytest.raises(ValueError): + normalise_issue_fix_maintainer_correction_input( + _correction_v0(source_event=event) + ) + with pytest.raises(ValueError): + normalise_issue_fix_maintainer_correction_input( + {k: v for k, v in _correction_v1(event).items() if k != "source_event"} + ) + with pytest.raises(ValueError): + normalise_issue_fix_maintainer_correction_input( + _correction_v1({**event, "updated_at": "2026-06-20 10:00"}) + ) + normalized = normalise_issue_fix_maintainer_correction_input(_correction_v1(event)) + assert normalized["admission_basis"] == "provider_verified" + assert normalized["source_event"]["body_captured"] is False + assert normalized["correction_fingerprint"] == ( + normalise_issue_fix_maintainer_correction_input( + _correction_v0(source_kind="maintainer_comment", source_ref=event["html_url"]) + )["correction_fingerprint"] + ) From 1c5477b984bb00f920b85fee60948897576372ee Mon Sep 17 00:00:00 2001 From: hhyykk <4408344+hhyykk@users.noreply.github.com> Date: Sun, 1 Nov 2026 19:24:49 +0800 Subject: [PATCH 02/11] Preserve superseded correction entries and canonicalize event revisions Address review findings on maintainer-correction admission: - a stale replay keeps the existing superseded entry, its todo_id and superseded_by link instead of rebuilding it - event timestamps are canonicalized to UTC before identity hashing, ordering and Todo text, so equivalent forms share one operation id - the Goal is resolved before the first ledger write; a missing Goal fails cleanly without leaving a pending entry - a replay with a different host summary settles from the stored todo_intent and reports host_summary_mismatch instead of erroring - the test-only fault hook fires only before a first Todo write - fold duplicate entry reader and readback receipt literals Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com> --- loopx/capabilities/issue_fix/README.md | 2 +- loopx/capabilities/issue_fix/cli.py | 77 +++++++++-------- loopx/capabilities/issue_fix/pr_lifecycle.py | 82 +++++++++---------- ...sue_fix_maintainer_correction_admission.py | 57 +++++++++++++ 4 files changed, 141 insertions(+), 77 deletions(-) diff --git a/loopx/capabilities/issue_fix/README.md b/loopx/capabilities/issue_fix/README.md index 019a5badcd..b9f0e4a270 100644 --- a/loopx/capabilities/issue_fix/README.md +++ b/loopx/capabilities/issue_fix/README.md @@ -439,7 +439,7 @@ these entries in `maintainer_corrections` `disposition`, `reason`, `todo_id`, `recorded_at`, `superseded_by`, `todo_intent`); the accepted entry is written with `todo_id: null` before the Todo write and filled afterwards, and any later run retries a still-pending -accepted entry under its operation id. Setting +accepted entry under its operation id. The test-only `LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO=1` aborts between those two writes and exists only to exercise that recovery. The legacy `maintainer_correction` fields keep the latest accepted correction for older diff --git a/loopx/capabilities/issue_fix/cli.py b/loopx/capabilities/issue_fix/cli.py index 711f8a67b9..17796b40ee 100644 --- a/loopx/capabilities/issue_fix/cli.py +++ b/loopx/capabilities/issue_fix/cli.py @@ -119,7 +119,10 @@ def _goal_stopped_for_correction(registry_path: Path | None, goal_id: str | None if registry_path is None or not goal_id: return False try: - return goal_is_stopped(load_goal_from_registry(registry_path, goal_id)) + goal = load_goal_from_registry(registry_path, goal_id) + if goal is None: + return True + return goal_is_stopped(goal) except ValueError: # An unreadable activation record cannot prove the Goal is active. return True @@ -175,6 +178,23 @@ def _maintainer_correction_todo_write_receipt( } +def _correction_todo_readback_receipt( + *, skip_reason: str, operation_id: str | None = None, todo_id: str | None = None, +) -> dict[str, Any]: + receipt = { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": False, + "already_exists": bool(todo_id), + "skip_reason": skip_reason, + "path_recorded": False, + } + if operation_id is not None: + receipt["operation_id"] = operation_id + if todo_id is not None: + receipt["todo_id"] = todo_id + return receipt + + def _settle_maintainer_correction_entries( *, payload: dict[str, Any], @@ -231,14 +251,9 @@ def _settle_maintainer_correction_entries( {"operation_id": operation_id, "status": "refused", "reason": refusal} ) if is_current: - current_todo_write = { - "schema_version": "issue_fix_maintainer_correction_todo_write_v0", - "write_performed": False, - "already_exists": False, - "skip_reason": refusal, - "operation_id": operation_id, - "path_recorded": False, - } + current_todo_write = _correction_todo_readback_receipt( + skip_reason=refusal, operation_id=operation_id, + ) continue if refusal is not None: # The Todo already exists for this revision; a replay only reads it @@ -247,17 +262,12 @@ def _settle_maintainer_correction_entries( {"operation_id": operation_id, "status": "settled_earlier", "reason": refusal} ) if is_current: - current_todo_write = { - "schema_version": "issue_fix_maintainer_correction_todo_write_v0", - "write_performed": False, - "already_exists": True, - "todo_id": entry.get("todo_id"), - "operation_id": operation_id, - "skip_reason": refusal, - "path_recorded": False, - } + current_todo_write = _correction_todo_readback_receipt( + skip_reason=refusal, operation_id=operation_id, + todo_id=entry.get("todo_id"), + ) continue - if os.environ.get(MAINTAINER_CORRECTION_FAULT_ENV) == "1": + if not entry.get("todo_id") and os.environ.get(MAINTAINER_CORRECTION_FAULT_ENV) == "1": raise RuntimeError( "maintainer correction Todo write aborted by " f"{MAINTAINER_CORRECTION_FAULT_ENV}" @@ -285,18 +295,14 @@ def _settle_maintainer_correction_entries( if is_current: current_todo_write = receipt if current_todo_write is None and current_operation_id: - current_todo_write = { - "schema_version": "issue_fix_maintainer_correction_todo_write_v0", - "write_performed": False, - "already_exists": False, - "skip_reason": ( + current_todo_write = _correction_todo_readback_receipt( + skip_reason=( str(admission.get("reason") or admission.get("disposition") or "") if admission.get("disposition") != "accepted" else "unchanged_monitor_quiet" ), - "operation_id": current_operation_id, - "path_recorded": False, - } + operation_id=current_operation_id, + ) if changed_entries: payload["maintainer_corrections"] = changed_entries upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) @@ -1465,6 +1471,13 @@ def handle_issue_fix_command( raise ValueError("--execute-transition requires --goal-id") if args.execute_transition and not args.claimed_by: raise ValueError("--execute-transition requires --claimed-by") + if args.maintainer_correction_json and args.execute_transition: + # Resolve the Goal before any ledger write so a missing Goal + # cannot leave a pending correction entry behind. + if load_goal_from_registry(registry_path, args.goal_id) is None: + raise ValueError( + f"goal {args.goal_id} is not resolvable from registry {registry_path}" + ) should_write_domain_state = bool( not args.no_write_domain_state and (args.goal_id or args.ledger_path) ) @@ -1575,13 +1588,9 @@ def handle_issue_fix_command( ) todo_changed = bool(todo_write_receipt.get("write_performed")) if not todo_write_receipt: - todo_write_receipt = { - "schema_version": "issue_fix_maintainer_correction_todo_write_v0", - "write_performed": False, - "already_exists": False, - "skip_reason": "terminal_or_no_followup", - "path_recorded": False, - } + todo_write_receipt = _correction_todo_readback_receipt( + skip_reason="terminal_or_no_followup", + ) if ( not todo_changed and "skip_reason" in todo_write_receipt diff --git a/loopx/capabilities/issue_fix/pr_lifecycle.py b/loopx/capabilities/issue_fix/pr_lifecycle.py index 7f8e569012..dcd0b03aa8 100644 --- a/loopx/capabilities/issue_fix/pr_lifecycle.py +++ b/loopx/capabilities/issue_fix/pr_lifecycle.py @@ -6,12 +6,13 @@ import re import subprocess from collections.abc import Mapping, Sequence -from datetime import datetime +from datetime import datetime, timezone from pathlib import PurePosixPath from typing import Any from urllib.parse import quote, urlsplit from ...control_plane.runtime.public_safety import public_safe_compact_text +from ...domain_packs.issue_fix import issue_fix_maintainer_correction_entries from .github_public import ( MAINTAINER_ASSOCIATIONS, @@ -296,11 +297,12 @@ def _normalise_author_login(value: Any, *, field: str) -> str: def _normalise_event_timestamp(value: Any, *, field: str) -> str: text = str(value or "").strip() - if _timestamp(text) is None: + parsed = _timestamp(text) + if parsed is None: raise ValueError( f"maintainer correction {field} must be a timezone-aware ISO timestamp" ) - return text[:40] + return parsed.astimezone(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") def normalise_issue_fix_maintainer_correction_source_event( @@ -516,6 +518,7 @@ def normalise_issue_fix_pr_source_events( or item.get("submitted_at") or item.get("submittedAt") ) + created_at = item.get("created_at") or item.get("createdAt") events.append( { "kind": kind, @@ -526,11 +529,14 @@ def normalise_issue_fix_pr_source_events( or item.get("authorAssociation"), "NONE", ), - "created_at": str( - item.get("created_at") or item.get("createdAt") or "" - )[:40] - or None, - "updated_at": str(updated_at or "")[:40] or None, + "created_at": ( + _normalise_event_timestamp(created_at, field="created_at") + if _timestamp(created_at) else None + ), + "updated_at": ( + _normalise_event_timestamp(updated_at, field="updated_at") + if _timestamp(updated_at) else None + ), "body_captured": False, } ) @@ -552,12 +558,6 @@ def _is_bot_or_self_login(login: Any) -> bool: return text.endswith(BOT_LOGIN_SUFFIX) -def _prior_entries(value: Any) -> list[dict[str, Any]]: - if not isinstance(value, Sequence) or isinstance(value, (str, bytes)): - return [] - return [dict(item) for item in value if isinstance(item, Mapping)] - - def _latest_accepted_revision( prior_entries: Sequence[Mapping[str, Any]], *, source_event: Mapping[str, Any] ) -> dict[str, Any] | None: @@ -605,7 +605,9 @@ def evaluate_issue_fix_maintainer_correction_admission( ) source_event = correction.get("source_event") admission_basis = str(correction.get("admission_basis") or "host_asserted") - prior = _prior_entries(prior_entries) + prior = issue_fix_maintainer_correction_entries( + {"maintainer_corrections": prior_entries} + ) existing_entry = next( (entry for entry in prior if entry.get("operation_id") == operation_id), None ) @@ -653,8 +655,9 @@ def record(name: str, passed: bool, failure_reason: str | None = None) -> None: provider_event.get("author_association"), str(source_event.get("author_association") or "NONE"), ), - "created_at": str( - provider_event.get("created_at") or source_event.get("created_at") + "created_at": _normalise_event_timestamp( + provider_event.get("created_at") or source_event.get("created_at"), + field="source_event created_at", ), } effective_event = verified_event or dict(source_event) @@ -696,23 +699,10 @@ def record(name: str, passed: bool, failure_reason: str | None = None) -> None: ) accepted = reason is None - same_revision = existing_entry is not None and revision_relation == "replay" - replay = bool( - same_revision - and existing_entry is not None - and existing_entry.get("disposition") in {"accepted", "superseded"} - ) - if replay and existing_entry is not None: + replay = existing_entry is not None + if existing_entry is not None: entry = dict(existing_entry) - if entry.get("disposition") == "accepted" and not accepted: - # A later poll can still see terminal PR state for an earlier - # accepted revision; keep the record and mark why it may no longer - # produce work. Its Todo, if any, is left to the Todo lifecycle. - entry["effect_recheck_reason"] = reason else: - # A previously rejected revision may be re-admitted once its gate - # input changes (for example provider events became observable); - # an unchanged rejection keeps its original recording time. entry = { "schema_version": ISSUE_FIX_MAINTAINER_CORRECTION_LEDGER_ENTRY_SCHEMA_VERSION, "operation_id": operation_id, @@ -723,11 +713,7 @@ def record(name: str, passed: bool, failure_reason: str | None = None) -> None: "disposition": "accepted" if accepted else "context_only", "reason": reason, "todo_id": None, - "recorded_at": ( - existing_entry.get("recorded_at") or recorded_at - if same_revision and existing_entry is not None and not accepted - else recorded_at - ), + "recorded_at": recorded_at, "superseded_by": None, "body_captured": False, } @@ -751,8 +737,13 @@ def record(name: str, passed: bool, failure_reason: str | None = None) -> None: "schema_version": ISSUE_FIX_MAINTAINER_CORRECTION_ADMISSION_SCHEMA_VERSION, "operation_id": operation_id, "admission_basis": admission_basis, - "disposition": entry.get("disposition"), - "reason": entry.get("reason") if not accepted else None, + "disposition": "accepted" if accepted else "context_only", + "reason": reason, + "host_summary_mismatch": bool( + existing_entry is not None + and correction.get("correction_fingerprint") + != existing_entry.get("correction_fingerprint") + ), "gates": gates, "revision_relation": revision_relation, "replay": replay, @@ -777,7 +768,10 @@ def recheck_issue_fix_maintainer_correction_effect_gates( source_event = entry.get("source_event") if isinstance(source_event, Mapping): latest = _latest_accepted_revision( - _prior_entries(ledger_entries), source_event=source_event + issue_fix_maintainer_correction_entries( + {"maintainer_corrections": ledger_entries} + ), + source_event=source_event, ) if latest is not None and latest.get("operation_id") != entry.get("operation_id"): latest_time = _timestamp(latest["source_event"].get("updated_at")) @@ -1600,7 +1594,10 @@ def build_issue_fix_pr_lifecycle_monitor_packet( or observation["pr_ref"] ), ) - if transition["decision"] in {"runnable_successor", "user_gate"}: + if ( + transition["decision"] in {"runnable_successor", "user_gate"} + and not maintainer_correction_admission["replay"] + ): # The entry carries its own Todo intent so a later run can # finish an interrupted write without the original input. maintainer_correction_admission["entries"][0]["todo_intent"] = { @@ -1627,7 +1624,8 @@ def build_issue_fix_pr_lifecycle_monitor_packet( if maintainer_correction is not None else None ), - "transition": transition, "grouped_monitor_projection": _grouped_monitor_projection( + "transition": transition, + "grouped_monitor_projection": _grouped_monitor_projection( observation, transition ), "first_screen": { diff --git a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py index cd5eda65f9..0b822f7596 100644 --- a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py +++ b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py @@ -567,3 +567,60 @@ def test_v1_input_rejects_body_fields_and_v0_rejects_source_event() -> None: _correction_v0(source_kind="maintainer_comment", source_ref=event["html_url"]) )["correction_fingerprint"] ) + + +def test_superseded_revision_replay_preserves_entry(harness: Harness) -> None: + old = _event("3101", updated_at="2026-06-20T10:00:00Z") + new = _event("3101", updated_at="2026-06-20T12:00:00Z") + first = harness.run_json(metadata=_metadata([old]), correction=_correction_v1(old)) + operation = first["maintainer_correction_admission"]["operation_id"] + todo = _correction_write(first)["todo_id"] + second = harness.run_json(metadata=_metadata([new]), correction=_correction_v1(new)) + before = harness.entries() + replay = harness.run_json(metadata=_metadata([old]), correction=_correction_v1(old)) + assert replay["maintainer_correction_admission"]["reason"] == "stale_revision" + assert harness.entries() == before + entry = next(e for e in before if e["operation_id"] == operation) + assert entry["disposition"] == "superseded" + assert entry["superseded_by"] == second["maintainer_correction_admission"]["operation_id"] + assert entry["todo_id"] == todo + assert len(harness.correction_todos()) == 2 + + +def test_equivalent_timestamp_replay(harness: Harness) -> None: + event = _event("3201") + first = harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + equivalent = {**event, "created_at": "2026-06-20T10:00:00+00:00", + "updated_at": "2026-06-20T10:00:00+00:00"} + replay = harness.run_json(metadata=_metadata([equivalent]), correction=_correction_v1(equivalent)) + assert first["maintainer_correction_admission"]["operation_id"] == replay["maintainer_correction_admission"]["operation_id"] + assert _correction_write(replay)["already_exists"] is True + assert len(harness.entries()) == len(harness.correction_todos()) == 1 + + +def test_replay_summary_uses_stored_intent_and_ignores_fault(harness: Harness) -> None: + event = _event("3301") + harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + before = harness.entries() + replay = harness.run_json( + metadata=_metadata([event]), + correction=_correction_v1(event, summary="Add a different regression assertion."), + env={"LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO": "1"}, + ) + assert replay["maintainer_correction_admission"]["host_summary_mismatch"] is True + assert _correction_write(replay)["already_exists"] is True + assert harness.entries() == before + assert len(harness.correction_todos()) == 1 + + +def test_missing_goal_does_not_write_ledger(harness: Harness) -> None: + registry = json.loads(harness.registry.read_text()) + registry["goals"] = [] + harness.registry.write_text(json.dumps(registry)) + event = _event("3401") + ledger = default_issue_fix_domain_state_ledger_path(project=harness.project, goal_id=GOAL_ID) + before = ledger.read_bytes() if ledger.exists() else None + result = harness.run(metadata=_metadata([event]), correction=_correction_v1(event), check=False) + assert result.returncode == 1 + assert json.loads(result.stdout)["ok"] is False + assert (ledger.read_bytes() if ledger.exists() else None) == before From 32d43fca73b4db2b7a4ae763d38d7ec428cc6172 Mon Sep 17 00:00:00 2001 From: hhyykk <4408344+hhyykk@users.noreply.github.com> Date: Sun, 1 Nov 2026 20:05:40 +0800 Subject: [PATCH 03/11] issue_fix: bind maintainer-correction patches to origin requests and return results A maintainer-correction patch can record which source-side request it answers, with the origin runtime root and registry recorded in the project-local ledger. Cancellation of the origin request is checked against the source registry before any effect and fails closed when the origin record is unreadable. Bound identity (goal_id, agent_id, goal_ref) is resolved from the origin request entry itself, never from project-side CLI arguments, and is re-resolved and compared on every settlement run. Terminal outcomes return to the origin conversation through the existing roundtrip report owner with explicit receipt/authority/misroute reason codes; disabled origins change nothing. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com> --- loopx/capabilities/issue_fix/README.md | 17 + loopx/capabilities/issue_fix/cli.py | 168 +++++++++- loopx/capabilities/issue_fix/pr_lifecycle.py | 3 + loopx/domain_packs/issue_fix.py | 7 +- ...test_issue_fix_correction_origin_return.py | 292 ++++++++++++++++++ 5 files changed, 473 insertions(+), 14 deletions(-) create mode 100644 tests/capabilities/test_issue_fix_correction_origin_return.py diff --git a/loopx/capabilities/issue_fix/README.md b/loopx/capabilities/issue_fix/README.md index b9f0e4a270..ea5be08e0b 100644 --- a/loopx/capabilities/issue_fix/README.md +++ b/loopx/capabilities/issue_fix/README.md @@ -425,6 +425,23 @@ no configured LoopX actor login exists yet), `author_not_maintainer` Immediately before the Todo write the gates run again against the ledger as it stands, and a stopped Goal refuses with `goal_not_active`. +Bind the row with `--origin-request-id`, `--origin-registry` and +`--origin-runtime-root` alongside `--goal-id` and `--claimed-by`: the origin +uses a separate lifecycle-only `source_session_v1` registry while issue-fix +uses the project registry for Goal/Todo operations. All origin flags are +required together; the binding survives reruns without them, and a different +request id, registry or runtime root fails with `origin_request_conflict`. +The request record supplies the origin Goal, receiver agent and optional Goal reference; +`--goal-id` and `--claimed-by` identify only the project side. Binding requires +one readable matching entry in the origin runtime; recipient addressing is unsupported. +Settlement resolves the entry again and refuses missing or changed identities. +Local locators remain ledger-only. Before Todo creation, origin inspection +fails closed with `origin_request_cancelled` or `origin_request_unreadable`; +existing Todos retain readback-only behavior. A completed Todo observed at a +new head queues a body-free conclusion through the origin runtime, deduplicated +by operation/head in `returns`, with `return_delivery` readback; cancellation +or unreadability prevents enqueue. The return service owns actual delivery. + With `--execute-transition`, an accepted `actionable_patch` creates exactly one `issue_fix_maintainer_correction_patch` todo claimed by the registered agent. `semantic_ambiguity` and `missing_authority` create a concrete user gate that diff --git a/loopx/capabilities/issue_fix/cli.py b/loopx/capabilities/issue_fix/cli.py index 17796b40ee..65af458024 100644 --- a/loopx/capabilities/issue_fix/cli.py +++ b/loopx/capabilities/issue_fix/cli.py @@ -1,11 +1,14 @@ from __future__ import annotations import argparse +import json import os from collections.abc import Callable from pathlib import Path from typing import Any +from ...paths import resolve_runtime_root +from ...control_plane.projects.registry_codec import load_project_registry from ...agent_registry import load_goal_from_registry from ...boundary_authority import checkpointed_boundary_authority_summary from ...control_plane.goals.activation import goal_is_stopped @@ -195,6 +198,28 @@ def _correction_todo_readback_receipt( return receipt +def _resolve_origin_request_identity(runtime_root: Path, request_id: str) -> dict[str, Any]: + paths = list((runtime_root / ".local/manager-context/entries").glob(f"*/{request_id}.json")) + if not paths: + raise ValueError("origin_request_not_found") + if len(paths) != 1: + raise ValueError("origin_request_ambiguous") + try: + entry = json.loads(paths[0].read_text(encoding="utf-8")) + except (OSError, ValueError) as exc: + raise ValueError("origin_request_not_found") from exc + if not isinstance(entry, dict): + raise ValueError("origin_request_not_found") + if isinstance(entry.get("recipient"), dict): + raise ValueError("origin_request_unsupported_target") + if not all(isinstance(entry.get(key), str) and entry[key] for key in ("goal_id", "agent_id")): + raise ValueError("origin_request_unsupported_target") + identity = {key: entry[key] for key in ("goal_id", "agent_id")} + if isinstance(entry.get("goal_ref"), dict): + identity["goal_ref"] = entry["goal_ref"] + return identity + + def _settle_maintainer_correction_entries( *, payload: dict[str, Any], @@ -223,10 +248,40 @@ def _settle_maintainer_correction_entries( row = read_issue_fix_pr_lifecycle_ledger_row(ledger_path, repo=repo, pr_ref=pr_ref) ledger_entries = issue_fix_maintainer_correction_entries(row) + origin = (row or {}).get("origin_request") + view = None + origin_refusal = None + if origin: + from ...control_plane.collaboration.goal_instance_scope import collaboration_goal_scope + from ...control_plane.collaboration.request_control_source import execute_request_control + + try: + identity = _resolve_origin_request_identity(Path(origin["runtime_root"]), origin["request_id"]) + if any(identity[key] != origin[key] for key in ("goal_id", "agent_id")): + raise ValueError("origin request identity changed") + with collaboration_goal_scope(Path(origin["registry_path"]), goal_id=identity["goal_id"], + agents=(), require_active=False) as scope: + goal_ref = identity.get("goal_ref") or scope.current_goal_ref or {"goal_id": identity["goal_id"]} + result = execute_request_control(registry_path=Path(origin["registry_path"]), + runtime_root=Path(origin["runtime_root"]), goal_ref=goal_ref, + receiver_agent_id=identity["agent_id"], request_id=origin["request_id"], + actor_agent_id=identity["agent_id"], operation="inspect", execute=False) + if not result.get("ok"): + raise ValueError("request inspection failed") + view = result["view"] + if not view.get("control_state"): + raise ValueError("request control state unavailable") + if view["control_state"] in {"cancelled", "cancellation_pending"}: + origin_refusal = "origin_request_cancelled" + except Exception: + origin_refusal = "origin_request_unreadable" settlements: list[dict[str, Any]] = [] current_todo_write: dict[str, Any] | None = None changed_entries: list[dict[str, Any]] = [] for entry in ledger_entries: + if origin and entry.get("disposition") == "accepted" and "accepted_head_sha" not in entry: + entry["accepted_head_sha"] = observation.get("head_commit_ref") + changed_entries.append(dict(entry)) operation_id = str(entry.get("operation_id") or "") is_current = operation_id == current_operation_id intent = entry.get("todo_intent") @@ -242,7 +297,10 @@ def _settle_maintainer_correction_entries( ledger_entries=ledger_entries, goal_stopped=goal_stopped, ) + refusal = origin_refusal or refusal if refusal is not None and not entry.get("todo_id"): + if is_current and origin_refusal: + admission.update(disposition="context_only", reason=refusal) changed_entries.append( {**entry, "disposition": "context_only", "reason": refusal, "effect_recheck": "refused"} @@ -310,6 +368,49 @@ def _settle_maintainer_correction_entries( ledger_path, repo=repo, pr_ref=pr_ref ) payload["maintainer_corrections"] = issue_fix_maintainer_correction_entries(row) + if origin: + from ...todos import list_goal_todos + from ...control_plane.collaboration.inbox import reply_status + from ..manager_context.roundtrip import report + + todos = {} if origin_refusal else {t["todo_id"]: t for t in list_goal_todos( + registry_path=registry_path, goal_id=goal_id)["todos"]} + head = str(observation.get("head_commit_ref") or "") + entries = payload["maintainer_corrections"] + for entry in entries: + if (origin_refusal or not view or entry.get("disposition") != "accepted" + or not entry.get("todo_id") or not head + or head == entry.get("accepted_head_sha") + or todos.get(entry["todo_id"], {}).get("status") != "done"): + continue + returns = entry.setdefault("returns", []) + if any(r["head_sha"] == head for r in returns): + continue + operation_id = entry["operation_id"] + update_id = f"issue-fix-correction:{operation_id}:{head[:12]}" + event = entry.get("source_event") or {} + checks = observation.get("checks") or {} + text = (f"Maintainer correction {entry.get('correction_kind')} " + f"({event.get('kind')} {event.get('id')}) applied in " + f"{observation.get('permalink')} at head {head[:12]}; " + f"PR state {observation.get('state')}, checks {checks.get('aggregate')}; " + f"todo {entry['todo_id']} done.") + origin_root = resolve_runtime_root( + load_project_registry(Path(origin["registry_path"])), + origin["runtime_root"], registry_path=Path(origin["registry_path"])) + result = report(origin_root, identity["goal_id"], identity["agent_id"], + origin["request_id"], "conclusion", text, registry=Path(origin["registry_path"]), + update_id=(update_id if any( + r["phase"] == "conclusion" for r in reply_status(origin_root, origin) + ) else None), workspace=project, request_revision=view["revision"]) + returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), + "enqueued_at": now_utc_iso(), "head_sha": head}) + upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) + statuses = {r.get("result_key", r["phase"]): r["status"] + for r in reply_status(Path(origin["runtime_root"]), origin)} + payload["return_delivery"] = [{"operation_id": e["operation_id"], + "update_id": r["update_id"], "status": statuses.get(r["result_key"], "explicit_unverified")} + for e in entries for r in e.get("returns", [])] return { "schema_version": "issue_fix_maintainer_correction_settlement_v0", "settlements": settlements, @@ -786,6 +887,12 @@ def register_issue_fix_commands( "Bodies, comments, raw logs, and provider responses stay out." ), ) + pr_lifecycle_parser.add_argument("--origin-registry", help="Origin lifecycle registry path.") + pr_lifecycle_parser.add_argument("--origin-runtime-root", help="Origin runtime root path.") + pr_lifecycle_parser.add_argument( + "--origin-request-id", + help="Bind this PR row to the originating steward request (requires Goal and agent).", + ) pr_lifecycle_parser.add_argument( "--maintainer-correction-json", default=None, @@ -1483,6 +1590,20 @@ def handle_issue_fix_command( ) ledger_path: Path | None = None prior_maintainer_corrections = None + prior_row = None + if any((args.origin_request_id, args.origin_registry, args.origin_runtime_root)) and not all( + (args.origin_request_id, args.origin_registry, args.origin_runtime_root) + ): + raise ValueError("--origin-request-id requires --origin-registry and --origin-runtime-root") + if args.origin_request_id: + from ...control_plane.content_digest import BARE_SHA256_PATTERN + + if not BARE_SHA256_PATTERN.fullmatch(args.origin_request_id): + raise ValueError("invalid origin request id") + origin_registry = Path(args.origin_registry).expanduser().resolve() + origin_runtime = Path(args.origin_runtime_root).expanduser().resolve() + if not args.goal_id or not args.claimed_by or not should_write_domain_state: + raise ValueError("--origin-request-id requires --goal-id and --claimed-by and ledger writes") if should_write_domain_state: ledger_path = ( Path(args.ledger_path).expanduser() @@ -1492,18 +1613,38 @@ def handle_issue_fix_command( goal_id=args.goal_id, ) ) - if args.maintainer_correction_json: - prior_reference = normalise_github_issue_reference( - repo=args.repo, issue_ref=args.pr_ref, url=args.url - ) - prior_row = read_issue_fix_pr_lifecycle_ledger_row( - ledger_path, - repo=str(prior_reference["repo"]), - pr_ref=str(prior_reference["issue_ref"]), - ) - prior_maintainer_corrections = ( - issue_fix_maintainer_correction_entries(prior_row) - ) + prior_reference = normalise_github_issue_reference( + repo=args.repo, issue_ref=args.pr_ref, url=args.url + ) + prior_row = read_issue_fix_pr_lifecycle_ledger_row( + ledger_path, + repo=str(prior_reference["repo"]), + pr_ref=str(prior_reference["issue_ref"]), + ) + prior_maintainer_corrections = ( + issue_fix_maintainer_correction_entries(prior_row) + ) + origin = (prior_row or {}).get("origin_request") + if args.origin_request_id: + if origin and (origin["request_id"] != args.origin_request_id + or origin["registry_path"] != str(origin_registry) + or origin["runtime_root"] != str(origin_runtime)): + raise ValueError("origin_request_conflict") + if not origin_registry.is_file() or not origin_runtime.is_dir(): + raise ValueError("origin registry and runtime root must exist") + try: + origin_profile = load_project_registry(origin_registry).get("profile_id") + except (ValueError, OSError) as exc: + raise ValueError("origin registry is unreadable") from exc + if origin_profile != "source_session_v1": + raise ValueError("origin registry requires source_session_v1") + candidate = {"request_id": args.origin_request_id, + "registry_path": str(origin_registry), "runtime_root": str(origin_runtime), + **_resolve_origin_request_identity(origin_runtime, args.origin_request_id), + "recorded_at": origin["recorded_at"] if origin else generated_at} + if origin and origin != candidate: + raise ValueError("origin_request_conflict") + origin = candidate payload = build_issue_fix_pr_lifecycle_monitor_packet( repo=args.repo, pr_ref=args.pr_ref, @@ -1522,6 +1663,8 @@ def handle_issue_fix_command( prior_maintainer_corrections=prior_maintainer_corrections, generated_at=generated_at, ) + if origin: + payload["origin_request"] = origin if should_write_domain_state and ledger_path is not None: upsert_issue_fix_pr_lifecycle_ledger_jsonl( ledger_path, @@ -2093,5 +2236,6 @@ def handle_issue_fix_command( if getattr(args, "issue_fix_command", None) in REVIEWER_COMMANDS else render_issue_fix_acceptance_loop_markdown ) + payload.pop("origin_request", None) print_payload(payload, output_format(args), renderer) return 0 if payload.get("ok") else 1 diff --git a/loopx/capabilities/issue_fix/pr_lifecycle.py b/loopx/capabilities/issue_fix/pr_lifecycle.py index dcd0b03aa8..8ff652b795 100644 --- a/loopx/capabilities/issue_fix/pr_lifecycle.py +++ b/loopx/capabilities/issue_fix/pr_lifecycle.py @@ -76,6 +76,8 @@ "author_not_maintainer", "terminal_pr_state", "goal_not_active", + "origin_request_cancelled", + "origin_request_unreadable", } # GitHub Apps post as `[bot]`; without a configured LoopX actor login this # suffix is the only self/bot signal available from body-free event metadata. @@ -713,6 +715,7 @@ def record(name: str, passed: bool, failure_reason: str | None = None) -> None: "disposition": "accepted" if accepted else "context_only", "reason": reason, "todo_id": None, + "accepted_head_sha": observation.get("head_commit_ref") if accepted else None, "recorded_at": recorded_at, "superseded_by": None, "body_captured": False, diff --git a/loopx/domain_packs/issue_fix.py b/loopx/domain_packs/issue_fix.py index 7ccae497ac..d8244e15d6 100644 --- a/loopx/domain_packs/issue_fix.py +++ b/loopx/domain_packs/issue_fix.py @@ -259,7 +259,7 @@ def merge_issue_fix_maintainer_correction_entries( operation_id = str(entry.get("operation_id") or "") if not operation_id: continue - merged[operation_id] = dict(entry) + merged[operation_id] = {**merged.get(operation_id, {}), **entry} return list(merged.values()) @@ -345,7 +345,8 @@ def unchanged_quiet_observation( existing_queue = existing.get("reviewer_notification_queue") incoming_queue = incoming.get("reviewer_notification_queue") return bool( - existing_fingerprint + existing.get("origin_request") == incoming.get("origin_request") + and existing_fingerprint and existing_fingerprint == incoming_fingerprint and existing_correction_fingerprint == incoming_correction_fingerprint and existing_receipts == incoming_receipts @@ -359,6 +360,8 @@ def preserve_compact_lifecycle_evidence( incoming: dict[str, Any], ) -> dict[str, Any]: merged = incoming + if existing.get("origin_request"): + merged = {**merged, "origin_request": existing["origin_request"]} receipts = existing.get("reviewer_notification_receipts") if ( "reviewer_notification_receipts" not in merged diff --git a/tests/capabilities/test_issue_fix_correction_origin_return.py b/tests/capabilities/test_issue_fix_correction_origin_return.py new file mode 100644 index 0000000000..f4968c83f3 --- /dev/null +++ b/tests/capabilities/test_issue_fix_correction_origin_return.py @@ -0,0 +1,292 @@ +"""Origin binding, controlled cancellation and durable return through real owners.""" +from __future__ import annotations + +import json +import os +import subprocess +import sys + +import pytest + +from test_issue_fix_maintainer_correction_admission import ( + Harness, GOAL_ID, AGENT_ID, PR_URL, ROOT, _event, _metadata, _correction_v1, +) +from loopx.domain_packs.issue_fix import default_issue_fix_domain_state_ledger_path +from loopx.control_plane.collaboration.inbox import acknowledge +from loopx.control_plane.collaboration.peers import request as peer_request +from loopx.control_plane.collaboration.request_control_source import execute_request_control + + +ORIGIN_GOAL_ID = "steward-goal" +ORIGIN_AGENT_ID = "steward-reviewer" + + +class BoundHarness(Harness): + run = None # assigned below after the shared CLI adapter is defined + + +class OriginHarness(Harness): + def __init__(self, root, provider): + super().__init__(root, provider) + self.origin_registry = root / "origin" / "registry.json" + self.runtime = root / "origin-runtime" + self._counter = 0 + args = [sys.executable, "-m", "loopx.cli", "--registry", str(self.origin_registry), + "--runtime-root", str(self.runtime), "--format", "json", "project", "register", + "--project-id", "correction-project", "--project-kind", "work", + "--knowledge-root", str(root / "origin"), "--goal-id", ORIGIN_GOAL_ID, + "--objective", "Apply bounded corrections", "--non-goal", "No remote changes", + "--acceptance", "Independent validation passes", "--next-effect", "Inspect request", + "--stop-condition", "Stop on cancellation", "--goal-instance-profile", "source_session_v1", + "--operation-id", "create-goal", "--registered-agent", ORIGIN_AGENT_ID, + "--registered-agent", "requester", "--canonical-storage", provider] + result = subprocess.run(args, cwd=ROOT, capture_output=True, text=True) + assert result.returncode == 0, result.stdout + result.stderr + self.ref = json.loads(result.stdout)["goal_ref"] + brief = {"schema_version": "collaboration_brief_v0", "purpose": "Validate correction", + "context": "Preserve compatibility", "constraints": ["No remote changes"], "inputs": [], + "acceptance": ["Tests pass"], "return_requirement": "Return validation result"} + self.request_id = peer_request(self.runtime, self.origin_registry, ORIGIN_GOAL_ID, "requester", + ORIGIN_AGENT_ID, "correction-request", brief, caller_goal_ref=self.ref)["request_id"] + acknowledge(self.runtime, ORIGIN_GOAL_ID, ORIGIN_AGENT_ID, self.request_id, "adopt", + "Validate bounded correction", registry=self.origin_registry, caller_goal_ref=self.ref) + + def control(self, operation, parameters=None): + return execute_request_control(registry_path=self.origin_registry, runtime_root=self.runtime, + goal_ref=self.ref, receiver_agent_id=ORIGIN_AGENT_ID, request_id=self.request_id, + actor_agent_id="requester", operation=operation, parameters=parameters, + execute=operation != "inspect")["view"] + + def cancel(self): + view = self.control("inspect") + return self.control("cancel", {"operation_id": "cancel-request", "expected_revision": view["revision"], + "expected_digest": view["revision_digest"], "reason": "Stop corrections"}) + + def run(self, *, metadata, correction, origin=None, env=None, check=True, origin_registry=None, origin_runtime=None, extra_args=()): + args = [sys.executable, "-m", "loopx.cli", "--registry", str(self.registry), + "--format", "json", "issue-fix", "pr-lifecycle", "--url", PR_URL, + "--metadata-json", str(self._write_json("metadata", metadata)), "--goal-id", GOAL_ID, + "--project", str(self.project), "--claimed-by", AGENT_ID, "--execute-transition"] + if origin: + args += ["--origin-request-id", origin, "--origin-registry", str(origin_registry or self.origin_registry), + "--origin-runtime-root", str(origin_runtime or self.runtime)] + if correction is not None: + args += ["--maintainer-correction-json", str(self._write_json("correction", correction))] + args += list(extra_args) + result = subprocess.run(args, cwd=ROOT, capture_output=True, text=True, + env={**os.environ, **(env or {})}) + if check: + assert result.returncode == 0, result.stdout + result.stderr + return result + + def replies(self): + return list((self.runtime / ".local/manager-context/replies" / self.request_id).glob("conclusion*.json")) + + +BoundHarness.run = OriginHarness.run +BoundHarness.replies = OriginHarness.replies + + +@pytest.fixture(params=["file", "sqlite"]) +def legacy(tmp_path, request): + h = OriginHarness(tmp_path, request.param) + return h + + +@pytest.fixture(params=["file", "sqlite"]) +def h(tmp_path, request): + return OriginHarness(tmp_path, request.param) + + +def observation(events, head="a" * 40): + return {**_metadata(events), "headRefOid": head} + + +def test_binding_conflict_and_unreadable(legacy): + h = legacy + event = _event("301") + h.run(metadata=observation([event]), correction=None, origin=h.request_id) + binding = h.ledger_row()["origin_request"] + assert binding["goal_id"] == ORIGIN_GOAL_ID + assert binding["agent_id"] == ORIGIN_AGENT_ID + assert binding["goal_ref"] == h.ref + h.run(metadata=observation([event]), correction=None) + assert h.ledger_row()["origin_request"] == binding + before = h.ledger_row() + result = h.run(metadata=observation([event]), correction=None, origin="f" * 64, check=False) + assert result.returncode == 1 and "origin_request_conflict" in result.stdout + assert h.ledger_row() == before + import shutil + other_registry = h.root / "other-registry.json" + shutil.copyfile(h.origin_registry, other_registry) + other_runtime = h.root / "other-runtime" + other_runtime.mkdir() + for overrides in ({"origin_registry": other_registry}, {"origin_runtime": other_runtime}): + result = h.run(metadata=observation([event]), correction=None, origin=h.request_id, + check=False, **overrides) + assert result.returncode == 1 and "origin_request_conflict" in result.stdout + assert h.ledger_row() == before + + +def test_cancelled_before_admission(h): + h.cancel() + event = _event("302") + result = h.run_json(metadata=observation([event]), correction=_correction_v1(event), origin=h.request_id) + assert result["maintainer_correction_admission"]["reason"] == "origin_request_cancelled" + assert h.entries()[0]["disposition"] == "context_only" + assert h.correction_todos() == [] + assert h.replies() == [] + + +def test_return_once_per_head(h): + event = _event("303") + h.run_json(metadata=observation([event]), correction=_correction_v1(event), origin=h.request_id) + entry = h.entries()[0] + assert entry["accepted_head_sha"] == "a" * 40 + h.mark_done(entry["todo_id"]) + for _ in range(3): + packet = h.run_json(metadata=observation([event], "b" * 40), correction=None) + assert len(h.replies()) == len(h.entries()[0]["returns"]) == 1 + assert packet["return_delivery"][0]["status"] == "queued" + assert str(h.origin_registry) not in json.dumps(packet) + assert str(h.runtime) not in json.dumps(packet) + reply = json.loads(h.replies()[0].read_text()) + assert PR_URL in reply["text"] and "bbbbbbbbbbbb" in reply["text"] + assert str(h.origin_registry) not in reply["text"] + assert str(h.runtime) not in reply["text"] + h.run_json(metadata=observation([event], "c" * 40), correction=None) + assert len(h.replies()) == len(h.entries()[0]["returns"]) == 2 + + +def test_cancel_after_existing_todo(h): + event = _event("304") + h.run_json(metadata=observation([event]), correction=_correction_v1(event), origin=h.request_id) + todo = h.entries()[0]["todo_id"] + h.mark_done(todo) + h.cancel() + h.run_json(metadata=observation([event], "b" * 40), correction=_correction_v1(event)) + assert [t["todo_id"] for t in h.correction_todos()] == [todo] + assert not h.replies() + + +def test_unbound_legacy_return_is_absent(legacy): + h = legacy + event = _event("305") + h.run_json(metadata=observation([event]), correction=_correction_v1(event)) + h.mark_done(h.entries()[0]["todo_id"]) + h.run_json(metadata=observation([event], "b" * 40), correction=None) + assert not h.replies() + + +def test_two_entry_crash_recovery(legacy): + h = legacy + first, second = _event("306"), _event("307") + h.run_json(metadata=observation([first, second]), correction=_correction_v1(first)) + failed = h.run(metadata=observation([first, second]), correction=_correction_v1(second), + env={"LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO": "1"}, check=False) + assert failed.returncode == 1 + assert sum(bool(e["todo_id"]) for e in h.entries()) == 1 + for _ in range(2): + h.run_json(metadata=observation([first, second]), correction=None) + assert len(h.correction_todos()) == 2 + assert len({e["todo_id"] for e in h.entries()}) == 2 + + +@pytest.mark.parametrize("damage", ["delete", "rename", "tamper"]) +def test_unreadable_request_fails_closed(h, damage): + event = _event("308") + h.run_json(metadata=observation([event]), correction=None, origin=h.request_id) + path = next((h.runtime / ".local/manager-context/entries").glob(f"*/{h.request_id}.json")) + if damage == "delete": + path.unlink() + elif damage == "rename": + path.rename(path.with_suffix(".missing")) + else: + ledger = default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID) + row = h.ledger_row() + row["origin_request"]["goal_id"] = "tampered-goal" + ledger.write_text(json.dumps(row) + "\n") + packet = h.run_json(metadata=observation([event]), correction=_correction_v1(event)) + assert packet["maintainer_correction_admission"]["reason"] == "origin_request_unreadable" + assert h.entries()[0]["disposition"] == "context_only" + assert not h.correction_todos() + assert not h.replies() + + +@pytest.mark.parametrize("existing", [False, True]) +@pytest.mark.parametrize("location", ["nowhere", "project"]) +def test_missing_origin_request_writes_nothing(h, existing, location): + if existing: + h.run_json(metadata=observation([]), correction=None) + request_id = "a" * 64 + if location == "project": + path = h.root / "runtime/.local/manager-context/entries/project" / f"{request_id}.json" + path.parent.mkdir(parents=True) + path.write_text(json.dumps({"goal_id": GOAL_ID, "agent_id": AGENT_ID})) + ledger = default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID) + before = ledger.read_bytes() if ledger.exists() else None + result = h.run(metadata=observation([]), correction=None, origin=request_id, check=False) + assert result.returncode == 1 + assert json.loads(result.stdout)["ok"] is False + assert json.loads(result.stdout)["error"] == "origin_request_not_found" + assert (ledger.read_bytes() if ledger.exists() else None) == before + + +@pytest.mark.parametrize("flags", [ + ["--origin-request-id", "a" * 64], + ["--origin-registry", "unused"], + ["--origin-runtime-root", "unused"], + ["--origin-request-id", "a" * 64, "--origin-registry", "unused"], + ["--origin-request-id", "a" * 64, "--origin-runtime-root", "unused"], +]) +def test_missing_origin_companions_write_nothing(tmp_path, flags): + harness = BoundHarness(tmp_path, "file") + result = harness.run(metadata=observation([]), correction=None, check=False, extra_args=flags) + assert result.returncode == 1 + assert json.loads(result.stdout)["ok"] is False + assert not default_issue_fix_domain_state_ledger_path(project=harness.project, goal_id=GOAL_ID).exists() + + +@pytest.mark.parametrize("invalid", ["id", "registry_missing", "runtime_missing", "profile", "decode"]) +def test_invalid_origin_write_nothing(h, invalid): + overrides = {} + origin = h.request_id + if invalid == "id": + origin = "invalid" + elif invalid == "registry_missing": + overrides["origin_registry"] = h.root / "missing" + elif invalid == "runtime_missing": + overrides["origin_runtime"] = h.root / "missing" + elif invalid == "profile": + overrides["origin_registry"] = h.registry + else: + broken = h.root / "broken.json" + broken.write_text("invalid json") + overrides["origin_registry"] = broken + result = h.run(metadata=observation([]), correction=None, origin=origin, check=False, **overrides) + assert result.returncode == 1 + assert json.loads(result.stdout)["ok"] is False + assert str(h.runtime) not in result.stdout + assert not default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID).exists() + + +@pytest.mark.parametrize("damage,error", [ + ("duplicate", "origin_request_ambiguous"), + ("json", "origin_request_not_found"), + ("recipient", "origin_request_unsupported_target"), +]) +def test_invalid_request_entry_writes_nothing(h, damage, error): + path = next((h.runtime / ".local/manager-context/entries").glob(f"*/{h.request_id}.json")) + if damage == "duplicate": + duplicate = path.parent.parent / "duplicate" / path.name + duplicate.parent.mkdir() + duplicate.write_bytes(path.read_bytes()) + elif damage == "json": + path.write_text("invalid json") + else: + path.write_text(json.dumps({"recipient": {"agent_id": "receiver"}})) + result = h.run(metadata=observation([]), correction=None, origin=h.request_id, check=False) + assert result.returncode == 1 + assert json.loads(result.stdout)["ok"] is False + assert json.loads(result.stdout)["error"] == error + assert not default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID).exists() From e1708d80d6402cb264165121b13830af906f5a0b Mon Sep 17 00:00:00 2001 From: hhyykk <4408344+hhyykk@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:33:37 +0800 Subject: [PATCH 04/11] issue_fix: key origin returns by full head and narrow origin refusal Use the full head SHA in the return update id so distinct heads sharing a short prefix each enqueue their own result. Compare the freshly resolved goal ref with the stored binding and pass it to the return report. Only OSError/ValueError/KeyError become origin_request_unreadable; programming errors propagate. Document that cancellation suppresses later returns. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com> --- loopx/capabilities/issue_fix/README.md | 3 + loopx/capabilities/issue_fix/cli.py | 9 ++- ...test_issue_fix_correction_origin_return.py | 70 +++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/loopx/capabilities/issue_fix/README.md b/loopx/capabilities/issue_fix/README.md index ea5be08e0b..d3acec4911 100644 --- a/loopx/capabilities/issue_fix/README.md +++ b/loopx/capabilities/issue_fix/README.md @@ -441,6 +441,9 @@ existing Todos retain readback-only behavior. A completed Todo observed at a new head queues a body-free conclusion through the origin runtime, deduplicated by operation/head in `returns`, with `return_delivery` readback; cancellation or unreadability prevents enqueue. The return service owns actual delivery. +Once the origin request is cancelled, no further result is returned even if an +accepted correction completes later; the request's cancellation record is the +accountability trail. With `--execute-transition`, an accepted `actionable_patch` creates exactly one `issue_fix_maintainer_correction_patch` todo claimed by the registered agent. diff --git a/loopx/capabilities/issue_fix/cli.py b/loopx/capabilities/issue_fix/cli.py index 65af458024..b5f543435d 100644 --- a/loopx/capabilities/issue_fix/cli.py +++ b/loopx/capabilities/issue_fix/cli.py @@ -262,6 +262,8 @@ def _settle_maintainer_correction_entries( with collaboration_goal_scope(Path(origin["registry_path"]), goal_id=identity["goal_id"], agents=(), require_active=False) as scope: goal_ref = identity.get("goal_ref") or scope.current_goal_ref or {"goal_id": identity["goal_id"]} + if origin.get("goal_ref") and goal_ref != origin["goal_ref"]: + raise ValueError("origin request identity changed") result = execute_request_control(registry_path=Path(origin["registry_path"]), runtime_root=Path(origin["runtime_root"]), goal_ref=goal_ref, receiver_agent_id=identity["agent_id"], request_id=origin["request_id"], @@ -273,7 +275,7 @@ def _settle_maintainer_correction_entries( raise ValueError("request control state unavailable") if view["control_state"] in {"cancelled", "cancellation_pending"}: origin_refusal = "origin_request_cancelled" - except Exception: + except (OSError, ValueError, KeyError): origin_refusal = "origin_request_unreadable" settlements: list[dict[str, Any]] = [] current_todo_write: dict[str, Any] | None = None @@ -387,7 +389,7 @@ def _settle_maintainer_correction_entries( if any(r["head_sha"] == head for r in returns): continue operation_id = entry["operation_id"] - update_id = f"issue-fix-correction:{operation_id}:{head[:12]}" + update_id = f"issue-fix-correction:{operation_id}:{head}" event = entry.get("source_event") or {} checks = observation.get("checks") or {} text = (f"Maintainer correction {entry.get('correction_kind')} " @@ -402,7 +404,8 @@ def _settle_maintainer_correction_entries( origin["request_id"], "conclusion", text, registry=Path(origin["registry_path"]), update_id=(update_id if any( r["phase"] == "conclusion" for r in reply_status(origin_root, origin) - ) else None), workspace=project, request_revision=view["revision"]) + ) else None), workspace=project, request_revision=view["revision"], + caller_goal_ref=goal_ref) returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), "enqueued_at": now_utc_iso(), "head_sha": head}) upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) diff --git a/tests/capabilities/test_issue_fix_correction_origin_return.py b/tests/capabilities/test_issue_fix_correction_origin_return.py index f4968c83f3..a9a76d82df 100644 --- a/tests/capabilities/test_issue_fix_correction_origin_return.py +++ b/tests/capabilities/test_issue_fix_correction_origin_return.py @@ -15,6 +15,7 @@ from loopx.control_plane.collaboration.inbox import acknowledge from loopx.control_plane.collaboration.peers import request as peer_request from loopx.control_plane.collaboration.request_control_source import execute_request_control +from loopx.capabilities.issue_fix.cli import _settle_maintainer_correction_entries ORIGIN_GOAL_ID = "steward-goal" @@ -102,6 +103,75 @@ def observation(events, head="a" * 40): return {**_metadata(events), "headRefOid": head} +def settle(h, packet): + return _settle_maintainer_correction_entries(payload=packet, + ledger_path=default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID), + registry_path=h.registry, goal_id=GOAL_ID, claimed_by=AGENT_ID, project=h.project) + + +def test_return_head_prefix_collision(h): + event = _event("309") + h.run_json(metadata=observation([event]), correction=_correction_v1(event), origin=h.request_id) + h.mark_done(h.entries()[0]["todo_id"]) + for suffix in "123": + h.run_json(metadata=observation([event], "b" * 12 + suffix * 28), correction=None) + returns = h.entries()[0]["returns"] + assert len(h.replies()) == len(returns) == 3 + assert len({r["update_id"] for r in returns}) == 3 + replies = [json.loads(path.read_text()) for path in h.replies()] + assert len({r.get("update_id") for r in replies}) == 3 + + +def test_stored_goal_ref_mismatch(h): + event = _event("310") + h.run_json(metadata=observation([event]), correction=None, origin=h.request_id) + row = h.ledger_row() + row["origin_request"]["goal_ref"]["goal_instance_id"] = "other-instance" + ledger = default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID) + ledger.write_text(json.dumps(row) + "\n") + packet = h.run_json(metadata=observation([event]), correction=_correction_v1(event)) + assert packet["maintainer_correction_admission"]["reason"] == "origin_request_unreadable" + assert not h.correction_todos() + assert not h.replies() + + +@pytest.mark.parametrize("owner,name", [ + ("loopx.capabilities.issue_fix.cli", "_resolve_origin_request_identity"), + ("loopx.control_plane.collaboration.request_control_source", "execute_request_control"), +]) +def test_programming_error_propagates(h, monkeypatch, owner, name): + event = _event("311") + packet = h.run_json(metadata=observation([event]), correction=None, origin=h.request_id) + + def broken(*args, **kwargs): + raise TypeError("programming error") + + monkeypatch.setattr(f"{owner}.{name}", broken) + with pytest.raises(TypeError, match="programming error"): + settle(h, packet) + + +def test_report_receives_goal_ref(h, monkeypatch): + from loopx.capabilities.manager_context import roundtrip + + event = _event("312") + packet = h.run_json(metadata=observation([event]), correction=_correction_v1(event), origin=h.request_id) + h.mark_done(h.entries()[0]["todo_id"]) + packet["observation"]["head_commit_ref"] = "b" * 40 + received = [] + original = roundtrip.report + + def report(*args, **kwargs): + if kwargs.get("registry") is not None: + received.append(kwargs["caller_goal_ref"]) + return original(*args, **kwargs) + + monkeypatch.setattr(roundtrip, "report", report) + settle(h, packet) + assert received == [h.ref] + assert len(h.replies()) == 1 + + def test_binding_conflict_and_unreadable(legacy): h = legacy event = _event("301") From d413b52b0ac5f7c8e43ece5581c4012bc6785b11 Mon Sep 17 00:00:00 2001 From: hhyykk <4408344+hhyykk@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:10:57 +0800 Subject: [PATCH 05/11] docs(issue_fix): sync Chinese README with correction admission and origin return Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com> --- loopx/capabilities/issue_fix/README.zh-CN.md | 55 +++++++++++++++++--- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/loopx/capabilities/issue_fix/README.zh-CN.md b/loopx/capabilities/issue_fix/README.zh-CN.md index 1d5d59449b..7e7a4102d2 100644 --- a/loopx/capabilities/issue_fix/README.zh-CN.md +++ b/loopx/capabilities/issue_fix/README.zh-CN.md @@ -389,15 +389,55 @@ LoopX 才会等待复审而不再创建重复 patch successor；缺失、部分 monitor-only。 当 review 或公开 maintainer comment 包含具体 correction 时，host 将其压缩成 -`issue_fix_maintainer_correction_input_v0` 再交给 `pr-lifecycle`。输入只保留 correction +`issue_fix_maintainer_correction_input_v1` 再交给 `pr-lifecycle`。输入只保留 correction 类型、公开 source reference、有限摘要，以及三类必要信息之一：验证计划与 PR 更新路径、 -具体歧义问题、或缺失 authority scope；不会复制 review/comment 原文。 - -显式使用 `--execute-transition` 时，`actionable_patch` 只创建一个由当前注册 agent 认领的 +具体歧义问题、或缺失 authority scope，另加一个标识承载该 correction 的 provider event +的 `source_event`：`{kind: issue_comment|review|review_comment, id, html_url, +author_login, author_association, created_at, updated_at}`。不会复制 review/comment +原文。不含 `source_event` 的旧 `issue_fix_maintainer_correction_input_v0` 输入仍被接受， +并记录为 `admission_basis: host_asserted`；v1 输入为 `provider_verified`，因为 LoopX +会将 event id 和 `updated_at` 与 PR 的无正文 comment/review metadata 匹配（来自同一 +`--metadata-json` fixture 的 `comments`、`reviews`、`reviewComments` key，或在 +`--fetch-metadata` 下通过 `gh api` 获取；review 使用 `submitted_at` 作为其 revision）。 + +每个 correction 在任何 Todo 写入前都必须通过确定性的 admission gate；每次拒绝都会 +作为带有 `reason` 的 `context_only` 条目记录在 ledger row 中，不会被丢弃： +`source_event_not_observed`、`source_event_revision_mismatch`、`stale_revision` +（已接纳 event 的更旧 `updated_at`）、`self_or_bot_author`（login 以 `[bot]` 结尾； +目前尚无已配置的 LoopX actor login）、`author_not_maintainer`（association 不属于 +`COLLABORATOR` / `MEMBER` / `OWNER`）以及 `terminal_pr_state`。 +紧接 Todo 写入之前，会基于 ledger 的当时状态再次执行这些 gate；已停止的 Goal 会以 +`goal_not_active` 拒绝。 + +在 `--goal-id` 和 `--claimed-by` 之外，使用 `--origin-request-id`、`--origin-registry` +与 `--origin-runtime-root` 绑定该 row：origin 使用独立的、仅用于 lifecycle 的 +`source_session_v1` registry，而 issue-fix 使用 project registry 执行 Goal/Todo 操作。 +所有 origin flag 必须一起提供；后续重跑即使不再提供这些 flag，绑定仍会保留；request id、 +registry 或 runtime root 不同会以 `origin_request_conflict` 失败。 +request record 提供 origin Goal、receiver agent 和可选 Goal reference；`--goal-id` +与 `--claimed-by` 只标识 project 一侧。绑定要求 origin runtime 中存在一个可读且匹配的 +entry；不支持 recipient addressing。Settlement 会再次解析该 entry，并拒绝缺失或已变化的 +身份。本地 locator 只保留在 ledger 中。在创建 Todo 之前，origin 检查会以 +`origin_request_cancelled` 或 `origin_request_unreadable` fail closed；已有 Todo +保持只读回读行为。在新 head 观察到已完成 Todo 时，会通过 origin runtime 将无正文的结论 +入队，在 `returns` 中按 operation/head 去重，并提供 `return_delivery` 回读；取消或不可读 +会阻止入队。Return service 负责实际投递。Origin request 一旦取消，即使已接纳的 correction +稍后完成，也不再返回任何结果；request 的 cancellation record 是其 accountability trail。 + +显式使用 `--execute-transition` 时，已接纳的 `actionable_patch` 只创建一个由当前注册 agent 认领的 `issue_fix_maintainer_correction_patch` todo；`semantic_ambiguity` 与 `missing_authority` 创建会阻塞同一 agent 的具体 user gate；`unchanged` 不创建 todo。 -标准化 correction fingerprint 与确定性 todo 文本保证重试幂等，merged/closed terminal -state 仍优先于迟到反馈。 +Todo 使用由 repository、PR、event id 和 `updated_at`（v0 则使用 correction fingerprint） +派生的稳定 operation id 写入，因此完全相同的重放即使在 Todo 已完成后也返回 +`already_exists`；两个内容相同但不同的 comment 会生成两个 Todo；同一 comment 的更新 +revision 会生成新 Todo，并把先前条目标记为 `superseded`。Ledger row 在 +`maintainer_corrections` 中保留这些条目（`operation_id`、`source_event`、 +`correction_fingerprint`、`admission_basis`、`disposition`、`reason`、`todo_id`、 +`recorded_at`、`superseded_by`、`todo_intent`）；已接纳条目在 Todo 写入前先以 +`todo_id: null` 写入，随后补齐；任何后续运行都会按其 operation id 重试仍处于 pending +状态的已接纳条目。仅供测试的 `LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO=1` +会在这两次写入之间中止，仅用于演练该恢复路径。旧 `maintainer_correction` 字段为旧版 +reader 保留最新接纳的 correction。Merged/closed terminal state 仍优先于迟到反馈。 当 connected lifecycle writeback 观察到 `MERGED`，LoopX 还会追加一条带 repository scope、public-safe 且幂等的 `pr_merge` rollout event。Todo resume 投影消费这条事件：例如 @@ -973,7 +1013,8 @@ loopx issue-fix pr-lifecycle \ Correction source 是 provider-neutral 的：可以使用任意公开 HTTPS 或 repo-relative reference，而当前 PR monitor 仍是 lifecycle authority。完全相同的重试既不会新增 -successor，也不会重写同一 lifecycle row。 +successor，也不会重写同一 lifecycle row；输出中的 `maintainer_correction_admission` +给出本次输入的 operation id、admission basis、disposition 和 reason。 ## 状态与产出视图 From f5f7fb72337f772c991288dfad993bfbf356faca Mon Sep 17 00:00:00 2001 From: hhyykk <4408344+hhyykk@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:21:33 +0800 Subject: [PATCH 06/11] refactor(issue_fix): type correction settlement and move it out of the CLI adapter Move maintainer-correction settlement and origin-request identity resolution into issue_fix/correction_settlement.py unchanged, and replace the new Any annotations with TypedDicts and object-typed mappings. Keeps cli.py within its reviewed Any ceiling and the default module line limit. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com> --- loopx/capabilities/issue_fix/cli.py | 322 +--------------- .../issue_fix/correction_settlement.py | 347 ++++++++++++++++++ ...test_issue_fix_correction_origin_return.py | 4 +- 3 files changed, 355 insertions(+), 318 deletions(-) create mode 100644 loopx/capabilities/issue_fix/correction_settlement.py diff --git a/loopx/capabilities/issue_fix/cli.py b/loopx/capabilities/issue_fix/cli.py index b5f543435d..3c969c3c79 100644 --- a/loopx/capabilities/issue_fix/cli.py +++ b/loopx/capabilities/issue_fix/cli.py @@ -1,17 +1,13 @@ from __future__ import annotations import argparse -import json -import os from collections.abc import Callable from pathlib import Path from typing import Any -from ...paths import resolve_runtime_root from ...control_plane.projects.registry_codec import load_project_registry from ...agent_registry import load_goal_from_registry from ...boundary_authority import checkpointed_boundary_authority_summary -from ...control_plane.goals.activation import goal_is_stopped from ...control_plane.runtime.time import now_utc_iso from ...domain_packs.issue_fix import ( default_issue_fix_candidate_preflight_ledger_path, @@ -33,6 +29,11 @@ render_issue_fix_acceptance_loop_markdown, ) from .candidate_evidence import collect_public_github_candidate_preflight_input +from .correction_settlement import ( + _correction_todo_readback_receipt, + _resolve_origin_request_identity, + _settle_maintainer_correction_entries, +) from .feasibility import ( build_issue_fix_feasibility_packet, render_issue_fix_feasibility_markdown, @@ -67,7 +68,6 @@ from .metadata_preview import normalise_github_issue_reference from .pr_lifecycle import ( build_issue_fix_pr_lifecycle_monitor_packet, - recheck_issue_fix_maintainer_correction_effect_gates, render_issue_fix_pr_lifecycle_monitor_markdown, validate_issue_fix_pr_lifecycle_monitor_packet, ) @@ -113,316 +113,6 @@ FormatSelector = Callable[..., str] AddFormat = Callable[[argparse.ArgumentParser], None] -# Set to "1" to abort between the accepted ledger entry write and its Todo -# write. Exists only to prove the next run reconciles the pending entry. -MAINTAINER_CORRECTION_FAULT_ENV = "LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO" - - -def _goal_stopped_for_correction(registry_path: Path | None, goal_id: str | None) -> bool: - if registry_path is None or not goal_id: - return False - try: - goal = load_goal_from_registry(registry_path, goal_id) - if goal is None: - return True - return goal_is_stopped(goal) - except ValueError: - # An unreadable activation record cannot prove the Goal is active. - return True - - -def _write_maintainer_correction_todo( - *, - registry_path: Path, - goal_id: str, - claimed_by: str, - project: Path, - operation_id: str, - intent: dict[str, Any], -) -> dict[str, Any]: - role = str(intent.get("role") or "agent") - return add_goal_todo( - registry_path=registry_path, - goal_id=goal_id, - role=role, - text=str(intent.get("text") or ""), - task_class=str(intent.get("task_class") or "advancement_task"), - action_kind=str(intent.get("action_kind") or ""), - required_write_scopes=( - list(intent.get("required_write_scopes") or []) if role == "agent" else None - ), - claimed_by=claimed_by if role == "agent" else None, - agent_id=claimed_by if role == "user" else None, - project=project, - operation_id=operation_id, - ) - - -def _maintainer_correction_todo_write_receipt( - todo_write: dict[str, Any], *, operation_id: str -) -> dict[str, Any]: - todo_changed = bool( - todo_write.get("added") - or todo_write.get("metadata_updated") - or todo_write.get("status_changed") - ) - created_todo = todo_write.get("todo") - created_todo = created_todo if isinstance(created_todo, dict) else {} - return { - "schema_version": "issue_fix_maintainer_correction_todo_write_v0", - "write_performed": todo_changed, - "already_exists": bool(todo_write.get("already_exists")), - "todo_id": todo_write.get("todo_id"), - "operation_id": operation_id, - "role": todo_write.get("role"), - "claimed_by": created_todo.get("claimed_by"), - "blocks_agent": created_todo.get("blocks_agent"), - "path_recorded": False, - } - - -def _correction_todo_readback_receipt( - *, skip_reason: str, operation_id: str | None = None, todo_id: str | None = None, -) -> dict[str, Any]: - receipt = { - "schema_version": "issue_fix_maintainer_correction_todo_write_v0", - "write_performed": False, - "already_exists": bool(todo_id), - "skip_reason": skip_reason, - "path_recorded": False, - } - if operation_id is not None: - receipt["operation_id"] = operation_id - if todo_id is not None: - receipt["todo_id"] = todo_id - return receipt - - -def _resolve_origin_request_identity(runtime_root: Path, request_id: str) -> dict[str, Any]: - paths = list((runtime_root / ".local/manager-context/entries").glob(f"*/{request_id}.json")) - if not paths: - raise ValueError("origin_request_not_found") - if len(paths) != 1: - raise ValueError("origin_request_ambiguous") - try: - entry = json.loads(paths[0].read_text(encoding="utf-8")) - except (OSError, ValueError) as exc: - raise ValueError("origin_request_not_found") from exc - if not isinstance(entry, dict): - raise ValueError("origin_request_not_found") - if isinstance(entry.get("recipient"), dict): - raise ValueError("origin_request_unsupported_target") - if not all(isinstance(entry.get(key), str) and entry[key] for key in ("goal_id", "agent_id")): - raise ValueError("origin_request_unsupported_target") - identity = {key: entry[key] for key in ("goal_id", "agent_id")} - if isinstance(entry.get("goal_ref"), dict): - identity["goal_ref"] = entry["goal_ref"] - return identity - - -def _settle_maintainer_correction_entries( - *, - payload: dict[str, Any], - ledger_path: Path, - registry_path: Path, - goal_id: str, - claimed_by: str, - project: Path, -) -> dict[str, Any]: - """Write Todos for accepted ledger entries that still lack a todo_id. - - Each accepted entry stores its own `todo_intent`, so an entry left pending - by an interrupted run is retried under its original operation id on any - later run. Every candidate is re-gated against the ledger as it stands - now, the same observation, and the Goal activation state. - """ - - observation = payload.get("observation") - observation = observation if isinstance(observation, dict) else {} - admission = payload.get("maintainer_correction_admission") - admission = admission if isinstance(admission, dict) else {} - current_operation_id = str(admission.get("operation_id") or "") - goal_stopped = _goal_stopped_for_correction(registry_path, goal_id) - repo = str(observation.get("repo") or "") - pr_ref = str(observation.get("pr_ref") or "") - - row = read_issue_fix_pr_lifecycle_ledger_row(ledger_path, repo=repo, pr_ref=pr_ref) - ledger_entries = issue_fix_maintainer_correction_entries(row) - origin = (row or {}).get("origin_request") - view = None - origin_refusal = None - if origin: - from ...control_plane.collaboration.goal_instance_scope import collaboration_goal_scope - from ...control_plane.collaboration.request_control_source import execute_request_control - - try: - identity = _resolve_origin_request_identity(Path(origin["runtime_root"]), origin["request_id"]) - if any(identity[key] != origin[key] for key in ("goal_id", "agent_id")): - raise ValueError("origin request identity changed") - with collaboration_goal_scope(Path(origin["registry_path"]), goal_id=identity["goal_id"], - agents=(), require_active=False) as scope: - goal_ref = identity.get("goal_ref") or scope.current_goal_ref or {"goal_id": identity["goal_id"]} - if origin.get("goal_ref") and goal_ref != origin["goal_ref"]: - raise ValueError("origin request identity changed") - result = execute_request_control(registry_path=Path(origin["registry_path"]), - runtime_root=Path(origin["runtime_root"]), goal_ref=goal_ref, - receiver_agent_id=identity["agent_id"], request_id=origin["request_id"], - actor_agent_id=identity["agent_id"], operation="inspect", execute=False) - if not result.get("ok"): - raise ValueError("request inspection failed") - view = result["view"] - if not view.get("control_state"): - raise ValueError("request control state unavailable") - if view["control_state"] in {"cancelled", "cancellation_pending"}: - origin_refusal = "origin_request_cancelled" - except (OSError, ValueError, KeyError): - origin_refusal = "origin_request_unreadable" - settlements: list[dict[str, Any]] = [] - current_todo_write: dict[str, Any] | None = None - changed_entries: list[dict[str, Any]] = [] - for entry in ledger_entries: - if origin and entry.get("disposition") == "accepted" and "accepted_head_sha" not in entry: - entry["accepted_head_sha"] = observation.get("head_commit_ref") - changed_entries.append(dict(entry)) - operation_id = str(entry.get("operation_id") or "") - is_current = operation_id == current_operation_id - intent = entry.get("todo_intent") - if ( - entry.get("disposition") != "accepted" - or not isinstance(intent, dict) - or (entry.get("todo_id") and not is_current) - ): - continue - refusal = recheck_issue_fix_maintainer_correction_effect_gates( - entry, - observation=observation, - ledger_entries=ledger_entries, - goal_stopped=goal_stopped, - ) - refusal = origin_refusal or refusal - if refusal is not None and not entry.get("todo_id"): - if is_current and origin_refusal: - admission.update(disposition="context_only", reason=refusal) - changed_entries.append( - {**entry, "disposition": "context_only", "reason": refusal, - "effect_recheck": "refused"} - ) - settlements.append( - {"operation_id": operation_id, "status": "refused", "reason": refusal} - ) - if is_current: - current_todo_write = _correction_todo_readback_receipt( - skip_reason=refusal, operation_id=operation_id, - ) - continue - if refusal is not None: - # The Todo already exists for this revision; a replay only reads it - # back and leaves its lifecycle to the Todo owner. - settlements.append( - {"operation_id": operation_id, "status": "settled_earlier", "reason": refusal} - ) - if is_current: - current_todo_write = _correction_todo_readback_receipt( - skip_reason=refusal, operation_id=operation_id, - todo_id=entry.get("todo_id"), - ) - continue - if not entry.get("todo_id") and os.environ.get(MAINTAINER_CORRECTION_FAULT_ENV) == "1": - raise RuntimeError( - "maintainer correction Todo write aborted by " - f"{MAINTAINER_CORRECTION_FAULT_ENV}" - ) - todo_write = _write_maintainer_correction_todo( - registry_path=registry_path, - goal_id=goal_id, - claimed_by=claimed_by, - project=project, - operation_id=operation_id, - intent=intent, - ) - receipt = _maintainer_correction_todo_write_receipt( - todo_write, operation_id=operation_id - ) - if entry.get("todo_id") != receipt["todo_id"]: - changed_entries.append({**entry, "todo_id": receipt["todo_id"]}) - settlements.append( - { - "operation_id": operation_id, - "status": "written" if receipt["write_performed"] else "already_exists", - "todo_id": receipt["todo_id"], - } - ) - if is_current: - current_todo_write = receipt - if current_todo_write is None and current_operation_id: - current_todo_write = _correction_todo_readback_receipt( - skip_reason=( - str(admission.get("reason") or admission.get("disposition") or "") - if admission.get("disposition") != "accepted" - else "unchanged_monitor_quiet" - ), - operation_id=current_operation_id, - ) - if changed_entries: - payload["maintainer_corrections"] = changed_entries - upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) - row = read_issue_fix_pr_lifecycle_ledger_row( - ledger_path, repo=repo, pr_ref=pr_ref - ) - payload["maintainer_corrections"] = issue_fix_maintainer_correction_entries(row) - if origin: - from ...todos import list_goal_todos - from ...control_plane.collaboration.inbox import reply_status - from ..manager_context.roundtrip import report - - todos = {} if origin_refusal else {t["todo_id"]: t for t in list_goal_todos( - registry_path=registry_path, goal_id=goal_id)["todos"]} - head = str(observation.get("head_commit_ref") or "") - entries = payload["maintainer_corrections"] - for entry in entries: - if (origin_refusal or not view or entry.get("disposition") != "accepted" - or not entry.get("todo_id") or not head - or head == entry.get("accepted_head_sha") - or todos.get(entry["todo_id"], {}).get("status") != "done"): - continue - returns = entry.setdefault("returns", []) - if any(r["head_sha"] == head for r in returns): - continue - operation_id = entry["operation_id"] - update_id = f"issue-fix-correction:{operation_id}:{head}" - event = entry.get("source_event") or {} - checks = observation.get("checks") or {} - text = (f"Maintainer correction {entry.get('correction_kind')} " - f"({event.get('kind')} {event.get('id')}) applied in " - f"{observation.get('permalink')} at head {head[:12]}; " - f"PR state {observation.get('state')}, checks {checks.get('aggregate')}; " - f"todo {entry['todo_id']} done.") - origin_root = resolve_runtime_root( - load_project_registry(Path(origin["registry_path"])), - origin["runtime_root"], registry_path=Path(origin["registry_path"])) - result = report(origin_root, identity["goal_id"], identity["agent_id"], - origin["request_id"], "conclusion", text, registry=Path(origin["registry_path"]), - update_id=(update_id if any( - r["phase"] == "conclusion" for r in reply_status(origin_root, origin) - ) else None), workspace=project, request_revision=view["revision"], - caller_goal_ref=goal_ref) - returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), - "enqueued_at": now_utc_iso(), "head_sha": head}) - upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) - statuses = {r.get("result_key", r["phase"]): r["status"] - for r in reply_status(Path(origin["runtime_root"]), origin)} - payload["return_delivery"] = [{"operation_id": e["operation_id"], - "update_id": r["update_id"], "status": statuses.get(r["result_key"], "explicit_unverified")} - for e in entries for r in e.get("returns", [])] - return { - "schema_version": "issue_fix_maintainer_correction_settlement_v0", - "settlements": settlements, - "goal_stopped": goal_stopped, - "todo_write": current_todo_write, - "path_recorded": False, - } - - def _comparable_delivery_evidence( value: dict[str, Any] | None, ) -> dict[str, Any] | None: if value is None: @@ -1713,7 +1403,7 @@ def handle_issue_fix_command( ) decision = str(transition.get("decision") or "") correction_admission = payload.get("maintainer_correction_admission") - settlement: dict[str, Any] | None = None + settlement: dict[str, object] | None = None if ledger_path is not None: settlement = _settle_maintainer_correction_entries( payload=payload, diff --git a/loopx/capabilities/issue_fix/correction_settlement.py b/loopx/capabilities/issue_fix/correction_settlement.py new file mode 100644 index 0000000000..4c367413a0 --- /dev/null +++ b/loopx/capabilities/issue_fix/correction_settlement.py @@ -0,0 +1,347 @@ +"""Maintainer-correction settlement and origin-request binding helpers.""" +from __future__ import annotations + +import json +import os +from collections.abc import Mapping +from pathlib import Path +from typing import NotRequired, TypedDict + +from ...agent_registry import load_goal_from_registry +from ...control_plane.goals.activation import goal_is_stopped +from ...control_plane.projects.registry_codec import load_project_registry +from ...control_plane.runtime.time import now_utc_iso +from ...control_plane.todos.mutation_api import add_goal_todo +from ...domain_packs.issue_fix import ( + issue_fix_maintainer_correction_entries, + read_issue_fix_pr_lifecycle_ledger_row, + upsert_issue_fix_pr_lifecycle_ledger_jsonl, +) +from ...paths import resolve_runtime_root +from .pr_lifecycle import recheck_issue_fix_maintainer_correction_effect_gates + + +# Set to "1" to abort between the accepted ledger entry write and its Todo +# write. Exists only to prove the next run reconciles the pending entry. +MAINTAINER_CORRECTION_FAULT_ENV = "LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO" + + +def _goal_stopped_for_correction(registry_path: Path | None, goal_id: str | None) -> bool: + if registry_path is None or not goal_id: + return False + try: + goal = load_goal_from_registry(registry_path, goal_id) + if goal is None: + return True + return goal_is_stopped(goal) + except ValueError: + # An unreadable activation record cannot prove the Goal is active. + return True + + +class _CorrectionTodoIntent(TypedDict, total=False): + role: str + text: str + task_class: str + action_kind: str + required_write_scopes: list[str] + + +class _OriginRequestIdentity(TypedDict): + goal_id: str + agent_id: str + goal_ref: NotRequired[dict[str, object]] + + +def _write_maintainer_correction_todo( + *, + registry_path: Path, + goal_id: str, + claimed_by: str, + project: Path, + operation_id: str, + intent: _CorrectionTodoIntent, +) -> dict[str, object]: + role = str(intent.get("role") or "agent") + return add_goal_todo( + registry_path=registry_path, + goal_id=goal_id, + role=role, + text=str(intent.get("text") or ""), + task_class=str(intent.get("task_class") or "advancement_task"), + action_kind=str(intent.get("action_kind") or ""), + required_write_scopes=( + list(intent.get("required_write_scopes") or []) if role == "agent" else None + ), + claimed_by=claimed_by if role == "agent" else None, + agent_id=claimed_by if role == "user" else None, + project=project, + operation_id=operation_id, + ) + + +def _maintainer_correction_todo_write_receipt( + todo_write: Mapping[str, object], *, operation_id: str +) -> dict[str, object]: + todo_changed = bool( + todo_write.get("added") + or todo_write.get("metadata_updated") + or todo_write.get("status_changed") + ) + created_todo = todo_write.get("todo") + created_todo = created_todo if isinstance(created_todo, dict) else {} + return { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": todo_changed, + "already_exists": bool(todo_write.get("already_exists")), + "todo_id": todo_write.get("todo_id"), + "operation_id": operation_id, + "role": todo_write.get("role"), + "claimed_by": created_todo.get("claimed_by"), + "blocks_agent": created_todo.get("blocks_agent"), + "path_recorded": False, + } + + +def _correction_todo_readback_receipt( + *, skip_reason: str, operation_id: str | None = None, todo_id: str | None = None, +) -> dict[str, object]: + receipt: dict[str, object] = { + "schema_version": "issue_fix_maintainer_correction_todo_write_v0", + "write_performed": False, + "already_exists": bool(todo_id), + "skip_reason": skip_reason, + "path_recorded": False, + } + if operation_id is not None: + receipt["operation_id"] = operation_id + if todo_id is not None: + receipt["todo_id"] = todo_id + return receipt + + +def _resolve_origin_request_identity(runtime_root: Path, request_id: str) -> _OriginRequestIdentity: + paths = list((runtime_root / ".local/manager-context/entries").glob(f"*/{request_id}.json")) + if not paths: + raise ValueError("origin_request_not_found") + if len(paths) != 1: + raise ValueError("origin_request_ambiguous") + try: + entry = json.loads(paths[0].read_text(encoding="utf-8")) + except (OSError, ValueError) as exc: + raise ValueError("origin_request_not_found") from exc + if not isinstance(entry, dict): + raise ValueError("origin_request_not_found") + if isinstance(entry.get("recipient"), dict): + raise ValueError("origin_request_unsupported_target") + goal_id = entry.get("goal_id") + agent_id = entry.get("agent_id") + if not isinstance(goal_id, str) or not goal_id or not isinstance(agent_id, str) or not agent_id: + raise ValueError("origin_request_unsupported_target") + identity: _OriginRequestIdentity = {"goal_id": goal_id, "agent_id": agent_id} + if isinstance(entry.get("goal_ref"), dict): + identity["goal_ref"] = entry["goal_ref"] + return identity + + +def _settle_maintainer_correction_entries( + *, + payload: dict[str, object], + ledger_path: Path, + registry_path: Path, + goal_id: str, + claimed_by: str, + project: Path, +) -> dict[str, object]: + """Write Todos for accepted ledger entries that still lack a todo_id. + + Each accepted entry stores its own `todo_intent`, so an entry left pending + by an interrupted run is retried under its original operation id on any + later run. Every candidate is re-gated against the ledger as it stands + now, the same observation, and the Goal activation state. + """ + + observation = payload.get("observation") + observation = observation if isinstance(observation, dict) else {} + admission = payload.get("maintainer_correction_admission") + admission = admission if isinstance(admission, dict) else {} + current_operation_id = str(admission.get("operation_id") or "") + goal_stopped = _goal_stopped_for_correction(registry_path, goal_id) + repo = str(observation.get("repo") or "") + pr_ref = str(observation.get("pr_ref") or "") + + row = read_issue_fix_pr_lifecycle_ledger_row(ledger_path, repo=repo, pr_ref=pr_ref) + ledger_entries = issue_fix_maintainer_correction_entries(row) + origin = (row or {}).get("origin_request") + view = None + origin_refusal = None + if origin: + from ...control_plane.collaboration.goal_instance_scope import collaboration_goal_scope + from ...control_plane.collaboration.request_control_source import execute_request_control + + try: + identity = _resolve_origin_request_identity(Path(origin["runtime_root"]), origin["request_id"]) + if any(identity[key] != origin[key] for key in ("goal_id", "agent_id")): + raise ValueError("origin request identity changed") + with collaboration_goal_scope(Path(origin["registry_path"]), goal_id=identity["goal_id"], + agents=(), require_active=False) as scope: + goal_ref = identity.get("goal_ref") or scope.current_goal_ref or {"goal_id": identity["goal_id"]} + if origin.get("goal_ref") and goal_ref != origin["goal_ref"]: + raise ValueError("origin request identity changed") + result = execute_request_control(registry_path=Path(origin["registry_path"]), + runtime_root=Path(origin["runtime_root"]), goal_ref=goal_ref, + receiver_agent_id=identity["agent_id"], request_id=origin["request_id"], + actor_agent_id=identity["agent_id"], operation="inspect", execute=False) + if not result.get("ok"): + raise ValueError("request inspection failed") + view = result["view"] + if not view.get("control_state"): + raise ValueError("request control state unavailable") + if view["control_state"] in {"cancelled", "cancellation_pending"}: + origin_refusal = "origin_request_cancelled" + except (OSError, ValueError, KeyError): + origin_refusal = "origin_request_unreadable" + settlements: list[dict[str, object]] = [] + current_todo_write: dict[str, object] | None = None + changed_entries: list[dict[str, object]] = [] + for entry in ledger_entries: + if origin and entry.get("disposition") == "accepted" and "accepted_head_sha" not in entry: + entry["accepted_head_sha"] = observation.get("head_commit_ref") + changed_entries.append(dict(entry)) + operation_id = str(entry.get("operation_id") or "") + is_current = operation_id == current_operation_id + intent = entry.get("todo_intent") + if ( + entry.get("disposition") != "accepted" + or not isinstance(intent, dict) + or (entry.get("todo_id") and not is_current) + ): + continue + refusal = recheck_issue_fix_maintainer_correction_effect_gates( + entry, + observation=observation, + ledger_entries=ledger_entries, + goal_stopped=goal_stopped, + ) + refusal = origin_refusal or refusal + if refusal is not None and not entry.get("todo_id"): + if is_current and origin_refusal: + admission.update(disposition="context_only", reason=refusal) + changed_entries.append( + {**entry, "disposition": "context_only", "reason": refusal, + "effect_recheck": "refused"} + ) + settlements.append( + {"operation_id": operation_id, "status": "refused", "reason": refusal} + ) + if is_current: + current_todo_write = _correction_todo_readback_receipt( + skip_reason=refusal, operation_id=operation_id, + ) + continue + if refusal is not None: + # The Todo already exists for this revision; a replay only reads it + # back and leaves its lifecycle to the Todo owner. + settlements.append( + {"operation_id": operation_id, "status": "settled_earlier", "reason": refusal} + ) + if is_current: + current_todo_write = _correction_todo_readback_receipt( + skip_reason=refusal, operation_id=operation_id, + todo_id=entry.get("todo_id"), + ) + continue + if not entry.get("todo_id") and os.environ.get(MAINTAINER_CORRECTION_FAULT_ENV) == "1": + raise RuntimeError( + "maintainer correction Todo write aborted by " + f"{MAINTAINER_CORRECTION_FAULT_ENV}" + ) + todo_write = _write_maintainer_correction_todo( + registry_path=registry_path, + goal_id=goal_id, + claimed_by=claimed_by, + project=project, + operation_id=operation_id, + intent=intent, + ) + receipt = _maintainer_correction_todo_write_receipt( + todo_write, operation_id=operation_id + ) + if entry.get("todo_id") != receipt["todo_id"]: + changed_entries.append({**entry, "todo_id": receipt["todo_id"]}) + settlements.append( + { + "operation_id": operation_id, + "status": "written" if receipt["write_performed"] else "already_exists", + "todo_id": receipt["todo_id"], + } + ) + if is_current: + current_todo_write = receipt + if current_todo_write is None and current_operation_id: + current_todo_write = _correction_todo_readback_receipt( + skip_reason=( + str(admission.get("reason") or admission.get("disposition") or "") + if admission.get("disposition") != "accepted" + else "unchanged_monitor_quiet" + ), + operation_id=current_operation_id, + ) + if changed_entries: + payload["maintainer_corrections"] = changed_entries + upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) + row = read_issue_fix_pr_lifecycle_ledger_row( + ledger_path, repo=repo, pr_ref=pr_ref + ) + entries = issue_fix_maintainer_correction_entries(row) + payload["maintainer_corrections"] = entries + if origin: + from ...todos import list_goal_todos + from ...control_plane.collaboration.inbox import reply_status + from ..manager_context.roundtrip import report + + todos = {} if origin_refusal else {t["todo_id"]: t for t in list_goal_todos( + registry_path=registry_path, goal_id=goal_id)["todos"]} + head = str(observation.get("head_commit_ref") or "") + for entry in entries: + if (origin_refusal or not view or entry.get("disposition") != "accepted" + or not entry.get("todo_id") or not head + or head == entry.get("accepted_head_sha") + or todos.get(entry["todo_id"], {}).get("status") != "done"): + continue + returns = entry.setdefault("returns", []) + if any(r["head_sha"] == head for r in returns): + continue + operation_id = entry["operation_id"] + update_id = f"issue-fix-correction:{operation_id}:{head}" + event = entry.get("source_event") or {} + checks = observation.get("checks") or {} + text = (f"Maintainer correction {entry.get('correction_kind')} " + f"({event.get('kind')} {event.get('id')}) applied in " + f"{observation.get('permalink')} at head {head[:12]}; " + f"PR state {observation.get('state')}, checks {checks.get('aggregate')}; " + f"todo {entry['todo_id']} done.") + origin_root = resolve_runtime_root( + load_project_registry(Path(origin["registry_path"])), + origin["runtime_root"], registry_path=Path(origin["registry_path"])) + result = report(origin_root, identity["goal_id"], identity["agent_id"], + origin["request_id"], "conclusion", text, registry=Path(origin["registry_path"]), + update_id=(update_id if any( + r["phase"] == "conclusion" for r in reply_status(origin_root, origin) + ) else None), workspace=project, request_revision=view["revision"], + caller_goal_ref=goal_ref) + returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), + "enqueued_at": now_utc_iso(), "head_sha": head}) + upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) + statuses = {r.get("result_key", r["phase"]): r["status"] + for r in reply_status(Path(origin["runtime_root"]), origin)} + payload["return_delivery"] = [{"operation_id": e["operation_id"], + "update_id": r["update_id"], "status": statuses.get(r["result_key"], "explicit_unverified")} + for e in entries for r in e.get("returns", [])] + return { + "schema_version": "issue_fix_maintainer_correction_settlement_v0", + "settlements": settlements, + "goal_stopped": goal_stopped, + "todo_write": current_todo_write, + "path_recorded": False, + } diff --git a/tests/capabilities/test_issue_fix_correction_origin_return.py b/tests/capabilities/test_issue_fix_correction_origin_return.py index a9a76d82df..e46871e05c 100644 --- a/tests/capabilities/test_issue_fix_correction_origin_return.py +++ b/tests/capabilities/test_issue_fix_correction_origin_return.py @@ -15,7 +15,7 @@ from loopx.control_plane.collaboration.inbox import acknowledge from loopx.control_plane.collaboration.peers import request as peer_request from loopx.control_plane.collaboration.request_control_source import execute_request_control -from loopx.capabilities.issue_fix.cli import _settle_maintainer_correction_entries +from loopx.capabilities.issue_fix.correction_settlement import _settle_maintainer_correction_entries ORIGIN_GOAL_ID = "steward-goal" @@ -136,7 +136,7 @@ def test_stored_goal_ref_mismatch(h): @pytest.mark.parametrize("owner,name", [ - ("loopx.capabilities.issue_fix.cli", "_resolve_origin_request_identity"), + ("loopx.capabilities.issue_fix.correction_settlement", "_resolve_origin_request_identity"), ("loopx.control_plane.collaboration.request_control_source", "execute_request_control"), ]) def test_programming_error_propagates(h, monkeypatch, owner, name): From bc0e763ea49c32822bf70da02291a474d5065896 Mon Sep 17 00:00:00 2001 From: hhyykk <4408344+hhyykk@users.noreply.github.com> Date: Mon, 2 Nov 2026 09:36:08 +0800 Subject: [PATCH 07/11] fix(issue-fix): keep correction admission and return recovery fail-closed - A verified retry of an event first recorded as context_only now upgrades the ledger entry and emits Todo intent once; accepted entries still replay as no-ops. - Settling an accepted-but-unsettled correction honors the current observation: an absent event or revision mismatch leaves it pending without creating a Todo. - The return publication identity is persisted before report(), so a retry after an interrupted return-ledger write reuses it instead of publishing a duplicate conclusion; a new head still publishes an update. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com> --- .../issue_fix/correction_settlement.py | 25 +++++++++++-- loopx/capabilities/issue_fix/pr_lifecycle.py | 6 ++-- ...test_issue_fix_correction_origin_return.py | 26 ++++++++++++++ ...sue_fix_maintainer_correction_admission.py | 35 +++++++++++++++++++ 4 files changed, 87 insertions(+), 5 deletions(-) diff --git a/loopx/capabilities/issue_fix/correction_settlement.py b/loopx/capabilities/issue_fix/correction_settlement.py index 4c367413a0..ae8aacc3aa 100644 --- a/loopx/capabilities/issue_fix/correction_settlement.py +++ b/loopx/capabilities/issue_fix/correction_settlement.py @@ -217,6 +217,17 @@ def _settle_maintainer_correction_entries( or (entry.get("todo_id") and not is_current) ): continue + if is_current and not entry.get("todo_id") and admission.get("reason") in { + "source_event_not_observed", "source_event_revision_mismatch", + }: + refusal = str(admission["reason"]) + settlements.append( + {"operation_id": operation_id, "status": "refused", "reason": refusal} + ) + current_todo_write = _correction_todo_readback_receipt( + skip_reason=refusal, operation_id=operation_id, todo_id=entry.get("todo_id"), + ) + continue refusal = recheck_issue_fix_maintainer_correction_effect_gates( entry, observation=observation, @@ -324,12 +335,20 @@ def _settle_maintainer_correction_entries( origin_root = resolve_runtime_root( load_project_registry(Path(origin["registry_path"])), origin["runtime_root"], registry_path=Path(origin["registry_path"])) + pending = entry.get("pending_return") + if not isinstance(pending, dict) or pending.get("head_sha") != head: + # Select the initial/update identity durably before publication; + # a successful report may outlive the final return-ledger write. + pending = {"head_sha": head, "update_id": update_id if any( + r["phase"] == "conclusion" for r in reply_status(origin_root, origin) + ) else None} + entry["pending_return"] = pending + upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) result = report(origin_root, identity["goal_id"], identity["agent_id"], origin["request_id"], "conclusion", text, registry=Path(origin["registry_path"]), - update_id=(update_id if any( - r["phase"] == "conclusion" for r in reply_status(origin_root, origin) - ) else None), workspace=project, request_revision=view["revision"], + update_id=pending["update_id"], workspace=project, request_revision=view["revision"], caller_goal_ref=goal_ref) + entry["pending_return"] = None returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), "enqueued_at": now_utc_iso(), "head_sha": head}) upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) diff --git a/loopx/capabilities/issue_fix/pr_lifecycle.py b/loopx/capabilities/issue_fix/pr_lifecycle.py index 8ff652b795..817423c754 100644 --- a/loopx/capabilities/issue_fix/pr_lifecycle.py +++ b/loopx/capabilities/issue_fix/pr_lifecycle.py @@ -701,8 +701,10 @@ def record(name: str, passed: bool, failure_reason: str | None = None) -> None: ) accepted = reason is None - replay = existing_entry is not None - if existing_entry is not None: + replay = existing_entry is not None and ( + not accepted or existing_entry.get("disposition") in {"accepted", "superseded"} + ) + if replay: entry = dict(existing_entry) else: entry = { diff --git a/tests/capabilities/test_issue_fix_correction_origin_return.py b/tests/capabilities/test_issue_fix_correction_origin_return.py index e46871e05c..32153eb178 100644 --- a/tests/capabilities/test_issue_fix_correction_origin_return.py +++ b/tests/capabilities/test_issue_fix_correction_origin_return.py @@ -360,3 +360,29 @@ def test_invalid_request_entry_writes_nothing(h, damage, error): assert json.loads(result.stdout)["ok"] is False assert json.loads(result.stdout)["error"] == error assert not default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID).exists() + + +def test_return_ledger_failure_preserves_publication_identity(h, monkeypatch): + from loopx.capabilities.issue_fix import correction_settlement + + event = _event("313") + packet = h.run_json(metadata=observation([event]), correction=_correction_v1(event), origin=h.request_id) + h.mark_done(h.entries()[0]["todo_id"]) + packet["observation"]["head_commit_ref"] = "b" * 40 + original = correction_settlement.upsert_issue_fix_pr_lifecycle_ledger_jsonl + + def fail_return_write(path, payload): + if any(entry.get("returns") for entry in payload["maintainer_corrections"]): + raise OSError("return ledger interrupted") + return original(path, payload) + + with monkeypatch.context() as patch: + patch.setattr(correction_settlement, "upsert_issue_fix_pr_lifecycle_ledger_jsonl", fail_return_write) + with pytest.raises(OSError, match="return ledger interrupted"): + settle(h, packet) + assert len(h.replies()) == 1 + assert not h.entries()[0].get("returns") + h.run_json(metadata=observation([event], "b" * 40), correction=None) + assert len(h.replies()) == len(h.entries()[0]["returns"]) == 1 + h.run_json(metadata=observation([event], "c" * 40), correction=None) + assert len(h.replies()) == len(h.entries()[0]["returns"]) == 2 diff --git a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py index 0b822f7596..a45d28c578 100644 --- a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py +++ b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py @@ -624,3 +624,38 @@ def test_missing_goal_does_not_write_ledger(harness: Harness) -> None: assert result.returncode == 1 assert json.loads(result.stdout)["ok"] is False assert (ledger.read_bytes() if ledger.exists() else None) == before + + +def test_unverified_correction_is_accepted_on_verified_retry(harness: Harness) -> None: + event = _event("3501") + missing = _metadata([]) + for key in ("comments", "reviews", "reviewComments"): + missing.pop(key) + first = harness.run_json(metadata=missing, correction=_correction_v1(event)) + _assert_no_correction_todo(first, harness, "source_event_not_observed") + second = harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + assert second["maintainer_correction_admission"]["replay"] is False + assert harness.entries()[0]["disposition"] == "accepted" + assert _correction_write(second)["write_performed"] is True + third = harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + assert third["maintainer_correction_admission"]["replay"] is True + assert _correction_write(third)["write_performed"] is False + assert len(harness.correction_todos()) == 1 + + +def test_pending_correction_requires_current_event_verification(harness: Harness) -> None: + event = _event("3502") + failed = harness.run(metadata=_metadata([event]), correction=_correction_v1(event), + env={"LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO": "1"}, check=False) + assert failed.returncode != 0 + for events, reason in ( + ([], "source_event_not_observed"), + ([_event("3502", updated_at="2026-06-20T12:00:00Z")], "source_event_revision_mismatch"), + ): + packet = harness.run_json(metadata=_metadata(events), correction=_correction_v1(event)) + assert _correction_write(packet)["skip_reason"] == reason + assert harness.correction_todos() == [] + assert harness.entries()[0]["todo_id"] is None + harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + assert len(harness.correction_todos()) == 1 From d00e0670b83b6163aa59b4b69ca789d1d24346d9 Mon Sep 17 00:00:00 2001 From: huangruiteng Date: Wed, 4 Nov 2026 14:24:09 +0800 Subject: [PATCH 08/11] fix(issue-fix): reconcile correction recovery from durable evidence Signed-off-by: huangruiteng --- loopx/capabilities/issue_fix/README.md | 10 ++- loopx/capabilities/issue_fix/README.zh-CN.md | 8 +- loopx/capabilities/issue_fix/cli.py | 6 ++ .../issue_fix/correction_settlement.py | 78 ++++++++++++------- loopx/capabilities/issue_fix/pr_lifecycle.py | 55 +++++++++---- ...test_issue_fix_correction_origin_return.py | 29 +++++-- ...sue_fix_maintainer_correction_admission.py | 26 +++++-- 7 files changed, 158 insertions(+), 54 deletions(-) diff --git a/loopx/capabilities/issue_fix/README.md b/loopx/capabilities/issue_fix/README.md index d3acec4911..22b2d4525f 100644 --- a/loopx/capabilities/issue_fix/README.md +++ b/loopx/capabilities/issue_fix/README.md @@ -440,7 +440,10 @@ fails closed with `origin_request_cancelled` or `origin_request_unreadable`; existing Todos retain readback-only behavior. A completed Todo observed at a new head queues a body-free conclusion through the origin runtime, deduplicated by operation/head in `returns`, with `return_delivery` readback; cancellation -or unreadability prevents enqueue. The return service owns actual delivery. +or unreadability prevents enqueue. The complete conclusion text, request +revision and selected update identity are persisted before `report()`, so an +interrupted return-ledger write resumes the exact publication instead of +reconstructing it from newer PR state. The return service owns actual delivery. Once the origin request is cancelled, no further result is returned even if an accepted correction completes later; the request's cancellation record is the accountability trail. @@ -459,7 +462,10 @@ these entries in `maintainer_corrections` `disposition`, `reason`, `todo_id`, `recorded_at`, `superseded_by`, `todo_intent`); the accepted entry is written with `todo_id: null` before the Todo write and filled afterwards, and any later run retries a still-pending -accepted entry under its operation id. The test-only +accepted entry under its operation id. Every retry revalidates a pending v1 +source event against the current provider metadata, including an ordinary poll +that omits the correction input or reports a different current correction. The +test-only `LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO=1` aborts between those two writes and exists only to exercise that recovery. The legacy `maintainer_correction` fields keep the latest accepted correction for older diff --git a/loopx/capabilities/issue_fix/README.zh-CN.md b/loopx/capabilities/issue_fix/README.zh-CN.md index 7e7a4102d2..023dafbef2 100644 --- a/loopx/capabilities/issue_fix/README.zh-CN.md +++ b/loopx/capabilities/issue_fix/README.zh-CN.md @@ -421,7 +421,9 @@ entry；不支持 recipient addressing。Settlement 会再次解析该 entry， `origin_request_cancelled` 或 `origin_request_unreadable` fail closed；已有 Todo 保持只读回读行为。在新 head 观察到已完成 Todo 时，会通过 origin runtime 将无正文的结论 入队，在 `returns` 中按 operation/head 去重，并提供 `return_delivery` 回读；取消或不可读 -会阻止入队。Return service 负责实际投递。Origin request 一旦取消，即使已接纳的 correction +会阻止入队。完整 conclusion text、request revision 与所选 update identity 会在 +`report()` 前持久化，因此 return-ledger 写入中断后恢复的是同一份 publication，而不是 +根据更新的 PR state 重建文本。Return service 负责实际投递。Origin request 一旦取消，即使已接纳的 correction 稍后完成，也不再返回任何结果；request 的 cancellation record 是其 accountability trail。 显式使用 `--execute-transition` 时，已接纳的 `actionable_patch` 只创建一个由当前注册 agent 认领的 @@ -435,7 +437,9 @@ revision 会生成新 Todo，并把先前条目标记为 `superseded`。Ledger r `correction_fingerprint`、`admission_basis`、`disposition`、`reason`、`todo_id`、 `recorded_at`、`superseded_by`、`todo_intent`）；已接纳条目在 Todo 写入前先以 `todo_id: null` 写入，随后补齐；任何后续运行都会按其 operation id 重试仍处于 pending -状态的已接纳条目。仅供测试的 `LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO=1` +状态的已接纳条目。每次重试都会用当前 provider metadata 重新验证 pending v1 source event， +包括省略 correction input 的普通轮询，或报告了另一个 current correction 的运行。仅供测试的 +`LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO=1` 会在这两次写入之间中止，仅用于演练该恢复路径。旧 `maintainer_correction` 字段为旧版 reader 保留最新接纳的 correction。Merged/closed terminal state 仍优先于迟到反馈。 diff --git a/loopx/capabilities/issue_fix/cli.py b/loopx/capabilities/issue_fix/cli.py index 3c969c3c79..c62c54d245 100644 --- a/loopx/capabilities/issue_fix/cli.py +++ b/loopx/capabilities/issue_fix/cli.py @@ -1356,6 +1356,7 @@ def handle_issue_fix_command( prior_maintainer_corrections=prior_maintainer_corrections, generated_at=generated_at, ) + pending_source_events = payload.pop("_maintainer_correction_source_events", None) if origin: payload["origin_request"] = origin if should_write_domain_state and ledger_path is not None: @@ -1412,6 +1413,11 @@ def handle_issue_fix_command( goal_id=args.goal_id, claimed_by=args.claimed_by, project=Path(args.project).expanduser(), + source_events=( + pending_source_events + if isinstance(pending_source_events, dict) + else None + ), ) if settlement.get("settlements") or isinstance( correction_admission, dict diff --git a/loopx/capabilities/issue_fix/correction_settlement.py b/loopx/capabilities/issue_fix/correction_settlement.py index ae8aacc3aa..596fe57577 100644 --- a/loopx/capabilities/issue_fix/correction_settlement.py +++ b/loopx/capabilities/issue_fix/correction_settlement.py @@ -24,6 +24,10 @@ # Set to "1" to abort between the accepted ledger entry write and its Todo # write. Exists only to prove the next run reconciles the pending entry. MAINTAINER_CORRECTION_FAULT_ENV = "LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO" +_SOURCE_EVENT_REFUSALS = { + "source_event_not_observed", + "source_event_revision_mismatch", +} def _goal_stopped_for_correction(registry_path: Path | None, goal_id: str | None) -> bool: @@ -152,6 +156,7 @@ def _settle_maintainer_correction_entries( goal_id: str, claimed_by: str, project: Path, + source_events: Mapping[str, object] | None = None, ) -> dict[str, object]: """Write Todos for accepted ledger entries that still lack a todo_id. @@ -172,6 +177,9 @@ def _settle_maintainer_correction_entries( row = read_issue_fix_pr_lifecycle_ledger_row(ledger_path, repo=repo, pr_ref=pr_ref) ledger_entries = issue_fix_maintainer_correction_entries(row) + if source_events is None: + pending_source_events = payload.pop("_maintainer_correction_source_events", None) + source_events = pending_source_events if isinstance(pending_source_events, dict) else None origin = (row or {}).get("origin_request") view = None origin_refusal = None @@ -217,25 +225,24 @@ def _settle_maintainer_correction_entries( or (entry.get("todo_id") and not is_current) ): continue - if is_current and not entry.get("todo_id") and admission.get("reason") in { - "source_event_not_observed", "source_event_revision_mismatch", - }: - refusal = str(admission["reason"]) - settlements.append( - {"operation_id": operation_id, "status": "refused", "reason": refusal} - ) - current_todo_write = _correction_todo_readback_receipt( - skip_reason=refusal, operation_id=operation_id, todo_id=entry.get("todo_id"), - ) - continue refusal = recheck_issue_fix_maintainer_correction_effect_gates( entry, observation=observation, ledger_entries=ledger_entries, goal_stopped=goal_stopped, + source_events=source_events, ) refusal = origin_refusal or refusal if refusal is not None and not entry.get("todo_id"): + if refusal in _SOURCE_EVENT_REFUSALS: + settlements.append( + {"operation_id": operation_id, "status": "refused", "reason": refusal} + ) + if is_current: + current_todo_write = _correction_todo_readback_receipt( + skip_reason=refusal, operation_id=operation_id, + ) + continue if is_current and origin_refusal: admission.update(disposition="context_only", reason=refusal) changed_entries.append( @@ -325,30 +332,47 @@ def _settle_maintainer_correction_entries( continue operation_id = entry["operation_id"] update_id = f"issue-fix-correction:{operation_id}:{head}" - event = entry.get("source_event") or {} - checks = observation.get("checks") or {} - text = (f"Maintainer correction {entry.get('correction_kind')} " - f"({event.get('kind')} {event.get('id')}) applied in " - f"{observation.get('permalink')} at head {head[:12]}; " - f"PR state {observation.get('state')}, checks {checks.get('aggregate')}; " - f"todo {entry['todo_id']} done.") origin_root = resolve_runtime_root( load_project_registry(Path(origin["registry_path"])), origin["runtime_root"], registry_path=Path(origin["registry_path"])) pending = entry.get("pending_return") - if not isinstance(pending, dict) or pending.get("head_sha") != head: - # Select the initial/update identity durably before publication; - # a successful report may outlive the final return-ledger write. - pending = {"head_sha": head, "update_id": update_id if any( - r["phase"] == "conclusion" for r in reply_status(origin_root, origin) - ) else None} + if ( + not isinstance(pending, dict) + or pending.get("head_sha") != head + or not isinstance(pending.get("text"), str) + or not isinstance(pending.get("request_revision"), int) + ): + event = entry.get("source_event") or {} + checks = observation.get("checks") or {} + text = (f"Maintainer correction {entry.get('correction_kind')} " + f"({event.get('kind')} {event.get('id')}) applied in " + f"{observation.get('permalink')} at head {head[:12]}; " + f"PR state {observation.get('state')}, checks {checks.get('aggregate')}; " + f"todo {entry['todo_id']} done.") + # The complete audience-visible publication is durable before + # report(). A crash after report() must replay this exact result, + # not reconstruct it from newer, mutable PR observations. + pending = { + "schema_version": "issue_fix_maintainer_correction_publication_v1", + "head_sha": head, + "update_id": update_id if any( + r["phase"] == "conclusion" + for r in reply_status(origin_root, origin) + ) else None, + "text": text, + "request_revision": view["revision"], + "phase": "conclusion", + } entry["pending_return"] = pending upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) result = report(origin_root, identity["goal_id"], identity["agent_id"], - origin["request_id"], "conclusion", text, registry=Path(origin["registry_path"]), - update_id=pending["update_id"], workspace=project, request_revision=view["revision"], + origin["request_id"], str(pending.get("phase") or "conclusion"), + str(pending["text"]), + registry=Path(origin["registry_path"]), + update_id=pending.get("update_id"), workspace=project, + request_revision=pending["request_revision"], caller_goal_ref=goal_ref) - entry["pending_return"] = None + entry.pop("pending_return", None) returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), "enqueued_at": now_utc_iso(), "head_sha": head}) upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) diff --git a/loopx/capabilities/issue_fix/pr_lifecycle.py b/loopx/capabilities/issue_fix/pr_lifecycle.py index 817423c754..3cc0650f9f 100644 --- a/loopx/capabilities/issue_fix/pr_lifecycle.py +++ b/loopx/capabilities/issue_fix/pr_lifecycle.py @@ -767,11 +767,28 @@ def recheck_issue_fix_maintainer_correction_effect_gates( observation: Mapping[str, Any], ledger_entries: Sequence[Mapping[str, Any]] | None, goal_stopped: bool, + source_events: Mapping[str, Any] | None = None, ) -> str | None: """Re-run the effect-time gates for an accepted entry just before its Todo write.""" source_event = entry.get("source_event") if isinstance(source_event, Mapping): + observed = [ + event + for event in ( + source_events.get("events") if isinstance(source_events, Mapping) else [] + ) + if isinstance(event, Mapping) + and event.get("kind") == source_event.get("kind") + and str(event.get("id")) == str(source_event.get("id")) + ] + if not observed: + return "source_event_not_observed" + if not any( + _same_instant(event.get("updated_at"), source_event.get("updated_at")) + for event in observed + ): + return "source_event_revision_mismatch" latest = _latest_accepted_revision( issue_fix_maintainer_correction_entries( {"maintainer_corrections": ledger_entries} @@ -1569,14 +1586,25 @@ def build_issue_fix_pr_lifecycle_monitor_packet( ) maintainer_correction_admission: dict[str, Any] | None = None source_events: dict[str, Any] | None = None + pending_source_event_recheck = any( + entry.get("disposition") == "accepted" + and not entry.get("todo_id") + and isinstance(entry.get("source_event"), Mapping) + for entry in issue_fix_maintainer_correction_entries( + {"maintainer_corrections": prior_maintainer_corrections} + ) + ) + if pending_source_event_recheck or ( + maintainer_correction is not None + and isinstance(maintainer_correction.get("source_event"), Mapping) + ): + if fetch_metadata: + source_events = _fetch_pr_source_events( + reference, timeout_seconds=fetch_timeout_seconds + ) + else: + source_events = normalise_issue_fix_pr_source_events(payload) if maintainer_correction is not None: - if isinstance(maintainer_correction.get("source_event"), Mapping): - if fetch_metadata: - source_events = _fetch_pr_source_events( - reference, timeout_seconds=fetch_timeout_seconds - ) - else: - source_events = normalise_issue_fix_pr_source_events(payload) maintainer_correction_admission = ( evaluate_issue_fix_maintainer_correction_admission( maintainer_correction, @@ -1686,12 +1714,13 @@ def build_issue_fix_pr_lifecycle_monitor_packet( entries = maintainer_correction_admission.pop("entries") packet["maintainer_correction_admission"] = maintainer_correction_admission packet["maintainer_corrections"] = entries - if source_events is not None: - packet["source_events_observed"] = { - "status": source_events.get("status"), - "event_count": len(source_events.get("events") or []), - "body_captured": False, - } + if source_events is not None: + packet["source_events_observed"] = { + "status": source_events.get("status"), + "event_count": len(source_events.get("events") or []), + "body_captured": False, + } + packet["_maintainer_correction_source_events"] = source_events validation = validate_issue_fix_pr_lifecycle_monitor_packet(packet) packet["ok"] = bool(validation["ok"]) packet["validation"] = validation diff --git a/tests/capabilities/test_issue_fix_correction_origin_return.py b/tests/capabilities/test_issue_fix_correction_origin_return.py index 32153eb178..3d8f3ca84f 100644 --- a/tests/capabilities/test_issue_fix_correction_origin_return.py +++ b/tests/capabilities/test_issue_fix_correction_origin_return.py @@ -362,7 +362,8 @@ def test_invalid_request_entry_writes_nothing(h, damage, error): assert not default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID).exists() -def test_return_ledger_failure_preserves_publication_identity(h, monkeypatch): +@pytest.mark.parametrize("change", ["checks", "revision"]) +def test_return_ledger_failure_replays_the_immutable_publication(h, monkeypatch, change): from loopx.capabilities.issue_fix import correction_settlement event = _event("313") @@ -381,8 +382,26 @@ def fail_return_write(path, payload): with pytest.raises(OSError, match="return ledger interrupted"): settle(h, packet) assert len(h.replies()) == 1 - assert not h.entries()[0].get("returns") - h.run_json(metadata=observation([event], "b" * 40), correction=None) + pending = h.entries()[0] + assert not pending.get("returns") + publication = pending["pending_return"] + assert publication["head_sha"] == "b" * 40 + assert "checks PASSING" in publication["text"] + retry_observation = observation([event], "b" * 40) + if change == "checks": + retry_observation["statusCheckRollup"] = [{"name": "focused", "conclusion": "FAILURE"}] + else: + view = h.control("inspect") + h.control("revise", { + "operation_id": "revision-after-publication", + "expected_revision": view["revision"], + "expected_digest": view["revision_digest"], + "brief": {**view["brief"], "purpose": "Validate the changed correction"}, + "reason": "The request changed after publication", + }) + h.run_json(metadata=retry_observation, correction=None) assert len(h.replies()) == len(h.entries()[0]["returns"]) == 1 - h.run_json(metadata=observation([event], "c" * 40), correction=None) - assert len(h.replies()) == len(h.entries()[0]["returns"]) == 2 + assert json.loads(h.replies()[0].read_text())["text"] == publication["text"] + if change == "checks": + h.run_json(metadata=observation([event], "c" * 40), correction=None) + assert len(h.replies()) == len(h.entries()[0]["returns"]) == 2 diff --git a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py index a45d28c578..af81bcb2a8 100644 --- a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py +++ b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py @@ -652,10 +652,26 @@ def test_pending_correction_requires_current_event_verification(harness: Harness ([], "source_event_not_observed"), ([_event("3502", updated_at="2026-06-20T12:00:00Z")], "source_event_revision_mismatch"), ): - packet = harness.run_json(metadata=_metadata(events), correction=_correction_v1(event)) - assert _correction_write(packet)["skip_reason"] == reason + packet = harness.run_json(metadata=_metadata(events), correction=None) + refusal = next(item for item in packet["maintainer_correction_settlement"]["settlements"] + if item["operation_id"] == harness.entries()[0]["operation_id"]) + assert refusal == { + "operation_id": harness.entries()[0]["operation_id"], + "status": "refused", + "reason": reason, + } assert harness.correction_todos() == [] - assert harness.entries()[0]["todo_id"] is None - harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) - harness.run_json(metadata=_metadata([event]), correction=_correction_v1(event)) + pending = harness.entries()[0] + assert pending["disposition"] == "accepted" and pending["todo_id"] is None + different = _event("3599") + packet = harness.run_json(metadata=_metadata([]), correction=_correction_v1(different)) + original_operation = harness.entries()[0]["operation_id"] + assert next(item for item in packet["maintainer_correction_settlement"]["settlements"] + if item["operation_id"] == original_operation)["status"] == "refused" + assert harness.entries()[0]["todo_id"] is None + recovered = harness.run_json(metadata=_metadata([event]), correction=None) + original_settlement = next(item for item in recovered["maintainer_correction_settlement"]["settlements"] + if item["operation_id"] == harness.entries()[0]["operation_id"]) + assert original_settlement["status"] == "written" + harness.run_json(metadata=_metadata([event]), correction=None) assert len(harness.correction_todos()) == 1 From 8d686eb4c72ebdad8a821028779d44890cbeccd8 Mon Sep 17 00:00:00 2001 From: huangruiteng Date: Wed, 4 Nov 2026 14:27:17 +0800 Subject: [PATCH 09/11] chore(issue-fix): record reviewed lifecycle module ceiling Signed-off-by: huangruiteng --- loopx/canary/module_metric_baseline.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/loopx/canary/module_metric_baseline.json b/loopx/canary/module_metric_baseline.json index 51ded7160f..70ddafe3cc 100644 --- a/loopx/canary/module_metric_baseline.json +++ b/loopx/canary/module_metric_baseline.json @@ -26,6 +26,9 @@ "any_count": 5, "dict_any_count": 0 }, + "loopx/capabilities/issue_fix/pr_lifecycle.py": { + "lines": 2054 + }, "loopx/chat_actions.py": { "any_count": 52, "dict_any_count": 0 From 14985e3d3a3153f825753e8c12506755b2fd9a3c Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:58:50 +0800 Subject: [PATCH 10/11] fix(issue-fix): settle pending returns before advancing heads Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .../issue_fix/correction_settlement.py | 91 ++++++++++--------- ...test_issue_fix_correction_origin_return.py | 42 +++++++-- 2 files changed, 80 insertions(+), 53 deletions(-) diff --git a/loopx/capabilities/issue_fix/correction_settlement.py b/loopx/capabilities/issue_fix/correction_settlement.py index 596fe57577..46adb077da 100644 --- a/loopx/capabilities/issue_fix/correction_settlement.py +++ b/loopx/capabilities/issue_fix/correction_settlement.py @@ -323,59 +323,62 @@ def _settle_maintainer_correction_entries( head = str(observation.get("head_commit_ref") or "") for entry in entries: if (origin_refusal or not view or entry.get("disposition") != "accepted" - or not entry.get("todo_id") or not head - or head == entry.get("accepted_head_sha") + or not entry.get("todo_id") or todos.get(entry["todo_id"], {}).get("status") != "done"): continue returns = entry.setdefault("returns", []) - if any(r["head_sha"] == head for r in returns): - continue operation_id = entry["operation_id"] - update_id = f"issue-fix-correction:{operation_id}:{head}" origin_root = resolve_runtime_root( load_project_registry(Path(origin["registry_path"])), origin["runtime_root"], registry_path=Path(origin["registry_path"])) pending = entry.get("pending_return") - if ( - not isinstance(pending, dict) - or pending.get("head_sha") != head - or not isinstance(pending.get("text"), str) - or not isinstance(pending.get("request_revision"), int) - ): - event = entry.get("source_event") or {} - checks = observation.get("checks") or {} - text = (f"Maintainer correction {entry.get('correction_kind')} " - f"({event.get('kind')} {event.get('id')}) applied in " - f"{observation.get('permalink')} at head {head[:12]}; " - f"PR state {observation.get('state')}, checks {checks.get('aggregate')}; " - f"todo {entry['todo_id']} done.") - # The complete audience-visible publication is durable before - # report(). A crash after report() must replay this exact result, - # not reconstruct it from newer, mutable PR observations. - pending = { - "schema_version": "issue_fix_maintainer_correction_publication_v1", - "head_sha": head, - "update_id": update_id if any( - r["phase"] == "conclusion" - for r in reply_status(origin_root, origin) - ) else None, - "text": text, - "request_revision": view["revision"], - "phase": "conclusion", - } - entry["pending_return"] = pending + pending_head = pending.get("head_sha") if ( + isinstance(pending, dict) + and isinstance(pending.get("head_sha"), str) + and isinstance(pending.get("text"), str) + and isinstance(pending.get("request_revision"), int) + ) else None + # Finish the immutable publication before a newer observation can + # replace it, and record the return against its original full head. + for return_head in dict.fromkeys((pending_head, head)): + if (not return_head or return_head == entry.get("accepted_head_sha") + or any(r["head_sha"] == return_head for r in returns)): + continue + update_id = f"issue-fix-correction:{operation_id}:{return_head}" + if return_head != pending_head: + event = entry.get("source_event") or {} + checks = observation.get("checks") or {} + text = (f"Maintainer correction {entry.get('correction_kind')} " + f"({event.get('kind')} {event.get('id')}) applied in " + f"{observation.get('permalink')} at head {return_head[:12]}; " + f"PR state {observation.get('state')}, checks {checks.get('aggregate')}; " + f"todo {entry['todo_id']} done.") + # Persist the entire publication before report(), including + # the first conclusion's absence of an update_id. + pending = { + "schema_version": "issue_fix_maintainer_correction_publication_v1", + "head_sha": return_head, + "update_id": update_id if any( + r["phase"] == "conclusion" + for r in reply_status(origin_root, origin) + ) else None, + "text": text, + "request_revision": view["revision"], + "phase": "conclusion", + } + entry["pending_return"] = pending + upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) + result = report(origin_root, identity["goal_id"], identity["agent_id"], + origin["request_id"], str(pending.get("phase") or "conclusion"), + str(pending["text"]), + registry=Path(origin["registry_path"]), + update_id=pending.get("update_id"), workspace=project, + request_revision=pending["request_revision"], + caller_goal_ref=goal_ref) + entry["pending_return"] = None + returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), + "enqueued_at": now_utc_iso(), "head_sha": return_head}) upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) - result = report(origin_root, identity["goal_id"], identity["agent_id"], - origin["request_id"], str(pending.get("phase") or "conclusion"), - str(pending["text"]), - registry=Path(origin["registry_path"]), - update_id=pending.get("update_id"), workspace=project, - request_revision=pending["request_revision"], - caller_goal_ref=goal_ref) - entry.pop("pending_return", None) - returns.append({"update_id": update_id, "result_key": result.get("result_key", "conclusion"), - "enqueued_at": now_utc_iso(), "head_sha": head}) - upsert_issue_fix_pr_lifecycle_ledger_jsonl(ledger_path, payload) statuses = {r.get("result_key", r["phase"]): r["status"] for r in reply_status(Path(origin["runtime_root"]), origin)} payload["return_delivery"] = [{"operation_id": e["operation_id"], diff --git a/tests/capabilities/test_issue_fix_correction_origin_return.py b/tests/capabilities/test_issue_fix_correction_origin_return.py index 3d8f3ca84f..a6347dff79 100644 --- a/tests/capabilities/test_issue_fix_correction_origin_return.py +++ b/tests/capabilities/test_issue_fix_correction_origin_return.py @@ -362,18 +362,25 @@ def test_invalid_request_entry_writes_nothing(h, damage, error): assert not default_issue_fix_domain_state_ledger_path(project=h.project, goal_id=GOAL_ID).exists() -@pytest.mark.parametrize("change", ["checks", "revision"]) -def test_return_ledger_failure_replays_the_immutable_publication(h, monkeypatch, change): +@pytest.mark.parametrize("change,prior_return", [ + ("checks", False), ("revision", False), ("head", False), ("head", True), +]) +def test_return_ledger_failure_replays_the_immutable_publication(h, monkeypatch, change, prior_return): from loopx.capabilities.issue_fix import correction_settlement event = _event("313") packet = h.run_json(metadata=observation([event]), correction=_correction_v1(event), origin=h.request_id) h.mark_done(h.entries()[0]["todo_id"]) + if prior_return: + h.run_json(metadata=observation([event], "d" * 40), correction=None) + prior_replies = set(h.replies()) + prior_returns = h.entries()[0].get("returns", []) packet["observation"]["head_commit_ref"] = "b" * 40 original = correction_settlement.upsert_issue_fix_pr_lifecycle_ledger_jsonl def fail_return_write(path, payload): - if any(entry.get("returns") for entry in payload["maintainer_corrections"]): + if any(any(r["head_sha"] == "b" * 40 for r in entry.get("returns", [])) + for entry in payload["maintainer_corrections"]): raise OSError("return ledger interrupted") return original(path, payload) @@ -381,16 +388,18 @@ def fail_return_write(path, payload): patch.setattr(correction_settlement, "upsert_issue_fix_pr_lifecycle_ledger_jsonl", fail_return_write) with pytest.raises(OSError, match="return ledger interrupted"): settle(h, packet) - assert len(h.replies()) == 1 + assert len(h.replies()) == len(prior_replies) + 1 + published_path, = set(h.replies()) - prior_replies + published_reply = json.loads(published_path.read_text()) pending = h.entries()[0] - assert not pending.get("returns") + assert pending.get("returns", []) == prior_returns publication = pending["pending_return"] assert publication["head_sha"] == "b" * 40 assert "checks PASSING" in publication["text"] retry_observation = observation([event], "b" * 40) if change == "checks": retry_observation["statusCheckRollup"] = [{"name": "focused", "conclusion": "FAILURE"}] - else: + elif change == "revision": view = h.control("inspect") h.control("revise", { "operation_id": "revision-after-publication", @@ -399,9 +408,24 @@ def fail_return_write(path, payload): "brief": {**view["brief"], "purpose": "Validate the changed correction"}, "reason": "The request changed after publication", }) + else: + retry_observation["headRefOid"] = "c" * 40 h.run_json(metadata=retry_observation, correction=None) - assert len(h.replies()) == len(h.entries()[0]["returns"]) == 1 - assert json.loads(h.replies()[0].read_text())["text"] == publication["text"] + assert json.loads(published_path.read_text()) == published_reply + assert published_reply["text"] == publication["text"] + settled = h.entries()[0] + assert settled["pending_return"] is None + returns = settled["returns"] + assert len(h.replies()) == len(returns) == len(prior_returns) + (2 if change == "head" else 1) + recovered = next(r for r in returns if r["head_sha"] == "b" * 40) + assert recovered["update_id"] == f"issue-fix-correction:{pending['operation_id']}:{'b' * 40}" + assert recovered["result_key"] == published_path.stem + if change == "head": + assert [r["head_sha"] for r in returns] == [r["head_sha"] for r in prior_returns] + ["b" * 40, "c" * 40] + h.run_json(metadata=observation([event], "b" * 40), correction=None) + assert h.entries()[0]["returns"] == returns + assert len(h.replies()) == len(returns) if change == "checks": - h.run_json(metadata=observation([event], "c" * 40), correction=None) + # Advance the head without also reactivating a completed checks monitor. + h.run_json(metadata={**retry_observation, "headRefOid": "c" * 40}, correction=None) assert len(h.replies()) == len(h.entries()[0]["returns"]) == 2 From 0bb063333437a56f75c345e8759ef7574060d7a8 Mon Sep 17 00:00:00 2001 From: aihop <4408344+hhyykk@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:34:29 +0800 Subject: [PATCH 11/11] fix(issue-fix): recheck current author before recovering Todo writes Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com> --- loopx/canary/module_metric_baseline.json | 2 +- .../issue_fix/correction_settlement.py | 2 + loopx/capabilities/issue_fix/pr_lifecycle.py | 14 ++++--- ...sue_fix_maintainer_correction_admission.py | 40 +++++++++++++++++++ 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/loopx/canary/module_metric_baseline.json b/loopx/canary/module_metric_baseline.json index cfc4fe9269..fbb9932e77 100644 --- a/loopx/canary/module_metric_baseline.json +++ b/loopx/canary/module_metric_baseline.json @@ -27,7 +27,7 @@ "dict_any_count": 0 }, "loopx/capabilities/issue_fix/pr_lifecycle.py": { - "lines": 2054 + "lines": 2056 }, "loopx/chat_actions.py": { "any_count": 52, diff --git a/loopx/capabilities/issue_fix/correction_settlement.py b/loopx/capabilities/issue_fix/correction_settlement.py index 46adb077da..17b12cffd7 100644 --- a/loopx/capabilities/issue_fix/correction_settlement.py +++ b/loopx/capabilities/issue_fix/correction_settlement.py @@ -27,6 +27,8 @@ _SOURCE_EVENT_REFUSALS = { "source_event_not_observed", "source_event_revision_mismatch", + "self_or_bot_author", + "author_not_maintainer", } diff --git a/loopx/capabilities/issue_fix/pr_lifecycle.py b/loopx/capabilities/issue_fix/pr_lifecycle.py index 3cc0650f9f..febfca6428 100644 --- a/loopx/capabilities/issue_fix/pr_lifecycle.py +++ b/loopx/capabilities/issue_fix/pr_lifecycle.py @@ -784,10 +784,12 @@ def recheck_issue_fix_maintainer_correction_effect_gates( ] if not observed: return "source_event_not_observed" - if not any( - _same_instant(event.get("updated_at"), source_event.get("updated_at")) - for event in observed - ): + current_event = next( + (event for event in observed + if _same_instant(event.get("updated_at"), source_event.get("updated_at"))), + None, + ) + if current_event is None: return "source_event_revision_mismatch" latest = _latest_accepted_revision( issue_fix_maintainer_correction_entries( @@ -800,10 +802,10 @@ def recheck_issue_fix_maintainer_correction_effect_gates( own_time = _timestamp(source_event.get("updated_at")) if latest_time is not None and own_time is not None and own_time < latest_time: return "stale_revision" - if _is_bot_or_self_login(source_event.get("author_login")): + if _is_bot_or_self_login(current_event.get("author_login")): return "self_or_bot_author" if ( - _upper_label(source_event.get("author_association"), "NONE") + _upper_label(current_event.get("author_association"), "NONE") not in MAINTAINER_ASSOCIATIONS ): return "author_not_maintainer" diff --git a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py index af81bcb2a8..a8fa7106f7 100644 --- a/tests/capabilities/test_issue_fix_maintainer_correction_admission.py +++ b/tests/capabilities/test_issue_fix_maintainer_correction_admission.py @@ -443,6 +443,46 @@ def test_t06_terminal_state_refuses_pending_entry_at_effect_time(harness: Harnes assert entries[0]["reason"] == "terminal_pr_state" +@pytest.mark.parametrize("resend_correction", [False, True], ids=["poll", "retry"]) +@pytest.mark.parametrize( + ("login", "association", "reason"), + [ + ("maintainer-a", "NONE", "author_not_maintainer"), + ("automation[bot]", "MEMBER", "self_or_bot_author"), + ], + ids=["non-maintainer", "bot"], +) +def test_pending_correction_rechecks_current_author_before_todo_write( + harness: Harness, resend_correction: bool, login: str, association: str, reason: str, +) -> None: + event = _event("6102") + correction = _correction_v1(event) + faulted = harness.run( + metadata=_metadata([event]), correction=correction, + env={"LOOPX_ISSUE_FIX_FAULT_BEFORE_CORRECTION_TODO": "1"}, check=False, + ) + assert faulted.returncode != 0 + pending = harness.entries()[0] + assert pending["disposition"] == "accepted" and pending["todo_id"] is None + + current_event = _event("6102", login=login, association=association) + refused = harness.run_json( + metadata=_metadata([current_event]), + correction=correction if resend_correction else None, + ) + assert refused["maintainer_correction_settlement"]["settlements"] == [ + {"operation_id": pending["operation_id"], "status": "refused", "reason": reason} + ] + assert harness.correction_todos() == [] + assert harness.entries()[0] == pending + + recovered = harness.run_json(metadata=_metadata([event]), correction=None) + assert recovered["maintainer_correction_settlement"]["settlements"][0]["status"] == "written" + harness.run_json(metadata=_metadata([event]), correction=correction) + assert len(harness.correction_todos()) == 1 + assert harness.entries()[0]["operation_id"] == pending["operation_id"] + + def test_t11_crash_between_ledger_entry_and_todo_write_is_reconciled(harness: Harness) -> None: event = _event("11001") metadata = _metadata([event])