Skip to content

Make the merge gates data, not control flow #64

Description

@daniel-kindl

Problem

docs/agentq.md:347-364 documents ten merge gates and states that the coordinator re-reads
all of them at merge time. The gates exist in four different shapes, so one configuration
skips six of them.

lib/agentqueue/coordinator.py:902-941 (_merge_gates) evaluates six gates. The other four
are control flow inside _implement_and_deliver, a 234-line procedure
(coordinator.py:465-698):

Gate Where it lives
the deterministic checks the while loop at :541-582
the GitHub checks the while True loop at :605-663
the credential scan _require_clean_diff at :847, which raises
the branch adoption audit git.audit_branch at :481-487

Because four gates are control flow, autoMerge: false returns before _merge_gates runs:

# --- the merge gates -------------------------------------------------
if not policy.autoMerge:
    result.outcome = Outcome.SUCCESS
    result.detail = (
        f"pull request #{pull.number} is green. autoMerge is off, so a "
        "human merges it."
    )

The checks_passed gate exists only inside _merge_gates (:929-930). A repair in the CI
loop is therefore never re-checked against it. A repair that leaves the in-sandbox checks
red, but whose push makes GitHub green, reports to the operator as is green.

Corroborating detail

The same block appears three times, at :518-526, :571-580 and :637-646.

The security stop has two protocols. _require_clean_diff and _pull_request_for raise
SecurityStop. The three blocks above return a result whose outcome stops the queue.
_guarded reconciles them afterwards at :349-350.

Four parameters are dead. _merge_gates takes a branch it never reads. _flag_human,
_flag_failed and _after_budget_breach each take a pull= they never read, and four call
sites pass pull=pull.number.

Coordinator.__init__ takes 14 parameters (:107-163).

A related gap in the tests

lib/agentqueue/ghapi.py:39-57 names its own seam:

GhTransport — the one place that runs the gh binary. A test replaces the transport,
or replaces the whole class with an in-memory double that has the same surface.

Every test takes the second option. No test in verify/probes/ instantiates
agentqueue.ghapi.GitHub. The only GhTransport double, at
verify/probes/agentqueue-setup.test.py:424-430, returns (0, "", "") for gh auth status
and never drives GitHub. So 437 lines never execute, including:

  • _pull_from_api, which maps True/False/None to MERGEABLE/CONFLICTING/UNKNOWN.
    Gate E4 depends on it.
  • blocked_by, whose None-instead-of-[] result is what makes the dependency rule
    fail closed. lib/agentqueue/schedule.py:143-149 is the only reader.
  • check_runs, which merges the check-runs endpoint with the commit-status endpoint
    (ghapi.py:266-300).

The Git seam is the counter-example. It has two adapters, and
verify/probes/agentqueue-integration.test.py:162-341 drives the real one against a
disposable bare remote.

A second output channel with one adapter

Coordinator, AgentboxRunner and ci.wait_for_checks each take an emit parameter beside
ui. In production both bindings are the same lambda (lib/agentqueue/cli.py:604,613):

emit=lambda line: ui.note(line, level="verbose"),

In tests emit is lambda line: None, which NullUi already covers. emit has one
adapter, so it is a seam that nothing varies across.

Goal

Let one module answer "may this merge", over one value, on every path that ends a run.

Proposed direction

  • Collect the delivery state into one record: the issue, the branch audit, the agent run,
    the pull request, the head SHA, the CI result and the diff findings.
  • Give MergeGates one function, evaluate(state), which returns one result per documented
    gate. lib/agentqueue/setup.py already uses this shape with Finding, inspect() and
    render().
  • Evaluate the gates on every terminal path. When autoMerge is off, print the gate results
    instead of merging.
  • Let the credential scan and the adoption audit return a gate result. Keep SecurityStop
    for the queue-stopping effect, and raise it from one place.
  • Remove the four dead parameters.
  • Give GhTransport a second adapter that replays captured gh responses, and add
    transport-level tests for the decoding above.
  • Remove emit, and call self.ui.note(...) at the five call sites in coordinator.py.

Acceptance criteria

  • One function returns one result per documented merge gate.
  • The gates evaluate when autoMerge is off, and the run prints them.
  • A repair that leaves the deterministic checks red cannot report is green.
  • The number of gates the code evaluates equals the number of rows in the docs/agentq.md table, and a test asserts it.
  • One gate is testable as one case, without a full process_issue run.
  • The security stop has one protocol.
  • _merge_gates, _flag_human, _flag_failed and _after_budget_breach take no parameter they do not read.
  • A test drives the real GitHub class through a replaying GhTransport.
  • Tests cover the three-valued mergeable mapping, blocked_by returning None, and a commit whose only signal is a commit status.
  • emit is gone, and NullUi covers the silent case.
  • ./verify.sh remains green.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething is not workingrefactorStructural change without intended behavior change

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions