Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,15 @@ capability-owned review.
Control-plane, runtime and product surfaces (`loopx/**`, `apps/**`, `packages/**`,
permissions/authority, persisted state/receipts, quota/scheduler/todo rules, and
CLI/API contracts) require explicit maintainer merge authorization. Propose and
review the exact head, pass required validation, and obtain ready merge-readiness
review the exact head, pass required validation, and run merge-readiness
(`uv run --extra test loopx pr-review --check-merge-readiness NUMBER@HEAD_OID`,
plus `--goal-id GOAL` for Goal-managed work) for that unchanged head before any
explicitly authorized merge or admin bypass. Small diffs, green CI or published
reviews do not grant merge authority; without authorization, leave the merge to
the maintainer.
plus `--goal-id GOAL` for Goal-managed work) for that unchanged head. A
`ready=true` result permits the normal path. Explicit maintainer authorization
may use admin bypass to override a `ready=false` readiness or GitHub policy
hold, but never missing exact-head review, required validation, unresolved
findings or threads, a changed head, an unmergeable conflict, or absent merge
authority. Small diffs, green CI or published reviews do not grant merge
authority; without authorization, leave the merge to the maintainer.

For author-owned PRs, GitHub blocks formal self-approval: publish a `COMMENTED`
review on the exact head carrying the approval conclusion and an English verdict.
Expand Down
6 changes: 3 additions & 3 deletions examples/pr-review-command-smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def main() -> int:
"Read the published review back",
"Route approval, merge, self-merge, and admin bypass to `loopx-pr-merge`",
"--check-merge-readiness NUMBER@HEAD_OID",
"admin bypass never overrides this gate",
"admin bypass may override a readiness or GitHub policy hold",
"Full PR Review And Bilingual Format",
"findings-only or blocker-only body is incomplete",
"Cover every changed surface and key symbols",
Expand Down Expand Up @@ -161,9 +161,9 @@ def main() -> int:
"loopx --format json pr-review --state all",
"agent_response_contract.review_execution_contract",
"--check-merge-readiness NUMBER@HEAD_OID",
"ready=true",
"Record the readiness result",
"completion_gate",
"admin bypass never overrides this gate",
"admin bypass may override a readiness or GitHub policy hold",
"A merge decision without this evidence is not authorized",
):
assert phrase in merge_text, phrase
Expand Down
25 changes: 17 additions & 8 deletions loopx/capabilities/pr_review_queue/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -933,10 +933,15 @@ update is not required merely because GitHub reports `BEHIND`: the gate also
reads GitHub's independent `mergeable` result and permits a conflict-free
`MERGEABLE` head when every other check passes. `CONFLICTING` or `DIRTY` still
requires repair; `BEHIND` with missing or unknown mergeability remains unverified.
Branch protection may still require a separately authorized admin bypass. An admin
bypass may satisfy GitHub's author-owned
self-review limitation, but it never overrides this capability gate or supplies
user merge authority.
Branch protection may still require a separately authorized admin bypass. An
explicit maintainer admin bypass may override a `ready=false` readiness or
GitHub policy hold, including GitHub's author-owned self-review limitation. It
does not make missing exact-head review, required validation, unresolved review
threads or findings, a changed head, or an unmergeable conflict optional, and
it never supplies merge authority by itself.
The `admin_bypass_overrides_this_gate` authority field describes whether every
blocker on this exact observation is in that override scope; it is false when
non-bypassable review, thread, head, or mergeability evidence is missing.

`pull_request_merge_readiness_observation_v0` is the Goal-scoped scheduling
receipt for that gate. It persists only the exact-head material fingerprint and
Expand Down Expand Up @@ -1262,10 +1267,14 @@ The packet should let a reviewer move through PRs in order:
7. Treat `metadata_risk_hint` only as queue-ordering metadata. It must not be
copied as the final risk judgement.
8. Recheck the exact head, then decide `approve`, `request changes`, `defer`, or
`merge after checks`. Immediately before merge, require
`loopx pr-review --check-merge-readiness NUMBER@HEAD_OID` to
return `ready=true`. Add `--goal-id GOAL` only for already Goal-managed work;
standalone delivery requires no registration or binding.
`merge after checks`. Immediately before merge, run
`loopx pr-review --check-merge-readiness NUMBER@HEAD_OID` and record its
result. Add `--goal-id GOAL` only for already Goal-managed work; standalone
delivery requires no registration or binding. `ready=true` permits the
normal path. Explicit maintainer authorization may use admin bypass to
override a `ready=false` readiness or GitHub policy hold, but never missing
exact-head review, required validation, unresolved findings or threads, a
changed head, an unmergeable conflict, or absent merge authority.

A response that only lists `Open` and `Merged` PRs, scale, and recommended next
order is incomplete for `/loopx-pr-review`; it should continue into the
Expand Down
24 changes: 22 additions & 2 deletions loopx/capabilities/pr_review_queue/merge_readiness.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@


SCHEMA_VERSION = "pull_request_merge_readiness_v0"
_ADMIN_BYPASS_OVERRIDABLE_BLOCKERS = frozenset(
{
"github_review_decision_not_approved",
"repository_merge_state_blocked",
"status_checks_failed",
"status_checks_incomplete",
"status_checks_missing",
"status_checks_pending",
}
)


def _mapping(value: object) -> Mapping[str, Any]:
Expand All @@ -31,7 +41,7 @@ def build_merge_readiness(
source: str,
wait_for_ci: bool = True,
) -> dict[str, Any]:
"""Return a read-only pre-merge gate without granting merge authority."""
"""Report readiness; explicit admin bypass can cover policy holds, not evidence."""

number = item.get("number")
head_oid = _text(item.get("head_oid"))
Expand Down Expand Up @@ -97,6 +107,9 @@ def build_merge_readiness(
blockers.append("repository_merge_state_blocked")

blockers = list(dict.fromkeys(blockers))
admin_bypass_overrides_this_gate = not any(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 合并后发现的独立反例:有效同头审查、完整线程和成功合成检查均保持不变,仅令 mergeStateStatus=BLOCKED,mergeable=UNKNOWN 或省略。实际 fixture CLI 两种情况都返回 admin_bypass_overrides_this_gate=true;不可变基线为 false。这里的未知合并性没有被前面的分支加入非策略 blocker,所以仅检查现有 blocker 的集合不足以保证新 README 声明的 open_unchanged_mergeable_head。请先确认 MERGEABLE 或把未知/缺失归入不可绕过的 unverified,并保留 ready=false、authority=false 与对应 CLI 控制用例。

blocker not in _ADMIN_BYPASS_OVERRIDABLE_BLOCKERS for blocker in blockers
)
material_state = readiness_material_state(
repository=repository,
item=item,
Expand Down Expand Up @@ -131,7 +144,14 @@ def build_merge_readiness(
"material_fingerprint": readiness_material_fingerprint(material_state),
"authority": {
"grants_merge_authority": False,
"admin_bypass_overrides_this_gate": False,
"admin_bypass_overrides_this_gate": admin_bypass_overrides_this_gate,
"admin_bypass_scope": "readiness_and_github_review_policy",
"admin_bypass_requires": [
"explicit_user_or_maintainer_merge_authorization",
"valid_exact_head_review_and_required_validation",
"complete_review_threads_without_unresolved_findings",
"open_unchanged_mergeable_head",
],
"required_timing": "immediately_before_merge",
"head_change_action": "restart_review_and_rerun_gate",
},
Expand Down
34 changes: 19 additions & 15 deletions skills/loopx-pr-merge/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,18 @@ authorized: a diff read, green CI, or pull-request metadata is not a substitute.
remote reviewed head. The source-checkout `loopx canary premerge` includes
the same direct blocking check. A failed check holds merge regardless of CI
waiting or authorized admin bypass. Working-tree changes cannot satisfy it.
4. Immediately before the merge, run `loopx --format json pr-review --repo
OWNER/REPO --check-merge-readiness NUMBER@HEAD_OID`. Merge only when it
returns `ready=true` for that unchanged head. For work already managed by a
Goal, pass `--goal-id GOAL` to retain its configured CI policy and record the
compact readiness observation. Standalone work needs no Goal, registration
or binding; its readiness check performs no Goal write.

A rebase or head update restarts review, and admin bypass never overrides this
gate: it needs explicit owner authorization and never substitutes for the
evidence.
4. Record the readiness result immediately before merge: run `loopx --format
json pr-review --repo OWNER/REPO --check-merge-readiness NUMBER@HEAD_OID`.
`ready=true` permits the normal merge path. With explicit maintainer
authorization, admin bypass may override a readiness or GitHub policy hold.
The bypass does not replace valid exact-head review,
required validation, complete review threads without unresolved findings, an
open unchanged mergeable head, or the separately granted merge authority.
For Goal-managed work, pass `--goal-id GOAL` to retain its configured CI
policy and record the compact readiness observation. Standalone work needs
no Goal, registration or binding; its readiness check performs no Goal write.

A rebase or head update restarts review and readiness qualification.

## Configured CI Waiting

Expand Down Expand Up @@ -82,11 +84,13 @@ Control-plane, runtime and product surfaces require explicit maintainer merge
authorization. This includes behavior in `loopx/**`, `apps/**` or `packages/**`,
permissions or authority, persisted state/receipts, quota/scheduler/todo rules,
and CLI/API contracts. Propose and review the exact head, pass required
validation, and obtain ready merge-readiness for that unchanged head. Only then
may an explicitly authorized merge or delivery batch use admin bypass. Small
diffs, green CI or published reviews do not grant merge authority. Unclear
affected boundaries use the same authorization rule; without it, leave the
merge to the maintainer. Bypass never replaces review, validation or readiness.
validation, and run readiness for that unchanged head. An explicit maintainer
authorization may then use admin bypass when readiness remains false for a
readiness or GitHub policy hold. Small diffs, green CI or published reviews do
not grant merge authority. Unclear affected boundaries use the same
authorization rule; without it, leave the merge to the maintainer. Bypass never
replaces exact-head review, validation, thread/finding resolution or merge
authority.

Maintainers may self-merge single-purpose, low-risk public-doc, contributor
metadata, test, example or narrow-cleanup PRs when all these conditions hold:
Expand Down
12 changes: 7 additions & 5 deletions skills/loopx-pr-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,13 @@ owner/GitHub authority, preserving discussion and unresolved reviews. Merge rout
`loopx-pr-merge`; `APPROVE` is not merge authority; a public blocker belongs on the PR.

Immediately before every merge, run `loopx --format json pr-review --repo OWNER/REPO
--check-merge-readiness NUMBER@HEAD_OID`; require `ready=true`. Pass `--goal-id GOAL`
when the work is already Goal-managed; its compact observation suppresses only unchanged
requalification. Standalone work requires no Goal and writes no Goal observation.
Material change reopens qualification, admin bypass never overrides this gate, and author
fallback needs user authority.
--check-merge-readiness NUMBER@HEAD_OID`. Pass `--goal-id GOAL` when the work is already
Goal-managed; its compact observation suppresses only unchanged requalification.
Standalone work requires no Goal and writes no Goal observation. `ready=true` needs no
bypass. Explicit maintainer authorization means admin bypass may override a readiness
or GitHub policy hold, but never missing exact-head review,
required validation, unresolved findings or threads, a changed head, an unmergeable
conflict, or absent merge authority. Material change reopens qualification.

## Full PR Review And Bilingual Format

Expand Down
14 changes: 14 additions & 0 deletions skills/loopx-self-repair/references/repair-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,20 @@ raw logs and private traces stay in ignored local paths.
| A prompt migration reports success but the App restores the old body | A running host caches automation rows; disk CAS cannot invalidate scheduler memory. Use native automation_update and read back through that host, or close the host for offline migration and verify after restart. | Keep update-time reconciliation fail-closed while the App runs, and cover unchanged SQLite/TOML plus a complete native update request. |
| A newly started heartbeat persists execution rules despite a successful runtime update | Startup and migration used different renderers, and installed skill guidance still told the agent to save raw thin output. Generate the v2 loader from the activation command and replace stale lifecycle instructions. | Exercise onboarding to bootstrap to one-level fresh CLI load, authentic legacy migration, and absent/duplicate-command rejection. |

## Readiness bypass is misreported as impossible

Symptom: a reviewer has explicit maintainer authorization for an admin-bypass
merge and completed exact-head review plus required validation, but the
readiness payload, skills, AGENTS policy, and smoke independently describe
admin bypass as unable to override `ready=false`. Treat readiness as a truthful
diagnostic and two separate decisions: whether the normal merge path is ready,
and whether an authorized bypass may cover a readiness or GitHub policy hold.
Keep exact-head review, required validation, changed-head detection, merge
conflicts, review-thread/finding resolution, and merge authority as separate
non-bypass prerequisites. Repair the payload field plus every instruction and
smoke that repeats the old rule, then exercise one denied-by-default path and
one explicitly authorized bypass path.

## Review ignores a Goal's CI waiting configuration

Symptom: a managed review ignores `pull_request_review.wait_for_ci=false` and waits on remote CI after local evidence is complete. Read the named Goal's configuration and pass `--goal-id` through both review and readiness. Repair the capability transport, packet, and readiness owner together; retain required local checks, exact-head binding, valid approval, unresolved-thread rejection, and merge authority. Never change the global default to repair one Goal. Validate both default-on and explicit-off paths through the production CLI and the shared configuration editor.
Expand Down
36 changes: 36 additions & 0 deletions tests/test_pr_review_github_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -1062,9 +1062,44 @@ def test_merge_readiness_binds_review_body_checks_and_threads_to_exact_head() ->
assert ready["ready"] is True, ready
assert ready["blocking_reasons"] == [], ready
assert ready["authority"]["grants_merge_authority"] is False, ready
assert ready["authority"]["admin_bypass_overrides_this_gate"] is True, ready
assert ready["authority"]["admin_bypass_scope"] == (
"readiness_and_github_review_policy"
), ready
assert ready["review_conclusion"]["verdict"] == "APPROVE", ready


def test_explicit_admin_bypass_may_override_a_readiness_policy_hold() -> None:
pr = _merge_ready_pr(review_decision="REVIEW_REQUIRED")
pr["mergeStateStatus"] = "BLOCKED"
readiness = merge_readiness_module.build_pr_merge_readiness_packet(
pull_request=pr,
repository="owner/repo",
expected_exact_head=f"4110@{HEAD_1}",
reviewer_login="maintainer",
review_threads=_complete_review_threads(),
source="fixture",
wait_for_ci=False,
)

assert readiness["ready"] is False, readiness
assert readiness["blocking_reasons"] == ["github_review_decision_not_approved"]
assert readiness["admin_bypass_required"] is True
assert readiness["authority"] == {
"grants_merge_authority": False,
"admin_bypass_overrides_this_gate": True,
"admin_bypass_scope": "readiness_and_github_review_policy",
"admin_bypass_requires": [
"explicit_user_or_maintainer_merge_authorization",
"valid_exact_head_review_and_required_validation",
"complete_review_threads_without_unresolved_findings",
"open_unchanged_mergeable_head",
],
"required_timing": "immediately_before_merge",
"head_change_action": "restart_review_and_rerun_gate",
}


def test_merge_readiness_rejects_review_text_for_pre_update_head() -> None:
# GitHub may associate a preserved review with the merge-from-main commit even
# though its public evidence still names the earlier reviewed head.
Expand All @@ -1083,6 +1118,7 @@ def test_merge_readiness_rejects_review_text_for_pre_update_head() -> None:

assert stale["ready"] is False, stale
assert "current_head_review_missing_or_invalid" in stale["blocking_reasons"]
assert stale["authority"]["admin_bypass_overrides_this_gate"] is False
assert (
"review_body_missing_standalone_bilingual_format"
in (stale["review_conclusion"]["invalid_reasons"])
Expand Down
Loading