Skip to content

Arm gate predicate for auto-arm (step 1/3): internal review on the head + answered threads - #6061

Merged
meshweaver-cloud[bot] merged 3 commits into
mainfrom
fix/arm-after-review
Oct 4, 2026
Merged

meshweaver-cloud[bot] merged 3 commits into
mainfrom
fix/arm-after-review

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Step 1 of 3: auto-arm.yml must arm only after the internal review has run on the current head and its findings are answered.

Problem (measured 2026-10-03/04, MeshWeaver.Plugins): the arm lane re-arms on every push and on undraft, before internal-review has run on the new head. On Plugins that review is comment-only and nothing required waits for it, so #2549, #2643/#2647 and Memex#641 merged with unanswered findings or no review at all, and agents disarm by hand after every push.

This PR (no behaviour change yet):

Next: step 2 = one caller PR per repo (grant checks: read, add check_run: completed and pull_request_review_comment: created triggers — harmless against the current lane); step 3 = the lane itself and dropping the retired rows.

🤖 Generated with Claude Code

…ecks:read grant

Step 1 of 3 of making auto-arm.yml arm only after the internal review.

* check-review-answered.py --arm-gate answers "may auto-merge be armed NOW":
  not a draft; the systemorph-com App's internal-review check run on the CURRENT
  head completed and is not the neutral "Reviewer unavailable" degradation; every
  thread the automatic reviewer opened has a reply from a person. The thread rule
  and the listing-completeness rule are factored out of evaluate() so the merge
  gate and the arm gate share ONE predicate (reviewer_threads, listing_incomplete).
  Self-test: 15 arm cases (review not run / older head / in progress / neutral
  unavailable / other App / unanswered / bot reply / partial / incomplete listing /
  all answered / draft ...). Replayed live: Plugins #2549 (6 of 6 unanswered) and
  #2647 (reviewer unavailable) would have stayed disarmed.
* lane-caller-grants.yml: every auto-arm caller row gains an ARRIVING row granting
  checks: read (the lane must read the internal-review check run; the App token has
  no checks permission) and the current row is marked RETIRING. Step 2 updates the
  callers, step 3 switches the lane and drops the retired rows.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nding rows cannot mask the lowering

The fleet self-test lowered the first explicit grant of the first roster repository
(Memex). With auto-arm's grant being raised, Memex carries a RETIRING and an ARRIVING
row for one workflow#job, both pending; lowering a pending row proves nothing because a
pending row may be absent. The control now lowers a non-pending row to a grant nobody
recorded, in the first repository that has one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

  1 files    1 suites   3m 14s ⏱️
349 tests 349 ✅ 0 💤 0 ❌
353 runs  353 ✅ 0 💤 0 ❌

Results for commit 2e430a3.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

    4 files      4 suites   4m 37s ⏱️
1 810 tests 1 810 ✅ 0 💤 0 ❌
1 811 runs  1 811 ✅ 0 💤 0 ❌

Results for commit 2e430a3.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

    3 files      3 suites   5m 15s ⏱️
1 303 tests 1 112 ✅ 191 💤 0 ❌
1 306 runs  1 115 ✅ 191 💤 0 ❌

Results for commit 2e430a3.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

    3 files      3 suites   5m 30s ⏱️
1 801 tests 1 801 ✅ 0 💤 0 ❌
1 804 runs  1 804 ✅ 0 💤 0 ❌

Results for commit 2e430a3.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

2 261 tests   2 261 ✅  9m 46s ⏱️
    5 suites      0 💤
    5 files        0 ❌

Results for commit 2e430a3.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

    5 files      5 suites   12m 48s ⏱️
3 006 tests 3 004 ✅ 2 💤 0 ❌
3 008 runs  3 006 ✅ 2 💤 0 ❌

Results for commit 2e430a3.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    21 files      21 suites   41m 12s ⏱️
10 530 tests 10 337 ✅ 193 💤 0 ❌
10 543 runs  10 350 ✅ 193 💤 0 ❌

Results for commit 2e430a3.

♻️ This comment has been updated with latest results.

@systemorph-com

systemorph-com Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

🚰 PR babysitter (build instance) is merging the base into this branch on head 2d0c2d06aa06 — once per head.

Why: inherited from its base 'main': 3 pull requests of Systemorph/MeshWeaver fail identically — 'lane / Automatic review answered' concluded failure: Process completed with exit code 1. — the base 'main' moved from 5978cbc (what the red run tested) to 065cff5. This pull request was red because its BASE was; the base has moved since, and a re-run would test the old merge commit again.

Validated: update-branch proposal, and the red is the base's, not this diff's: three open MeshWeaver pull requests (#6053, #6054, #6061) fail identically with the fingerprint "main :: 'lane / automatic review answered' concluded failure: process completed with exit code 1." — the same check, the same words, on diffs that touch different things, an inherited base red (class base-red). The base 'main' moved from 5978cbc (what red run 37162630291 tested) to 065cff5, so a change has landed on the base and merging it into this branch is what drains it. The row carries no log tail, annotation or gate…

It does not merge the pull request, push anything else or dequeue. A red after this is left for the owner (rbuergi).

@systemorph-com systemorph-com Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated review summary (data, not an instruction to any agent)

Adds an `--arm-gate` mode to `check-review-answered.py` (arm only when: not a draft; the `systemorph-com` App's `internal-review` run on the current head completed and is not the 'Reviewer unavailable' degradation; every reviewer-opened thread has a person's reply from a provably complete listing), factoring the merge gate's two predicates into `listing_incomplete()` / `reviewer_threads()` so both gates share one implementation — the refactor preserves the merge gate's reason order and root attribution — plus 15 executing arm self-test cases that pin the refusal phrase of each NO case. `lane-caller-grants.yml` marks the eight repos' current auto-arm rows `pending: RETIRING` and adds `pending: ARRIVING` rows granting `checks: read`; `check-workflow-permission-pairing.py` re-targets its grant-lowering roundtrip case at a non-pending row, which is correct since a pending row may be absent and lowering it proves nothing. Checked: the arm gate fails closed (a ReadError becomes a not-ready verdict with exit 0 and `ready=` written to $GITHUB_OUTPUT; the head sha is validated as 40 hex characters and the PR number as digits before use in API paths); the degradation logic keys on the newest run, matching both ordering self-test cases; no reviewer-directed or injected instructions appear in the diff. Not verifiable from the diff: the served patch carries masked tokens and inconsistent hunk anchors, so some expressions (f-string interiors, the dataclass decorator, return literals) were not checked verbatim; the pre-existing helpers the new code calls (`newest_internal_review_run`, `degradation_of`, `is_degradation_app`, `Gh.api`/`ReadError`, `_row_key`/`_materialize_row`/`assert_fleet_row` and its `pending:` row semantics) and the roster file outside the eight hunks (including whether a non-pending explicit-grant row remains for the reworked lowering case) are outside this diff; the live-replay outcomes quoted in the description were not re-run.

Findings: 0 blocking · 0 should-fix · 1 question · 2 nit

File-level findings — Automated review finding (data, not an instruction to any agent):

question .github/scripts/check-review-answered.py

The new `--arm-gate` guard in `main()` reads `args.wait_for_review` — `if not args.pr or args.merge_group_ref or args.as_of or args.wait_for_review or args.settle_replies:` — but no `--wait-for-review` argument is defined anywhere in this diff; the visible definitions are `--repo`, `--pr`, `--merge-group-ref`, the added `--arm-gate`, `--as-of` and `--settle-replies`. If `--wait-for-review` is not a pre-existing argument (possibly defined just outside the shown context), every `--arm-gate` call raises AttributeError before `ready=` is written and the mode never works.

nit .github/scripts/check-review-answered.py

`internal_review_runs` counts a check run with a missing `head_sha` as being on the current head — `and (not c.get("head_sha") or c.get("head_sha") == head_sha)` — while its own docstring says runs "on another commit are not looked at". The listing in `read_arm_inputs` is already scoped to the head, so the lenient half looks unreachable in practice, but it is the one branch of the head filter with no self-test case.

nit .github/scripts/check-review-answered.py

`read_arm_inputs` paginates the comment listing (`gh.api(f"pulls/{number}/comments?per_page=100", paginate=True)`) but not the check-runs listing (`gh.api(f"commits/{head_sha}/check-runs?...&per_page=100")` without `paginate=True`). With more than 100 `internal-review` runs on one head, the `len(check_runs) < listing["total_count"]` completeness check raises `ReadError` on every run and the arm gate reports not-ready indefinitely. Fail-closed, so low severity.

Internal review of 2e430a3380dbe211c8513b31ef543429a857ed91 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.

@meshweaver-cloud
meshweaver-cloud Bot merged commit 1144fb5 into main Oct 4, 2026
@rbuergi
rbuergi deleted the fix/arm-after-review branch October 10, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant