Compare commits
13 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 7ccd59e630 | |||
| d768d8667b | |||
| dcf9d3cdc1 | |||
| b1475b1f71 | |||
| b2d5f88f98 | |||
| 31283a292a | |||
| bc7c45f3d6 | |||
| bf0db08c7c | |||
| e441def8b3 | |||
| 51f83260df | |||
| 2fa68b1f23 | |||
| 48b6011e17 | |||
| cc99d3fff4 |
@@ -4,7 +4,11 @@
|
||||
Gitea 1.22.6+ has auto-merge (`pull_auto_merge`) but no GitHub-style merge
|
||||
queue. This script provides the missing serialized policy in user space:
|
||||
|
||||
1. Pick the oldest open PR carrying QUEUE_LABEL (skipping HOLD_LABEL).
|
||||
1. Scan open same-repo PRs that are NOT opted out (auto-discovery, see below),
|
||||
oldest-first, skipping drafts, until an ACTIONABLE one is found. A non-ready
|
||||
candidate (REQUEST_CHANGES, mergeable!=True, insufficient genuine approvals,
|
||||
or red required CI) is SKIPPED so it cannot head-of-line block newer ready
|
||||
PRs; the scan continues to the next candidate.
|
||||
2. Refuse to act unless main's BP-required contexts are green.
|
||||
3. Refuse fork PRs; the queue may only mutate same-repo branches.
|
||||
4. If the PR branch does not contain current main, call Gitea's
|
||||
@@ -29,13 +33,42 @@ Authoritative gates (fail-closed):
|
||||
approvals present). It is NEVER used to bypass a failing REQUIRED context
|
||||
or missing approvals.
|
||||
|
||||
Head-of-line (HOL) safety: a permanent permission/4xx merge error
|
||||
(403/404/405) HOLDS the PR (applies HOLD_LABEL) so the queue advances to the
|
||||
next PR instead of re-selecting the same wedged PR every tick. Likewise, a
|
||||
persistent branch-update conflict (the /update endpoint returns HTTP 409
|
||||
because the PR branch cannot be merged with main without manual rebase) HOLDS
|
||||
the PR — a conflict will not self-resolve, so retrying it every tick would
|
||||
HOL-block every ready PR behind it (issue #2352).
|
||||
Auto-discovery (opt-OUT, label-optional):
|
||||
The queue is SELF-SUSTAINING — a ready PR does NOT need a human (or an agent)
|
||||
to add the `merge-queue` label first. When AUTO_DISCOVER is on (default), the
|
||||
queue enumerates ALL open same-repo PRs and considers any that meets the full
|
||||
merge bar (genuine approvals on current head + BP-required green + mergeable +
|
||||
no open REQUEST_CHANGES). The merge bar above is UNCHANGED; auto-discovery only
|
||||
changes WHICH PRs are considered, not whether they are mergeable.
|
||||
|
||||
This deliberately removes the historical dependency on an agent adding the
|
||||
`merge-queue` label — agent Gitea tokens lack `write:issue` (labels are
|
||||
issue-scoped), so they could never self-label and the queue stalled. The label
|
||||
is now OPTIONAL metadata, not a gate.
|
||||
|
||||
SAFETY is preserved as opt-OUT: any PR carrying an opt-out label
|
||||
(OPT_OUT_LABELS — `merge-queue-hold`, `do-not-auto-merge`, `wip`, `draft` by
|
||||
default) is skipped (never auto-considered, never merged). Draft PRs
|
||||
(draft=true STATE) are also skipped; the literal `draft` LABEL is an
|
||||
additional explicit opt-out a human can apply without converting to a draft.
|
||||
A human who wants to keep a PR out of autonomous merging just adds one of
|
||||
those labels. Setting AUTO_DISCOVER=0 restores the legacy opt-IN behaviour
|
||||
(only PRs already carrying QUEUE_LABEL are considered).
|
||||
|
||||
Head-of-line (HOL) safety has two complementary layers:
|
||||
(a) The queue SCANS THROUGH the FIFO candidate list and skips any non-ready
|
||||
PR (REQUEST_CHANGES, mergeable!=True, insufficient genuine approvals, or
|
||||
red required CI) instead of locking on the oldest and waiting, so a PR
|
||||
that can never become ready without human action does not block newer
|
||||
ready PRs.
|
||||
(b) For the candidate the scan acts on, two permanent failure modes HOLD the
|
||||
PR (apply HOLD_LABEL) and let the scan CONTINUE to the next candidate
|
||||
rather than re-selecting the same wedged PR every tick:
|
||||
- a permanent permission/4xx merge error (403/404/405), and
|
||||
- a persistent branch-update conflict (the /update endpoint returns
|
||||
HTTP 409 because the PR branch cannot be merged with main without a
|
||||
manual rebase). A conflict will not self-resolve, so retrying it
|
||||
every tick would HOL-block every ready PR behind it (issue #2352).
|
||||
|
||||
Status-fetch is fail-closed: if the combined status for a sha cannot be
|
||||
fetched, the PR is skipped this tick (never treated as green).
|
||||
@@ -68,6 +101,33 @@ WATCH_BRANCH = _env("WATCH_BRANCH", default="main")
|
||||
QUEUE_LABEL = _env("QUEUE_LABEL", default="merge-queue")
|
||||
HOLD_LABEL = _env("HOLD_LABEL", default="merge-queue-hold")
|
||||
UPDATE_STYLE = _env("UPDATE_STYLE", default="merge")
|
||||
# Auto-discovery (opt-OUT). When truthy (default), the queue considers ALL open
|
||||
# same-repo PRs that meet the merge bar, not only PRs already carrying
|
||||
# QUEUE_LABEL — so the queue is self-sustaining without any human/agent labeling
|
||||
# (agent tokens lack write:issue and cannot self-label). Set AUTO_DISCOVER=0 to
|
||||
# restore the legacy opt-IN behaviour (QUEUE_LABEL required to be considered).
|
||||
AUTO_DISCOVER = _env("AUTO_DISCOVER", default="1").strip().lower() not in {
|
||||
"0",
|
||||
"false",
|
||||
"no",
|
||||
"off",
|
||||
"",
|
||||
}
|
||||
# Opt-OUT labels. A PR carrying ANY of these is skipped (never auto-considered,
|
||||
# never merged) — the human escape hatch from autonomous merging. HOLD_LABEL is
|
||||
# always included so the existing hold semantics keep working. `do-not-auto-merge`
|
||||
# and `wip` let a human keep a PR out of the auto-merge path without removing it.
|
||||
# `draft` is included as a literal label too: Gitea draft STATE (draft=true) is
|
||||
# already skipped via _issue_is_draft, but a "draft" LABEL is an additional,
|
||||
# explicit opt-out signal a human can apply without converting the PR to a draft.
|
||||
OPT_OUT_LABELS = {
|
||||
name.strip()
|
||||
for name in _env(
|
||||
"OPT_OUT_LABELS",
|
||||
default="do-not-auto-merge,wip,draft",
|
||||
).split(",")
|
||||
if name.strip()
|
||||
} | ({HOLD_LABEL} if HOLD_LABEL else set())
|
||||
REQUIRED_CONTEXTS_RAW = _env(
|
||||
"REQUIRED_CONTEXTS",
|
||||
default=(
|
||||
@@ -410,6 +470,85 @@ def choose_next_queued_issue(
|
||||
return candidates[0] if candidates else None
|
||||
|
||||
|
||||
def _issue_is_draft(issue: dict) -> bool:
|
||||
"""True if the issue/PR is a draft.
|
||||
|
||||
The /issues listing exposes draft state under the `pull_request` sub-object
|
||||
(`{"draft": true}`); some Gitea versions also surface a top-level `draft`.
|
||||
Either is honoured. Drafts are never auto-considered for merging.
|
||||
"""
|
||||
pr = issue.get("pull_request")
|
||||
if isinstance(pr, dict) and pr.get("draft") is True:
|
||||
return True
|
||||
return issue.get("draft") is True
|
||||
|
||||
|
||||
def choose_candidate_issues(
|
||||
issues: list[dict],
|
||||
*,
|
||||
queue_label: str,
|
||||
opt_out_labels: set[str],
|
||||
auto_discover: bool,
|
||||
) -> list[dict]:
|
||||
"""All open PRs eligible for a merge attempt this tick, oldest-first.
|
||||
|
||||
This is the auto-discovery selector. It does NOT change the merge bar — it
|
||||
only changes WHICH PRs are considered:
|
||||
|
||||
- auto_discover=True (default): every open same-repo PR is a candidate,
|
||||
EXCEPT those carrying an opt-out label or marked draft. The QUEUE_LABEL
|
||||
is optional metadata, not a gate, so a ready PR reaches the queue with no
|
||||
human/agent labeling (the write:issue gap is removed).
|
||||
- auto_discover=False: legacy opt-IN — only PRs carrying queue_label are
|
||||
candidates (still skipping opt-out labels and drafts).
|
||||
|
||||
Opt-out is the safety escape hatch: any opt_out_labels member present skips
|
||||
the PR entirely (never considered, never merged). Ordering is oldest-first
|
||||
(created_at, then number) to preserve the serialized FIFO ordering.
|
||||
|
||||
Returns the FULL ordered list (not just the head) so process_once can SCAN
|
||||
THROUGH non-ready candidates instead of locking on the oldest. A non-ready
|
||||
auto-discovered PR (e.g. one with REQUEST_CHANGES or mergeable=false, which
|
||||
can never become ready without human action) must NOT head-of-line block the
|
||||
newer ready PRs behind it — the readiness check happens per-candidate in
|
||||
process_once, and a `wait` candidate is skipped to the next one.
|
||||
"""
|
||||
candidates = []
|
||||
for issue in issues:
|
||||
if "pull_request" not in issue:
|
||||
continue
|
||||
labels = label_names(issue)
|
||||
if opt_out_labels & labels:
|
||||
continue # opt-out: human kept this PR out of autonomous merging
|
||||
if _issue_is_draft(issue):
|
||||
continue # drafts are never auto-merged
|
||||
if not auto_discover and queue_label not in labels:
|
||||
continue # legacy opt-IN: require the queue label
|
||||
candidates.append(issue)
|
||||
candidates.sort(key=lambda issue: (issue.get("created_at") or "", int(issue["number"])))
|
||||
return candidates
|
||||
|
||||
|
||||
def choose_next_candidate_issue(
|
||||
issues: list[dict],
|
||||
*,
|
||||
queue_label: str,
|
||||
opt_out_labels: set[str],
|
||||
auto_discover: bool,
|
||||
) -> dict | None:
|
||||
"""The oldest eligible candidate, or None. Thin head-of-list wrapper around
|
||||
choose_candidate_issues; retained for callers/tests that only want the head.
|
||||
process_once uses the full list (choose_candidate_issues) so it can scan past
|
||||
non-ready PRs rather than HOL-block on the oldest."""
|
||||
candidates = choose_candidate_issues(
|
||||
issues,
|
||||
queue_label=queue_label,
|
||||
opt_out_labels=opt_out_labels,
|
||||
auto_discover=auto_discover,
|
||||
)
|
||||
return candidates[0] if candidates else None
|
||||
|
||||
|
||||
def pr_contains_base_sha(commits: list[dict], base_sha: str) -> bool:
|
||||
for commit in commits:
|
||||
sha = commit.get("sha") or commit.get("id")
|
||||
@@ -577,6 +716,31 @@ def list_queued_issues() -> list[dict]:
|
||||
return body
|
||||
|
||||
|
||||
def list_candidate_issues(*, auto_discover: bool) -> list[dict]:
|
||||
"""Open PR issues eligible for consideration this tick.
|
||||
|
||||
With auto_discover=True (default) this enumerates ALL open PRs (no label
|
||||
filter) so the queue is self-sustaining — a ready PR is considered without
|
||||
any human/agent first adding QUEUE_LABEL. With auto_discover=False it falls
|
||||
back to the legacy label-filtered listing (opt-IN). Opt-out filtering and
|
||||
draft-skipping happen in choose_next_candidate_issue, not here.
|
||||
"""
|
||||
if not auto_discover:
|
||||
return list_queued_issues()
|
||||
_, body = api(
|
||||
"GET",
|
||||
f"/repos/{OWNER}/{NAME}/issues",
|
||||
query={
|
||||
"state": "open",
|
||||
"type": "pulls",
|
||||
"limit": "50",
|
||||
},
|
||||
)
|
||||
if not isinstance(body, list):
|
||||
raise ApiError("candidate issues response not list")
|
||||
return body
|
||||
|
||||
|
||||
def get_pull(pr_number: int) -> dict:
|
||||
_, body = api("GET", f"/repos/{OWNER}/{NAME}/pulls/{pr_number}")
|
||||
if not isinstance(body, dict):
|
||||
@@ -731,45 +895,181 @@ def process_once(*, dry_run: bool = False) -> int:
|
||||
print(f"::notice::queue paused: {WATCH_BRANCH}@{main_sha[:8]} required contexts not green: {', '.join(main_bad)}")
|
||||
return 0
|
||||
|
||||
issue = choose_next_queued_issue(
|
||||
list_queued_issues(),
|
||||
candidates = choose_candidate_issues(
|
||||
list_candidate_issues(auto_discover=AUTO_DISCOVER),
|
||||
queue_label=QUEUE_LABEL,
|
||||
hold_label=HOLD_LABEL,
|
||||
opt_out_labels=OPT_OUT_LABELS,
|
||||
auto_discover=AUTO_DISCOVER,
|
||||
)
|
||||
if not issue:
|
||||
print("::notice::merge queue empty")
|
||||
if not candidates:
|
||||
print(
|
||||
"::notice::no merge candidates "
|
||||
f"(auto_discover={'on' if AUTO_DISCOVER else 'off'})"
|
||||
)
|
||||
return 0
|
||||
|
||||
# HOL fix: SCAN THROUGH the FIFO candidate list until a PR we can ACT on is
|
||||
# found, instead of locking on the oldest and waiting. A non-ready candidate
|
||||
# (decision.action == "wait": REQUEST_CHANGES, mergeable!=True, insufficient
|
||||
# genuine approvals, or red required CI) is SKIPPED — it must NOT head-of-line
|
||||
# block the newer ready PRs behind it. The merge bar is unchanged: a skipped
|
||||
# PR is never merged, and the first ACTIONABLE candidate (an "update" that
|
||||
# advances a stale branch, or a fully-ready "merge") terminates the scan.
|
||||
#
|
||||
# `update` is treated as actionable, not skippable: a PR whose head merely
|
||||
# lacks current main is in a legitimate in-progress state (updating it +
|
||||
# rerunning CI moves it toward ready), unlike a PR that can never become
|
||||
# ready without a human (RC / conflict), which is a `wait` and gets skipped.
|
||||
for issue in candidates:
|
||||
decision, ctx = _evaluate_candidate(
|
||||
issue,
|
||||
main_sha=main_sha,
|
||||
main_status=main_status,
|
||||
required_contexts=contexts,
|
||||
required_approvals=required_approvals,
|
||||
dry_run=dry_run,
|
||||
)
|
||||
if decision is None:
|
||||
continue # not merge-eligible (not-open / opted-out / fork / wrong base)
|
||||
pr_number = ctx["pr_number"]
|
||||
print(f"::notice::PR #{pr_number} decision={decision.action}: {decision.reason}")
|
||||
if decision.action == "wait":
|
||||
# Non-ready: skip to the next candidate (no HOL block, no merge).
|
||||
continue
|
||||
if decision.action == "update":
|
||||
try:
|
||||
update_pull(pr_number, dry_run=dry_run)
|
||||
except BranchUpdateConflictError as exc:
|
||||
# The branch cannot be updated with main because of a real
|
||||
# conflict (HTTP 409 from /update). This is the #2352 HOL guard:
|
||||
# a conflict will not self-resolve without a human/agent rebase,
|
||||
# so re-attempting the update every tick would head-of-line block
|
||||
# every ready PR behind it. HOLD this PR (apply HOLD_LABEL, which
|
||||
# is an opt-out label so later ticks skip it) and CONTINUE the
|
||||
# scan so a newer ready PR can still merge this tick. Fail-closed:
|
||||
# a held PR is skipped, never merged.
|
||||
sys.stderr.write(
|
||||
f"::error::branch-update conflict for PR #{pr_number}: {exc}\n"
|
||||
)
|
||||
hold_note = (
|
||||
"merge-queue: could not update this branch with "
|
||||
f"`{WATCH_BRANCH}` — the update returned a merge conflict "
|
||||
f"(HTTP 409) that the queue cannot auto-resolve ({exc}). "
|
||||
f"Applied `{HOLD_LABEL}` to unblock the queue (HOL guard). "
|
||||
f"Fix: rebase/merge `{WATCH_BRANCH}` into this branch and "
|
||||
f"resolve the conflicts, then remove `{HOLD_LABEL}` to requeue."
|
||||
)
|
||||
hold_pr(pr_number, hold_note, dry_run=dry_run)
|
||||
continue # held — keep scanning for a mergeable candidate
|
||||
post_comment(
|
||||
pr_number,
|
||||
(
|
||||
f"merge-queue: updated this branch with `{WATCH_BRANCH}` at "
|
||||
f"`{main_sha[:12]}`. Waiting for CI on the refreshed head."
|
||||
),
|
||||
dry_run=dry_run,
|
||||
)
|
||||
return 0
|
||||
if decision.ready:
|
||||
latest_main_sha = get_branch_head(WATCH_BRANCH)
|
||||
if latest_main_sha != main_sha:
|
||||
print(
|
||||
f"::notice::main moved {main_sha[:8]} -> {latest_main_sha[:8]}; "
|
||||
"deferring to next tick"
|
||||
)
|
||||
return 0
|
||||
try:
|
||||
merge_pull(pr_number, dry_run=dry_run, force=decision.force)
|
||||
except MergePermissionError as exc:
|
||||
# Permanent merge failure (HTTP 403/404/405). HOLD this PR by
|
||||
# applying HOLD_LABEL (it becomes an opt-out label, so subsequent
|
||||
# ticks skip it) and CONTINUE scanning so the queue still advances
|
||||
# to the next ready PR this tick rather than stalling.
|
||||
sys.stderr.write(f"::error::merge permission error for PR #{pr_number}: {exc}\n")
|
||||
hold_note = (
|
||||
"merge-queue: merge failed with a permanent permission error "
|
||||
f"({exc}). No available token has Can-merge permission for this "
|
||||
f"PR. Applied `{HOLD_LABEL}` to unblock the queue (HOL guard). "
|
||||
f"Fix: grant Can-merge to the queue token, then remove "
|
||||
f"`{HOLD_LABEL}` to requeue."
|
||||
)
|
||||
try:
|
||||
add_label_by_name(pr_number, HOLD_LABEL, dry_run=dry_run)
|
||||
except ApiError as label_exc:
|
||||
# If we cannot even apply the hold label, fall back to a comment
|
||||
# so the wedge is at least visible; do NOT loop on this PR.
|
||||
sys.stderr.write(
|
||||
f"::error::could not apply HOLD_LABEL to PR #{pr_number}: {label_exc}\n"
|
||||
)
|
||||
hold_note += (
|
||||
f"\n\n(NOTE: could not apply the hold label automatically: "
|
||||
f"{label_exc}. Please add `{HOLD_LABEL}` manually.)"
|
||||
)
|
||||
post_comment(pr_number, hold_note, dry_run=dry_run)
|
||||
continue # held — keep scanning for a mergeable candidate
|
||||
return 0
|
||||
return 0
|
||||
|
||||
|
||||
def _evaluate_candidate(
|
||||
issue: dict,
|
||||
*,
|
||||
main_sha: str,
|
||||
main_status: dict,
|
||||
required_contexts: list[str],
|
||||
required_approvals: int,
|
||||
dry_run: bool,
|
||||
) -> tuple[MergeDecision | None, dict]:
|
||||
"""Evaluate a single auto-discovered candidate against the full merge bar.
|
||||
|
||||
Returns (decision, ctx) where ctx carries {"pr_number"}. A None decision
|
||||
means the PR is not merge-eligible at all (not open / opted-out / draft /
|
||||
fork / wrong base) and the caller should skip to the next candidate; for
|
||||
fork / wrong-base the explanatory comment is posted here before returning.
|
||||
|
||||
The merge bar is UNCHANGED from the single-PR path — this only factors the
|
||||
per-PR evaluation out so process_once can scan multiple candidates. A failed
|
||||
status fetch still raises (fail-closed): it propagates to the caller so the
|
||||
PR is never treated as green.
|
||||
"""
|
||||
pr_number = int(issue["number"])
|
||||
ctx = {"pr_number": pr_number}
|
||||
pr = get_pull(pr_number)
|
||||
if pr.get("state") != "open":
|
||||
print(f"::notice::PR #{pr_number} is not open; skipping")
|
||||
return 0
|
||||
return None, ctx
|
||||
# Defensive opt-out/draft re-check on the authoritative pull payload: the
|
||||
# /issues listing's label/draft view can lag, but the merge bar must respect
|
||||
# the live pull state. (choose_candidate_issues already filtered on the
|
||||
# listing; this guards against a stale listing racing a just-added opt-out.)
|
||||
if OPT_OUT_LABELS & label_names(pr):
|
||||
print(f"::notice::PR #{pr_number} carries an opt-out label; skipping")
|
||||
return None, ctx
|
||||
if pr.get("draft") is True:
|
||||
print(f"::notice::PR #{pr_number} is a draft; skipping")
|
||||
return None, ctx
|
||||
if pr.get("base", {}).get("ref") != WATCH_BRANCH:
|
||||
post_comment(pr_number, f"merge-queue: skipped; base branch is not `{WATCH_BRANCH}`.", dry_run=dry_run)
|
||||
return 0
|
||||
return None, ctx
|
||||
if pr.get("head", {}).get("repo_id") != pr.get("base", {}).get("repo_id"):
|
||||
post_comment(pr_number, "merge-queue: skipped; fork PRs are not supported by the serialized queue.", dry_run=dry_run)
|
||||
return 0
|
||||
return None, ctx
|
||||
|
||||
head_sha = pr.get("head", {}).get("sha")
|
||||
if not isinstance(head_sha, str) or len(head_sha) < 7:
|
||||
raise ApiError(f"PR #{pr_number} missing head sha")
|
||||
commits = get_pull_commits(pr_number)
|
||||
current_base = pr_has_current_base(pr, commits, main_sha)
|
||||
# Fail-closed: a failed status fetch raises here and the tick is skipped
|
||||
# (the PR is never treated as green).
|
||||
# Fail-closed: a failed status fetch raises here and propagates (the PR is
|
||||
# never treated as green).
|
||||
pr_status = get_combined_status(head_sha)
|
||||
pr_labels = label_names(pr)
|
||||
# FAIL-CLOSED: Gitea returns mergeable=None (or omits the field) while it is
|
||||
# still COMPUTING conflict state. Only the literal True is decisive proof the
|
||||
# PR is conflict-free; None and False both mean "not (yet) mergeable". We must
|
||||
# NOT autonomously merge on an unknown — treat anything but True as not-yet-
|
||||
# mergeable so evaluate_merge_readiness returns a transient "wait" decision.
|
||||
# This is transient: process_once returns 0 (no hold label, no dequeue) and
|
||||
# the PR is re-checked next tick once Gitea has finished computing mergeability.
|
||||
mergeable_field = pr.get("mergeable")
|
||||
mergeable = mergeable_field is True
|
||||
# mergeable so evaluate_merge_readiness returns a "wait" decision.
|
||||
mergeable = pr.get("mergeable") is True
|
||||
|
||||
reviews = get_pull_reviews(pr_number)
|
||||
approvers, request_changes = genuine_approvals(
|
||||
@@ -779,7 +1079,7 @@ def process_once(*, dry_run: bool = False) -> int:
|
||||
decision = evaluate_merge_readiness(
|
||||
main_status=main_status,
|
||||
pr_status=pr_status,
|
||||
required_contexts=contexts,
|
||||
required_contexts=required_contexts,
|
||||
required_approvals=required_approvals,
|
||||
approvers=approvers,
|
||||
request_changes=request_changes,
|
||||
@@ -787,72 +1087,7 @@ def process_once(*, dry_run: bool = False) -> int:
|
||||
mergeable=mergeable,
|
||||
pr_labels=pr_labels,
|
||||
)
|
||||
|
||||
print(f"::notice::PR #{pr_number} decision={decision.action}: {decision.reason}")
|
||||
if decision.action == "update":
|
||||
try:
|
||||
update_pull(pr_number, dry_run=dry_run)
|
||||
except BranchUpdateConflictError as exc:
|
||||
# The branch cannot be updated with main because of a real conflict
|
||||
# (HTTP 409). This is the HOL fix for issue #2352: previously the
|
||||
# 409 propagated to main() and the tick exited 0 with the PR still
|
||||
# queued, so the NEXT tick re-selected the SAME conflicted PR and
|
||||
# retried the failing update forever — head-of-line-blocking every
|
||||
# ready PR behind it. A conflict will not self-resolve; it needs a
|
||||
# human/agent rebase. So HOLD this PR (HOL guard) and advance to the
|
||||
# next candidate. Fail-closed: a held PR is skipped, never merged.
|
||||
sys.stderr.write(
|
||||
f"::error::branch-update conflict for PR #{pr_number}: {exc}\n"
|
||||
)
|
||||
hold_note = (
|
||||
"merge-queue: could not update this branch with "
|
||||
f"`{WATCH_BRANCH}` — the update returned a merge conflict "
|
||||
f"(HTTP 409) that the queue cannot auto-resolve ({exc}). "
|
||||
f"Applied `{HOLD_LABEL}` to unblock the queue (HOL guard). "
|
||||
f"Fix: rebase/merge `{WATCH_BRANCH}` into this branch and "
|
||||
f"resolve the conflicts, then remove `{HOLD_LABEL}` to requeue."
|
||||
)
|
||||
hold_pr(pr_number, hold_note, dry_run=dry_run)
|
||||
return 0
|
||||
post_comment(
|
||||
pr_number,
|
||||
(
|
||||
f"merge-queue: updated this branch with `{WATCH_BRANCH}` at "
|
||||
f"`{main_sha[:12]}`. Waiting for CI on the refreshed head."
|
||||
),
|
||||
dry_run=dry_run,
|
||||
)
|
||||
return 0
|
||||
if decision.ready:
|
||||
latest_main_sha = get_branch_head(WATCH_BRANCH)
|
||||
if latest_main_sha != main_sha:
|
||||
print(
|
||||
f"::notice::main moved {main_sha[:8]} -> {latest_main_sha[:8]}; "
|
||||
"deferring to next tick"
|
||||
)
|
||||
return 0
|
||||
try:
|
||||
merge_pull(pr_number, dry_run=dry_run, force=decision.force)
|
||||
except MergePermissionError as exc:
|
||||
# Permanent merge failure (HTTP 403/404/405). This is the
|
||||
# head-of-line (HOL) bug fix: previously we returned 0 with the PR
|
||||
# still queued, so the next tick re-selected the SAME wedged PR
|
||||
# forever and the queue never advanced. Instead, HOLD this PR by
|
||||
# applying HOLD_LABEL (choose_next_queued_issue skips held PRs), so
|
||||
# the queue moves on to the next candidate. A maintainer removes
|
||||
# the hold once the permission issue is fixed.
|
||||
sys.stderr.write(f"::error::merge permission error for PR #{pr_number}: {exc}\n")
|
||||
hold_note = (
|
||||
"merge-queue: merge failed with a permanent permission error "
|
||||
f"({exc}). No available token has Can-merge permission for this "
|
||||
f"PR. Applied `{HOLD_LABEL}` to unblock the queue (HOL guard). "
|
||||
f"Fix: grant Can-merge to the queue token, then remove "
|
||||
f"`{HOLD_LABEL}` to requeue."
|
||||
)
|
||||
hold_pr(pr_number, hold_note, dry_run=dry_run)
|
||||
return 0
|
||||
return 0
|
||||
return 0
|
||||
return decision, ctx
|
||||
|
||||
|
||||
def main() -> int:
|
||||
|
||||
@@ -197,19 +197,15 @@ if [ "$HTTP_CODE" != "200" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Filter: state=APPROVED, not-dismissed, non-author. Optionally strict-mode
|
||||
# adds commit_id==head.sha (off by default; see header).
|
||||
# Filter: state=APPROVED, official=true, not-dismissed, non-author,
|
||||
# commit_id matches current PR head. All conditions are mandatory.
|
||||
JQ_FILTER='.[]
|
||||
| select(.state == "APPROVED")
|
||||
| select(.official == true)
|
||||
| select(.dismissed != true)
|
||||
| select(.official != false)
|
||||
| select(.user.login != $author)'
|
||||
if [ "${REVIEW_CHECK_STRICT:-}" = "1" ]; then
|
||||
JQ_FILTER="${JQ_FILTER}
|
||||
| select(.commit_id == \$head)"
|
||||
fi
|
||||
JQ_FILTER="${JQ_FILTER}
|
||||
| .user.login"
|
||||
| select(.user.login != $author)
|
||||
| select(.commit_id == $head)
|
||||
| .user.login'
|
||||
|
||||
REVIEW_CANDIDATES=$(jq -r --arg author "$PR_AUTHOR" --arg head "$PR_HEAD_SHA" "$JQ_FILTER" "$REVIEWS_JSON" | sort -u)
|
||||
debug "candidate non-author approvers: $(echo "$REVIEW_CANDIDATES" | tr '\n' ' ')"
|
||||
@@ -241,49 +237,14 @@ if [ -z "$REVIEW_CANDIDATES" ]; then
|
||||
|
||||
fi
|
||||
|
||||
# --- Fallback/extension (internal#348): check issue comments for agent-approval ---
|
||||
# core-qa-agent and core-security-agent can approve via issue comments. Always
|
||||
# include comment candidates, even if the reviews API returned approvals for a
|
||||
# different team; team membership below is the authoritative filter.
|
||||
COMMENT_CANDIDATES=""
|
||||
AGENT_PATTERN=""
|
||||
case "$TEAM" in
|
||||
qa) AGENT_PATTERN="\\[core-qa-agent\\]" ;;
|
||||
security) AGENT_PATTERN="\\[core-security-agent\\]" ;;
|
||||
esac
|
||||
HTTP_CODE=$(curl -sS -o "$COMMENTS_JSON" -w '%{http_code}' \
|
||||
-K "$CURL_AUTH_FILE" "${API}/repos/${OWNER}/${NAME}/issues/${PR_NUMBER}/comments")
|
||||
debug "GET /issues/${PR_NUMBER}/comments → HTTP ${HTTP_CODE}"
|
||||
if [ "$HTTP_CODE" = "200" ]; then
|
||||
# JQ expression: select non-author comments that match either the
|
||||
# agent-prefix pattern (case-insensitive) OR a generic approval keyword.
|
||||
JQ_APPROVALS='
|
||||
.[] |
|
||||
select(.user.login != $author) |
|
||||
. as $cmt |
|
||||
if ($agent_pattern | length) > 0 and ($cmt.body // "" | test($agent_pattern; "i")) then
|
||||
$cmt.user.login
|
||||
elif ($cmt.body // "" | test("\\b(APPROVED|LGTM|ACCEPTED)\\b"; "i")) then
|
||||
$cmt.user.login
|
||||
else
|
||||
empty
|
||||
end
|
||||
'
|
||||
COMMENT_CANDIDATES=$(jq -r \
|
||||
--arg author "$PR_AUTHOR" \
|
||||
--arg agent_pattern "$AGENT_PATTERN" \
|
||||
"$JQ_APPROVALS" \
|
||||
"$COMMENTS_JSON" 2>/dev/null | sort -u)
|
||||
debug "comment-based approval candidates: $(echo "$COMMENT_CANDIDATES" | tr '\n' ' ')"
|
||||
|
||||
if [ -n "$COMMENT_CANDIDATES" ]; then
|
||||
echo "::notice::${TEAM}-review: found $(echo "$COMMENT_CANDIDATES" | wc -w | xargs) comment-based approval candidate(s) — verifying team membership..."
|
||||
fi
|
||||
else
|
||||
debug "could not fetch issue comments (HTTP ${HTTP_CODE})"
|
||||
fi
|
||||
|
||||
CANDIDATES=$(printf '%s\n%s\n' "$REVIEW_CANDIDATES" "$COMMENT_CANDIDATES" | sed '/^$/d' | sort -u)
|
||||
# --- COMMENT APPROVAL REMOVED (security hardening) ---
|
||||
# Previous versions accepted issue comments containing generic approval
|
||||
# keywords (APPROVED/LGTM/ACCEPTED) or agent prefixes ([core-qa-agent],
|
||||
# [core-security-agent]) as satisfying the gate. Both paths are bypasses:
|
||||
# a comment lacks the audit trail, dismissal, stale-review invalidation,
|
||||
# and commit_id binding that an official Gitea review provides.
|
||||
# Only APPROVED reviews from the Gitea reviews API count.
|
||||
CANDIDATES="$REVIEW_CANDIDATES"
|
||||
|
||||
if [ -z "${CANDIDATES:-}" ]; then
|
||||
echo "::error::${TEAM}-review awaiting non-author APPROVE from ${TEAM} team (no candidates from reviews API or issue comments)"
|
||||
|
||||
@@ -48,7 +48,6 @@ set -euo pipefail
|
||||
# workflow-level jq install can fail on runners with network restrictions
|
||||
# (GitHub releases not reachable from some runner networks — infra#241
|
||||
# follow-up). This fallback is idempotent — no-op when jq is already on PATH.
|
||||
# SOP_FAIL_OPEN=1 makes this always exit 0 so CI never blocks on jq absence.
|
||||
if ! command -v jq >/dev/null 2>&1; then
|
||||
echo "::notice::jq not found on PATH — attempting install..."
|
||||
_jq_installed="no"
|
||||
@@ -67,12 +66,6 @@ if ! command -v jq >/dev/null 2>&1; then
|
||||
if ! command -v jq >/dev/null 2>&1; then
|
||||
echo "::error::jq installation failed — apt-get and GitHub binary both failed."
|
||||
echo "::error::sop-tier-check requires jq for all JSON API parsing."
|
||||
# SOP_FAIL_OPEN=1 is set in the workflow step's env — makes script always
|
||||
# exit 0 so CI never blocks. The SOP-6 tier review gate remains enforced.
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
fi
|
||||
@@ -101,15 +94,10 @@ echo "::notice::tier-check start: repo=$OWNER/$NAME pr=$PR_NUMBER author=$PR_AUT
|
||||
# cause the script to exit prematurely when the token is empty/invalid — the
|
||||
# if check below handles that case gracefully. Without || true, a 401 from an
|
||||
# empty/invalid token causes jq to exit 1, triggering set -e and exiting the
|
||||
# entire script before SOP_FAIL_OPEN can be evaluated (the check is in the jq-
|
||||
# install block; if jq is already on PATH, that block is skipped entirely).
|
||||
# entire script before the error can be logged.
|
||||
WHOAMI=$(curl -sS -H "$AUTH" "${API}/user" | jq -r '.login // ""') || true
|
||||
if [ -z "$WHOAMI" ]; then
|
||||
echo "::error::GITEA_TOKEN cannot resolve a user via /api/v1/user — check the token scope and that the secret is wired correctly."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
echo "::notice::token resolves to user: $WHOAMI"
|
||||
@@ -119,10 +107,6 @@ echo "::notice::token resolves to user: $WHOAMI"
|
||||
HEAD_SHA=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}" | jq -r '.head.sha // ""') || true
|
||||
if [ -z "$HEAD_SHA" ]; then
|
||||
echo "::error::Failed to fetch PR head SHA — token may be invalid."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
debug "pr-head-sha=$HEAD_SHA"
|
||||
@@ -215,10 +199,6 @@ if [ "${SOP_DEBUG:-}" = "1" ]; then
|
||||
fi
|
||||
if [ "$_HTTP_EXIT" -ne 0 ] || [ "$HTTP_CODE" != "200" ]; then
|
||||
echo "::error::GET /orgs/${OWNER}/teams failed (curl exit=$_HTTP_EXIT HTTP=$HTTP_CODE) — token may lack read:org scope or be invalid."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -265,17 +245,13 @@ done
|
||||
|
||||
# 5. Read approving reviewers. set +e disables set -e temporarily so that curl
|
||||
# failures (e.g. empty/invalid token → HTTP 401) do not abort the script before
|
||||
# SOP_FAIL_OPEN is evaluated. set -e is restored immediately after.
|
||||
# set -e is restored immediately after.
|
||||
set +e
|
||||
REVIEWS=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}/reviews")
|
||||
_REVIEWS_EXIT=$?
|
||||
set -e
|
||||
if [ $_REVIEWS_EXIT -ne 0 ] || [ -z "$REVIEWS" ]; then
|
||||
echo "::error::Failed to fetch reviews (curl exit=$_REVIEWS_EXIT) — token may be invalid or unreachable."
|
||||
if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then
|
||||
echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block."
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
APPROVERS=$(echo "$REVIEWS" | jq -r --arg head_sha "$HEAD_SHA" '[.[] | select(.state=="APPROVED" and .commit_id == $head_sha) | .user.login] | unique | .[]') || true
|
||||
|
||||
@@ -109,23 +109,34 @@ class Handler(http.server.BaseHTTPRequestHandler):
|
||||
return self._json(200, [{
|
||||
"state": "APPROVED",
|
||||
"dismissed": True,
|
||||
"official": True,
|
||||
"user": {"login": "core-devops"},
|
||||
"commit_id": "abc1234",
|
||||
"commit_id": "deadbeef0000111122223333444455556666",
|
||||
}])
|
||||
if sc == "T3_reviews_approved_non_author":
|
||||
return self._json(200, [
|
||||
{"state": "CHANGES_REQUESTED", "dismissed": False, "user": {"login": "bob"}, "commit_id": "abc1234"},
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "core-devops"}, "commit_id": "abc1234"},
|
||||
{"state": "CHANGES_REQUESTED", "dismissed": False, "official": True, "user": {"login": "bob"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "core-devops"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
if sc == "T19_ai_sop_ack_approved":
|
||||
# ai-sop-ack member submitted APPROVED review — must NOT count
|
||||
# toward qa-review (team_id=20) or security-review (team_id=21).
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "ai-reviewer"}, "commit_id": "abc1234"},
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "ai-reviewer"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
# Default: one non-author APPROVED
|
||||
if sc == "T21_stale_head_approved":
|
||||
# APPROVED review but on an old commit (stale head) → must be rejected
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "core-devops"}, "commit_id": "oldsha0000000000000000000000000000"},
|
||||
])
|
||||
if sc == "T22_missing_official":
|
||||
# APPROVED review with no official field → must be rejected
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "core-devops"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
# Default: one non-author APPROVED (current head, official)
|
||||
return self._json(200, [
|
||||
{"state": "APPROVED", "dismissed": False, "user": {"login": "core-devops"}, "commit_id": "abc1234"},
|
||||
{"state": "APPROVED", "dismissed": False, "official": True, "user": {"login": "core-devops"}, "commit_id": "deadbeef0000111122223333444455556666"},
|
||||
])
|
||||
|
||||
# GET /repos/{owner}/{name}/issues/{pr_number}/comments
|
||||
|
||||
@@ -308,6 +308,8 @@ def test_process_once_holds_pr_on_permanent_merge_error(monkeypatch):
|
||||
monkeypatch.setattr(mq, "WATCH_BRANCH", "main")
|
||||
monkeypatch.setattr(mq, "QUEUE_LABEL", "merge-queue")
|
||||
monkeypatch.setattr(mq, "HOLD_LABEL", "merge-queue-hold")
|
||||
monkeypatch.setattr(mq, "AUTO_DISCOVER", True)
|
||||
monkeypatch.setattr(mq, "OPT_OUT_LABELS", {"merge-queue-hold", "do-not-auto-merge", "wip"})
|
||||
monkeypatch.setattr(mq, "REVIEWER_SET", REVIEWERS)
|
||||
|
||||
monkeypatch.setattr(mq, "get_branch_protection", lambda branch: mq.BranchProtection(
|
||||
@@ -324,7 +326,7 @@ def test_process_once_holds_pr_on_permanent_merge_error(monkeypatch):
|
||||
return {"state": "success", "statuses": [{"context": ctx, "status": "success"}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
|
||||
monkeypatch.setattr(mq, "list_queued_issues", lambda: [
|
||||
monkeypatch.setattr(mq, "list_candidate_issues", lambda *, auto_discover: [
|
||||
{"number": 100, "pull_request": {}, "labels": [{"name": "merge-queue"}],
|
||||
"created_at": "2026-06-01T00:00:00Z"},
|
||||
])
|
||||
@@ -374,6 +376,8 @@ def _fully_ready_process_once_monkeypatch(monkeypatch, mergeable, calls):
|
||||
monkeypatch.setattr(mq, "WATCH_BRANCH", "main")
|
||||
monkeypatch.setattr(mq, "QUEUE_LABEL", "merge-queue")
|
||||
monkeypatch.setattr(mq, "HOLD_LABEL", "merge-queue-hold")
|
||||
monkeypatch.setattr(mq, "AUTO_DISCOVER", True)
|
||||
monkeypatch.setattr(mq, "OPT_OUT_LABELS", {"merge-queue-hold", "do-not-auto-merge", "wip"})
|
||||
monkeypatch.setattr(mq, "REVIEWER_SET", REVIEWERS)
|
||||
monkeypatch.setattr(mq, "get_branch_protection", lambda branch: mq.BranchProtection(
|
||||
required_contexts=["CI / all-required (pull_request)"],
|
||||
@@ -389,7 +393,7 @@ def _fully_ready_process_once_monkeypatch(monkeypatch, mergeable, calls):
|
||||
return {"state": "success", "statuses": [{"context": ctx, "status": "success"}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
|
||||
monkeypatch.setattr(mq, "list_queued_issues", lambda: [
|
||||
monkeypatch.setattr(mq, "list_candidate_issues", lambda *, auto_discover: [
|
||||
{"number": 102, "pull_request": {}, "labels": [{"name": "merge-queue"}],
|
||||
"created_at": "2026-06-01T00:00:00Z"},
|
||||
])
|
||||
@@ -484,6 +488,8 @@ def test_status_fetch_failure_is_fail_closed(monkeypatch):
|
||||
monkeypatch.setattr(mq, "WATCH_BRANCH", "main")
|
||||
monkeypatch.setattr(mq, "QUEUE_LABEL", "merge-queue")
|
||||
monkeypatch.setattr(mq, "HOLD_LABEL", "merge-queue-hold")
|
||||
monkeypatch.setattr(mq, "AUTO_DISCOVER", True)
|
||||
monkeypatch.setattr(mq, "OPT_OUT_LABELS", {"merge-queue-hold", "do-not-auto-merge", "wip"})
|
||||
monkeypatch.setattr(mq, "REVIEWER_SET", REVIEWERS)
|
||||
monkeypatch.setattr(mq, "get_branch_protection", lambda branch: mq.BranchProtection(
|
||||
required_contexts=["CI / all-required (pull_request)"],
|
||||
@@ -501,7 +507,7 @@ def test_status_fetch_failure_is_fail_closed(monkeypatch):
|
||||
raise mq.ApiError("GET /commits/HEAD/status -> HTTP 502: bad gateway")
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
|
||||
monkeypatch.setattr(mq, "list_queued_issues", lambda: [
|
||||
monkeypatch.setattr(mq, "list_candidate_issues", lambda *, auto_discover: [
|
||||
{"number": 101, "pull_request": {}, "labels": [{"name": "merge-queue"}],
|
||||
"created_at": "2026-06-01T00:00:00Z"},
|
||||
])
|
||||
@@ -595,6 +601,8 @@ def _stale_pr_update_409_monkeypatch(monkeypatch, queued_issues, calls):
|
||||
monkeypatch.setattr(mq, "WATCH_BRANCH", "main")
|
||||
monkeypatch.setattr(mq, "QUEUE_LABEL", "merge-queue")
|
||||
monkeypatch.setattr(mq, "HOLD_LABEL", "merge-queue-hold")
|
||||
monkeypatch.setattr(mq, "AUTO_DISCOVER", True)
|
||||
monkeypatch.setattr(mq, "OPT_OUT_LABELS", {"merge-queue-hold", "do-not-auto-merge", "wip"})
|
||||
monkeypatch.setattr(mq, "REVIEWER_SET", REVIEWERS)
|
||||
monkeypatch.setattr(mq, "get_branch_protection", lambda branch: mq.BranchProtection(
|
||||
required_contexts=["CI / all-required (pull_request)"],
|
||||
@@ -610,7 +618,8 @@ def _stale_pr_update_409_monkeypatch(monkeypatch, queued_issues, calls):
|
||||
return {"state": "success", "statuses": [{"context": ctx, "status": "success"}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
|
||||
monkeypatch.setattr(mq, "list_queued_issues", lambda: queued_issues)
|
||||
# Scan-loop process_once enumerates candidates via list_candidate_issues.
|
||||
monkeypatch.setattr(mq, "list_candidate_issues", lambda *, auto_discover: queued_issues)
|
||||
monkeypatch.setattr(mq, "get_pull", lambda n: {
|
||||
"state": "open", "number": n, "mergeable": True,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
@@ -640,6 +649,7 @@ def _stale_pr_update_409_monkeypatch(monkeypatch, queued_issues, calls):
|
||||
|
||||
def fake_add_label(pr_number, label_name, *, dry_run):
|
||||
calls["hold_label"] = (pr_number, label_name)
|
||||
calls.setdefault("holds", []).append((pr_number, label_name))
|
||||
monkeypatch.setattr(mq, "add_label_by_name", fake_add_label)
|
||||
monkeypatch.setattr(mq, "post_comment", lambda *a, **k: None)
|
||||
|
||||
@@ -669,9 +679,12 @@ def test_process_once_holds_pr_on_409_conflict_on_update(monkeypatch):
|
||||
|
||||
|
||||
def test_queue_advances_past_held_conflicted_pr(monkeypatch):
|
||||
"""End-to-end HOL proof for #2352: PR #1409 (oldest) hits a 409-on-update
|
||||
and is held; on the NEXT tick choose_next_queued_issue must SKIP the held
|
||||
PR and select the next ready PR (#1500) instead of stalling on #1409."""
|
||||
"""End-to-end HOL proof for #2352 under the scan-loop architecture: PR #1409
|
||||
(oldest) hits a 409-on-update and is HELD (HOLD_LABEL applied); once held it
|
||||
carries an opt-out label so it is excluded from candidate selection and can
|
||||
never re-block the queue. The 409-conflict hold (#2354) and the
|
||||
scan-through-skip (#2356) coexist: a held conflicted PR is both held AND no
|
||||
longer a candidate, so newer ready PRs behind it are unblocked."""
|
||||
calls = {"update_attempts": 0, "merge_attempts": 0, "hold_label": None}
|
||||
conflicted = {"number": 1409, "pull_request": {},
|
||||
"labels": [{"name": "merge-queue"}],
|
||||
@@ -685,16 +698,30 @@ def test_queue_advances_past_held_conflicted_pr(monkeypatch):
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
# Tick 1: oldest (#1409) is selected, 409-on-update → held.
|
||||
# Tick 1: oldest (#1409) is selected, 409-on-update → held, then the scan
|
||||
# CONTINUES to #1500 (which also 409s in this fixture and is likewise held).
|
||||
# The key #2352 property: the conflicted oldest PR is held and does NOT stop
|
||||
# the scan from advancing past it.
|
||||
rc = mq.process_once(dry_run=False)
|
||||
assert rc == 0
|
||||
assert calls["hold_label"] == (1409, "merge-queue-hold")
|
||||
assert (1409, "merge-queue-hold") in calls["holds"]
|
||||
assert calls["merge_attempts"] == 0 # held, not merged — fail-closed
|
||||
|
||||
# Simulate the label now present on #1409 (as the real hold would persist).
|
||||
conflicted["labels"] = [{"name": "merge-queue"}, {"name": "merge-queue-hold"}]
|
||||
|
||||
# Tick 2: the queue must ADVANCE — choose_next_queued_issue skips the held
|
||||
# #1409 and selects the next ready candidate #1500, NOT re-select #1409.
|
||||
# Next selection: the scan-loop candidate selector must SKIP the now-held
|
||||
# #1409 (HOLD_LABEL is in OPT_OUT_LABELS) and surface the next ready
|
||||
# candidate #1500 — the held PR no longer head-of-line blocks. The legacy
|
||||
# opt-IN selector (choose_next_queued_issue) honours the same hold.
|
||||
opt_out = {"merge-queue-hold", "do-not-auto-merge", "wip"}
|
||||
remaining = mq.choose_candidate_issues(
|
||||
[conflicted, next_ready],
|
||||
queue_label="merge-queue",
|
||||
opt_out_labels=opt_out,
|
||||
auto_discover=True,
|
||||
)
|
||||
assert [i["number"] for i in remaining] == [1500]
|
||||
selected = mq.choose_next_queued_issue(
|
||||
[conflicted, next_ready],
|
||||
queue_label="merge-queue",
|
||||
@@ -702,3 +729,563 @@ def test_queue_advances_past_held_conflicted_pr(monkeypatch):
|
||||
)
|
||||
assert selected is not None
|
||||
assert selected["number"] == 1500
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# §SOP-22: AUTO-DISCOVERY (opt-OUT, label-optional). The queue must be
|
||||
# self-sustaining — a ready PR is considered/merged with NO `merge-queue`
|
||||
# label, while opt-out labels (merge-queue-hold / do-not-auto-merge / wip) and
|
||||
# drafts are skipped. The merge bar (approvals/required-green/mergeable) is
|
||||
# unchanged; only candidate selection changes.
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
OPT_OUT = {"merge-queue-hold", "do-not-auto-merge", "wip"}
|
||||
|
||||
|
||||
def _issue(number, labels, *, created="2026-06-01T00:00:00Z", draft=False, is_pr=True):
|
||||
pr = {"draft": draft} if is_pr else None
|
||||
out = {
|
||||
"number": number,
|
||||
"labels": [{"name": n} for n in labels],
|
||||
"created_at": created,
|
||||
}
|
||||
if pr is not None:
|
||||
out["pull_request"] = pr
|
||||
return out
|
||||
|
||||
|
||||
def test_auto_discover_selects_unlabeled_ready_pr():
|
||||
"""A ready PR with NO merge-queue label is auto-considered (the autonomy fix:
|
||||
agents cannot self-label because their token lacks write:issue)."""
|
||||
issues = [_issue(50, labels=[])] # no merge-queue label at all
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=True
|
||||
)
|
||||
assert selected is not None
|
||||
assert selected["number"] == 50
|
||||
|
||||
|
||||
def test_auto_discover_skips_opt_out_labels():
|
||||
"""Each opt-out label keeps a PR OUT of autonomous merging (the human escape
|
||||
hatch). A PR carrying any of them is never selected even though it is open."""
|
||||
for optout in OPT_OUT:
|
||||
issues = [_issue(60, labels=[optout])]
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=True
|
||||
)
|
||||
assert selected is None, f"{optout!r} should opt the PR out"
|
||||
|
||||
|
||||
def test_auto_discover_skips_opt_out_even_when_queue_labeled():
|
||||
"""An opt-out label beats the merge-queue label: a held/wip PR that also
|
||||
carries merge-queue is still skipped."""
|
||||
issues = [_issue(61, labels=["merge-queue", "wip"])]
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=True
|
||||
)
|
||||
assert selected is None
|
||||
|
||||
|
||||
def test_auto_discover_skips_drafts():
|
||||
issues = [_issue(62, labels=[], draft=True)]
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=True
|
||||
)
|
||||
assert selected is None
|
||||
|
||||
|
||||
def test_auto_discover_skips_non_pull_issues():
|
||||
"""A plain issue (no pull_request key) is never a merge candidate."""
|
||||
issues = [_issue(63, labels=[], is_pr=False)]
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=True
|
||||
)
|
||||
assert selected is None
|
||||
|
||||
|
||||
def test_auto_discover_oldest_first_skipping_opt_out():
|
||||
"""Selection is FIFO (oldest created_at first), and the opt-out PR is passed
|
||||
over for the next-oldest eligible PR."""
|
||||
issues = [
|
||||
_issue(70, labels=["do-not-auto-merge"], created="2026-06-01T01:00:00Z"),
|
||||
_issue(71, labels=[], created="2026-06-01T02:00:00Z"),
|
||||
_issue(72, labels=["merge-queue"], created="2026-06-01T03:00:00Z"),
|
||||
]
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=True
|
||||
)
|
||||
assert selected["number"] == 71 # 70 opted out, 71 is next-oldest eligible
|
||||
|
||||
|
||||
def test_opt_in_mode_requires_queue_label():
|
||||
"""AUTO_DISCOVER off restores legacy opt-IN: only merge-queue-labeled PRs are
|
||||
candidates; an unlabeled ready PR is NOT selected."""
|
||||
issues = [
|
||||
_issue(80, labels=[], created="2026-06-01T01:00:00Z"),
|
||||
_issue(81, labels=["merge-queue"], created="2026-06-01T02:00:00Z"),
|
||||
]
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=False
|
||||
)
|
||||
assert selected["number"] == 81
|
||||
|
||||
|
||||
def test_opt_in_mode_still_honours_opt_out():
|
||||
"""Even in opt-IN mode, an opt-out label on a queue-labeled PR skips it."""
|
||||
issues = [_issue(82, labels=["merge-queue", "merge-queue-hold"])]
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=False
|
||||
)
|
||||
assert selected is None
|
||||
|
||||
|
||||
def test_list_candidate_issues_omits_label_filter_when_auto_discover(monkeypatch):
|
||||
"""The auto-discovery listing must NOT pass a `labels` filter (so unlabeled
|
||||
PRs are enumerated); the opt-IN listing must keep filtering by QUEUE_LABEL."""
|
||||
captured = {}
|
||||
|
||||
def fake_api(method, path, *, query=None, **kw):
|
||||
captured["query"] = dict(query or {})
|
||||
return 200, []
|
||||
|
||||
monkeypatch.setattr(mq, "api", fake_api)
|
||||
monkeypatch.setattr(mq, "QUEUE_LABEL", "merge-queue")
|
||||
|
||||
mq.list_candidate_issues(auto_discover=True)
|
||||
assert "labels" not in captured["query"]
|
||||
assert captured["query"].get("type") == "pulls"
|
||||
|
||||
mq.list_candidate_issues(auto_discover=False)
|
||||
assert captured["query"].get("labels") == "merge-queue"
|
||||
|
||||
|
||||
def _wire_ready_process_once(monkeypatch, *, issues, pr_payload, calls):
|
||||
"""Wire process_once fully green EXCEPT candidate selection / pull payload,
|
||||
which the caller supplies to exercise auto-discovery end-to-end."""
|
||||
monkeypatch.setattr(mq, "OWNER", "molecule-ai")
|
||||
monkeypatch.setattr(mq, "NAME", "molecule-core")
|
||||
monkeypatch.setattr(mq, "WATCH_BRANCH", "main")
|
||||
monkeypatch.setattr(mq, "QUEUE_LABEL", "merge-queue")
|
||||
monkeypatch.setattr(mq, "HOLD_LABEL", "merge-queue-hold")
|
||||
monkeypatch.setattr(mq, "AUTO_DISCOVER", True)
|
||||
monkeypatch.setattr(mq, "OPT_OUT_LABELS", OPT_OUT)
|
||||
monkeypatch.setattr(mq, "REVIEWER_SET", REVIEWERS)
|
||||
monkeypatch.setattr(mq, "get_branch_protection", lambda branch: mq.BranchProtection(
|
||||
required_contexts=["CI / all-required (pull_request)"],
|
||||
required_approvals=2, block_on_rejected_reviews=True,
|
||||
))
|
||||
main_sha = "b" * 40
|
||||
head_sha = "a" * 40
|
||||
monkeypatch.setattr(mq, "get_branch_head", lambda branch: main_sha)
|
||||
|
||||
def fake_combined(sha):
|
||||
ctx = "CI / all-required (push)" if sha == main_sha else "CI / all-required (pull_request)"
|
||||
return {"state": "success", "statuses": [{"context": ctx, "status": "success"}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
monkeypatch.setattr(mq, "list_candidate_issues", lambda *, auto_discover: issues)
|
||||
monkeypatch.setattr(mq, "get_pull", lambda n: dict(pr_payload, number=n))
|
||||
monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": main_sha}, {"sha": head_sha}])
|
||||
monkeypatch.setattr(mq, "get_pull_reviews", lambda n: [
|
||||
{"state": "APPROVED", "user": {"login": "agent-researcher"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": head_sha},
|
||||
{"state": "APPROVED", "user": {"login": "agent-reviewer-cr2"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": head_sha},
|
||||
])
|
||||
|
||||
def fake_merge(pr_number, *, dry_run, force=False):
|
||||
calls["merged"] = pr_number
|
||||
monkeypatch.setattr(mq, "merge_pull", fake_merge)
|
||||
monkeypatch.setattr(mq, "update_pull", lambda *a, **k: calls.__setitem__("updated", True))
|
||||
monkeypatch.setattr(mq, "post_comment", lambda *a, **k: None)
|
||||
monkeypatch.setattr(mq, "add_label_by_name", lambda *a, **k: None)
|
||||
return main_sha, head_sha
|
||||
|
||||
|
||||
def test_process_once_auto_merges_unlabeled_ready_pr(monkeypatch):
|
||||
"""End-to-end: a fully-ready PR with NO merge-queue label is auto-merged.
|
||||
This is the core autonomy fix — no human/agent labeling required."""
|
||||
calls = {"merged": None, "updated": False}
|
||||
head_sha = "a" * 40
|
||||
_wire_ready_process_once(
|
||||
monkeypatch,
|
||||
issues=[_issue(90, labels=[])], # NO merge-queue label
|
||||
pr_payload={
|
||||
"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": head_sha, "repo_id": 1},
|
||||
"labels": [],
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merged"] == 90 # merged despite no merge-queue label
|
||||
|
||||
|
||||
def test_process_once_skips_opt_out_labeled_pr(monkeypatch):
|
||||
"""A fully-ready PR carrying an opt-out label is NOT merged (skipped)."""
|
||||
for optout in OPT_OUT:
|
||||
calls = {"merged": None, "updated": False}
|
||||
head_sha = "a" * 40
|
||||
_wire_ready_process_once(
|
||||
monkeypatch,
|
||||
issues=[_issue(91, labels=[optout])],
|
||||
pr_payload={
|
||||
"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": head_sha, "repo_id": 1},
|
||||
"labels": [{"name": optout}],
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
rc = mq.process_once(dry_run=False)
|
||||
assert rc == 0
|
||||
assert calls["merged"] is None, f"{optout!r} PR must not be merged"
|
||||
|
||||
|
||||
def test_process_once_does_not_merge_unapproved_pr(monkeypatch):
|
||||
"""A not-ready PR (only one genuine approval) is auto-considered but NOT
|
||||
merged — auto-discovery does not lower the merge bar."""
|
||||
calls = {"merged": None, "updated": False}
|
||||
head_sha = "a" * 40
|
||||
main_sha, _ = _wire_ready_process_once(
|
||||
monkeypatch,
|
||||
issues=[_issue(92, labels=[])],
|
||||
pr_payload={
|
||||
"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": head_sha, "repo_id": 1},
|
||||
"labels": [],
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
# Only ONE genuine approval → below the required 2.
|
||||
monkeypatch.setattr(mq, "get_pull_reviews", lambda n: [
|
||||
{"state": "APPROVED", "user": {"login": "agent-researcher"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": head_sha},
|
||||
])
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merged"] is None
|
||||
|
||||
|
||||
def test_process_once_does_not_merge_red_required_pr(monkeypatch):
|
||||
"""A not-ready PR (required context red) is auto-considered but NOT merged."""
|
||||
calls = {"merged": None, "updated": False}
|
||||
head_sha = "a" * 40
|
||||
main_sha = "b" * 40
|
||||
_wire_ready_process_once(
|
||||
monkeypatch,
|
||||
issues=[_issue(93, labels=[])],
|
||||
pr_payload={
|
||||
"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": head_sha, "repo_id": 1},
|
||||
"labels": [],
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
# Required PR context is FAILURE; main stays green.
|
||||
def fake_combined(sha):
|
||||
if sha == main_sha:
|
||||
return {"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (push)", "status": "success"}]}
|
||||
return {"state": "failure",
|
||||
"statuses": [{"context": "CI / all-required (pull_request)", "status": "failure"}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merged"] is None
|
||||
|
||||
|
||||
def test_process_once_does_not_merge_unmergeable_pr(monkeypatch):
|
||||
"""A not-ready PR (mergeable False = conflicts) is auto-considered but NOT
|
||||
merged."""
|
||||
calls = {"merged": None, "updated": False}
|
||||
head_sha = "a" * 40
|
||||
_wire_ready_process_once(
|
||||
monkeypatch,
|
||||
issues=[_issue(94, labels=[])],
|
||||
pr_payload={
|
||||
"state": "open", "mergeable": False, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": head_sha, "repo_id": 1},
|
||||
"labels": [],
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merged"] is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# §SOP-22 (cont.): HEAD-OF-LINE (HOL) — a non-ready auto-discovered candidate
|
||||
# must NOT block the newer ready PRs behind it. The queue SCANS THROUGH the
|
||||
# FIFO candidate list, skipping `wait` candidates (REQUEST_CHANGES, mergeable
|
||||
# != True, insufficient genuine approvals, or red required CI), and merges the
|
||||
# first ready PR in the SAME tick. (Regression for the #1519-style false
|
||||
# candidate the reviewer caught: open + unlabeled + mergeable=false + current-
|
||||
# head official REQUEST_CHANGES + <2 genuine approvals.)
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
MAIN_SHA = "b" * 40
|
||||
|
||||
|
||||
def _wire_multi_candidate_process_once(monkeypatch, *, issues, pulls, reviews, calls):
|
||||
"""Wire process_once for MULTIPLE candidates, dispatching get_pull /
|
||||
get_pull_reviews / head-status BY PR NUMBER so each candidate can have a
|
||||
different readiness. `pulls` maps number -> pull payload; `reviews` maps
|
||||
number -> reviews list. Main is green; each PR head status is green."""
|
||||
monkeypatch.setattr(mq, "OWNER", "molecule-ai")
|
||||
monkeypatch.setattr(mq, "NAME", "molecule-core")
|
||||
monkeypatch.setattr(mq, "WATCH_BRANCH", "main")
|
||||
monkeypatch.setattr(mq, "QUEUE_LABEL", "merge-queue")
|
||||
monkeypatch.setattr(mq, "HOLD_LABEL", "merge-queue-hold")
|
||||
monkeypatch.setattr(mq, "AUTO_DISCOVER", True)
|
||||
monkeypatch.setattr(mq, "OPT_OUT_LABELS", OPT_OUT)
|
||||
monkeypatch.setattr(mq, "REVIEWER_SET", REVIEWERS)
|
||||
monkeypatch.setattr(mq, "get_branch_protection", lambda branch: mq.BranchProtection(
|
||||
required_contexts=["CI / all-required (pull_request)"],
|
||||
required_approvals=2, block_on_rejected_reviews=True,
|
||||
))
|
||||
monkeypatch.setattr(mq, "get_branch_head", lambda branch: MAIN_SHA)
|
||||
|
||||
def fake_combined(sha):
|
||||
ctx = "CI / all-required (push)" if sha == MAIN_SHA else "CI / all-required (pull_request)"
|
||||
return {"state": "success", "statuses": [{"context": ctx, "status": "success"}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
|
||||
monkeypatch.setattr(mq, "list_candidate_issues", lambda *, auto_discover: issues)
|
||||
monkeypatch.setattr(mq, "get_pull", lambda n: dict(pulls[n], number=n))
|
||||
# Each PR head contains current main (so no candidate needs an update; the
|
||||
# only differentiator is readiness). head sha is the pull's own head.
|
||||
monkeypatch.setattr(
|
||||
mq, "get_pull_commits",
|
||||
lambda n: [{"sha": MAIN_SHA}, {"sha": pulls[n]["head"]["sha"]}],
|
||||
)
|
||||
monkeypatch.setattr(mq, "get_pull_reviews", lambda n: reviews[n])
|
||||
|
||||
def fake_merge(pr_number, *, dry_run, force=False):
|
||||
calls.setdefault("merged", [])
|
||||
calls["merged"].append(pr_number)
|
||||
monkeypatch.setattr(mq, "merge_pull", fake_merge)
|
||||
monkeypatch.setattr(mq, "update_pull", lambda *a, **k: calls.__setitem__("updated", True))
|
||||
monkeypatch.setattr(mq, "post_comment", lambda *a, **k: None)
|
||||
monkeypatch.setattr(mq, "add_label_by_name", lambda *a, **k: None)
|
||||
|
||||
|
||||
def _two_approvals(head_sha):
|
||||
return [
|
||||
{"state": "APPROVED", "user": {"login": "agent-researcher"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": head_sha},
|
||||
{"state": "APPROVED", "user": {"login": "agent-reviewer-cr2"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": head_sha},
|
||||
]
|
||||
|
||||
|
||||
def test_hol_unready_oldest_does_not_block_newer_ready_pr(monkeypatch):
|
||||
"""The OLDEST auto-discovered candidate is NOT ready (mergeable=false). The
|
||||
queue must SKIP it and merge the NEWER ready PR in the SAME tick — no HOL."""
|
||||
calls = {"updated": False}
|
||||
old_head, new_head = "a" * 40, "c" * 40
|
||||
_wire_multi_candidate_process_once(
|
||||
monkeypatch,
|
||||
issues=[
|
||||
_issue(500, labels=[], created="2026-06-01T01:00:00Z"), # oldest, NOT ready
|
||||
_issue(501, labels=[], created="2026-06-01T02:00:00Z"), # newer, READY
|
||||
],
|
||||
pulls={
|
||||
500: {"state": "open", "mergeable": False, "draft": False, # conflict
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": old_head, "repo_id": 1}, "labels": []},
|
||||
501: {"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": new_head, "repo_id": 1}, "labels": []},
|
||||
},
|
||||
reviews={500: _two_approvals(old_head), 501: _two_approvals(new_head)},
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
# The newer ready PR merged; the non-ready oldest did not block it.
|
||||
assert calls.get("merged") == [501]
|
||||
|
||||
|
||||
def test_hol_1519_style_false_candidate_never_merged_and_never_blocks(monkeypatch):
|
||||
"""Live #1519 repro: oldest, open, UNLABELED, but mergeable=false + a
|
||||
current-head official REQUEST_CHANGES + only ONE genuine approval. It must
|
||||
NEVER be merged and must NEVER block the newer ready PR behind it."""
|
||||
calls = {"updated": False}
|
||||
false_head, ready_head = "a" * 40, "c" * 40
|
||||
_wire_multi_candidate_process_once(
|
||||
monkeypatch,
|
||||
issues=[
|
||||
_issue(1519, labels=[], created="2026-05-20T00:00:00Z"), # oldest false candidate
|
||||
_issue(2000, labels=[], created="2026-06-01T00:00:00Z"), # newer, READY
|
||||
],
|
||||
pulls={
|
||||
1519: {"state": "open", "mergeable": False, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": false_head, "repo_id": 1}, "labels": []},
|
||||
2000: {"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": ready_head, "repo_id": 1}, "labels": []},
|
||||
},
|
||||
reviews={
|
||||
1519: [
|
||||
# one genuine approval (below 2) ...
|
||||
{"state": "APPROVED", "user": {"login": "agent-researcher"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": false_head},
|
||||
# ... plus a current-head official REQUEST_CHANGES (human action needed)
|
||||
{"state": "REQUEST_CHANGES", "user": {"login": "agent-reviewer"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": false_head},
|
||||
],
|
||||
2000: _two_approvals(ready_head),
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
# #1519 is never merged; the ready PR behind it merges this same tick.
|
||||
assert calls.get("merged") == [2000]
|
||||
assert 1519 not in calls.get("merged", [])
|
||||
|
||||
|
||||
def test_hol_unready_red_required_ci_is_skipped_for_ready_pr(monkeypatch):
|
||||
"""A candidate whose required CI is RED is skipped (not waited-on) so the
|
||||
newer ready PR merges in the same tick."""
|
||||
calls = {"updated": False}
|
||||
red_head, ready_head = "a" * 40, "c" * 40
|
||||
_wire_multi_candidate_process_once(
|
||||
monkeypatch,
|
||||
issues=[
|
||||
_issue(600, labels=[], created="2026-06-01T01:00:00Z"), # required CI red
|
||||
_issue(601, labels=[], created="2026-06-01T02:00:00Z"), # ready
|
||||
],
|
||||
pulls={
|
||||
600: {"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": red_head, "repo_id": 1}, "labels": []},
|
||||
601: {"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": ready_head, "repo_id": 1}, "labels": []},
|
||||
},
|
||||
reviews={600: _two_approvals(red_head), 601: _two_approvals(ready_head)},
|
||||
calls=calls,
|
||||
)
|
||||
# PR 600's required PR context is FAILURE; 601 (and main) stay green.
|
||||
def fake_combined(sha):
|
||||
if sha == MAIN_SHA:
|
||||
return {"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (push)", "status": "success"}]}
|
||||
state = "failure" if sha == red_head else "success"
|
||||
return {"state": state,
|
||||
"statuses": [{"context": "CI / all-required (pull_request)", "status": state}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", fake_combined)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls.get("merged") == [601]
|
||||
|
||||
|
||||
def test_hol_all_candidates_unready_merges_nothing(monkeypatch):
|
||||
"""If EVERY candidate is non-ready, the queue merges nothing (fail-closed)
|
||||
and does not loop — it simply finds no actionable PR this tick."""
|
||||
calls = {"updated": False}
|
||||
h1, h2 = "a" * 40, "c" * 40
|
||||
_wire_multi_candidate_process_once(
|
||||
monkeypatch,
|
||||
issues=[
|
||||
_issue(700, labels=[], created="2026-06-01T01:00:00Z"), # RC
|
||||
_issue(701, labels=[], created="2026-06-01T02:00:00Z"), # unmergeable
|
||||
],
|
||||
pulls={
|
||||
700: {"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": h1, "repo_id": 1}, "labels": []},
|
||||
701: {"state": "open", "mergeable": False, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": h2, "repo_id": 1}, "labels": []},
|
||||
},
|
||||
reviews={
|
||||
700: _two_approvals(h1) + [
|
||||
{"state": "REQUEST_CHANGES", "user": {"login": "agent-reviewer"},
|
||||
"official": True, "stale": False, "dismissed": False, "commit_id": h1},
|
||||
],
|
||||
701: _two_approvals(h2),
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls.get("merged") is None # nothing merged; no HOL loop
|
||||
|
||||
|
||||
def test_opt_out_draft_label_excludes_candidate():
|
||||
"""The literal `draft` label is now an opt-out label (added to the default
|
||||
OPT_OUT_LABELS), independent of Gitea draft STATE — a human can opt a PR out
|
||||
by labeling it `draft` without converting it to a draft PR."""
|
||||
# `draft` must be in the shipped default opt-out set.
|
||||
assert "draft" in mq.OPT_OUT_LABELS
|
||||
opt_out = OPT_OUT | {"draft"}
|
||||
issues = [_issue(800, labels=["draft"], draft=False)] # label only, not draft STATE
|
||||
selected = mq.choose_next_candidate_issue(
|
||||
issues, queue_label="merge-queue", opt_out_labels=opt_out, auto_discover=True
|
||||
)
|
||||
assert selected is None
|
||||
|
||||
|
||||
def test_choose_candidate_issues_returns_full_fifo_list_skipping_opt_out():
|
||||
"""choose_candidate_issues returns ALL eligible candidates oldest-first (so
|
||||
process_once can scan past non-ready ones), skipping opt-out/draft/non-PR."""
|
||||
issues = [
|
||||
_issue(72, labels=["merge-queue"], created="2026-06-01T03:00:00Z"),
|
||||
_issue(70, labels=["do-not-auto-merge"], created="2026-06-01T01:00:00Z"), # opt-out
|
||||
_issue(71, labels=[], created="2026-06-01T02:00:00Z"),
|
||||
_issue(73, labels=[], draft=True, created="2026-06-01T00:30:00Z"), # draft
|
||||
_issue(74, labels=[], is_pr=False, created="2026-06-01T00:00:00Z"), # not a PR
|
||||
]
|
||||
ordered = mq.choose_candidate_issues(
|
||||
issues, queue_label="merge-queue", opt_out_labels=OPT_OUT, auto_discover=True
|
||||
)
|
||||
assert [i["number"] for i in ordered] == [71, 72] # FIFO, opt-out/draft/non-PR dropped
|
||||
|
||||
|
||||
def test_process_once_defensive_skip_when_pull_payload_opted_out(monkeypatch):
|
||||
"""If the listing missed an opt-out label but the authoritative pull payload
|
||||
carries it (stale listing race), process_once must still skip the merge."""
|
||||
calls = {"merged": None, "updated": False}
|
||||
head_sha = "a" * 40
|
||||
_wire_ready_process_once(
|
||||
monkeypatch,
|
||||
issues=[_issue(95, labels=[])], # listing shows no opt-out
|
||||
pr_payload={
|
||||
"state": "open", "mergeable": True, "draft": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": head_sha, "repo_id": 1},
|
||||
"labels": [{"name": "do-not-auto-merge"}], # live pull is opted out
|
||||
},
|
||||
calls=calls,
|
||||
)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merged"] is None
|
||||
|
||||
@@ -14,10 +14,17 @@
|
||||
# T9 — team membership probe → 403 (token not in team) → script exits 1 (fail closed)
|
||||
# T10 — CURL_AUTH_FILE created with mode 600 and correct header content
|
||||
# T11 — bash syntax check (bash -n passes)
|
||||
# T12 — jq filter: non-author APPROVED → in candidate list; dismissed → excluded
|
||||
# T12 — jq filter: non-author APPROVED official current-head → in candidate list; dismissed → excluded
|
||||
# T13 — missing required env GITEA_TOKEN → exits 1 with error
|
||||
# T14 — non-default-base PR exits 0 without requiring review
|
||||
# T18 — wrong-team review candidate does not block right-team comment approval
|
||||
# T15 — comment agent-prefix approval → exit 1
|
||||
# T16 — comment generic keyword approval → exit 1
|
||||
# T17 — comments with no approval keywords → exit 1
|
||||
# T18 — wrong-team review + right-team comment → exit 1
|
||||
# T19 — ai-sop-ack APPROVED review excluded from qa-review gate
|
||||
# T20 — ai-sop-ack APPROVED review excluded from security-review gate
|
||||
# T21 — stale-head APPROVED review → exit 1 (commit_id mismatch)
|
||||
# T22 — missing/non-official APPROVED review → exit 1 (official != true)
|
||||
#
|
||||
# Hostile-self-review (per feedback_assert_exact_not_substring):
|
||||
# this test MUST FAIL if the script is absent. Verified by running
|
||||
@@ -319,41 +326,50 @@ assert_file_contains "T10b printf header format (CURL_AUTH_FILE content)" "$T10_
|
||||
assert_file_contains "T10c 'header =' curl-config syntax" "$T10_AUTHFILE" 'header = "Authorization: token '
|
||||
rm -f "$T10_AUTHFILE"
|
||||
|
||||
# T12 — jq filter: non-author APPROVED included, dismissed excluded
|
||||
# T12 — jq filter: non-author APPROVED official current-head included; dismissed/stale/missing-official excluded
|
||||
echo
|
||||
echo "== T12 jq filter =="
|
||||
# These are tested indirectly via T3 and T6 above, but let's also test
|
||||
# the jq expression directly.
|
||||
JQ_FILTER='.[]
|
||||
| select(.state == "APPROVED")
|
||||
| select(.official == true)
|
||||
| select(.dismissed != true)
|
||||
| select(.user.login != "alice")
|
||||
| select(.commit_id == $head)
|
||||
| .user.login'
|
||||
|
||||
T12_INPUT='[{"state":"APPROVED","dismissed":false,"user":{"login":"core-devops"}},{"state":"CHANGES_REQUESTED","dismissed":false,"user":{"login":"bob"}},{"state":"APPROVED","dismissed":false,"user":{"login":"alice"}},{"state":"APPROVED","dismissed":true,"user":{"login":"carol"}}]'
|
||||
T12_INPUT='[{"state":"APPROVED","official":true,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"core-devops"}},{"state":"CHANGES_REQUESTED","official":true,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"bob"}},{"state":"APPROVED","official":true,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"alice"}},{"state":"APPROVED","official":true,"dismissed":true,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"carol"}},{"state":"APPROVED","official":false,"dismissed":false,"commit_id":"deadbeef0000111122223333444455556666","user":{"login":"dave"}},{"state":"APPROVED","official":true,"dismissed":false,"commit_id":"oldsha0000000000000000000000000000","user":{"login":"eve"}}]'
|
||||
|
||||
JQ_CMD=$(command -v jq 2>/dev/null || echo /tmp/jq)
|
||||
T12_CANDIDATES=$(echo "$T12_INPUT" | "$JQ_CMD" -r "$JQ_FILTER" 2>/dev/null | sort -u)
|
||||
assert_contains "T12 jq: core-devops (non-author APPROVED) in candidates" "core-devops" "$T12_CANDIDATES"
|
||||
T12_CANDIDATES=$(echo "$T12_INPUT" | "$JQ_CMD" -r --arg head "deadbeef0000111122223333444455556666" "$JQ_FILTER" 2>/dev/null | sort -u)
|
||||
assert_contains "T12 jq: core-devops (non-author APPROVED official current-head) in candidates" "core-devops" "$T12_CANDIDATES"
|
||||
assert_eq "T12 jq: alice (author) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^alice$' || true)"
|
||||
assert_eq "T12 jq: carol (dismissed) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^carol$' || true)"
|
||||
assert_eq "T12 jq: dave (official=false) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^dave$' || true)"
|
||||
assert_eq "T12 jq: eve (stale head) NOT in candidates" "" "$(echo "$T12_CANDIDATES" | grep '^eve$' || true)"
|
||||
|
||||
# T15 — comment-based approval via agent prefix pattern → exit 0
|
||||
# T15 — comment-based approval via agent prefix pattern → exit 1
|
||||
# SECURITY: agent-prefix comments are also removed. A text prefix in an
|
||||
# issue comment is spoofable (any team member can type "[core-qa-agent]")
|
||||
# and lacks the audit trail of an official Gitea review.
|
||||
echo
|
||||
echo "== T15 comment agent-prefix approval =="
|
||||
T15_OUT=$(run_review_check "T15_comments_agent_approval")
|
||||
T15_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T15 exit code 0 (agent-comment approval + team member)" "0" "$T15_RC"
|
||||
assert_contains "T15 comment fallback notice" "comment-based approval" "$T15_OUT"
|
||||
assert_contains "T15 core-qa-agent APPROVED" "APPROVED by core-qa-agent" "$T15_OUT"
|
||||
assert_eq "T15 exit code 1 (agent-prefix comment rejected — not an official review)" "1" "$T15_RC"
|
||||
assert_contains "T15 no candidates error" "no candidates from reviews API or issue comments" "$T15_OUT"
|
||||
|
||||
# T16 — comment-based approval via generic APPROVED keyword → exit 0
|
||||
# T16 — comment-based approval via generic APPROVED keyword → exit 1
|
||||
# SECURITY: generic keywords (APPROVED/LGTM/ACCEPTED) must NOT satisfy the
|
||||
# gate — only official Gitea reviews or agent-prefix comments count. A plain
|
||||
# comment from a team member is a bypass if it skips the review UI.
|
||||
echo
|
||||
echo "== T16 comment generic keyword approval =="
|
||||
T16_OUT=$(run_review_check "T16_comments_generic_approval")
|
||||
T16_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T16 exit code 0 (generic-approval comment + team member)" "0" "$T16_RC"
|
||||
assert_contains "T16 comment fallback notice" "comment-based approval" "$T16_OUT"
|
||||
assert_eq "T16 exit code 1 (generic-approval comment rejected — not an official review)" "1" "$T16_RC"
|
||||
assert_contains "T16 no candidates error" "no candidates from reviews API or issue comments" "$T16_OUT"
|
||||
|
||||
# T17 — no approval keywords in comments → exit 1
|
||||
echo
|
||||
@@ -363,16 +379,16 @@ T17_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T17 exit code 1 (no candidates from comments)" "1" "$T17_RC"
|
||||
assert_contains "T17 no candidates error" "no candidates from reviews API or issue comments" "$T17_OUT"
|
||||
|
||||
# T18 — a wrong-team PR review candidate must not suppress a right-team
|
||||
# comment approval. This matches PR #1790, where QA had an APPROVED review
|
||||
# and security approved via the agent comment convention.
|
||||
# T18 — wrong-team review + right-team comment → exit 1
|
||||
# SECURITY: with comment approval fully removed, a wrong-team review plus
|
||||
# a right-team comment yields NO valid candidates. Only official reviews
|
||||
# from the target team count.
|
||||
echo
|
||||
echo "== T18 review candidate wrong team, comment candidate right team =="
|
||||
T18_OUT=$(run_review_check "T18_review_wrong_team_comment_right_team")
|
||||
T18_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T18 exit code 0 (comment approval still considered)" "0" "$T18_RC"
|
||||
assert_contains "T18 comment candidate notice" "comment-based approval" "$T18_OUT"
|
||||
assert_contains "T18 comment approver accepted" "APPROVED by core-qa-agent" "$T18_OUT"
|
||||
assert_eq "T18 exit code 1 (comment approval removed — no valid candidates)" "1" "$T18_RC"
|
||||
assert_contains "T18 none are in team" "none are in team" "$T18_OUT"
|
||||
|
||||
# T19 — ai-sop-ack member APPROVED review must NOT count toward qa-review
|
||||
# or security-review (R1 hardening refinement, msg 1388c76f).
|
||||
@@ -393,6 +409,24 @@ assert_eq "T20 exit code 1 (ai-sop-ack not in security team)" "1" "$T20_RC"
|
||||
assert_contains "T20 ai-reviewer excluded from security" "candidates: ai-reviewer" "$T20_OUT"
|
||||
assert_contains "T20 none are in security team" "none are in team" "$T20_OUT"
|
||||
|
||||
# T21 — stale-head APPROVED review must be rejected (commit_id mismatch).
|
||||
# SECURITY: an approval on an old commit does not cover the current head.
|
||||
echo
|
||||
echo "== T21 stale-head APPROVED review rejected =="
|
||||
T21_OUT=$(run_review_check "T21_stale_head_approved")
|
||||
T21_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T21 exit code 1 (stale-head approval rejected)" "1" "$T21_RC"
|
||||
assert_contains "T21 no candidates error" "no candidates from reviews API or issue comments" "$T21_OUT"
|
||||
|
||||
# T22 — missing/non-official APPROVED review must be rejected.
|
||||
# SECURITY: only official Gitea reviews count; comments and non-official reviews lack audit trail.
|
||||
echo
|
||||
echo "== T22 missing official flag APPROVED review rejected =="
|
||||
T22_OUT=$(run_review_check "T22_missing_official")
|
||||
T22_RC=$(cat "$FIX_STATE_DIR/last_rc")
|
||||
assert_eq "T22 exit code 1 (missing official rejected)" "1" "$T22_RC"
|
||||
assert_contains "T22 no candidates error" "no candidates from reviews API or issue comments" "$T22_OUT"
|
||||
|
||||
echo
|
||||
echo "------"
|
||||
echo "PASS=$PASS FAIL=$FAIL"
|
||||
|
||||
@@ -7,10 +7,13 @@ name: gitea-merge-queue
|
||||
# the user-space queue bot, one PR per tick, using the non-bypass merge actor.
|
||||
#
|
||||
# Queue contract:
|
||||
# - add label `merge-queue` to an open same-repo PR
|
||||
# - auto-discovery (default): any open same-repo PR is considered — no
|
||||
# `merge-queue` label required (the label is optional metadata now)
|
||||
# - bot updates stale PR heads with current main, then waits for CI
|
||||
# - bot merges only when current main is green and required PR contexts pass
|
||||
# - add `merge-queue-hold` to pause a queued PR without removing it
|
||||
# - bot merges only when current main is green, genuine approvals are present
|
||||
# on the current head, required PR contexts pass, and the PR is mergeable
|
||||
# - add `merge-queue-hold`, `do-not-auto-merge`, or `wip` to keep a PR OUT of
|
||||
# autonomous merging; draft PRs are also skipped
|
||||
|
||||
on:
|
||||
# Schedule moved to operator-config:
|
||||
@@ -48,6 +51,19 @@ jobs:
|
||||
WATCH_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
QUEUE_LABEL: merge-queue
|
||||
HOLD_LABEL: merge-queue-hold
|
||||
# Auto-discovery (opt-OUT). When on (default), the queue considers ALL
|
||||
# open same-repo PRs that meet the merge bar — it does NOT wait for a
|
||||
# human/agent to add `merge-queue`. Agent Gitea tokens lack
|
||||
# write:issue (labels are issue-scoped) and could never self-label,
|
||||
# which stalled the queue; the label is now OPTIONAL metadata. The
|
||||
# merge bar is UNCHANGED — only candidate selection widens. Set
|
||||
# AUTO_DISCOVER=0 to restore legacy opt-IN (require the merge-queue
|
||||
# label to be considered).
|
||||
AUTO_DISCOVER: "1"
|
||||
# Opt-OUT labels: any of these on a PR keeps it OUT of autonomous
|
||||
# merging (the human escape hatch). HOLD_LABEL is always also honoured.
|
||||
# A human who wants a PR held just adds one of these labels.
|
||||
OPT_OUT_LABELS: do-not-auto-merge,wip
|
||||
UPDATE_STYLE: merge
|
||||
# Recognised official-reviewer set. A merge needs >= required_approvals
|
||||
# DISTINCT genuine official approvals from these accounts on the
|
||||
|
||||
@@ -1,293 +0,0 @@
|
||||
# RFC: Org-level Platform Agent — a tenant-resident concierge
|
||||
|
||||
**Perspective:** CTO + Backend Engineer + DevOps
|
||||
**Status:** Draft — pre-implementation, **CTO sign-off required before any implementation PR**
|
||||
**Scope:** `molecule-core` (workspace-server), `molecule-controlplane`, workspace runtime, `molecule-app`
|
||||
**This document is the single source of truth (SSOT) for the feature.** Code, OpenAPI, the platform
|
||||
MCP, and end-user docs reconcile to this RFC — not to each other.
|
||||
|
||||
---
|
||||
|
||||
## 1. Summary
|
||||
|
||||
Today a Molecule tenant is a control/router box: one EC2 runs the `workspace-server`
|
||||
(`molecule-tenant` container) + Postgres + Redis, and **each workspace is its own separate EC2**
|
||||
running a runtime image that joins the tenant's A2A mesh. A2A has exactly two participant kinds:
|
||||
**workspaces** (agents) and the **user** (the canvas, modeled implicitly as `activity_logs.source_id
|
||||
IS NULL`). A user who wants to *do* anything must drive individual workspaces directly — create them,
|
||||
assign agents, wire channels/schedules/secrets — i.e. they must carry a lot of platform knowledge.
|
||||
|
||||
This RFC introduces a **platform agent**: an always-on org-level agent that
|
||||
|
||||
1. runs as a **container on the tenant EC2** itself (beside `molecule-tenant`),
|
||||
2. natively holds the **platform-management MCP** (the org-admin tool surface) so it can do anything
|
||||
in the org,
|
||||
3. joins A2A as a **first-class third participant** (`kind='platform'`) that sits at the org root, and
|
||||
4. becomes the **user's default chat target** — a concierge the user talks to like a chatbot, which
|
||||
then orchestrates the org on their behalf.
|
||||
|
||||
Destructive actions the concierge triggers are **human-approved** through the existing approvals
|
||||
subsystem.
|
||||
|
||||
## 2. Motivation
|
||||
|
||||
- **Lower the knowledge floor.** "Spin up an SEO team and have them publish weekly" should be a
|
||||
sentence, not a sequence of workspace/agent/schedule/secret operations.
|
||||
- **One front door.** A single conversational entry point that *is* the org, instead of N per-workspace
|
||||
chats the user has to coordinate.
|
||||
- **Reuse, don't rebuild.** The agent runtime, A2A mesh, the 87-tool platform MCP, and the approvals
|
||||
subsystem already exist. This feature is mostly *composition* plus one honest new participant kind.
|
||||
|
||||
## 3. Goals / Non-Goals
|
||||
|
||||
**Goals**
|
||||
- A per-tenant platform agent, provisioned automatically, that controls the org via the platform MCP.
|
||||
- A first-class `platform` participant in A2A with correct routing and tenant isolation.
|
||||
- Server-side approval gating for destructive org operations.
|
||||
- Parity with normal workspaces for runtime/model/provider/billing (no special-casing).
|
||||
|
||||
**Non-Goals (this RFC)**
|
||||
- Replacing the canvas. The canvas remains the advanced/power-user surface.
|
||||
- Multi-concierge / per-team concierges. Exactly **one** platform agent per org.
|
||||
- A new scoped-down token system for the MCP (tracked separately; see §10 Open Questions).
|
||||
|
||||
## 4. Current-state ground truth (verified, with references)
|
||||
|
||||
- **Topology.** Tenant EC2 runs `molecule-tenant` (workspace-server) + Postgres + Redis;
|
||||
`controlplane/internal/provisioner/ec2.go:buildTenantUserDataSM()` `docker run`s it with
|
||||
`--network host`, `PORT=8080`. Each **workspace is its own EC2** (`ec2.go:ProvisionWorkspace`).
|
||||
- **No `org_id` column.** An "org" is the `parent_id IS NULL` subtree root;
|
||||
`workspace-server/internal/handlers/org_scope.go` resolves it with a recursive CTE (`orgRootID`) and
|
||||
`sameOrg()` compares two workspaces' resolved roots for tenant isolation (#1953/OFFSEC-015).
|
||||
- **A2A authorization is hierarchy-based.** `workspace-server/internal/registry/access.go:CanCommunicate`
|
||||
permits self / siblings / ancestor↔descendant. Root-level rows are "siblings" but every routing path
|
||||
is additionally gated by `sameOrg()`.
|
||||
- **No participant-kind discriminator.** `workspaces.role` is a free-form string; the user is implicit
|
||||
(`activity_logs.source_id IS NULL`). `migrations/001_workspaces.sql`.
|
||||
- **Runtime injects MCP servers** in the claude-code executor's `mcp_servers` dict — today exactly one
|
||||
entry, `"a2a"` (`molecule-ai-workspace-template-claude-code/claude_sdk_executor.py`,
|
||||
`molecule_runtime/claude_sdk_executor.py`). The agent self-registers via `POST /registry/register`
|
||||
(`molecule_runtime/main.py`) and is identified by `WORKSPACE_ID` + `X-Molecule-Org-Id`.
|
||||
- **Platform MCP** (`molecule-mcp-server`, stdio Node) authenticates purely from env
|
||||
(`MOLECULE_API_KEY` = org-admin token, `MOLECULE_API_URL`, `MOLECULE_ORG_ID`; `src/api.ts`), is a
|
||||
thin proxy over the tenant REST/A2A API (`chat_with_agent` → `POST /workspaces/:id/a2a`,
|
||||
`async_delegate` → `/delegate`), and has **zero embeddability blockers**.
|
||||
- **Billing** is a per-workspace resolver — `ResolveLLMBillingModeDerived`
|
||||
(`workspace-server/internal/handlers/workspace_provision.go`, `llm_billing_mode.go`), defaulting
|
||||
closed to `platform_managed`; `byok` runs on the tenant's own provider key (see
|
||||
`docs/architecture/byok-fail-closed-billing.md`).
|
||||
- **Approvals** exist: `migrations/007_approvals.sql`, `internal/handlers/approvals.go`,
|
||||
`EventApprovalRequested`, decide route `POST /workspaces/:id/approvals/:approvalId/decide`.
|
||||
|
||||
## 5. Design
|
||||
|
||||
### 5.1 The platform agent IS the org root
|
||||
|
||||
Because `sameOrg()` resolves each workspace to its topmost `parent_id IS NULL` root, a platform agent
|
||||
added as a *second* root would resolve to a *different* root than the existing team and be **blocked**
|
||||
by `sameOrg`. Therefore the platform agent **becomes the single org root**, and the org's existing
|
||||
root is **re-parented under it**. Consequences:
|
||||
|
||||
- `orgRootID(any workspace) == platform-agent-id`; `sameOrg(platform, any in-org ws) == true`.
|
||||
- The platform agent reaches every workspace (and is reachable) via the **existing**
|
||||
ancestor↔descendant rules — **no `CanCommunicate` change**, and tenant isolation is unchanged.
|
||||
|
||||
This is the honest realization of "a third participant above workspace and user": the concierge is
|
||||
literally the org.
|
||||
|
||||
### 5.2 `kind` discriminator (the only new marker)
|
||||
|
||||
Add a single column `workspaces.kind TEXT NOT NULL DEFAULT 'workspace'`, constrained to
|
||||
`('workspace','platform')`. It is the **only** marker of the platform agent — we do **not** also
|
||||
encode identity in `role`/`tier` (those stay descriptive). The enum is defined once: the migration
|
||||
`CHECK` and the Go constants `KindWorkspace`/`KindPlatform` (+ one `IsValidKind`) are kept in lockstep.
|
||||
|
||||
Invariants (handler-enforced, since there is no `org_id` for a pure-SQL unique):
|
||||
- `kind='platform' ⇒ parent_id IS NULL`.
|
||||
- A row may be `kind='platform'` only if it is its own org root (`orgRootID(self) == self`), giving
|
||||
"exactly one platform agent per org". Guard the check+write in a tx with `FOR UPDATE` on the root.
|
||||
|
||||
### 5.3 Identity & registration
|
||||
|
||||
- **ID** = derived `uuidv5(org-namespace, "platform-agent")` — reproducible, no stored-vs-derived
|
||||
drift, lowercase so it satisfies the runtime's `WORKSPACE_ID` validator.
|
||||
- CP **pre-seeds** the `workspaces` row (`kind='platform'`, `parent_id=NULL`, `tier=0`) before the
|
||||
agent boots; the agent self-registers (`POST /registry/register`) into that row. `Register` accepts
|
||||
an optional `kind` and reconciles it, enforcing the §5.2 invariants.
|
||||
|
||||
### 5.4 Default-target resolver
|
||||
|
||||
New `GET /registry/platform-agent` (handler `internal/handlers/platform_agent.go`): resolve the
|
||||
caller's `orgRootID()` and return it iff `kind='platform'`. This is the server hook the dashboard
|
||||
targets by default; no change to `ProxyA2A`. **Authored in the OpenAPI SSOT first**; MCP/CLI/docs
|
||||
derive from it.
|
||||
|
||||
### 5.5 Runtime: two MCPs, config-driven
|
||||
|
||||
Make the runtime's `mcp_servers` **config-driven** rather than hardcoded:
|
||||
- `molecule_runtime/config.py`: add `extra_mcp_servers: list[dict]` to `WorkspaceConfig`, read
|
||||
`raw.get("mcp_servers", [])`.
|
||||
- Both executors merge `extra_mcp_servers` into the `mcp_servers` dict after the always-on `"a2a"`
|
||||
entry (the template `claude_sdk_executor.py` is the live one; the runtime-package copy is the
|
||||
fallback).
|
||||
|
||||
The platform agent's `config.yaml` then declares:
|
||||
|
||||
```yaml
|
||||
runtime: claude-code
|
||||
model: sonnet # default; user-switchable model AND provider via providers.yaml
|
||||
a2a:
|
||||
port: 8090 # avoid the workspace default 8000 under host networking
|
||||
mcp_servers:
|
||||
- name: platform
|
||||
command: node
|
||||
args: ["/opt/molecule-mcp-server/dist/index.js"]
|
||||
```
|
||||
|
||||
The `platform` MCP reads `MOLECULE_API_KEY`/`MOLECULE_API_URL`/`MOLECULE_ORG_ID` from the container
|
||||
env (passed through to the stdio child) — no per-server `env` block needed.
|
||||
|
||||
### 5.6 Hosting & provisioning (tenant EC2 container)
|
||||
|
||||
In `ec2.go:buildTenantUserDataSM()` add a `start_platform_agent` stage **after** `wait_platform_health`
|
||||
(the agent registers against `localhost:8080` on boot):
|
||||
|
||||
```bash
|
||||
docker run -d --restart=always --name molecule-platform-agent --network host \
|
||||
-v /data/platform-agent/configs:/configs \
|
||||
-e WORKSPACE_ID=<platform-uuid> -e WORKSPACE_CONFIG_PATH=/configs \
|
||||
-e PLATFORM_URL=http://localhost:8080 \
|
||||
-e MOLECULE_API_URL=http://localhost:8080 -e MOLECULE_API_KEY=$ADMIN_TOKEN -e MOLECULE_ORG_ID=<orgID> \
|
||||
-e ANTHROPIC_AUTH_TOKEN=$ADMIN_TOKEN -e MOLECULE_LLM_ANTHROPIC_BASE_URL=$MOLECULE_LLM_ANTHROPIC_BASE_URL \
|
||||
<platform-agent-image>
|
||||
```
|
||||
|
||||
- The org `admin_token` is already on the box (Secrets Manager `molecule/tenant/{orgID}`).
|
||||
- `--restart=always` provides Docker-level supervision (matches `molecule-tenant`).
|
||||
- Mirror the block into the redeploy path (`buildRedeployScript`) so existing tenants backfill it.
|
||||
|
||||
### 5.7 Image
|
||||
|
||||
A **dedicated `molecule-platform-agent` image**: `FROM workspace-template-claude-code`, `COPY` the
|
||||
prebuilt `molecule-mcp-server/dist` + `node_modules` into `/opt/molecule-mcp-server`, and **pin Node
|
||||
20** (the slim base ships Node 18; the MCP expects ≥20). A dedicated image keeps the org-admin MCP
|
||||
**out of** ordinary workspace images (security hygiene) and lets us set concierge defaults without
|
||||
touching the workspace template. `molecule-ci` publishes it.
|
||||
|
||||
### 5.8 Approval gate (server-side trust boundary)
|
||||
|
||||
The MCP is a *client* of the tenant handlers, so enforcement lives in the **handlers**, not the MCP.
|
||||
|
||||
- `internal/approvals/policy.go` (new): one auditable map of gated actions —
|
||||
`delete_workspace`, `deprovision`, `secret_write`, `org_token_mint`.
|
||||
- `requireApproval(ctx, workspaceID, action, contextHash)` reuses the existing approvals
|
||||
INSERT/broadcast/escalate. If an `approved`+unconsumed row matches → consume it → proceed. Else
|
||||
create a `pending` row, broadcast `EventApprovalRequested`, and return **HTTP 202
|
||||
`{approval_id, status:"pending"}`** instead of executing. The human decides via the existing decide
|
||||
route; the agent retries and the gate now passes.
|
||||
- Add `approval_requests.consumed_at` (single-use) and optional `request_hash` (dedupe identical
|
||||
pending requests).
|
||||
- **Escalation:** the platform agent's `parent_id` is NULL, so platform-originated approvals escalate
|
||||
to the **user** (canvas notify), not a parent.
|
||||
- The 202 response shape is authored in the **OpenAPI SSOT**.
|
||||
|
||||
### 5.9 Billing & model/provider parity
|
||||
|
||||
The platform agent is a `workspaces` row, so it inherits the one billing resolver and the
|
||||
`providers.yaml` runtime matrix unchanged:
|
||||
- **Default `platform_managed`** (metered CP proxy, billed to org credits) — the env wiring in §5.6.
|
||||
- **`byok`** = flip `/admin/workspaces/:id/llm-billing-mode` + supply the org's `ANTHROPIC_API_KEY`
|
||||
secret (workspace or global). Exposed as a provisioning flag so a tenant can choose at create time.
|
||||
- Model **and provider** are switchable (Claude, Kimi-for-coding, …) via the same dashboard
|
||||
model-switcher any workspace uses.
|
||||
|
||||
### 5.10 UX (summary; detailed in app RFC / Phase 5)
|
||||
|
||||
The **dashboard** (`molecule-app`) becomes the primary entry: a concierge chat (default-targeting the
|
||||
§5.4 resolver) plus a live org overview, with pending approvals surfaced inline. The **canvas** stays
|
||||
for advanced users. First UI version is produced in Claude Design and iterated before build.
|
||||
|
||||
## 6. SSOT mapping (derive, don't fork)
|
||||
|
||||
| Concern | Single source of truth | This RFC's rule |
|
||||
|---|---|---|
|
||||
| "The org" | `orgRootID()`/`sameOrg()` (`org_scope.go`) | platform agent *becomes* the root; no `org_id` column |
|
||||
| Platform marker | `workspaces.kind` | `kind` only; never also `role`/`tier` |
|
||||
| Model/provider | `providers.yaml` runtime matrix | concierge switches via the same registry |
|
||||
| LLM billing | `ResolveLLMBillingModeDerived` | inherits the one resolver; no new path |
|
||||
| Config/secrets delivery | tenant Secrets Manager bundle (`seedWorkspaceConfigSecret`) | no new S3 prefix / second store |
|
||||
| Management API | OpenAPI spec | new endpoints authored there first; MCP/CLI/docs derive |
|
||||
| Gated actions | `internal/approvals/policy.go` | one map |
|
||||
| Platform-agent id | `uuidv5(org, "platform-agent")` | derived, never stored separately |
|
||||
|
||||
## 7. Security & blast radius
|
||||
|
||||
The concierge holds the org **admin token** (full tenant-root, self-minting) and is driven by
|
||||
end-user chat. Mitigations:
|
||||
- **Approval gate (§5.8)** must ship *with* the agent going user-facing, not after. Until then the
|
||||
agent is operator-only.
|
||||
- **Tenant isolation** is unchanged — every reach path still passes `sameOrg()`.
|
||||
- **MCP not in workspace images** (dedicated image, §5.7); the admin token lives only in the
|
||||
platform-agent container env on the tenant box.
|
||||
- **Token rotation:** the MCP reads env once at spawn → rotation = `docker restart
|
||||
molecule-platform-agent` (runbook item).
|
||||
- Future: a scoped-down org token (no delete/billing/member) — see §10.
|
||||
|
||||
## 8. Migration & rollout
|
||||
|
||||
Phase ordering is the rollout contract:
|
||||
- **Phase 0** (schema) ships and bakes before anything writes `kind`. Backward-compatible: every
|
||||
existing row defaults to `kind='workspace'`; the `CHECK` is added `NOT VALID` then validated.
|
||||
- **Phase 1 re-parenting backfill** is the one real watch-item. **Before** running it, audit whether
|
||||
any org-scoped table keys off the *root workspace id* (e.g. `org_api_tokens`, `org_plugin_allowlist`)
|
||||
versus the CP org UUID. If they reference the root workspace id, re-parenting changes "the root" and
|
||||
those refs must migrate too. The backfill is per-org, idempotent, and reversible.
|
||||
- New orgs get the platform agent from first boot; existing orgs backfill via `/admin/tenants
|
||||
redeploy` + a one-time re-parent migration.
|
||||
|
||||
## 9. Implementation phases
|
||||
|
||||
0. **Schema + model** (`molecule-core`): `kind` column + `approval_requests.consumed_at`; model field +
|
||||
constants; `Register` accepts/validates `kind` with invariants.
|
||||
1. **Platform-as-root + resolver** (`molecule-core` + CP): CP pre-seeds the platform row and creates
|
||||
teams under it; per-org re-parent backfill (after the §8 audit); `GET /registry/platform-agent`.
|
||||
2. **Config-driven two-MCP runtime** (runtime + claude-code template).
|
||||
3. **Image + tenant provisioning** (CP + image + `molecule-ci`): dedicated image; `start_platform_agent`
|
||||
in user-data + redeploy; config via the tenant Secrets Manager bundle; billing knob.
|
||||
4. **Approval gate** (`molecule-core`): policy map + `requireApproval` at destructive handlers; OpenAPI
|
||||
202 shape.
|
||||
5. **Dashboard concierge UX** (`molecule-app`): design-first, then build against the resolver.
|
||||
6. **Cleanup**: exclude the platform agent from billable counts; canvas visibility; rotation runbook.
|
||||
|
||||
## 10. Open questions
|
||||
|
||||
- **Scoped-down token.** Should the concierge hold a reduced-scope token (no delete/billing/member)
|
||||
instead of full admin + an approval gate? The token-scope system does not exist yet (`orgtoken`
|
||||
TODO). Recommendation: ship admin-token + approval gate now; add scope-down as a follow-up.
|
||||
- **Re-parenting vs. wrapper.** If product later wants a platform agent that is *not* the topological
|
||||
root, a `CanCommunicateWithKind` wrapper (guarded by `sameOrg`) is the alternative. Deferred —
|
||||
platform-as-root is lower-risk and needs zero access-control change.
|
||||
- **Canvas visibility** of the root concierge node (hide vs. show as the org anchor).
|
||||
|
||||
## 11. Verification (end-to-end on a staging tenant)
|
||||
|
||||
1. **Schema:** Phase-0 migrations applied; existing workspaces report `kind='workspace'`; `go test
|
||||
./...` + `-tags=integration` green.
|
||||
2. **Provision:** redeploy a staging tenant; `docker ps` shows `molecule-platform-agent` healthy; its
|
||||
logs show a successful `/registry/register`.
|
||||
3. **Identity:** the platform row is `kind='platform'`, `parent_id IS NULL`; the former root now has
|
||||
`parent_id = <platform id>`; `GET /registry/platform-agent` returns it.
|
||||
4. **Reach:** chat the platform agent → it `list_workspaces` then `create_workspace` via the platform
|
||||
MCP and reports back via `send_message_to_user`.
|
||||
5. **Isolation:** it reaches every workspace in its org and **cannot** reach another tenant's
|
||||
workspace.
|
||||
6. **Approval gate:** `delete_workspace` → HTTP 202 pending + approval event; decide-approve →
|
||||
completes; a second delete with the same approval is rejected (consumed).
|
||||
7. Drive a real concierge flow ("spin up a PM + engineer to build X") and watch the delegation/activity
|
||||
ledger.
|
||||
|
||||
---
|
||||
|
||||
*Derived from a read-only multi-agent source audit of `molecule-core`, `molecule-controlplane`,
|
||||
`molecule-ai-workspace-runtime`, `molecule-ai-workspace-template-claude-code`, and
|
||||
`molecule-mcp-server`. No secret values recorded.*
|
||||
@@ -8,26 +8,39 @@ against the latest `main`.
|
||||
|
||||
## Queue Contract
|
||||
|
||||
Add the `merge-queue` label to an open PR when it is ready to merge.
|
||||
**Auto-discovery (opt-OUT, default).** You do NOT need to label a PR. The bot
|
||||
auto-discovers every open same-repo PR and merges any that meets the bar. The
|
||||
`merge-queue` label is now optional metadata, not a gate. This removed the
|
||||
historical autonomy gap: agent Gitea tokens lack `write:issue` (labels are
|
||||
issue-scoped), so agents could never self-label and ready PRs stalled.
|
||||
|
||||
To keep a PR OUT of autonomous merging, add an opt-OUT label:
|
||||
`merge-queue-hold`, `do-not-auto-merge`, or `wip`. Draft PRs are also skipped.
|
||||
|
||||
The bot processes one PR per tick:
|
||||
|
||||
1. Confirms `main` is green.
|
||||
2. Selects the oldest open PR carrying `merge-queue`.
|
||||
3. Skips PRs with `merge-queue-hold`.
|
||||
4. Rejects fork PRs because the queue may only update same-repo branches.
|
||||
5. If the PR head does not contain current `main`, calls Gitea's
|
||||
1. Confirms `main`'s branch-protection-required push contexts are green.
|
||||
2. Selects the oldest open same-repo PR that is NOT opt-out-labeled and NOT a
|
||||
draft (auto-discovery). With `AUTO_DISCOVER=0` it falls back to legacy
|
||||
opt-IN: only PRs carrying `merge-queue` are considered.
|
||||
3. Rejects fork PRs because the queue may only update same-repo branches.
|
||||
4. If the PR head does not contain current `main`, calls Gitea's
|
||||
`/pulls/{n}/update?style=merge` endpoint and waits for CI on the new head.
|
||||
6. Merges only after the current PR head has required contexts green:
|
||||
- `CI / all-required (pull_request)`
|
||||
- `sop-checklist / all-items-acked (pull_request)`
|
||||
5. Merges only when, on the PR's CURRENT head sha:
|
||||
- `>= required_approvals` distinct genuine official `APPROVED` reviews from
|
||||
the recognised reviewer set (read from branch protection; default 2),
|
||||
- no open official `REQUEST_CHANGES`,
|
||||
- every branch-protection-required status context is green, and
|
||||
- the PR is `mergeable` (Gitea returns `True`; `None`/`False` = wait).
|
||||
|
||||
The workflow is serialized with `concurrency`, so two queued PRs cannot be
|
||||
The merge bar is unchanged by auto-discovery — only WHICH PRs are considered
|
||||
changes. The workflow is serialized with `concurrency`, so two PRs cannot be
|
||||
merged against the same observed `main`.
|
||||
|
||||
## Operator Commands
|
||||
|
||||
Queue a PR:
|
||||
Queue a PR (optional — auto-discovery already considers every ready PR; the
|
||||
label is just visible metadata):
|
||||
|
||||
```bash
|
||||
curl -fsS -X POST \
|
||||
@@ -37,7 +50,8 @@ curl -fsS -X POST \
|
||||
-d '{"labels":["merge-queue"]}'
|
||||
```
|
||||
|
||||
Temporarily hold a queued PR:
|
||||
Keep a PR OUT of autonomous merging (opt-OUT — use `merge-queue-hold`,
|
||||
`do-not-auto-merge`, or `wip`):
|
||||
|
||||
```bash
|
||||
curl -fsS -X POST \
|
||||
@@ -56,9 +70,11 @@ REPO=molecule-ai/molecule-core \
|
||||
WATCH_BRANCH=main \
|
||||
QUEUE_LABEL=merge-queue \
|
||||
HOLD_LABEL=merge-queue-hold \
|
||||
AUTO_DISCOVER=1 \
|
||||
OPT_OUT_LABELS=do-not-auto-merge,wip \
|
||||
REVIEWER_SET=agent-reviewer,agent-researcher,agent-reviewer-cr2 \
|
||||
UPDATE_STYLE=merge \
|
||||
REQUIRED_CONTEXTS='CI / all-required (pull_request),sop-checklist / all-items-acked (pull_request)' \
|
||||
python3 .gitea/scripts/gitea-merge-queue.py
|
||||
python3 .gitea/scripts/gitea-merge-queue.py --dry-run
|
||||
```
|
||||
|
||||
Dry run:
|
||||
|
||||
@@ -161,7 +161,7 @@ func (h *PluginsHandler) uninstallViaDocker(ctx context.Context, c *gin.Context,
|
||||
// 1. Strip plugin's rule/fragment markers from CLAUDE.md (mirrors
|
||||
// AgentskillsAdaptor.uninstall lines 184-188). Best-effort: if
|
||||
// the user edited CLAUDE.md, our marker stays untouched.
|
||||
h.stripPluginMarkersFromMemory(ctx, containerName, pluginName)
|
||||
h.stripPluginMarkersFromMemory(ctx, workspaceID, containerName, pluginName)
|
||||
|
||||
// 2. Remove copied skill dirs declared in the plugin's plugin.yaml.
|
||||
for _, skill := range skillNames {
|
||||
@@ -171,9 +171,11 @@ func (h *PluginsHandler) uninstallViaDocker(ctx context.Context, c *gin.Context,
|
||||
log.Printf("Plugin uninstall: skipping invalid skill name %q in %s: %v", skill, pluginName, err)
|
||||
continue
|
||||
}
|
||||
_, _ = h.execAsRoot(ctx, containerName, []string{
|
||||
if _, rmErr := h.execAsRoot(ctx, containerName, []string{
|
||||
"rm", "-rf", "/configs/skills/" + skill,
|
||||
})
|
||||
}); rmErr != nil {
|
||||
log.Printf("Plugin uninstall: failed to remove skill %s from %s: %v", skill, workspaceID, rmErr)
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Delete the plugin directory itself (as root to handle file ownership).
|
||||
|
||||
@@ -393,7 +393,7 @@ func (h *PluginsHandler) readPluginSkillsFromContainer(ctx context.Context, cont
|
||||
// `# Plugin: <name> /` — mirrors AgentskillsAdaptor.uninstall's stripping
|
||||
// logic so install/uninstall are symmetric. Best-effort: silent on read or
|
||||
// write failure, since the rest of uninstall must still succeed.
|
||||
func (h *PluginsHandler) stripPluginMarkersFromMemory(ctx context.Context, containerName, pluginName string) {
|
||||
func (h *PluginsHandler) stripPluginMarkersFromMemory(ctx context.Context, workspaceID, containerName, pluginName string) {
|
||||
// Use sed via bash -c for atomic in-place delete: drop the marker line
|
||||
// and the blank line that follows it (install adds a leading blank line
|
||||
// before the marker via append_to_memory). Three sed passes mirror the
|
||||
@@ -417,7 +417,9 @@ func (h *PluginsHandler) stripPluginMarkersFromMemory(ctx context.Context, conta
|
||||
`awk 'BEGIN{skip=0; blanks=0} /^%s/{skip=1; blanks=0; next} skip==1 && /^[[:space:]]*$/{blanks++; if(blanks>=2){skip=0; print; next} next} /^# Plugin: /{if(skip==1)skip=0} skip==1{next} {print}' /configs/CLAUDE.md > /tmp/claude.new && mv /tmp/claude.new /configs/CLAUDE.md`,
|
||||
regexpEscapeForAwk(marker),
|
||||
)
|
||||
_, _ = h.execAsRoot(ctx, containerName, []string{"bash", "-c", script})
|
||||
if _, awkErr := h.execAsRoot(ctx, containerName, []string{"bash", "-c", script}); awkErr != nil {
|
||||
log.Printf("Plugin uninstall: failed to strip markers from CLAUDE.md for %s in %s: %v", pluginName, workspaceID, awkErr)
|
||||
}
|
||||
}
|
||||
|
||||
// regexpEscapeForAwk escapes characters that have special meaning inside an
|
||||
|
||||
Reference in New Issue
Block a user