Compare commits
9 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| e0b6939e11 | |||
| 45dcc665d3 | |||
| c90b44fc47 | |||
| a89e2680c9 | |||
| a2d9549de2 | |||
| c967edd162 | |||
| 52ab1cc715 | |||
| 6dcde313d1 | |||
| 0aa18baecc |
@@ -9,17 +9,33 @@ queue. This script provides the missing serialized policy in user space:
|
||||
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.
|
||||
2. Refuse to act unless main's BP-required contexts are green. This is also
|
||||
the serialized backstop for direct-merge (see below): after a direct merge,
|
||||
main re-runs push CI and this gate PAUSES the queue if main goes red, so no
|
||||
merge piles onto an unverified/red main (issue #2358).
|
||||
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
|
||||
/pulls/{n}/update endpoint and stop. CI must rerun on the updated head.
|
||||
4. DIRECT-MERGE when conflict-free (issue #2358). When Gitea reports the PR
|
||||
conflict-free (mergeable is True) and the merge bar below is met, MERGE IT
|
||||
DIRECTLY — even if its head does not contain current main. We do NOT call
|
||||
/pulls/{n}/update first: branch protection does not require strict
|
||||
up-to-date, so behind-main conflict-free PRs merge cleanly, and calling
|
||||
/update would trigger Gitea dismiss_stale_approvals (dismissing the genuine
|
||||
approvals and forcing a re-review every tick — the rebase-churn bottleneck).
|
||||
The /update path is used ONLY when the PR is DEFINITIVELY not mergeable
|
||||
(mergeable is literal False) AND its head lacks current main — refreshing the
|
||||
branch may resolve a behind-main non-conflict; a real conflict returns HTTP
|
||||
409 and the PR is HELD per #2352. mergeable=None/missing (Gitea STILL
|
||||
COMPUTING conflict state) is a distinct fail-closed WAIT: never merged AND
|
||||
never /update'd — calling /update during the compute window would dismiss the
|
||||
PR's genuine approvals (dismiss_stale_approvals) and re-introduce the exact
|
||||
rebase-churn this queue eliminates. None is re-checked next tick.
|
||||
5. Merge ONLY when, on the PR's CURRENT head sha:
|
||||
- >= REQUIRED_APPROVALS distinct GENUINE official APPROVED reviews from
|
||||
the recognised reviewer set (not stale, not dismissed, commit_id ==
|
||||
current head), AND
|
||||
- no open official REQUEST_CHANGES on the current head, AND
|
||||
- every BP-required status context is green, AND
|
||||
- the PR is mergeable.
|
||||
- the PR is mergeable (Gitea reports it conflict-free).
|
||||
|
||||
Authoritative gates (fail-closed):
|
||||
- The REQUIRED status contexts come from BRANCH PROTECTION
|
||||
@@ -622,29 +638,32 @@ def evaluate_merge_readiness(
|
||||
approvers: set[str],
|
||||
request_changes: list[str],
|
||||
pr_has_current_base: bool,
|
||||
mergeable: bool,
|
||||
mergeable: bool | None,
|
||||
pr_labels: set[str] | None = None,
|
||||
) -> MergeDecision:
|
||||
# 1) Main's push-required contexts must be green. Combined state can be
|
||||
# "failure" due to non-blocking jobs (continue-on-error: true) that do
|
||||
# not gate merges, so check the explicit required set, not combined.
|
||||
#
|
||||
# This main-green gate is ALSO the serialized backstop that makes the
|
||||
# direct-merge (no update) path safe (issue #2358): after a direct merge
|
||||
# of a behind-main PR, main re-runs its push CI; if a semantic main-break
|
||||
# slips through (PR green standalone but broken when combined with newer
|
||||
# main), main's required contexts go red and this gate PAUSES the queue —
|
||||
# no further merge piles onto an unverified/red main until it is green.
|
||||
main_latest = latest_statuses_by_context(main_status.get("statuses") or [])
|
||||
main_ok, main_bad = required_contexts_green(main_latest, push_required_contexts())
|
||||
if not main_ok:
|
||||
return MergeDecision(False, "pause", "main required contexts not green: " + ", ".join(main_bad))
|
||||
|
||||
# 2) PR head must contain current main.
|
||||
if not pr_has_current_base:
|
||||
return MergeDecision(False, "update", "PR head does not contain current main")
|
||||
|
||||
# 3) No open official REQUEST_CHANGES on the current head.
|
||||
# 2) No open official REQUEST_CHANGES on the current head.
|
||||
if request_changes:
|
||||
return MergeDecision(
|
||||
False, "wait",
|
||||
"open REQUEST_CHANGES on current head from: " + ", ".join(sorted(request_changes)),
|
||||
)
|
||||
|
||||
# 4) Enough distinct genuine official approvals on the current head.
|
||||
# 3) Enough distinct genuine official approvals on the current head.
|
||||
if len(approvers) < required_approvals:
|
||||
return MergeDecision(
|
||||
False, "wait",
|
||||
@@ -653,7 +672,7 @@ def evaluate_merge_readiness(
|
||||
f"need {required_approvals}",
|
||||
)
|
||||
|
||||
# 5) Every BRANCH-PROTECTION-REQUIRED status context must be green. This is
|
||||
# 4) Every BRANCH-PROTECTION-REQUIRED status context must be green. This is
|
||||
# the authoritative status gate — NON-required reds (qa-review,
|
||||
# security-review, sop-tier/sop-checklist when not BP-required, E2E Chat,
|
||||
# Staging SaaS, ci-arm64-advisory, continue-on-error jobs) are NOT
|
||||
@@ -663,16 +682,53 @@ def evaluate_merge_readiness(
|
||||
if not ok:
|
||||
return MergeDecision(False, "wait", "required contexts not green: " + ", ".join(missing_or_bad))
|
||||
|
||||
# 6) Gitea must consider the PR mergeable (no conflicts).
|
||||
if not mergeable:
|
||||
return MergeDecision(False, "wait", "PR is not mergeable (conflicts)")
|
||||
# 5) DIRECT-MERGE when conflict-free (issue #2358 — throughput fix).
|
||||
# If Gitea reports the PR conflict-free (mergeable is True), MERGE IT
|
||||
# DIRECTLY even if its head does not yet contain current main. Branch
|
||||
# protection does NOT require strict up-to-date, so a behind-main but
|
||||
# conflict-free PR merges cleanly. We deliberately do NOT call
|
||||
# /pulls/{n}/update first: update triggers Gitea dismiss_stale_approvals,
|
||||
# which would dismiss the PR's genuine approvals and force a full
|
||||
# re-review every tick — the rebase-churn bottleneck that collapsed
|
||||
# throughput to ~0/hr with dozens of mergeable PRs open.
|
||||
#
|
||||
# The merge bar is UNCHANGED: we only reach here with main green +
|
||||
# >= required genuine approvals on the current head + no open
|
||||
# REQUEST_CHANGES + every BP-required context green. The trade-off is
|
||||
# that the PR's CI ran on a possibly-behind base, so a SEMANTIC main-break
|
||||
# is caught by POST-merge main CI (step 1's pause backstop) rather than
|
||||
# pre-merge. force_merge is used ONLY for missing-but-non-required
|
||||
# governance reds (required are green + approvals genuine), never to
|
||||
# bypass a failing required context or an approval shortfall.
|
||||
if mergeable is True:
|
||||
force = _non_required_red_present(latest, required_contexts)
|
||||
return MergeDecision(True, "merge", "ready", force=force)
|
||||
|
||||
# Ready. Use force_merge ONLY if the merge would otherwise be blocked by
|
||||
# missing-but-non-required governance contexts. Required are green and
|
||||
# approvals are genuine, so force only bypasses non-required reds — never a
|
||||
# failing required context or missing approval.
|
||||
force = _non_required_red_present(latest, required_contexts)
|
||||
return MergeDecision(True, "merge", "ready", force=force)
|
||||
# 6) NOT (yet) mergeable. TRI-STATE, fail-closed — never merge on an unknown.
|
||||
# We MUST distinguish "still computing" (None/missing) from a "definitive
|
||||
# conflict" (False); collapsing them would route a behind-main but
|
||||
# STILL-COMPUTING PR into the /update path, whose dismiss_stale_approvals
|
||||
# is the rebase-churn this change eliminates.
|
||||
#
|
||||
# mergeable is None → Gitea has NOT finished computing conflict state.
|
||||
# WAIT: do nothing this tick — never /update (would dismiss genuine
|
||||
# approvals during the compute window → churn), never merge. Re-check next
|
||||
# tick once Gitea reports a decisive True/False.
|
||||
if mergeable is None:
|
||||
return MergeDecision(
|
||||
False, "wait",
|
||||
"PR mergeability is still being computed (mergeable=None) — waiting",
|
||||
)
|
||||
|
||||
# mergeable is False → DEFINITIVE not-mergeable. If the head also does not
|
||||
# contain current main, try the /update path to refresh the branch (this
|
||||
# may resolve a behind-main non-conflict; a real conflict returns HTTP 409
|
||||
# and process_once HOLDs the PR per #2352). If the head already contains
|
||||
# current main yet Gitea still reports not-mergeable, there is nothing the
|
||||
# queue can do (genuine conflict against current main) — WAIT.
|
||||
if not pr_has_current_base:
|
||||
return MergeDecision(False, "update", "PR not mergeable and head does not contain current main")
|
||||
return MergeDecision(False, "wait", "PR is not mergeable (conflicts)")
|
||||
|
||||
|
||||
def get_branch_head(branch: str) -> str:
|
||||
@@ -1076,12 +1132,20 @@ def _evaluate_candidate(
|
||||
# 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 "wait" decision.
|
||||
mergeable = pr.get("mergeable") is True
|
||||
# FAIL-CLOSED, TRI-STATE: Gitea returns mergeable=None (or omits the field)
|
||||
# while it is still COMPUTING conflict state, mergeable=False for a definitive
|
||||
# conflict, and mergeable=True only when it has proven the PR conflict-free.
|
||||
# We preserve all THREE states (do NOT collapse None/missing into False):
|
||||
# - True → direct-merge eligible (step 5).
|
||||
# - None / missing → still computing → WAIT (never merge, never update,
|
||||
# never dismiss approvals); re-check next tick.
|
||||
# - False → definitive conflict → the update/hold path (step 6).
|
||||
# Collapsing None→False would route a behind-main but STILL-COMPUTING PR into
|
||||
# the /update path, which triggers dismiss_stale_approvals — the exact
|
||||
# rebase-churn this change eliminates. Normalize only to the literal True /
|
||||
# False / None set (some Gitea versions omit the key entirely → None).
|
||||
raw_mergeable = pr.get("mergeable")
|
||||
mergeable: bool | None = raw_mergeable if isinstance(raw_mergeable, bool) else None
|
||||
|
||||
reviews = get_pull_reviews(pr_number)
|
||||
approvers, request_changes = genuine_approvals(
|
||||
|
||||
@@ -143,13 +143,72 @@ def test_merge_decision_requires_main_green_pr_green_and_current_base():
|
||||
assert decision.force is False # no non-required reds present
|
||||
|
||||
|
||||
def test_merge_decision_updates_stale_pr_before_merge():
|
||||
decision = mq.evaluate_merge_readiness(**_ready_kwargs(pr_has_current_base=False))
|
||||
def test_behind_main_but_mergeable_pr_merges_directly():
|
||||
"""§SOP-22 (#2358): a behind-main but CONFLICT-FREE PR (mergeable is True)
|
||||
merges DIRECTLY — no update step. Branch protection does not require strict
|
||||
up-to-date, and calling /update would dismiss the genuine approvals
|
||||
(dismiss_stale_approvals), forcing re-review every tick (the throughput
|
||||
bottleneck). This replaces the old update-before-merge behavior."""
|
||||
decision = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(pr_has_current_base=False, mergeable=True)
|
||||
)
|
||||
|
||||
assert decision.ready is True
|
||||
assert decision.action == "merge"
|
||||
|
||||
|
||||
def test_behind_main_and_not_mergeable_pr_updates():
|
||||
"""The /update path is reached ONLY when the PR is NOT mergeable AND its head
|
||||
lacks current main — refreshing the branch may resolve a behind-main
|
||||
non-conflict; a real conflict 409s and is held (#2352)."""
|
||||
decision = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(pr_has_current_base=False, mergeable=False)
|
||||
)
|
||||
|
||||
assert decision.ready is False
|
||||
assert decision.action == "update"
|
||||
|
||||
|
||||
def test_current_base_but_not_mergeable_pr_waits():
|
||||
"""Up-to-date with main yet Gitea reports not-mergeable → genuine conflict
|
||||
against current main (or still computing). The queue cannot act: WAIT,
|
||||
never update (update would not help) and never merge (fail-closed)."""
|
||||
decision = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(pr_has_current_base=True, mergeable=False)
|
||||
)
|
||||
|
||||
assert decision.ready is False
|
||||
assert decision.action == "wait"
|
||||
assert "not mergeable" in decision.reason
|
||||
|
||||
|
||||
def test_behind_main_and_mergeable_none_waits_not_update():
|
||||
"""§SOP-22 (CR2 #2374) — the churn-residual fix. A BEHIND-MAIN PR whose
|
||||
mergeability Gitea is STILL COMPUTING (mergeable is None) must WAIT, NOT take
|
||||
the /update path. The old code collapsed None→False, so a behind-main +
|
||||
None PR returned action="update" → /pulls/{n}/update → dismiss_stale_approvals
|
||||
→ the exact rebase-churn this change eliminates, fired during the compute
|
||||
window. None and False are now DISTINCT: None waits, False updates."""
|
||||
decision = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(pr_has_current_base=False, mergeable=None)
|
||||
)
|
||||
|
||||
assert decision.ready is False
|
||||
assert decision.action == "wait" # NOT "update" — no churn during compute
|
||||
assert "computed" in decision.reason
|
||||
|
||||
|
||||
def test_current_base_and_mergeable_none_waits():
|
||||
"""Up-to-date with main + mergeable None (still computing) → WAIT (unchanged
|
||||
fail-closed; just confirming None is never merged regardless of base)."""
|
||||
decision = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(pr_has_current_base=True, mergeable=None)
|
||||
)
|
||||
|
||||
assert decision.ready is False
|
||||
assert decision.action == "wait"
|
||||
|
||||
|
||||
def test_MergePermissionError_inherits_from_ApiError():
|
||||
assert issubclass(mq.MergePermissionError, mq.ApiError)
|
||||
|
||||
@@ -506,6 +565,131 @@ def test_process_once_merges_when_mergeable_is_true(monkeypatch):
|
||||
assert calls["hold_label"] is None
|
||||
|
||||
|
||||
def test_process_once_behind_main_mergeable_none_waits_no_update(monkeypatch):
|
||||
"""§SOP-22 (CR2 #2374) — end-to-end churn-residual regression. A BEHIND-MAIN
|
||||
PR (commits do NOT contain main_sha) whose mergeability Gitea is STILL
|
||||
COMPUTING (mergeable=None) must WAIT: process_once returns 0 and NEVER calls
|
||||
update_pull (which dismisses genuine approvals via dismiss_stale_approvals)
|
||||
NOR merge_pull NOR hold. The old None→False collapse routed this exact case
|
||||
into the /update path → approval-dismissing rebase churn during the compute
|
||||
window. This proves the durable churn elimination: no update, approvals
|
||||
preserved, re-checked next tick."""
|
||||
calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
|
||||
_fully_ready_process_once_monkeypatch(monkeypatch, mergeable=None, calls=calls)
|
||||
# Make the head BEHIND main: commits do NOT contain main_sha. This is the
|
||||
# case the bug missed (the prior None test had current base, masking it).
|
||||
behind_head = "a" * 40
|
||||
monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": behind_head}])
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["updated"] is False # NO /update → approvals NOT dismissed
|
||||
assert calls["merge_attempts"] == 0 # never merge on an unknown
|
||||
assert calls["hold_label"] is None # transient → not held, retried next tick
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# §SOP-22: DIRECT-MERGE throughput fix (#2358). A conflict-free 2-genuine PR
|
||||
# merges WITHOUT a pre-merge /update call, so its approvals are NOT dismissed by
|
||||
# dismiss_stale_approvals. The merge bar (2-genuine-on-current-head +
|
||||
# BP-required green + mergeable + no RC + opt-out) is UNCHANGED; only the
|
||||
# unnecessary update-before-merge churn is removed. The /update path survives
|
||||
# for the genuine case it is needed (not-mergeable + behind-main), where a real
|
||||
# conflict 409s and is held per #2352. mergeable=None stays fail-closed.
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_process_once_merges_conflict_free_pr_without_update(monkeypatch):
|
||||
"""§SOP-22(a) — the core throughput fix. A conflict-free, fully-approved PR
|
||||
merges WITHOUT update_pull ever being called. The old behavior called
|
||||
/update first whenever the head lacked current main, which dismissed the 2
|
||||
genuine approvals (dismiss_stale_approvals) and forced re-review every tick.
|
||||
Assert update_pull is NOT invoked and merge_pull IS invoked."""
|
||||
calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
|
||||
_fully_ready_process_once_monkeypatch(monkeypatch, mergeable=True, calls=calls)
|
||||
# Make the head BEHIND main: commits do NOT contain main_sha. Under the old
|
||||
# logic this alone forced an update_pull; under the fix it merges directly.
|
||||
head_sha = "a" * 40
|
||||
monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": head_sha}])
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merge_attempts"] == 1 # merged directly
|
||||
assert calls["updated"] is False # NO update_pull → approvals NOT dismissed
|
||||
assert calls["hold_label"] is None
|
||||
|
||||
|
||||
def test_process_once_behind_main_conflict_free_merges_directly(monkeypatch):
|
||||
"""§SOP-22(b) — explicit behind-main + conflict-free case: it still merges
|
||||
directly (branch protection does not require strict up-to-date)."""
|
||||
calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
|
||||
_fully_ready_process_once_monkeypatch(monkeypatch, mergeable=True, calls=calls)
|
||||
behind_head = "a" * 40
|
||||
monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": behind_head}])
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merge_attempts"] == 1
|
||||
assert calls["updated"] is False
|
||||
|
||||
|
||||
def test_process_once_pauses_when_main_not_green_no_direct_merge(monkeypatch):
|
||||
"""§SOP-22 backstop — the serialized safety that makes direct-merge safe:
|
||||
when main's required push contexts are NOT green (e.g. a prior direct merge
|
||||
introduced a semantic main-break caught by post-merge main CI), the queue
|
||||
PAUSES — it does NOT merge the next PR onto an unverified/red main."""
|
||||
calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
|
||||
_fully_ready_process_once_monkeypatch(monkeypatch, mergeable=True, calls=calls)
|
||||
main_sha = "b" * 40
|
||||
|
||||
def red_main_combined(sha):
|
||||
if sha == main_sha:
|
||||
return {"state": "failure",
|
||||
"statuses": [{"context": "CI / all-required (push)", "status": "failure"}]}
|
||||
return {"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}]}
|
||||
monkeypatch.setattr(mq, "get_combined_status", red_main_combined)
|
||||
|
||||
rc = mq.process_once(dry_run=False)
|
||||
|
||||
assert rc == 0
|
||||
assert calls["merge_attempts"] == 0 # paused — no merge onto red main
|
||||
assert calls["updated"] is False
|
||||
|
||||
|
||||
def test_direct_merge_bar_unchanged_behind_main(monkeypatch):
|
||||
"""§SOP-22(d) — the merge bar is UNCHANGED on the new direct-merge path. A
|
||||
behind-main + conflict-free PR is still rejected (no merge) when ANY gate
|
||||
fails: insufficient genuine approvals, red required context, open
|
||||
REQUEST_CHANGES, or opt-out label. Direct-merge removes the update churn, it
|
||||
does NOT weaken the bar — fail-closed on every gate."""
|
||||
head_sha = "a" * 40
|
||||
behind_main = dict(pr_has_current_base=False, mergeable=True)
|
||||
|
||||
# <2 genuine approvals → wait, not merge.
|
||||
d = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(approvers={"agent-researcher"}, **behind_main)
|
||||
)
|
||||
assert d.action == "wait" and d.ready is False
|
||||
|
||||
# Red required context → wait, not merge.
|
||||
red_required = {"state": "failure", "statuses": [
|
||||
{"context": "CI / all-required (pull_request)", "status": "failure"}]}
|
||||
d = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(pr_status=red_required, **behind_main)
|
||||
)
|
||||
assert d.action == "wait" and d.ready is False
|
||||
|
||||
# Open REQUEST_CHANGES on current head → wait, not merge.
|
||||
d = mq.evaluate_merge_readiness(
|
||||
**_ready_kwargs(request_changes=["agent-reviewer-cr2"], **behind_main)
|
||||
)
|
||||
assert d.action == "wait" and d.ready is False
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Fix 3: status fetch is fail-closed (failed fetch != green)
|
||||
# --------------------------------------------------------------------------
|
||||
@@ -707,13 +891,17 @@ def _stale_pr_update_409_monkeypatch(monkeypatch, queued_issues, calls):
|
||||
# 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,
|
||||
"state": "open", "number": n, "mergeable": False,
|
||||
"base": {"ref": "main", "repo_id": 1},
|
||||
"head": {"sha": head_sha, "repo_id": 1},
|
||||
"labels": [{"name": "merge-queue"}],
|
||||
})
|
||||
# NOTE: commits do NOT contain main_sha → pr_has_current_base is False →
|
||||
# decision.action == "update".
|
||||
# NOTE: mergeable is False (real conflict) AND commits do NOT contain
|
||||
# main_sha → pr_has_current_base is False → decision.action == "update".
|
||||
# Under the #2358 direct-merge fix the update path is reached ONLY when the
|
||||
# PR is NOT mergeable; a mergeable=True behind-main PR would merge directly,
|
||||
# so this fixture sets mergeable=False to exercise the #2352 409-on-update
|
||||
# hold path.
|
||||
monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": head_sha}])
|
||||
monkeypatch.setattr(mq, "get_pull_reviews", lambda n: [
|
||||
{"state": "APPROVED", "user": {"login": "agent-researcher"},
|
||||
|
||||
@@ -271,6 +271,11 @@ func (m *Manager) Reload(ctx context.Context) {
|
||||
ch.Config["_channel_id"] = ch.ID
|
||||
|
||||
go func(a ChannelAdapter, c ChannelRow, pCtx context.Context) {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("PANIC recovered in channel polling goroutine: %v", r)
|
||||
}
|
||||
}()
|
||||
if err := a.StartPolling(pCtx, c.Config, m.onInboundMessage); err != nil {
|
||||
log.Printf("Channels: polling error for %s/%s: %v", c.ChannelType, truncID(c.ID), err)
|
||||
}
|
||||
@@ -354,6 +359,11 @@ func (m *Manager) HandleInbound(ctx context.Context, ch ChannelRow, msg *Inbound
|
||||
typingCtx, typingCancel := context.WithCancel(fireCtx)
|
||||
defer typingCancel()
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("PANIC recovered in typing indicator goroutine: %v", r)
|
||||
}
|
||||
}()
|
||||
typer.SendTyping(ch.Config, msg.ChatID)
|
||||
ticker := time.NewTicker(4 * time.Second)
|
||||
defer ticker.Stop()
|
||||
|
||||
@@ -63,31 +63,6 @@ func TestSessionSearchReturnsActivityAndMemory(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSessionSearch_DBError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewActivityHandler(broadcaster)
|
||||
|
||||
mock.ExpectQuery("WITH session_items AS").
|
||||
WillReturnError(context.DeadlineExceeded)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/ws-123/session-search?q=test", bytes.NewBufferString(""))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-123"}}
|
||||
|
||||
handler.SessionSearch(c)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500 on DB error, got %d", w.Code)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- Activity List source filter ----------
|
||||
|
||||
func TestActivityList_SourceCanvas(t *testing.T) {
|
||||
|
||||
@@ -602,33 +602,6 @@ func TestDelegationRecord_RejectsInvalidUUID(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestDelegationRecord_DBInsertFails(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
h := NewDelegationHandler(wh, broadcaster)
|
||||
|
||||
mock.ExpectExec("INSERT INTO activity_logs").
|
||||
WillReturnError(fmt.Errorf("connection refused"))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "550e8400-e29b-41d4-a716-446655440000"}}
|
||||
body := `{"target_id":"550e8400-e29b-41d4-a716-446655440001","task":"hello","delegation_id":"del-xyz"}`
|
||||
c.Request = httptest.NewRequest("POST", "/delegations/record", bytes.NewBufferString(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Record(c)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500 on DB insert failure, got %d", w.Code)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDelegationUpdateStatus_CompletedInsertsResultRow(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime/debug"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
@@ -113,6 +114,11 @@ func (h *WorkspaceHandler) goAsync(fn func()) {
|
||||
h.asyncWG.Add(1)
|
||||
go func() {
|
||||
defer h.asyncWG.Done()
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("PANIC recovered in goAsync goroutine: %v\n%s", r, debug.Stack())
|
||||
}
|
||||
}()
|
||||
fn()
|
||||
}()
|
||||
}
|
||||
@@ -151,6 +157,11 @@ func globalGoAsync(fn func()) {
|
||||
globalAsync.Add(1)
|
||||
go func() {
|
||||
defer globalAsync.Done()
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("PANIC recovered in globalGoAsync goroutine: %v\n%s", r, debug.Stack())
|
||||
}
|
||||
}()
|
||||
fn()
|
||||
}()
|
||||
}
|
||||
|
||||
@@ -199,6 +199,11 @@ func (s *Scheduler) Start(ctx context.Context) {
|
||||
// entry/exit — those are kept as redundant signals but this pulse is the
|
||||
// one that guarantees liveness freshness regardless of tick state.
|
||||
go func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("PANIC recovered in scheduler heartbeat goroutine: %v", r)
|
||||
}
|
||||
}()
|
||||
pulseTicker := time.NewTicker(10 * time.Second)
|
||||
defer pulseTicker.Stop()
|
||||
for {
|
||||
@@ -638,6 +643,11 @@ func (s *Scheduler) fireSchedule(ctx context.Context, sched scheduleRow) {
|
||||
summary := s.extractResponseSummary(respBody)
|
||||
if summary != "" {
|
||||
go func(wsID, text string) {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
log.Printf("PANIC recovered in broadcast summary goroutine: %v", r)
|
||||
}
|
||||
}()
|
||||
postCtx, postCancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer postCancel()
|
||||
s.channels.BroadcastToWorkspaceChannels(postCtx, wsID, text)
|
||||
|
||||
Reference in New Issue
Block a user