Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2e17a5c329 | ||
|
|
da6fd233f4 | ||
|
|
253269c61b |
@@ -55,12 +55,3 @@ GITEA_MCP_PROFILE=prgs
|
|||||||
# GITEA_MERGER_WORKTREE=/path/to/repo/branches/merge-pr456
|
# GITEA_MERGER_WORKTREE=/path/to/repo/branches/merge-pr456
|
||||||
# GITEA_RECONCILER_WORKTREE=/path/to/repo/branches/reconcile-pr456
|
# GITEA_RECONCILER_WORKTREE=/path/to/repo/branches/reconcile-pr456
|
||||||
# GITEA_ACTIVE_WORKTREE=/path/to/repo/branches/session-override
|
# GITEA_ACTIVE_WORKTREE=/path/to/repo/branches/session-override
|
||||||
|
|
||||||
# Durable HMAC key for irrecoverable decision-lock provenance artifacts (#709 F4).
|
|
||||||
# REQUIRED in production native MCP processes that mint or verify recovery
|
|
||||||
# authorization (reconciler + merger must share the same key). Hex (preferred),
|
|
||||||
# base64, or utf-8 literal (>=16 bytes). Never generate an ephemeral per-process
|
|
||||||
# key — cross-process / post-restart verification would fail closed.
|
|
||||||
# GITEA_IRRECOVERABLE_AUTH_HMAC_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
|
|
||||||
# Optional key version string bound into the signature (for rotation).
|
|
||||||
# GITEA_IRRECOVERABLE_AUTH_HMAC_KEY_VERSION=v1
|
|
||||||
|
|||||||
@@ -134,22 +134,16 @@ _TARGET_BRANCH_SHA_RE = re.compile(
|
|||||||
r"target branch sha\s*:\s*[0-9a-f]{40}",
|
r"target branch sha\s*:\s*[0-9a-f]{40}",
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
# #698: structured proof is rendered in several equivalent shapes —
|
|
||||||
# `workflow_hash: abc...`, `workflow_hash=abc...`, or JSON
|
|
||||||
# `"workflow_hash": "abc..."`. Recognize all of them; demanding one exact
|
|
||||||
# punctuation style rejects legitimate structured workflow-load proof.
|
|
||||||
_WORKFLOW_LOAD_HELPER_RE = re.compile(
|
_WORKFLOW_LOAD_HELPER_RE = re.compile(
|
||||||
r"workflow[-_ ]load[-_ ]helper[-_ ]result\s*[:=]",
|
r"workflow[- ]load helper result\s*:",
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
_WORKFLOW_LOAD_HASH_RE = re.compile(
|
_WORKFLOW_LOAD_HASH_RE = re.compile(
|
||||||
r"workflow[-_ ]load[-_ ]helper[-_ ]result[\s\S]{0,400}?"
|
r"workflow[- ]load helper result[\s\S]{0,400}?workflow[_ ]hash\s*:\s*[0-9a-f]{12}",
|
||||||
r"workflow[_ ]hash\"?\s*[:=]\s*\"?[0-9a-f]{12}",
|
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
_WORKFLOW_LOAD_BOUNDARY_RE = re.compile(
|
_WORKFLOW_LOAD_BOUNDARY_RE = re.compile(
|
||||||
r"workflow[-_ ]load[-_ ]helper[-_ ]result[\s\S]{0,400}?"
|
r"workflow[- ]load helper result[\s\S]{0,400}?boundary[_ ]status\s*:\s*(?:clean|violation)",
|
||||||
r"boundary[_ ]status\"?\s*[:=]\s*\"?(?:clean|violation)",
|
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
_WORKFLOW_FILE_VIEW_NARRATIVE_RE = re.compile(
|
_WORKFLOW_FILE_VIEW_NARRATIVE_RE = re.compile(
|
||||||
@@ -250,59 +244,6 @@ def _normalize_task_kind(task_kind: str | None) -> str:
|
|||||||
return _TASK_KIND_ALIASES.get(raw, raw)
|
return _TASK_KIND_ALIASES.get(raw, raw)
|
||||||
|
|
||||||
|
|
||||||
def _iter_action_entries(action_log: list | None) -> list[dict]:
|
|
||||||
"""Yield only structured (dict) action-log entries (#698).
|
|
||||||
|
|
||||||
Callers must never crash on malformed entries (strings, numbers, null)
|
|
||||||
that reach the validator from LLM-composed or partially parsed logs;
|
|
||||||
:func:`sanitize_action_log` reports them separately.
|
|
||||||
"""
|
|
||||||
return [e for e in (action_log or []) if isinstance(e, dict)]
|
|
||||||
|
|
||||||
|
|
||||||
def sanitize_action_log(
|
|
||||||
action_log: list | None,
|
|
||||||
) -> tuple[list[dict], list[dict[str, str]]]:
|
|
||||||
"""Split an action log into structured entries and sanitized findings (#698).
|
|
||||||
|
|
||||||
Malformed entries become clear, sanitized ``warning`` findings — the
|
|
||||||
offending value's content is never echoed back (only its position and
|
|
||||||
type), so secrets or garbage in a broken log cannot leak into validation
|
|
||||||
errors, and validation itself proceeds without secondary exceptions.
|
|
||||||
"""
|
|
||||||
if action_log is None:
|
|
||||||
return [], []
|
|
||||||
if not isinstance(action_log, (list, tuple)):
|
|
||||||
return [], [
|
|
||||||
validator_finding(
|
|
||||||
"shared.action_log_malformed",
|
|
||||||
"downgrade",
|
|
||||||
"Action log",
|
|
||||||
"action_log is not a list of structured entries "
|
|
||||||
f"(got {type(action_log).__name__}); it was ignored",
|
|
||||||
"pass action_log as a list of dict entries",
|
|
||||||
)
|
|
||||||
]
|
|
||||||
entries: list[dict] = []
|
|
||||||
findings: list[dict[str, str]] = []
|
|
||||||
for index, entry in enumerate(action_log):
|
|
||||||
if isinstance(entry, dict):
|
|
||||||
entries.append(entry)
|
|
||||||
continue
|
|
||||||
findings.append(
|
|
||||||
validator_finding(
|
|
||||||
"shared.action_log_malformed",
|
|
||||||
"downgrade",
|
|
||||||
"Action log",
|
|
||||||
f"action_log entry {index} is not a structured mapping "
|
|
||||||
f"(got {type(entry).__name__}); the entry was ignored",
|
|
||||||
"repair the malformed action_log entry or drop it before "
|
|
||||||
"revalidating",
|
|
||||||
)
|
|
||||||
)
|
|
||||||
return entries, findings
|
|
||||||
|
|
||||||
|
|
||||||
def validator_finding(
|
def validator_finding(
|
||||||
rule_id: str,
|
rule_id: str,
|
||||||
severity: str,
|
severity: str,
|
||||||
@@ -480,7 +421,7 @@ def _rule_shared_canonical_comment_post_claim(
|
|||||||
rejected_in_report = bool(_CANONICAL_VALIDATION_REJECTED_RE.search(text))
|
rejected_in_report = bool(_CANONICAL_VALIDATION_REJECTED_RE.search(text))
|
||||||
rejected_in_log = False
|
rejected_in_log = False
|
||||||
if action_log:
|
if action_log:
|
||||||
for entry in _iter_action_entries(action_log):
|
for entry in action_log:
|
||||||
validation = entry.get("canonical_comment_validation") or {}
|
validation = entry.get("canonical_comment_validation") or {}
|
||||||
if validation.get("allowed") is False:
|
if validation.get("allowed") is False:
|
||||||
rejected_in_log = True
|
rejected_in_log = True
|
||||||
@@ -534,13 +475,9 @@ def _rule_reviewer_vague_mutations_none(
|
|||||||
action_log: list[dict] | None = None,
|
action_log: list[dict] | None = None,
|
||||||
mutations_observed: bool = False,
|
mutations_observed: bool = False,
|
||||||
) -> list[dict[str, str]]:
|
) -> list[dict[str, str]]:
|
||||||
# #698: infer review mutations only from authoritative evidence — an
|
|
||||||
# entry proves a mutation only when it affirmatively records
|
|
||||||
# performed=true and was not gated. Read-only diagnostics and pre-API
|
|
||||||
# rejections (entries without a performed flag) are not mutations.
|
|
||||||
performed = any(
|
performed = any(
|
||||||
e.get("performed") is True and not e.get("gated_rejected")
|
e.get("performed") is not False and not e.get("gated_rejected")
|
||||||
for e in _iter_action_entries(action_log)
|
for e in (action_log or [])
|
||||||
)
|
)
|
||||||
if not (mutations_observed or performed):
|
if not (mutations_observed or performed):
|
||||||
return []
|
return []
|
||||||
@@ -599,7 +536,7 @@ def _rule_reviewer_git_fetch_readonly(
|
|||||||
text = report_text or ""
|
text = report_text or ""
|
||||||
fetch_observed = any(
|
fetch_observed = any(
|
||||||
_GIT_FETCH_RE.search(str(e.get("command") or e.get("action") or ""))
|
_GIT_FETCH_RE.search(str(e.get("command") or e.get("action") or ""))
|
||||||
for e in _iter_action_entries(action_log)
|
for e in (action_log or [])
|
||||||
) or _GIT_FETCH_RE.search(text)
|
) or _GIT_FETCH_RE.search(text)
|
||||||
if not fetch_observed:
|
if not fetch_observed:
|
||||||
return []
|
return []
|
||||||
@@ -1040,7 +977,7 @@ def _rule_reviewer_target_branch_freshness(
|
|||||||
fields = _handoff_fields(text)
|
fields = _handoff_fields(text)
|
||||||
fetch_reported = bool(_GIT_FETCH_RE.search(text)) or any(
|
fetch_reported = bool(_GIT_FETCH_RE.search(text)) or any(
|
||||||
_GIT_FETCH_RE.search(str(e.get("command") or e.get("action") or ""))
|
_GIT_FETCH_RE.search(str(e.get("command") or e.get("action") or ""))
|
||||||
for e in _iter_action_entries(action_log)
|
for e in (action_log or [])
|
||||||
)
|
)
|
||||||
target_sha_reported = bool(_TARGET_BRANCH_SHA_RE.search(text)) or any(
|
target_sha_reported = bool(_TARGET_BRANCH_SHA_RE.search(text)) or any(
|
||||||
"target branch" in key and "sha" in key and _FULL_SHA_RE.search(value)
|
"target branch" in key and "sha" in key and _FULL_SHA_RE.search(value)
|
||||||
@@ -1798,12 +1735,6 @@ def assess_final_report_validator(
|
|||||||
checks: dict[str, Any] = {}
|
checks: dict[str, Any] = {}
|
||||||
findings: list[dict[str, str]] = []
|
findings: list[dict[str, str]] = []
|
||||||
|
|
||||||
# #698: malformed action_log data must never crash validation with a
|
|
||||||
# secondary exception; malformed entries surface as sanitized findings.
|
|
||||||
sanitized_action_log, action_log_findings = sanitize_action_log(action_log)
|
|
||||||
action_log = sanitized_action_log
|
|
||||||
findings.extend(action_log_findings)
|
|
||||||
|
|
||||||
if normalized_kind == "issue_filing" and issue_filing_lock is not None:
|
if normalized_kind == "issue_filing" and issue_filing_lock is not None:
|
||||||
checks["issue_filing"] = assess_issue_filing_final_report(
|
checks["issue_filing"] = assess_issue_filing_final_report(
|
||||||
report_text,
|
report_text,
|
||||||
@@ -1832,23 +1763,7 @@ def assess_final_report_validator(
|
|||||||
}
|
}
|
||||||
|
|
||||||
for rule in _RULES_BY_TASK.get(normalized_kind, ()):
|
for rule in _RULES_BY_TASK.get(normalized_kind, ()):
|
||||||
try:
|
findings.extend(_call_rule(rule, report_text, normalized_kind, rule_kwargs))
|
||||||
findings.extend(
|
|
||||||
_call_rule(rule, report_text, normalized_kind, rule_kwargs)
|
|
||||||
)
|
|
||||||
except Exception as exc: # #698: fail closed with a sanitized error
|
|
||||||
findings.append(
|
|
||||||
validator_finding(
|
|
||||||
"shared.validator_rule_error",
|
|
||||||
"block",
|
|
||||||
"Validator",
|
|
||||||
f"validator rule '{getattr(rule, '__name__', 'unknown')}' "
|
|
||||||
f"failed with {type(exc).__name__} (details withheld; "
|
|
||||||
"sanitized)",
|
|
||||||
"file a validator defect with the rule name; do not "
|
|
||||||
"bypass final-report validation",
|
|
||||||
)
|
|
||||||
)
|
|
||||||
|
|
||||||
grade, blocked, downgraded = _aggregate_grade(findings)
|
grade, blocked, downgraded = _aggregate_grade(findings)
|
||||||
reasons = [f"{f['rule_id']}: {f['reason']}" for f in findings]
|
reasons = [f"{f['rule_id']}: {f['reason']}" for f in findings]
|
||||||
|
|||||||
+26
-1507
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
+1
-196
@@ -36,24 +36,7 @@ KIND_REVIEW_DRAFT = "review_draft"
|
|||||||
# other session proofs; a contaminated session fails closed on gated mutations
|
# other session proofs; a contaminated session fails closed on gated mutations
|
||||||
# until a reconciler audits and clears it.
|
# until a reconciler audits and clears it.
|
||||||
KIND_STABLE_BRANCH_CONTAMINATION = "stable_branch_contamination"
|
KIND_STABLE_BRANCH_CONTAMINATION = "stable_branch_contamination"
|
||||||
# #709: archive prior terminal decision ledgers instead of silent overwrite.
|
|
||||||
KIND_DECISION_LOCK_ARCHIVE = "review_decision_lock_archive"
|
|
||||||
# #709: post-merge cleanup/audit reconciliation-required durable record.
|
|
||||||
KIND_POST_MERGE_DECISION_RECOVERY = "post_merge_decision_recovery"
|
|
||||||
# #709: truthful record when historical terminal evidence is irrecoverably gone.
|
|
||||||
KIND_IRRECOVERABLE_DECISION_PROVENANCE = "irrecoverable_decision_provenance"
|
|
||||||
# #709 F1: server-side non-forgeable authorization artifact for recovery.
|
|
||||||
KIND_IRRECOVERABLE_PROVENANCE_AUTH = "irrecoverable_provenance_authorization"
|
|
||||||
|
|
||||||
# Kinds that must survive the default session-state TTL (forensic / recovery).
|
|
||||||
RECOVERY_CRITICAL_KINDS = frozenset(
|
|
||||||
{
|
|
||||||
KIND_DECISION_LOCK_ARCHIVE,
|
|
||||||
KIND_POST_MERGE_DECISION_RECOVERY,
|
|
||||||
KIND_IRRECOVERABLE_DECISION_PROVENANCE,
|
|
||||||
KIND_IRRECOVERABLE_PROVENANCE_AUTH,
|
|
||||||
}
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
_SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+")
|
_SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+")
|
||||||
@@ -287,11 +270,7 @@ def identity_match_reasons(
|
|||||||
reasons.append("session state missing recorded_at timestamp (fail closed)")
|
reasons.append("session state missing recorded_at timestamp (fail closed)")
|
||||||
else:
|
else:
|
||||||
age = _now_utc() - recorded_at
|
age = _now_utc() - recorded_at
|
||||||
kind = (record.get("kind") or "").strip()
|
if age > timedelta(hours=ttl_hours()):
|
||||||
ttl_exempt = kind in RECOVERY_CRITICAL_KINDS or bool(
|
|
||||||
record.get("recovery_critical")
|
|
||||||
)
|
|
||||||
if age > timedelta(hours=ttl_hours()) and not ttl_exempt:
|
|
||||||
reasons.append(
|
reasons.append(
|
||||||
f"session state expired after {ttl_hours():g}h (fail closed)"
|
f"session state expired after {ttl_hours():g}h (fail closed)"
|
||||||
)
|
)
|
||||||
@@ -468,177 +447,3 @@ def clear_state(
|
|||||||
profile_identity=profile_identity,
|
profile_identity=profile_identity,
|
||||||
state_dir=state_dir,
|
state_dir=state_dir,
|
||||||
)
|
)
|
||||||
|
|
||||||
def list_decision_lock_profile_identities(
|
|
||||||
state_dir: str | None = None,
|
|
||||||
) -> list[str]:
|
|
||||||
"""Return profile identities that have a durable review_decision_lock file (#709).
|
|
||||||
|
|
||||||
Filename form: ``review_decision_lock-<profile>.json`` (see ``state_key``).
|
|
||||||
Does not validate TTL or identity — callers must load via ``load_state``.
|
|
||||||
"""
|
|
||||||
root = (state_dir or default_state_dir()).strip()
|
|
||||||
if not root or not os.path.isdir(root):
|
|
||||||
return []
|
|
||||||
prefix = f"{_sanitize_segment(KIND_DECISION_LOCK)}-"
|
|
||||||
suffix = ".json"
|
|
||||||
found: list[str] = []
|
|
||||||
try:
|
|
||||||
names = os.listdir(root)
|
|
||||||
except OSError:
|
|
||||||
return []
|
|
||||||
for name in names:
|
|
||||||
if not name.startswith(prefix) or not name.endswith(suffix):
|
|
||||||
continue
|
|
||||||
if name.endswith(".lock"):
|
|
||||||
continue
|
|
||||||
mid = name[len(prefix) : -len(suffix)]
|
|
||||||
if mid:
|
|
||||||
found.append(mid)
|
|
||||||
return sorted(set(found))
|
|
||||||
|
|
||||||
|
|
||||||
def load_state_for_profile(
|
|
||||||
*,
|
|
||||||
kind: str,
|
|
||||||
profile_identity: str,
|
|
||||||
remote: str | None = None,
|
|
||||||
org: str | None = None,
|
|
||||||
repo: str | None = None,
|
|
||||||
state_dir: str | None = None,
|
|
||||||
skip_identity_match: bool = False,
|
|
||||||
enforce_repo_scope: bool = True,
|
|
||||||
) -> dict[str, Any] | None:
|
|
||||||
"""Load durable state for an explicit profile identity (#709 cross-profile).
|
|
||||||
|
|
||||||
When *skip_identity_match* is True, still requires the file's recorded
|
|
||||||
profile_identity to equal the requested profile (anti-stomp), but does not
|
|
||||||
require the *active* session identity to match — needed so a merger can
|
|
||||||
inspect a reviewer lock after merge.
|
|
||||||
|
|
||||||
#709 F3: remote/org/repo filter reasons are **enforced** (not merely
|
|
||||||
computed). When *enforce_repo_scope* is True (default) and the caller
|
|
||||||
supplies remote/org/repo, mismatches fail closed with ``None``.
|
|
||||||
"""
|
|
||||||
# #709 F3: refuse traversal / malformed profile identities.
|
|
||||||
try:
|
|
||||||
from irrecoverable_provenance import assess_profile_path_identity
|
|
||||||
|
|
||||||
path_gate = assess_profile_path_identity(profile_identity)
|
|
||||||
if not path_gate.get("valid"):
|
|
||||||
return None
|
|
||||||
except Exception:
|
|
||||||
# Module may be mid-import in edge bootstraps; fall through to
|
|
||||||
# conservative checks below.
|
|
||||||
raw = str(profile_identity or "")
|
|
||||||
if ".." in raw or "/" in raw or "\\" in raw or "\x00" in raw:
|
|
||||||
return None
|
|
||||||
|
|
||||||
profile = current_profile_identity(profile_identity=profile_identity)
|
|
||||||
root = _ensure_state_dir(state_dir)
|
|
||||||
# Refuse symlink escape of the state root (#709 F3).
|
|
||||||
try:
|
|
||||||
real_root = os.path.realpath(root)
|
|
||||||
path = state_file_path(
|
|
||||||
kind=kind,
|
|
||||||
remote=remote,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
profile_identity=profile,
|
|
||||||
state_dir=root,
|
|
||||||
)
|
|
||||||
real_path = os.path.realpath(path) if os.path.exists(path) else path
|
|
||||||
if os.path.exists(path) and not str(real_path).startswith(
|
|
||||||
str(real_root) + os.sep
|
|
||||||
) and str(real_path) != str(real_root):
|
|
||||||
return None
|
|
||||||
except OSError:
|
|
||||||
return None
|
|
||||||
|
|
||||||
path = state_file_path(
|
|
||||||
kind=kind,
|
|
||||||
remote=remote,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
profile_identity=profile,
|
|
||||||
state_dir=root,
|
|
||||||
)
|
|
||||||
lock_path = f"{path}.lock"
|
|
||||||
with _exclusive_file_lock(lock_path):
|
|
||||||
envelope = _read_json(path)
|
|
||||||
if not envelope:
|
|
||||||
return None
|
|
||||||
payload = envelope.get("payload")
|
|
||||||
if not isinstance(payload, dict):
|
|
||||||
return None
|
|
||||||
merged = dict(payload)
|
|
||||||
for key in (
|
|
||||||
"kind",
|
|
||||||
"remote",
|
|
||||||
"org",
|
|
||||||
"repo",
|
|
||||||
"profile_identity",
|
|
||||||
"session_profile_lock",
|
|
||||||
"recorded_at",
|
|
||||||
"updated_at",
|
|
||||||
"writer_pid",
|
|
||||||
):
|
|
||||||
if key in envelope and key not in merged:
|
|
||||||
merged[key] = envelope[key]
|
|
||||||
stored = (merged.get("profile_identity") or "").strip()
|
|
||||||
if stored and stored != profile:
|
|
||||||
return None
|
|
||||||
if skip_identity_match:
|
|
||||||
# Enforce remote/org/repo when caller provides them (#709 F3).
|
|
||||||
# Drop only *active session* profile-identity mismatches; keep
|
|
||||||
# expiry, spoof, and repository-scope reasons.
|
|
||||||
scope_remote = remote if enforce_repo_scope else None
|
|
||||||
scope_org = org if enforce_repo_scope else None
|
|
||||||
scope_repo = repo if enforce_repo_scope else None
|
|
||||||
reasons = identity_match_reasons(
|
|
||||||
merged,
|
|
||||||
remote=scope_remote,
|
|
||||||
org=scope_org,
|
|
||||||
repo=scope_repo,
|
|
||||||
profile_identity=stored or profile,
|
|
||||||
)
|
|
||||||
filtered = [
|
|
||||||
r
|
|
||||||
for r in reasons
|
|
||||||
if "profile identity mismatch" not in r
|
|
||||||
or (stored and stored != profile)
|
|
||||||
]
|
|
||||||
# When caller requested a specific remote/org/repo, also fail if the
|
|
||||||
# stored record lacks those identity fields entirely (legacy incomplete).
|
|
||||||
if enforce_repo_scope:
|
|
||||||
for field, want in (
|
|
||||||
("remote", remote),
|
|
||||||
("org", org),
|
|
||||||
("repo", repo),
|
|
||||||
):
|
|
||||||
want_s = (want or "").strip()
|
|
||||||
if not want_s:
|
|
||||||
continue
|
|
||||||
have = (str(merged.get(field) or "")).strip()
|
|
||||||
if not have:
|
|
||||||
filtered.append(
|
|
||||||
f"session state {field} missing on durable record "
|
|
||||||
f"(expected={want_s!r}; fail closed, #709 F3)"
|
|
||||||
)
|
|
||||||
elif have != want_s:
|
|
||||||
# identity_match_reasons already adds mismatch; ensure kept
|
|
||||||
pass
|
|
||||||
if filtered:
|
|
||||||
return None
|
|
||||||
return merged
|
|
||||||
reasons = identity_match_reasons(
|
|
||||||
merged,
|
|
||||||
remote=remote,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
profile_identity=profile,
|
|
||||||
)
|
|
||||||
if reasons:
|
|
||||||
return None
|
|
||||||
return merged
|
|
||||||
|
|
||||||
|
|||||||
@@ -86,22 +86,6 @@ _WRONG_BRANCH_RE = re.compile(
|
|||||||
r"deleted branch (?:does not match|!=|differs from) (?:merged )?pr head",
|
r"deleted branch (?:does not match|!=|differs from) (?:merged )?pr head",
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
# #698: canonical reviewer lease lifecycle operations are NOT post-merge
|
|
||||||
# cleanup. Releasing a reviewer PR lease (or posting its terminal
|
|
||||||
# phase=released marker) happens after every review — merged or not — and
|
|
||||||
# must never trigger the post-merge branch/worktree cleanup checklist.
|
|
||||||
_LEASE_LIFECYCLE_RE = re.compile(
|
|
||||||
r"(?:gitea_release_reviewer_pr_lease|gitea_abandon_workflow_lease|"
|
|
||||||
r"release[d]? (?:the )?(?:reviewer|workflow) (?:pr )?lease|"
|
|
||||||
r"reviewer (?:pr )?lease release[d]?|"
|
|
||||||
r"lease (?:marker|comment).{0,40}phase\s*[:=]\s*released|"
|
|
||||||
r"phase\s*[:=]\s*released)",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_CLEANUP_MUTATIONS_VALUE_RE = re.compile(
|
|
||||||
r"cleanup mutations\s*:\s*([^\n]+)",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _claims_remote_delete(text: str) -> bool:
|
def _claims_remote_delete(text: str) -> bool:
|
||||||
@@ -220,19 +204,16 @@ def assess_post_merge_cleanup_proof(
|
|||||||
for field in _worktree_cleanup_fields_present(text)
|
for field in _worktree_cleanup_fields_present(text)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
if (remote_delete or worktree_remove) and not (remote_delete or worktree_remove):
|
||||||
|
pass
|
||||||
|
|
||||||
if not remote_delete and not worktree_remove:
|
if not remote_delete and not worktree_remove:
|
||||||
value_match = _CLEANUP_MUTATIONS_VALUE_RE.search(text)
|
cleanup_mutations = re.search(
|
||||||
value = (value_match.group(1).strip() if value_match else "")
|
r"cleanup mutations\s*:\s*(?!none\b)\S",
|
||||||
value_lower = value.lower()
|
text,
|
||||||
substantive = bool(value) and value_lower not in {
|
re.IGNORECASE,
|
||||||
"none", "n/a", "not applicable",
|
|
||||||
}
|
|
||||||
# #698: reviewer lease release / terminal lease markers are lease
|
|
||||||
# lifecycle, not post-merge cleanup — no checklist owed.
|
|
||||||
lease_lifecycle_only = substantive and bool(
|
|
||||||
_LEASE_LIFECYCLE_RE.search(value)
|
|
||||||
)
|
)
|
||||||
if substantive and not lease_lifecycle_only:
|
if cleanup_mutations:
|
||||||
reasons.append(
|
reasons.append(
|
||||||
"cleanup mutations reported without post-merge cleanup proof checklist"
|
"cleanup mutations reported without post-merge cleanup proof checklist"
|
||||||
)
|
)
|
||||||
|
|||||||
+6
-67
@@ -403,37 +403,10 @@ _REVIEWER_ACTIVE_RE = re.compile(
|
|||||||
r"whether any reviewer was active\s*:\s*(yes|no|true|false)",
|
r"whether any reviewer was active\s*:\s*(yes|no|true|false)",
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
# #698 phase detection for phase-specific head proofs.
|
|
||||||
_NO_REVIEWED_HEAD_RE = re.compile(
|
|
||||||
r"(?:reviewed head sha|candidate head sha)\s*:\s*none\b",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_VERDICT_RECORDED_RE = re.compile(
|
|
||||||
r"review decision\s*:\s*(?:approve[d]?|request[_ ]changes)\b"
|
|
||||||
r"|review_status\s*:\s*(?:approved|request_changes)\b"
|
|
||||||
r"|terminal review mutation\s*:\s*(?!none\b)\S",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_MERGE_ATTEMPTED_RE = re.compile(
|
|
||||||
r"merge result\s*:\s*(?:merged|success|performed|failed|attempted)\b"
|
|
||||||
r"|merge mutations\s*:\s*(?!none\b|not applicable\b)\S",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_VALIDATION_STARTED_RE = re.compile(
|
|
||||||
r"validation\s*:\s*(?!none\b|not run\b|not applicable\b|not started\b)"
|
|
||||||
r"[^\n]*(?:pass|fail|ran|executed|\d+\s+passed)",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
|
def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
|
||||||
"""Final-report proof for reviewed vs live head SHAs (#399 AC 6).
|
"""Final-report proof for reviewed vs live head SHAs (#399 AC 6)."""
|
||||||
|
|
||||||
#698: head proofs are phase-specific. A legitimately blocked run that
|
|
||||||
never began validation (no reviewed head, no formal verdict, no merge)
|
|
||||||
owes none of them; approval-time and merge-time live-head proofs are
|
|
||||||
owed only once the corresponding phase actually begins.
|
|
||||||
"""
|
|
||||||
text = report_text or ""
|
text = report_text or ""
|
||||||
reasons: list[str] = []
|
reasons: list[str] = []
|
||||||
reviewed = _normalize_sha(_REVIEWED_HEAD_RE.search(text).group(1) if _REVIEWED_HEAD_RE.search(text) else None)
|
reviewed = _normalize_sha(_REVIEWED_HEAD_RE.search(text).group(1) if _REVIEWED_HEAD_RE.search(text) else None)
|
||||||
@@ -449,49 +422,15 @@ def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
|
|||||||
)
|
)
|
||||||
push_during = _PUSH_DURING_VALIDATION_RE.search(text)
|
push_during = _PUSH_DURING_VALIDATION_RE.search(text)
|
||||||
|
|
||||||
# Phase detection from the report's own claims.
|
if not reviewed:
|
||||||
no_head_stated = bool(_NO_REVIEWED_HEAD_RE.search(text))
|
|
||||||
verdict_recorded = bool(_VERDICT_RECORDED_RE.search(text))
|
|
||||||
merge_attempted = bool(_MERGE_ATTEMPTED_RE.search(text))
|
|
||||||
validation_started = bool(reviewed) or bool(_VALIDATION_STARTED_RE.search(text))
|
|
||||||
blocked_before_validation = (
|
|
||||||
no_head_stated
|
|
||||||
and not reviewed
|
|
||||||
and not verdict_recorded
|
|
||||||
and not merge_attempted
|
|
||||||
and not validation_started
|
|
||||||
)
|
|
||||||
|
|
||||||
if blocked_before_validation:
|
|
||||||
return {
|
|
||||||
"proven": True,
|
|
||||||
"block": False,
|
|
||||||
"reasons": [],
|
|
||||||
"reviewed_head_sha": None,
|
|
||||||
"live_head_sha_before_approval": None,
|
|
||||||
"live_head_sha_before_merge": None,
|
|
||||||
"push_during_validation": (
|
|
||||||
push_during.group(1).lower() if push_during else None
|
|
||||||
),
|
|
||||||
"phase": "blocked_before_validation",
|
|
||||||
}
|
|
||||||
|
|
||||||
if not reviewed and not no_head_stated:
|
|
||||||
# The head must always be STATED — either a SHA or an explicit
|
|
||||||
# 'none'. Silence is not a phase claim and fails closed.
|
|
||||||
reasons.append(
|
|
||||||
"reviewed head SHA not stated in final report "
|
|
||||||
"(state the SHA or an explicit 'none')"
|
|
||||||
)
|
|
||||||
elif not reviewed and (validation_started or verdict_recorded or merge_attempted):
|
|
||||||
reasons.append("reviewed head SHA not stated in final report")
|
reasons.append("reviewed head SHA not stated in final report")
|
||||||
if verdict_recorded and not live_approval:
|
if not live_approval:
|
||||||
reasons.append("final live head SHA before approval not stated")
|
reasons.append("final live head SHA before approval not stated")
|
||||||
if merge_attempted and not live_merge:
|
if not live_merge:
|
||||||
reasons.append("final live head SHA before merge not stated")
|
reasons.append("final live head SHA before merge not stated")
|
||||||
if validation_started and not push_during:
|
if not push_during:
|
||||||
reasons.append("whether push occurred during validation not stated")
|
reasons.append("whether push occurred during validation not stated")
|
||||||
if reviewed and live_approval and reviewed != live_approval:
|
elif reviewed and live_approval and reviewed != live_approval:
|
||||||
reasons.append("live head before approval differs from reviewed head SHA")
|
reasons.append("live head before approval differs from reviewed head SHA")
|
||||||
elif reviewed and live_merge and reviewed != live_merge:
|
elif reviewed and live_merge and reviewed != live_merge:
|
||||||
reasons.append("live head before merge differs from reviewed head SHA")
|
reasons.append("live head before merge differs from reviewed head SHA")
|
||||||
|
|||||||
@@ -25,13 +25,8 @@ _REVIEWED_HEAD_RE = re.compile(
|
|||||||
r"(?:pinned reviewed head|reviewed head sha)\s*:\s*([0-9a-f]{7,40})",
|
r"(?:pinned reviewed head|reviewed head sha)\s*:\s*([0-9a-f]{7,40})",
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
# #698: validation pass proof appears in several legitimate shapes —
|
|
||||||
# "Validation: pass", "Validation: focused 50 passed; full 2665 passed",
|
|
||||||
# or structured "validation_status: pass". Accept pass evidence anywhere in
|
|
||||||
# the Validation field's value, not only as its first token.
|
|
||||||
_VALIDATION_PASS_RE = re.compile(
|
_VALIDATION_PASS_RE = re.compile(
|
||||||
r"validation(?:_status)?\s*:[^\n]{0,300}?"
|
r"validation\s*:\s*(?:pass|passed|strong|ok|green)",
|
||||||
r"(?:\bpass(?:ed)?\b|\bstrong\b|\bok\b|\bgreen\b|\d+\s+passed)",
|
|
||||||
re.IGNORECASE,
|
re.IGNORECASE,
|
||||||
)
|
)
|
||||||
_MERGED_CLAIM_RE = re.compile(
|
_MERGED_CLAIM_RE = re.compile(
|
||||||
|
|||||||
+9
-36
@@ -858,16 +858,9 @@ _WALKTHROUGH_ARTIFACT_RE = re.compile(r"walkthrough\.md", re.I)
|
|||||||
|
|
||||||
|
|
||||||
def _performed_file_mutations(action_log: list[dict] | None) -> list[dict]:
|
def _performed_file_mutations(action_log: list[dict] | None) -> list[dict]:
|
||||||
"""Return performed local file mutations, excluding gated rejections.
|
"""Return performed local file mutations, excluding gated rejections."""
|
||||||
|
|
||||||
Non-dict entries (malformed JSON, LLM mistakes) are ignored instead of
|
|
||||||
raising ``AttributeError`` (#698): a malformed ledger entry can never be
|
|
||||||
authoritative mutation evidence.
|
|
||||||
"""
|
|
||||||
performed: list[dict] = []
|
performed: list[dict] = []
|
||||||
for entry in action_log or []:
|
for entry in action_log or []:
|
||||||
if not isinstance(entry, dict):
|
|
||||||
continue
|
|
||||||
if entry.get("gated_rejected") or entry.get("performed") is False:
|
if entry.get("gated_rejected") or entry.get("performed") is False:
|
||||||
continue
|
continue
|
||||||
action = (entry.get("action") or "").strip().lower()
|
action = (entry.get("action") or "").strip().lower()
|
||||||
@@ -2235,31 +2228,24 @@ HANDOFF_REVIEW_MUTATION_FIELDS = (
|
|||||||
)
|
)
|
||||||
|
|
||||||
HANDOFF_ROLE_FIELDS = {
|
HANDOFF_ROLE_FIELDS = {
|
||||||
# #698: the review/merger required-field sets must stay aligned with the
|
|
||||||
# canonical schema (skills/llm-project-workflow/schemas/
|
|
||||||
# review-merge-final-report.md). The schema explicitly FORBIDS the legacy
|
|
||||||
# fields 'Pinned reviewed head', 'Scratch worktree used', and 'Workspace
|
|
||||||
# mutations' — a validator must never demand a field the schema bans.
|
|
||||||
"review": (
|
"review": (
|
||||||
("Selected PR", ("selected pr",)),
|
("Selected PR", ("selected pr",)),
|
||||||
("Reviewer eligibility", ("reviewer eligibility", "eligibility")),
|
("Reviewer eligibility", ("reviewer eligibility", "eligibility")),
|
||||||
("Reviewed head SHA", ("reviewed head sha", "candidate head sha")),
|
("Pinned reviewed head", ("pinned reviewed head", "pinned head")),
|
||||||
("Review worktree path", ("review worktree path", "worktree path",
|
("Worktree path", ("worktree path", "starting worktree path")),
|
||||||
"starting worktree path")),
|
("Worktree dirty", ("worktree dirty", "whether worktree was dirty")),
|
||||||
("Review worktree dirty", ("review worktree dirty", "worktree dirty",
|
("Scratch worktree used", ("scratch worktree used", "scratch clone used",
|
||||||
"whether worktree was dirty")),
|
"scratch worktree")),
|
||||||
("Unrelated local mutations", ("unrelated local mutations",
|
("Unrelated local mutations", ("unrelated local mutations",
|
||||||
"unrelated files modified",
|
"unrelated files modified")),
|
||||||
"file edits by reviewer")),
|
|
||||||
("Review decision", ("review decision", "decision")),
|
("Review decision", ("review decision", "decision")),
|
||||||
("Merge result", ("merge result",)),
|
("Merge result", ("merge result",)),
|
||||||
("Linked issue status", ("linked issue status", "linked issue")),
|
("Linked issue status", ("linked issue status", "linked issue")),
|
||||||
("Cleanup status", ("cleanup status", "cleanup")),
|
("Cleanup status", ("cleanup status", "cleanup")),
|
||||||
("Safe next action", ("safe next action", "next")),
|
|
||||||
) + HANDOFF_REVIEW_MUTATION_FIELDS,
|
) + HANDOFF_REVIEW_MUTATION_FIELDS,
|
||||||
"merger": (
|
"merger": (
|
||||||
("Selected PR", ("selected pr",)),
|
("Selected PR", ("selected pr",)),
|
||||||
("Reviewed head SHA", ("reviewed head sha", "candidate head sha")),
|
("Pinned reviewed head", ("pinned reviewed head", "pinned head")),
|
||||||
("Active profile", ("active profile",)),
|
("Active profile", ("active profile",)),
|
||||||
("Role kind", ("role kind",)),
|
("Role kind", ("role kind",)),
|
||||||
("Merge capability source", ("merge capability source",)),
|
("Merge capability source", ("merge capability source",)),
|
||||||
@@ -2447,22 +2433,9 @@ def assess_controller_handoff(report_text, role=None, local_edits=False):
|
|||||||
# Issue #320: reviewer and merger handoffs use the precise mutation categories
|
# Issue #320: reviewer and merger handoffs use the precise mutation categories
|
||||||
# in HANDOFF_REVIEW_MUTATION_FIELDS instead of the legacy ambiguous
|
# in HANDOFF_REVIEW_MUTATION_FIELDS instead of the legacy ambiguous
|
||||||
# "Workspace mutations" field, which is rejected below.
|
# "Workspace mutations" field, which is rejected below.
|
||||||
# Issue #698: the canonical review-merge schema has no 'Mutations',
|
|
||||||
# 'Next', 'Issue/PR', 'Branch/SHA', or 'Files changed' fields — their
|
|
||||||
# content lives in the precise mutation categories, 'Safe next
|
|
||||||
# action', 'Selected PR'/'Linked issue', head-SHA fields, and 'Files
|
|
||||||
# reviewed'. Requiring the legacy names rejects canonical reports.
|
|
||||||
_non_canonical_for_review = {
|
|
||||||
"Workspace mutations",
|
|
||||||
"Mutations",
|
|
||||||
"Next",
|
|
||||||
"Issue/PR",
|
|
||||||
"Branch/SHA",
|
|
||||||
"Files changed",
|
|
||||||
}
|
|
||||||
required = [
|
required = [
|
||||||
field for field in required
|
field for field in required
|
||||||
if field[0] not in _non_canonical_for_review
|
if field[0] != "Workspace mutations"
|
||||||
]
|
]
|
||||||
if any(label.startswith("workspace mutations") for label in labels):
|
if any(label.startswith("workspace mutations") for label in labels):
|
||||||
return {
|
return {
|
||||||
|
|||||||
@@ -438,252 +438,3 @@ def format_cleanup_audit_comment(audit: dict[str, Any]) -> str:
|
|||||||
"This path only clears a lock when the referenced PR is merged/closed.",
|
"This path only clears a lock when the referenced PR is merged/closed.",
|
||||||
]
|
]
|
||||||
return "\n".join(lines)
|
return "\n".join(lines)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
|
||||||
# #709 cross-profile cleanup, overwrite protection, recovery provenance
|
|
||||||
# ---------------------------------------------------------------------------
|
|
||||||
|
|
||||||
|
|
||||||
def has_unresolved_terminal_evidence(lock: dict | None) -> bool:
|
|
||||||
"""True when *lock* still carries a terminal live mutation ledger."""
|
|
||||||
return last_terminal_mutation(lock) is not None
|
|
||||||
|
|
||||||
|
|
||||||
def assess_init_overwrite(
|
|
||||||
existing_lock: dict | None,
|
|
||||||
*,
|
|
||||||
force: bool = False,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
"""Whether init_review_decision_lock may replace an existing durable lock (#709 AC2).
|
|
||||||
|
|
||||||
Unresolved terminal evidence must never be silently replaced by an empty
|
|
||||||
initialized lock. *force* does not authorize destruction of terminal
|
|
||||||
ledgers — only explicit cleanup / archive transitions may remove them.
|
|
||||||
"""
|
|
||||||
result: dict[str, Any] = {
|
|
||||||
"overwrite_allowed": True,
|
|
||||||
"has_existing": existing_lock is not None,
|
|
||||||
"has_unresolved_terminal": False,
|
|
||||||
"reasons": [],
|
|
||||||
"last_terminal_pr": None,
|
|
||||||
"last_terminal_action": None,
|
|
||||||
"existing_profile_identity": None,
|
|
||||||
}
|
|
||||||
if existing_lock is None:
|
|
||||||
result["reasons"].append("no existing lock; empty init allowed")
|
|
||||||
return result
|
|
||||||
result["existing_profile_identity"] = (
|
|
||||||
existing_lock.get("profile_identity")
|
|
||||||
or existing_lock.get("session_profile_lock")
|
|
||||||
or existing_lock.get("session_profile")
|
|
||||||
)
|
|
||||||
last = last_terminal_mutation(existing_lock)
|
|
||||||
if last is None:
|
|
||||||
result["reasons"].append(
|
|
||||||
"existing lock has no terminal mutations; re-init allowed"
|
|
||||||
)
|
|
||||||
return result
|
|
||||||
result["has_unresolved_terminal"] = True
|
|
||||||
result["overwrite_allowed"] = False
|
|
||||||
result["last_terminal_pr"] = last.get("pr_number")
|
|
||||||
result["last_terminal_action"] = last.get("action")
|
|
||||||
result["reasons"].append(
|
|
||||||
"refuse empty re-init: unresolved terminal decision-lock evidence "
|
|
||||||
f"present ({last.get('action')} on PR #{last.get('pr_number')}); "
|
|
||||||
"use gitea_cleanup_stale_review_decision_lock when moot, or archive "
|
|
||||||
f"via sanctioned recovery — force={force!r} does not authorize overwrite "
|
|
||||||
"(#709 AC2)"
|
|
||||||
)
|
|
||||||
return result
|
|
||||||
|
|
||||||
|
|
||||||
def lock_targets_merged_pr_approval(
|
|
||||||
lock: dict | None,
|
|
||||||
*,
|
|
||||||
pr_number: int,
|
|
||||||
expected_head_sha: str | None = None,
|
|
||||||
) -> bool:
|
|
||||||
"""True when *lock*'s last terminal mutation is approve of *pr_number*.
|
|
||||||
|
|
||||||
When *expected_head_sha* is provided, a **recorded** terminal head must
|
|
||||||
match. Legacy same-repo approve locks with no recorded head do **not**
|
|
||||||
match (fail closed, #709 F3 residual / review 435) — callers must use the
|
|
||||||
strict secondary path that refuses no-head clears.
|
|
||||||
"""
|
|
||||||
last = last_terminal_mutation(lock)
|
|
||||||
if last is None:
|
|
||||||
return False
|
|
||||||
if last.get("action") != "approve":
|
|
||||||
return False
|
|
||||||
if last.get("pr_number") != pr_number:
|
|
||||||
return False
|
|
||||||
if expected_head_sha:
|
|
||||||
want = normalize_head_sha(expected_head_sha)
|
|
||||||
if not want:
|
|
||||||
return False
|
|
||||||
locked = mutation_head_sha(last, lock)
|
|
||||||
# Require recorded-head match; unrecorded head is not a match (#709 F3).
|
|
||||||
if not locked or not heads_equal(locked, want):
|
|
||||||
return False
|
|
||||||
return True
|
|
||||||
|
|
||||||
|
|
||||||
def build_post_merge_recovery_record(
|
|
||||||
*,
|
|
||||||
pr_number: int,
|
|
||||||
head_sha: str | None,
|
|
||||||
merge_commit_sha: str | None,
|
|
||||||
target_profile_identity: str | None,
|
|
||||||
failed_step: str,
|
|
||||||
error: str | None,
|
|
||||||
remote: str | None,
|
|
||||||
org: str | None,
|
|
||||||
repo: str | None,
|
|
||||||
actor_username: str | None,
|
|
||||||
profile_name: str | None,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
"""Durable recovery-required payload after irreversible merge (#709 AC3)."""
|
|
||||||
return {
|
|
||||||
"event": "post_merge_decision_lock_recovery_required",
|
|
||||||
"status": "recovery_required",
|
|
||||||
"issue_ref": "#709",
|
|
||||||
"recovery_critical": True,
|
|
||||||
"applied": False, # never claim historical cleanup succeeded
|
|
||||||
"timestamp": datetime.now(timezone.utc).isoformat(),
|
|
||||||
"pr_number": pr_number,
|
|
||||||
"head_sha": normalize_head_sha(head_sha),
|
|
||||||
"merge_commit_sha": merge_commit_sha,
|
|
||||||
"target_profile_identity": target_profile_identity,
|
|
||||||
"failed_step": failed_step,
|
|
||||||
"error": error,
|
|
||||||
"remote": remote,
|
|
||||||
"org": org,
|
|
||||||
"repo": repo,
|
|
||||||
"actor_username": actor_username,
|
|
||||||
"profile_name": profile_name,
|
|
||||||
"required_recovery_action": (
|
|
||||||
"retry cross-profile decision-lock cleanup and audit publication "
|
|
||||||
"for the merged PR; if terminal evidence is gone, use "
|
|
||||||
"gitea_record_irrecoverable_decision_lock_provenance"
|
|
||||||
),
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def build_irrecoverable_provenance_record(
|
|
||||||
*,
|
|
||||||
pr_number: int,
|
|
||||||
head_sha: str | None,
|
|
||||||
remote: str | None,
|
|
||||||
org: str | None,
|
|
||||||
repo: str | None,
|
|
||||||
actor_username: str | None,
|
|
||||||
profile_name: str | None,
|
|
||||||
reason: str,
|
|
||||||
incident_ref: str | None = None,
|
|
||||||
# Deprecated kwargs retained only so stale call sites fail closed:
|
|
||||||
operator_authorized: bool | None = None,
|
|
||||||
# Required for merger-acceptable records (#709 F1):
|
|
||||||
authorization: dict[str, Any] | None = None,
|
|
||||||
incident_issue: int | None = None,
|
|
||||||
incident_comment_id: int | None = None,
|
|
||||||
destroyed_subject: str | None = None,
|
|
||||||
historical_provenance_subject: str | None = None,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
"""Truthful absence-of-proof record (#709 AC5). Never sets applied=True.
|
|
||||||
|
|
||||||
Caller-supplied ``operator_authorized`` is **ignored** as authorization
|
|
||||||
evidence (review 434 F1). Prefer
|
|
||||||
:func:`irrecoverable_provenance.build_irrecoverable_provenance_record`
|
|
||||||
with a verified server-side authorization artifact.
|
|
||||||
"""
|
|
||||||
# Explicitly ignore deprecated self-assertable Boolean.
|
|
||||||
_ = operator_authorized
|
|
||||||
if authorization is not None and incident_issue is not None and incident_comment_id is not None:
|
|
||||||
from irrecoverable_provenance import (
|
|
||||||
build_irrecoverable_provenance_record as _build,
|
|
||||||
)
|
|
||||||
|
|
||||||
return _build(
|
|
||||||
pr_number=int(pr_number),
|
|
||||||
head_sha=str(head_sha or ""),
|
|
||||||
remote=str(remote or ""),
|
|
||||||
org=str(org or ""),
|
|
||||||
repo=str(repo or ""),
|
|
||||||
actor_username=actor_username,
|
|
||||||
profile_name=profile_name,
|
|
||||||
reason=reason,
|
|
||||||
incident_issue=int(incident_issue),
|
|
||||||
incident_comment_id=int(incident_comment_id),
|
|
||||||
authorization=authorization,
|
|
||||||
destroyed_subject=destroyed_subject,
|
|
||||||
historical_provenance_subject=historical_provenance_subject
|
|
||||||
or destroyed_subject,
|
|
||||||
)
|
|
||||||
# Fail-closed skeleton when no server authorization is supplied: never
|
|
||||||
# sets merger_may_accept True (even if operator_authorized was True).
|
|
||||||
return {
|
|
||||||
"event": "irrecoverable_decision_lock_provenance",
|
|
||||||
"status": "provenance_irrecoverable",
|
|
||||||
"record_type": "irrecoverable_decision_provenance",
|
|
||||||
"operator_recovery_required": True,
|
|
||||||
"issue_ref": "#709",
|
|
||||||
"recovery_critical": True,
|
|
||||||
"applied": False,
|
|
||||||
"historical_cleanup_proven": False,
|
|
||||||
"timestamp": datetime.now(timezone.utc).isoformat(),
|
|
||||||
"pr_number": pr_number,
|
|
||||||
"head_sha": normalize_head_sha(head_sha),
|
|
||||||
"remote": remote,
|
|
||||||
"org": org,
|
|
||||||
"repo": repo,
|
|
||||||
"actor_username": actor_username,
|
|
||||||
"profile_name": profile_name,
|
|
||||||
"reason": reason,
|
|
||||||
"incident_ref": incident_ref,
|
|
||||||
"incident_issue": incident_issue,
|
|
||||||
"incident_comment_id": incident_comment_id,
|
|
||||||
"authorization_verified": False,
|
|
||||||
"merger_may_accept": False,
|
|
||||||
"acceptance_rule": (
|
|
||||||
"Merger may accept this record only when a server-side "
|
|
||||||
"authorization artifact verifies for remote/org/repo/PR/exact "
|
|
||||||
"head/incident, the record is durable and read back, and normal "
|
|
||||||
"merge gates still pass. Caller Booleans never authorize. This "
|
|
||||||
"does not prove historical cleanup."
|
|
||||||
),
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def format_irrecoverable_audit_comment(record: dict[str, Any]) -> str:
|
|
||||||
"""Markdown body for irrecoverable provenance audit (no applied=true claim)."""
|
|
||||||
from irrecoverable_provenance import (
|
|
||||||
format_irrecoverable_audit_comment as _fmt,
|
|
||||||
)
|
|
||||||
|
|
||||||
return _fmt(record)
|
|
||||||
|
|
||||||
|
|
||||||
def format_post_merge_recovery_comment(record: dict[str, Any]) -> str:
|
|
||||||
"""Markdown body for post-merge recovery-required audit."""
|
|
||||||
lines = [
|
|
||||||
"## Post-merge decision-lock recovery required (#709)",
|
|
||||||
"",
|
|
||||||
"Status: **RECOVERY_REQUIRED** (merge is irreversible; cleanup/audit incomplete)",
|
|
||||||
"",
|
|
||||||
f"- actor: `{record.get('actor_username')}`",
|
|
||||||
f"- profile: `{record.get('profile_name')}`",
|
|
||||||
f"- timestamp: `{record.get('timestamp')}`",
|
|
||||||
f"- PR: `#{record.get('pr_number')}`",
|
|
||||||
f"- head_sha: `{record.get('head_sha')}`",
|
|
||||||
f"- merge_commit_sha: `{record.get('merge_commit_sha')}`",
|
|
||||||
f"- target_profile_identity: `{record.get('target_profile_identity')}`",
|
|
||||||
f"- failed_step: `{record.get('failed_step')}`",
|
|
||||||
f"- error: `{record.get('error')}`",
|
|
||||||
"",
|
|
||||||
f"Required action: {record.get('required_recovery_action')}",
|
|
||||||
"",
|
|
||||||
"applied=false — do not treat this as successful cleanup evidence.",
|
|
||||||
]
|
|
||||||
return "\n".join(lines)
|
|
||||||
|
|
||||||
|
|||||||
@@ -127,32 +127,6 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
|||||||
"permission": "gitea.pr.review",
|
"permission": "gitea.pr.review",
|
||||||
"role": "reviewer",
|
"role": "reviewer",
|
||||||
},
|
},
|
||||||
# #709: truthful absence-of-proof recovery (server-side auth + record + consume).
|
|
||||||
# Dedicated mutation capability — gitea.read is insufficient (review 434 F1).
|
|
||||||
"issue_irrecoverable_provenance_authorization": {
|
|
||||||
"permission": "gitea.decision_lock.irrecoverable_recovery",
|
|
||||||
"role": "reconciler",
|
|
||||||
},
|
|
||||||
"gitea_issue_irrecoverable_provenance_authorization": {
|
|
||||||
"permission": "gitea.decision_lock.irrecoverable_recovery",
|
|
||||||
"role": "reconciler",
|
|
||||||
},
|
|
||||||
"record_irrecoverable_decision_lock_provenance": {
|
|
||||||
"permission": "gitea.decision_lock.irrecoverable_recovery",
|
|
||||||
"role": "reconciler",
|
|
||||||
},
|
|
||||||
"gitea_record_irrecoverable_decision_lock_provenance": {
|
|
||||||
"permission": "gitea.decision_lock.irrecoverable_recovery",
|
|
||||||
"role": "reconciler",
|
|
||||||
},
|
|
||||||
"consume_irrecoverable_decision_lock_provenance": {
|
|
||||||
"permission": "gitea.pr.merge",
|
|
||||||
"role": "merger",
|
|
||||||
},
|
|
||||||
"gitea_consume_irrecoverable_decision_lock_provenance": {
|
|
||||||
"permission": "gitea.pr.merge",
|
|
||||||
"role": "merger",
|
|
||||||
},
|
|
||||||
"delete_branch": {
|
"delete_branch": {
|
||||||
"permission": "gitea.branch.delete",
|
"permission": "gitea.branch.delete",
|
||||||
"role": "author",
|
"role": "author",
|
||||||
|
|||||||
@@ -1,461 +0,0 @@
|
|||||||
"""Regression tests for #698: final-report validator vs canonical schema.
|
|
||||||
|
|
||||||
Covers the original #698 lead plus the independent reproduction recorded
|
|
||||||
during the PR #703 formal review (issue #698 comment 11246):
|
|
||||||
|
|
||||||
1. non-dict ``action_log`` entries must fail structured, never crash;
|
|
||||||
2. the validator must not demand legacy fields the canonical schema forbids
|
|
||||||
(``Pinned reviewed head``, ``Scratch worktree used``, ``Worktree path``,
|
|
||||||
``Worktree dirty``, ``Mutations``, ``Next``);
|
|
||||||
3. a legitimately blocked report (``Candidate head SHA: none``, no formal
|
|
||||||
verdict) must not owe approval/merge live-head proofs;
|
|
||||||
4. canonical reviewer lease release must not be misclassified as post-merge
|
|
||||||
cleanup;
|
|
||||||
5. structured workflow-load and validation proof must be recognized;
|
|
||||||
6. review mutations are inferred only from authoritative evidence.
|
|
||||||
"""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import sys
|
|
||||||
import unittest
|
|
||||||
from pathlib import Path
|
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
import final_report_validator as frv # noqa: E402
|
|
||||||
import post_merge_cleanup_proof as pmcp # noqa: E402
|
|
||||||
import pr_work_lease as pwl # noqa: E402
|
|
||||||
import review_proofs as rp # noqa: E402
|
|
||||||
from review_final_report_schema import ( # noqa: E402
|
|
||||||
assess_review_final_report_schema,
|
|
||||||
)
|
|
||||||
|
|
||||||
REVIEWED_HEAD = "a" * 40
|
|
||||||
LIVE_HEAD = "a" * 40
|
|
||||||
|
|
||||||
|
|
||||||
def _pr703_style_report(
|
|
||||||
*,
|
|
||||||
decision: str = "request_changes",
|
|
||||||
cleanup_mutations: str = (
|
|
||||||
"released reviewer PR lease via gitea_release_reviewer_pr_lease "
|
|
||||||
"(terminal lease marker phase=released posted)"
|
|
||||||
),
|
|
||||||
) -> str:
|
|
||||||
"""Canonical-schema report modeled on the PR #703 formal review handoff."""
|
|
||||||
return f"""
|
|
||||||
Formal review completed with a REQUEST_CHANGES verdict submitted and read
|
|
||||||
back via the native review API.
|
|
||||||
|
|
||||||
## Controller Handoff
|
|
||||||
|
|
||||||
- Task: review-merge-pr
|
|
||||||
- Repo: Scaled-Tech-Consulting/Gitea-Tools
|
|
||||||
- Role: reviewer
|
|
||||||
- Identity: sysadmin / prgs-reviewer
|
|
||||||
- Active profile: prgs-reviewer
|
|
||||||
- Runtime context: neutral workspace binding
|
|
||||||
- Selected PR: 703
|
|
||||||
- Linked issue: #702 open
|
|
||||||
- Eligibility class: reviewable
|
|
||||||
- Queue ordering policy: oldest eligible first
|
|
||||||
- Inventory pagination proof: has_more=false, total_count=8
|
|
||||||
- Earlier PRs skipped: none
|
|
||||||
- Candidate head SHA: {REVIEWED_HEAD}
|
|
||||||
- Reviewed head SHA: {REVIEWED_HEAD}
|
|
||||||
- Target branch: master
|
|
||||||
- Target branch SHA: {"2" * 40}
|
|
||||||
- Already-landed gate: not landed
|
|
||||||
- Author-safety result: pass (author differs from reviewer)
|
|
||||||
- Prior request-changes state: none
|
|
||||||
- Review worktree used: true
|
|
||||||
- Review worktree path: branches/review-pr-703-independent
|
|
||||||
- Review worktree inside branches: true
|
|
||||||
- Review worktree HEAD state: detached at pinned head
|
|
||||||
- Review worktree dirty before validation: clean
|
|
||||||
- Review worktree dirty after validation: clean
|
|
||||||
- Baseline worktree used: false
|
|
||||||
- Baseline worktree path: none
|
|
||||||
- Files reviewed: 4
|
|
||||||
- Validation: focused 50 passed; related 94 passed; full 2665 passed, 6 skipped
|
|
||||||
- Official validation integrity status: intact
|
|
||||||
- Terminal review mutation: one REQUEST_CHANGES review submitted and read back
|
|
||||||
- Review decision: {decision}
|
|
||||||
- Merge preflight: not run
|
|
||||||
- Merge result: none
|
|
||||||
- Linked issue status: open (live fetch proof: gitea_view_issue)
|
|
||||||
- Main checkout branch: master
|
|
||||||
- Main checkout dirty state: clean
|
|
||||||
- Main checkout updated: false
|
|
||||||
- File edits by reviewer: none
|
|
||||||
- Worktree/index mutations: none
|
|
||||||
- Git ref mutations: git fetch prgs (recorded)
|
|
||||||
- MCP/Gitea mutations: review submission and lease comments only
|
|
||||||
- Review mutations: one formal REQUEST_CHANGES verdict
|
|
||||||
- Merge mutations: none
|
|
||||||
- Cleanup mutations: {cleanup_mutations}
|
|
||||||
- External-state mutations: none
|
|
||||||
- Read-only diagnostics: gitea_view_pr, gitea_get_pr_review_feedback
|
|
||||||
- Blockers: findings F1-F6 recorded on the PR thread
|
|
||||||
- Current status: review complete; author remediation required
|
|
||||||
- Safe next action: author addresses findings and pushes a new head
|
|
||||||
- Safety statement: no merge attempted; no self-review; no root-checkout edits
|
|
||||||
- Workflow-load helper result: workflow_hash=da045d1e1f1f boundary_status=clean
|
|
||||||
- Live head SHA before approval: {LIVE_HEAD}
|
|
||||||
- Pushes occurred during validation: no
|
|
||||||
"""
|
|
||||||
|
|
||||||
|
|
||||||
def _blocked_preflight_report() -> str:
|
|
||||||
"""Blocked-run report modeled on the #702 comment 11164 reproduction."""
|
|
||||||
return """
|
|
||||||
Fresh review preflight stopped before any worktree or validation work.
|
|
||||||
|
|
||||||
## Controller Handoff
|
|
||||||
|
|
||||||
- Task: review-merge-pr
|
|
||||||
- Repo: Scaled-Tech-Consulting/Gitea-Tools
|
|
||||||
- Role: reviewer
|
|
||||||
- Identity: sysadmin / prgs-reviewer
|
|
||||||
- Active profile: prgs-reviewer
|
|
||||||
- Runtime context: stale workspace binding detected
|
|
||||||
- Selected PR: 701
|
|
||||||
- Linked issue: #699 open
|
|
||||||
- Eligibility class: blocked-before-validation
|
|
||||||
- Queue ordering policy: oldest eligible first
|
|
||||||
- Inventory pagination proof: has_more=false, total_count=8
|
|
||||||
- Earlier PRs skipped: none
|
|
||||||
- Candidate head SHA: none
|
|
||||||
- Reviewed head SHA: none
|
|
||||||
- Target branch: master
|
|
||||||
- Target branch SHA: none
|
|
||||||
- Already-landed gate: not run
|
|
||||||
- Author-safety result: not run
|
|
||||||
- Prior request-changes state: none
|
|
||||||
- Review worktree used: false
|
|
||||||
- Review worktree path: none
|
|
||||||
- Review worktree inside branches: not applicable
|
|
||||||
- Review worktree HEAD state: not applicable
|
|
||||||
- Review worktree dirty before validation: not applicable
|
|
||||||
- Review worktree dirty after validation: not applicable
|
|
||||||
- Baseline worktree used: false
|
|
||||||
- Baseline worktree path: none
|
|
||||||
- Files reviewed: 0
|
|
||||||
- Validation: not run
|
|
||||||
- Official validation integrity status: not applicable
|
|
||||||
- Terminal review mutation: none
|
|
||||||
- Review decision: none
|
|
||||||
- Merge preflight: not run
|
|
||||||
- Merge result: none
|
|
||||||
- Linked issue status: open (live fetch proof: gitea_view_issue)
|
|
||||||
- Main checkout branch: master
|
|
||||||
- Main checkout dirty state: clean
|
|
||||||
- Main checkout updated: false
|
|
||||||
- File edits by reviewer: none
|
|
||||||
- Worktree/index mutations: none
|
|
||||||
- Git ref mutations: none
|
|
||||||
- MCP/Gitea mutations: none
|
|
||||||
- Review mutations: none
|
|
||||||
- Merge mutations: none
|
|
||||||
- Cleanup mutations: none
|
|
||||||
- External-state mutations: none
|
|
||||||
- Read-only diagnostics: gitea_view_pr, gitea_get_runtime_context
|
|
||||||
- Blockers: runtime bound to a foreign task worktree; mutation prohibited
|
|
||||||
- Current status: stopped before validation began
|
|
||||||
- Next actor: operator
|
|
||||||
- Next action: repair the runtime workspace binding, then rerun the full
|
|
||||||
review workflow in a fresh reviewer session
|
|
||||||
- Next prompt: Act as REVIEWER for PR 701 after the operator repairs the
|
|
||||||
runtime binding; acquire the lease before any validation.
|
|
||||||
- Safe next action: operator repairs runtime binding, then a fresh reviewer
|
|
||||||
reruns the full workflow
|
|
||||||
- Safety statement: no lease acquired; no verdict recorded; no source edits
|
|
||||||
- Workflow-load helper result: workflow_hash=da045d1e1f1f boundary_status=clean
|
|
||||||
"""
|
|
||||||
|
|
||||||
|
|
||||||
class TestActionLogRobustness(unittest.TestCase):
|
|
||||||
"""#698 original lead: non-dict action_log must not crash validation."""
|
|
||||||
|
|
||||||
MALFORMED = [
|
|
||||||
"git fetch prgs",
|
|
||||||
42,
|
|
||||||
None,
|
|
||||||
{"action": "edit", "path": "x.py", "performed": True, "tracked": True},
|
|
||||||
]
|
|
||||||
|
|
||||||
def test_assess_final_report_validator_survives_malformed_entries(self):
|
|
||||||
result = frv.assess_final_report_validator(
|
|
||||||
_pr703_style_report(),
|
|
||||||
"review_pr",
|
|
||||||
action_log=self.MALFORMED,
|
|
||||||
)
|
|
||||||
self.assertIsInstance(result, dict)
|
|
||||||
rule_ids = {f["rule_id"] for f in result["findings"]}
|
|
||||||
self.assertIn("shared.action_log_malformed", rule_ids)
|
|
||||||
|
|
||||||
def test_malformed_entry_errors_are_sanitized(self):
|
|
||||||
_entries, findings = frv.sanitize_action_log(["secret-token-abc123"])
|
|
||||||
self.assertEqual(len(findings), 1)
|
|
||||||
reason = findings[0]["reason"]
|
|
||||||
self.assertNotIn("secret-token-abc123", reason)
|
|
||||||
self.assertIn("str", reason)
|
|
||||||
self.assertIn("entry 0", reason)
|
|
||||||
|
|
||||||
def test_non_list_action_log_is_reported_not_raised(self):
|
|
||||||
entries, findings = frv.sanitize_action_log("not-a-list")
|
|
||||||
self.assertEqual(entries, [])
|
|
||||||
self.assertEqual(len(findings), 1)
|
|
||||||
self.assertIn("not a list", findings[0]["reason"])
|
|
||||||
|
|
||||||
def test_performed_file_mutations_skips_non_dict_entries(self):
|
|
||||||
performed = rp._performed_file_mutations(
|
|
||||||
["oops", {"action": "edited", "path": "a.py"}]
|
|
||||||
)
|
|
||||||
self.assertEqual(len(performed), 1)
|
|
||||||
self.assertEqual(performed[0]["path"], "a.py")
|
|
||||||
|
|
||||||
def test_schema_entrypoint_survives_string_only_log(self):
|
|
||||||
result = assess_review_final_report_schema(
|
|
||||||
_pr703_style_report(),
|
|
||||||
action_log=["just a string", "another string"],
|
|
||||||
)
|
|
||||||
self.assertIsInstance(result, dict)
|
|
||||||
|
|
||||||
|
|
||||||
class TestLegacyFieldRequirementsRemoved(unittest.TestCase):
|
|
||||||
"""#698: prohibited legacy fields must not be REQUIRED of reports."""
|
|
||||||
|
|
||||||
PROHIBITED = (
|
|
||||||
"Pinned reviewed head",
|
|
||||||
"Scratch worktree used",
|
|
||||||
"Worktree path",
|
|
||||||
"Worktree dirty",
|
|
||||||
"Workspace mutations",
|
|
||||||
"Mutations",
|
|
||||||
"Next",
|
|
||||||
"Issue/PR",
|
|
||||||
"Branch/SHA",
|
|
||||||
"Files changed",
|
|
||||||
)
|
|
||||||
|
|
||||||
def test_review_role_field_table_has_no_prohibited_requirements(self):
|
|
||||||
names = [name for name, _ in rp.HANDOFF_ROLE_FIELDS["review"]]
|
|
||||||
for prohibited in ("Pinned reviewed head", "Scratch worktree used",
|
|
||||||
"Worktree path", "Worktree dirty"):
|
|
||||||
self.assertNotIn(prohibited, names)
|
|
||||||
|
|
||||||
def test_merger_role_field_table_has_no_pinned_reviewed_head(self):
|
|
||||||
names = [name for name, _ in rp.HANDOFF_ROLE_FIELDS["merger"]]
|
|
||||||
self.assertNotIn("Pinned reviewed head", names)
|
|
||||||
|
|
||||||
def test_canonical_report_missing_fields_never_include_prohibited(self):
|
|
||||||
result = rp.assess_controller_handoff(
|
|
||||||
_pr703_style_report(), role="review"
|
|
||||||
)
|
|
||||||
for prohibited in self.PROHIBITED:
|
|
||||||
self.assertNotIn(prohibited, result.get("missing_fields") or [])
|
|
||||||
|
|
||||||
def test_canonical_pr703_report_satisfies_required_fields(self):
|
|
||||||
result = rp.assess_controller_handoff(
|
|
||||||
_pr703_style_report(), role="review"
|
|
||||||
)
|
|
||||||
self.assertEqual(result.get("missing_fields") or [], [])
|
|
||||||
self.assertEqual(result.get("verdict"), "complete")
|
|
||||||
|
|
||||||
|
|
||||||
class TestBlockedReportAccepted(unittest.TestCase):
|
|
||||||
"""#698: blocked run with no reviewed head / verdict is legitimate."""
|
|
||||||
|
|
||||||
def test_stale_head_proof_waived_before_validation(self):
|
|
||||||
result = pwl.assess_reviewer_stale_head_final_report(
|
|
||||||
_blocked_preflight_report()
|
|
||||||
)
|
|
||||||
self.assertTrue(result["proven"])
|
|
||||||
self.assertEqual(result.get("phase"), "blocked_before_validation")
|
|
||||||
|
|
||||||
def test_blocked_report_passes_schema_validation(self):
|
|
||||||
result = assess_review_final_report_schema(_blocked_preflight_report())
|
|
||||||
blocking = [
|
|
||||||
f for f in result["findings"] if f["severity"] == "block"
|
|
||||||
]
|
|
||||||
self.assertEqual(blocking, [], blocking)
|
|
||||||
|
|
||||||
def test_verdict_phase_still_demands_approval_head_proof(self):
|
|
||||||
report = _blocked_preflight_report().replace(
|
|
||||||
"- Review decision: none",
|
|
||||||
"- Review decision: approve",
|
|
||||||
).replace(
|
|
||||||
"- Candidate head SHA: none",
|
|
||||||
f"- Candidate head SHA: {REVIEWED_HEAD}",
|
|
||||||
)
|
|
||||||
result = pwl.assess_reviewer_stale_head_final_report(report)
|
|
||||||
self.assertFalse(result["proven"])
|
|
||||||
joined = " ".join(result["reasons"])
|
|
||||||
self.assertIn("before approval", joined)
|
|
||||||
|
|
||||||
def test_merge_phase_still_demands_merge_head_proof(self):
|
|
||||||
report = _pr703_style_report().replace(
|
|
||||||
"- Merge result: none",
|
|
||||||
"- Merge result: merged",
|
|
||||||
)
|
|
||||||
result = pwl.assess_reviewer_stale_head_final_report(report)
|
|
||||||
self.assertFalse(result["proven"])
|
|
||||||
self.assertIn(
|
|
||||||
"final live head SHA before merge not stated",
|
|
||||||
result["reasons"],
|
|
||||||
)
|
|
||||||
|
|
||||||
def test_validation_phase_demands_push_disclosure(self):
|
|
||||||
report = _pr703_style_report().replace(
|
|
||||||
"- Pushes occurred during validation: no\n", ""
|
|
||||||
)
|
|
||||||
result = pwl.assess_reviewer_stale_head_final_report(report)
|
|
||||||
self.assertFalse(result["proven"])
|
|
||||||
self.assertIn(
|
|
||||||
"whether push occurred during validation not stated",
|
|
||||||
result["reasons"],
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
class TestLeaseReleaseVsPostMergeCleanup(unittest.TestCase):
|
|
||||||
"""#698 (PR #703 review reproduction): lease release is not cleanup."""
|
|
||||||
|
|
||||||
def test_lease_release_cleanup_mutations_do_not_demand_checklist(self):
|
|
||||||
result = pmcp.assess_post_merge_cleanup_proof(_pr703_style_report())
|
|
||||||
self.assertFalse(result["block"], result["reasons"])
|
|
||||||
|
|
||||||
def test_release_tool_name_alone_is_recognized(self):
|
|
||||||
report = _pr703_style_report(
|
|
||||||
cleanup_mutations="gitea_release_reviewer_pr_lease comment 11244"
|
|
||||||
)
|
|
||||||
result = pmcp.assess_post_merge_cleanup_proof(report)
|
|
||||||
self.assertFalse(result["block"], result["reasons"])
|
|
||||||
|
|
||||||
def test_substantive_non_lease_cleanup_still_demands_checklist(self):
|
|
||||||
report = _pr703_style_report(
|
|
||||||
cleanup_mutations="deleted stale scratch directory manually"
|
|
||||||
)
|
|
||||||
result = pmcp.assess_post_merge_cleanup_proof(report)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
def test_remote_branch_delete_claims_still_demand_full_proof(self):
|
|
||||||
report = _pr703_style_report(
|
|
||||||
cleanup_mutations="gitea_delete_branch removed the remote branch"
|
|
||||||
)
|
|
||||||
result = pmcp.assess_post_merge_cleanup_proof(report)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertTrue(
|
|
||||||
any("remote branch deletion missing" in r for r in result["reasons"])
|
|
||||||
)
|
|
||||||
|
|
||||||
def test_full_schema_run_accepts_lease_release_report(self):
|
|
||||||
result = assess_review_final_report_schema(_pr703_style_report())
|
|
||||||
lease_cleanup_blocks = [
|
|
||||||
f for f in result["findings"]
|
|
||||||
if f["rule_id"] == "reviewer.post_merge_cleanup_proof"
|
|
||||||
]
|
|
||||||
self.assertEqual(lease_cleanup_blocks, [], lease_cleanup_blocks)
|
|
||||||
|
|
||||||
|
|
||||||
class TestStructuredProofRecognition(unittest.TestCase):
|
|
||||||
"""#698: structured workflow-load and validation proof must be accepted."""
|
|
||||||
|
|
||||||
def test_key_value_workflow_proof_recognized(self):
|
|
||||||
findings = frv._rule_reviewer_workflow_load_boundary(
|
|
||||||
_pr703_style_report()
|
|
||||||
)
|
|
||||||
self.assertEqual(findings, [], findings)
|
|
||||||
|
|
||||||
def test_colon_form_workflow_proof_still_recognized(self):
|
|
||||||
report = _pr703_style_report().replace(
|
|
||||||
"- Workflow-load helper result: workflow_hash=da045d1e1f1f "
|
|
||||||
"boundary_status=clean",
|
|
||||||
"- Workflow-load helper result: workflow_hash: da045d1e1f1f, "
|
|
||||||
"boundary_status: clean",
|
|
||||||
)
|
|
||||||
findings = frv._rule_reviewer_workflow_load_boundary(report)
|
|
||||||
self.assertEqual(findings, [], findings)
|
|
||||||
|
|
||||||
def test_incomplete_structured_proof_still_blocks(self):
|
|
||||||
report = _pr703_style_report().replace(
|
|
||||||
"workflow_hash=da045d1e1f1f boundary_status=clean",
|
|
||||||
"workflow_hash=da045d1e1f1f",
|
|
||||||
)
|
|
||||||
findings = frv._rule_reviewer_workflow_load_boundary(report)
|
|
||||||
self.assertTrue(findings)
|
|
||||||
self.assertIn("boundary_status", findings[0]["reason"])
|
|
||||||
|
|
||||||
def test_validation_counts_accepted_as_pass_proof(self):
|
|
||||||
# "Validation: focused 50 passed; ..." must satisfy the reviewed-head
|
|
||||||
# validation-proof rule (PR #703 reproduction).
|
|
||||||
result = assess_review_final_report_schema(_pr703_style_report())
|
|
||||||
head_blocks = [
|
|
||||||
f for f in result["findings"]
|
|
||||||
if f["rule_id"] == "reviewer.reviewed_head_without_validation"
|
|
||||||
]
|
|
||||||
self.assertEqual(head_blocks, [], head_blocks)
|
|
||||||
|
|
||||||
|
|
||||||
class TestAuthoritativeMutationInference(unittest.TestCase):
|
|
||||||
"""#698: review mutations inferred only from authoritative evidence."""
|
|
||||||
|
|
||||||
def test_read_only_entries_do_not_imply_mutations(self):
|
|
||||||
report = "## Controller Handoff\n- Mutations: none\n"
|
|
||||||
findings = frv._rule_reviewer_vague_mutations_none(
|
|
||||||
report,
|
|
||||||
action_log=[
|
|
||||||
{"action": "gitea_view_pr"},
|
|
||||||
{"action": "gitea_get_pr_review_feedback", "performed": False},
|
|
||||||
],
|
|
||||||
)
|
|
||||||
self.assertEqual(findings, [], findings)
|
|
||||||
|
|
||||||
def test_performed_mutation_still_blocks_vague_none(self):
|
|
||||||
report = "## Controller Handoff\n- Mutations: none\n"
|
|
||||||
findings = frv._rule_reviewer_vague_mutations_none(
|
|
||||||
report,
|
|
||||||
action_log=[{"action": "edit", "path": "a.py", "performed": True}],
|
|
||||||
)
|
|
||||||
self.assertTrue(findings)
|
|
||||||
|
|
||||||
def test_gated_rejection_is_not_a_mutation(self):
|
|
||||||
report = "## Controller Handoff\n- Mutations: none\n"
|
|
||||||
findings = frv._rule_reviewer_vague_mutations_none(
|
|
||||||
report,
|
|
||||||
action_log=[
|
|
||||||
{"action": "edit", "path": "a.py", "performed": True,
|
|
||||||
"gated_rejected": True},
|
|
||||||
],
|
|
||||||
)
|
|
||||||
self.assertEqual(findings, [], findings)
|
|
||||||
|
|
||||||
|
|
||||||
class TestValidatorRuleErrorContainment(unittest.TestCase):
|
|
||||||
"""#698: a defective rule fails closed with a sanitized error."""
|
|
||||||
|
|
||||||
def test_rule_exception_becomes_sanitized_block_finding(self):
|
|
||||||
def _boom(report_text):
|
|
||||||
raise ValueError("raw secret detail that must not leak")
|
|
||||||
|
|
||||||
original = frv._RULES_BY_TASK["review_pr"]
|
|
||||||
frv._RULES_BY_TASK["review_pr"] = [_boom]
|
|
||||||
try:
|
|
||||||
result = frv.assess_final_report_validator(
|
|
||||||
"report body", "review_pr"
|
|
||||||
)
|
|
||||||
finally:
|
|
||||||
frv._RULES_BY_TASK["review_pr"] = original
|
|
||||||
self.assertTrue(result["blocked"])
|
|
||||||
finding = next(
|
|
||||||
f for f in result["findings"]
|
|
||||||
if f["rule_id"] == "shared.validator_rule_error"
|
|
||||||
)
|
|
||||||
self.assertNotIn("raw secret detail", finding["reason"])
|
|
||||||
self.assertIn("ValueError", finding["reason"])
|
|
||||||
self.assertIn("_boom", finding["reason"])
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
unittest.main()
|
|
||||||
File diff suppressed because it is too large
Load Diff
@@ -957,20 +957,17 @@ class TestControllerHandoff(unittest.TestCase):
|
|||||||
if not line.startswith("- Workspace mutations:"))
|
if not line.startswith("- Workspace mutations:"))
|
||||||
result = assess_controller_handoff(review_base, role="review")
|
result = assess_controller_handoff(review_base, role="review")
|
||||||
self.assertEqual(result["verdict"], "incomplete")
|
self.assertEqual(result["verdict"], "incomplete")
|
||||||
# #698: the canonical schema forbids the legacy fields, so the
|
self.assertIn("Pinned reviewed head", result["missing_fields"])
|
||||||
# validator must demand the canonical names instead.
|
self.assertIn("Worktree path", result["missing_fields"])
|
||||||
self.assertIn("Reviewed head SHA", result["missing_fields"])
|
|
||||||
self.assertIn("Review worktree path", result["missing_fields"])
|
|
||||||
self.assertIn("Merge result", result["missing_fields"])
|
self.assertIn("Merge result", result["missing_fields"])
|
||||||
for legacy in ("Pinned reviewed head", "Scratch worktree used"):
|
|
||||||
self.assertNotIn(legacy, result["missing_fields"])
|
|
||||||
|
|
||||||
complete = review_base + "\n" + "\n".join([
|
complete = review_base + "\n" + "\n".join([
|
||||||
"- Selected PR: #999",
|
"- Selected PR: #999",
|
||||||
"- Reviewer eligibility: passed",
|
"- Reviewer eligibility: passed",
|
||||||
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||||
"- Review worktree path: /repo/branches/review-pr-999",
|
"- Worktree path: /repo/branches/review-pr-999",
|
||||||
"- Review worktree dirty before validation: no",
|
"- Worktree dirty: no",
|
||||||
|
"- Scratch worktree used: yes (/repo/branches/review-pr-999)",
|
||||||
"- Unrelated local mutations: none",
|
"- Unrelated local mutations: none",
|
||||||
"- Review decision: approve",
|
"- Review decision: approve",
|
||||||
"- Merge result: merged",
|
"- Merge result: merged",
|
||||||
@@ -1128,9 +1125,10 @@ class TestReviewHandoffPreciseMutationCategories(unittest.TestCase):
|
|||||||
"- Safety: no self-review; no self-merge; no secrets",
|
"- Safety: no self-review; no self-merge; no secrets",
|
||||||
"- Selected PR: #999",
|
"- Selected PR: #999",
|
||||||
"- Reviewer eligibility: passed",
|
"- Reviewer eligibility: passed",
|
||||||
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||||
"- Worktree path: /repo/branches/review-pr-999",
|
"- Worktree path: /repo/branches/review-pr-999",
|
||||||
"- Worktree dirty: no",
|
"- Worktree dirty: no",
|
||||||
|
"- Scratch worktree used: yes (/repo/branches/review-pr-999)",
|
||||||
"- Unrelated local mutations: none",
|
"- Unrelated local mutations: none",
|
||||||
"- Review decision: approve",
|
"- Review decision: approve",
|
||||||
"- Merge result: none",
|
"- Merge result: none",
|
||||||
|
|||||||
Reference in New Issue
Block a user