Skip to content

Commit de4f869

Browse files
wolfgang-auraclaude
andcommitted
wip: close the last two PRHunt delay defects and update the procedure
Batch refresh for candidates whose freshness evidence aged out while another candidate was still in review (#69). Validation of the verification passthrough, so a Mailman option can no longer reach the test runner and read as a candidate failure (#70). Procedure raised to version 2: coordinator ownership, checkpoint pages, the cumulative review budget, health states, narrow-first duplicate search and the pre-finish refresh. The skill and command carry the ownership rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 5bdda48 commit de4f869

6 files changed

Lines changed: 261 additions & 15 deletions

File tree

.agents/skills/prhunt/SKILL.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ Read `mailman/procedure.md` from the repository root, or run
88
model. Follow it through `hunt finish`; do not stop after the agents finish.
99

1010
Read existing hunt state with `python -m mailman hunt list` before starting.
11+
One hunt has one coordinator. If `hunt status` shows a live lease you do not
12+
hold, another task owns it: take a separate hunt, or take that one over with
13+
`hunt lease --takeover --reason`. Never sit in a loop polling someone else's
14+
hunt. Show the checkpoint page as soon as any candidate is ready rather than
15+
holding finished work until the quota is met.
1116
If model choices are missing, ask which primary and reviewer adapter/model IDs
1217
the user wants. Do not assume defaults. N counts complete PR candidates, not
1318
attempts. Repair routine failures or replace candidates without asking the user.

mailman/cli.py

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ def _build_parser() -> argparse.ArgumentParser:
122122
"action",
123123
choices=(
124124
"init", "add", "drop", "restore", "status", "finish", "list",
125-
"escalate", "refresh-procedure", "lease", "release",
125+
"escalate", "refresh-procedure", "lease", "release", "refresh",
126126
),
127127
)
128128
hunt.add_argument("hunt_id", nargs="?")
@@ -714,7 +714,7 @@ def _hunt(arguments: argparse.Namespace) -> int:
714714
hunt.release_lease(root, record, owner=arguments.owner)
715715
print(json.dumps({"hunt_id": record["hunt_id"], "lease": None}, indent=2))
716716
return 0
717-
if arguments.action in ("add", "drop", "restore", "escalate", "finish"):
717+
if arguments.action in ("add", "drop", "restore", "escalate", "finish", "refresh"):
718718
hunt.require_lease(record, arguments.owner)
719719
if arguments.action == "add":
720720
if not arguments.run_id:
@@ -742,7 +742,12 @@ def _hunt(arguments: argparse.Namespace) -> int:
742742
"attempted": arguments.attempted, "why_user": arguments.why_user,
743743
"user_action": arguments.user_action})
744744
hunt.save(hunt.hunt_path(root, record["hunt_id"]), record)
745-
result = hunt.finish(root, record) if arguments.action == "finish" else hunt.status(root, record)
745+
if arguments.action == "refresh":
746+
result = hunt.refresh(root, record)
747+
elif arguments.action == "finish":
748+
result = hunt.finish(root, record)
749+
else:
750+
result = hunt.status(root, record)
746751
print(json.dumps(result, indent=2))
747752
return 1 if arguments.action == "finish" and not result["complete"] else 0
748753

@@ -1966,6 +1971,36 @@ def _tail(text: str, lines: int = 20) -> str:
19661971
return "\n".join(text.strip().splitlines()[-lines:])
19671972

19681973

1974+
#: Options only Mailman defines. One of these after `--` is always a mistake:
1975+
#: it reaches the test runner, which fails collection, and the failure reads
1976+
#: like a broken candidate. https://github.com/wolfgang-aura/Mailman/issues/70
1977+
_MAILMAN_ONLY_OPTIONS = frozenset({
1978+
"--data-root", "--max-revisions", "--max-review-cycles", "--reasoning-effort",
1979+
"--max-turns", "--agent-timeout", "--verification-timeout", "--owner",
1980+
"--acknowledge-prior-attempts", "--acknowledge-claims", "--verification",
1981+
})
1982+
1983+
1984+
def check_verification_command(command: list[str]) -> None:
1985+
"""Refuse a passthrough that cannot be what the operator meant."""
1986+
if not command:
1987+
return
1988+
if command[0].startswith("-"):
1989+
raise ValueError(
1990+
f"the verification command starts with {command[0]!r}. Everything "
1991+
"after `--` is run as a program, so the first token must be an "
1992+
"executable, not an option."
1993+
)
1994+
for token in command:
1995+
name = token.split("=", 1)[0]
1996+
if name in _MAILMAN_ONLY_OPTIONS:
1997+
raise ValueError(
1998+
f"{name} is a Mailman option, but it appears after `--`, so the "
1999+
"test runner would receive it and fail collection. Move it "
2000+
"before the `--` separator."
2001+
)
2002+
2003+
19692004
def main(arguments: list[str] | None = None) -> int:
19702005
raw_arguments = list(arguments if arguments is not None else sys.argv[1:])
19712006
verification_command: list[str] | None = None
@@ -1980,6 +2015,8 @@ def main(arguments: list[str] | None = None) -> int:
19802015
if parsed.subcommand in ("verify", "orchestrate", "resume-review", "reproduce", "build-prompts"):
19812016
parsed.command = verification_command or []
19822017
try:
2018+
if parsed.subcommand in ("verify", "orchestrate", "resume-review", "reproduce", "build-prompts"):
2019+
check_verification_command(parsed.command)
19832020
if parsed.subcommand == "doctor":
19842021
return _doctor()
19852022
if parsed.subcommand == "procedure":

mailman/hunt.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,60 @@ def write_checkpoint(root: Path, record: dict, result: dict) -> Path | None:
338338
return destination
339339

340340

341+
def refresh(root: Path, record: dict) -> dict:
342+
"""Re-run the aging evidence for every ready candidate, as one batch.
343+
344+
Duplicate searches and claim reads expire in an hour. Two finished
345+
candidates were withheld while a third was still in review, aged out one at
346+
a time, and the visible ready count fell from two to zero. Refreshing them
347+
together is what the coordinator was doing by hand.
348+
349+
This does not weaken the filing gate. `hunt finish` still re-checks every
350+
candidate and still refuses stale evidence; this only makes getting them
351+
fresh together a single command.
352+
See https://github.com/wolfgang-aura/Mailman/issues/69.
353+
"""
354+
from mailman.claims import read_claims
355+
from mailman.submission import record_duplicate_search
356+
357+
before = status(root, record)
358+
refreshed: list[dict] = []
359+
for row in before["runs"]:
360+
if row.get("dropped"):
361+
continue
362+
run, directory = load_run(row["run_id"], root)
363+
# A candidate that is ready needs no refresh, and one that never
364+
# reached a handoff has an earlier problem than aging. The runs this is
365+
# for are the finished ones whose evidence expired while another
366+
# candidate was still in review: complete packages reading as failures.
367+
if row["ready"] or load_handoff(directory) is None:
368+
continue
369+
outcome = {"run_id": run.run_id}
370+
search = read_object(directory / "duplicate-search.json")
371+
if not search.get("query"):
372+
outcome["duplicate_search"] = "no recorded query; run duplicate-search first"
373+
else:
374+
fresh = record_duplicate_search(
375+
directory,
376+
repository=run.repository,
377+
query=search["query"],
378+
issue_number=search.get("issue_number"),
379+
symbols=search.get("symbols") or (),
380+
)
381+
outcome["duplicate_search"] = {
382+
"success": fresh["success"], "complete": fresh["complete"],
383+
"matches": fresh.get("match_count", 0),
384+
"decided_by": fresh.get("decided_by"),
385+
}
386+
claims = read_claims(directory)
387+
outcome["claims"] = {"success": claims.get("success"),
388+
"assignees": claims.get("assignees")}
389+
refreshed.append(outcome)
390+
after = status(root, record)
391+
return {**after, "refreshed": refreshed,
392+
"ready_before_refresh": before["ready"]}
393+
394+
341395
def finish(root: Path, record: dict) -> dict:
342396
from mailman.review_packet import write_packet_page
343397
from mailman.review_page import write_run_page

mailman/procedure.md

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# PRHunt procedure, version 1
1+
# PRHunt procedure, version 2
22

33
This is the procedure for every Mailman coordinator, regardless of model.
44
Read it on `/PRHunt N`, `$prhunt N`, or a request to hunt for N pull requests.
@@ -16,20 +16,35 @@ Claude CLI adapters. Confirm each selected CLI is installed and authenticated
1616
with a harmless local fixture before spending a target run.
1717

1818
Create the hunt with `mailman hunt init N --primary ADAPTER --primary-model ID
19-
--reviewer ADAPTER --reviewer-model ID`. `mailman hunt status HUNT_ID` gives the
20-
next missing step and last-check timestamp. Update it after each run. Its
21-
records survive a new conversation. The coordinator performs the actions;
22-
the command does not spawn a background agent or discover targets itself.
19+
--reviewer ADAPTER --reviewer-model ID`. It prints a lease with an `owner`
20+
token. Keep that token: every action that changes the hunt takes `--owner
21+
TOKEN`. `mailman hunt status HUNT_ID` gives the next missing step and last-check
22+
timestamp. Update it after each run. Its records survive a new conversation.
23+
The coordinator performs the actions; the command does not spawn a background
24+
agent or discover targets itself.
25+
26+
One hunt has one coordinator. If `hunt status` shows a live lease you do not
27+
hold, you are the second task on someone else's hunt. Do not poll it and do not
28+
work around it. Either take a separate hunt of your own, or, when the other
29+
coordinator is genuinely gone, take over with `mailman hunt lease HUNT_ID
30+
--owner YOUR_TOKEN --takeover --reason "..."`, which records what you took and
31+
why. Waiting on another coordinator is not progress; it costs the same model
32+
allowance and produces nothing.
33+
34+
Renew the lease with `mailman hunt lease` during long stages. Release it with
35+
`mailman hunt release --owner TOKEN` when you stop.
2336

2437
## Find and screen
2538

2639
1. Read the target's contributor instructions and AI policy. Run
2740
`mailman screen-target OWNER/REPO --refresh`. Reject a failed screen.
2841
Human-only authorship declarations, assignment requirements and bans on
2942
generated descriptions are reasons to pick another target for this flow.
30-
2. Search open and closed PRs by exact affected symbol, behaviour and issue
31-
number, using more than one query where needed. Read overlapping patches
32-
and maintainer responses. A broad empty search is not sufficient evidence.
43+
2. Search narrow first. Give `duplicate-search` the issue number and the
44+
symbols the change touches with `--symbol`; the broad listing runs after.
45+
A record whose `decided_by` is `narrow` already found a duplicate and the
46+
candidate is finished. Read overlapping patches and maintainer responses. A
47+
broad empty search is not sufficient evidence, and neither is a narrow one.
3348
3. Initialize a run at an exact current upstream commit with the hunt's model
3449
configuration. Add it with `mailman hunt add HUNT_ID RUN_ID`.
3550
4. Run `fetch-issue`, `duplicate-search`, `prior-art`, `target-intel` and
@@ -48,9 +63,11 @@ the command does not spawn a background agent or discover targets itself.
4863
missing dependencies from the reported defect. Run `reproduce`, then
4964
`check-target`. A bug that no longer reproduces means replace the candidate.
5065
8. Use `build-prompts RUN_ID -- EXECUTABLE ARG ...` to record verification argv.
51-
`orchestrate RUN_ID` reads that same command. Do not supply custom prompts
52-
or call `run-agent` to bypass this sequence. Both model roles must use the
53-
same recorded procedure and the independent verification gate.
66+
Everything after `--` is run as a program, so it starts with an executable
67+
and carries no Mailman option; the CLI refuses the common mistakes but not
68+
all of them. `orchestrate RUN_ID` reads that same command. Do not supply
69+
custom prompts or call `run-agent` to bypass this sequence. Both model roles
70+
must use the same recorded procedure and the independent verification gate.
5471

5572
## Repair without escalating routine work
5673

@@ -61,6 +78,16 @@ the command does not spawn a background agent or discover targets itself.
6178
before another edit. Never retry an identical command indefinitely.
6279
For an unusable review, preserve the patch, repair the environment, then
6380
run `resume-review`. Do not restart a dirty primary workspace.
81+
Reviewer passes are budgeted per run, not per command: `--max-review-cycles`
82+
counts across every `orchestrate` and `resume-review`. When a run blocks on
83+
a spent budget, replace the candidate or raise the budget deliberately and
84+
say why. Do not resume repeatedly to buy more passes.
85+
A run whose `hunt status` carries a `health` state stopped for a reason
86+
outside the candidate. `USAGE_LIMIT` means the account, not the code, and
87+
the record holds the stage and the exact resume command; retrying the
88+
candidate spends the same allowance again. `INFRASTRUCTURE` means the host,
89+
such as an unwritable temporary directory. Neither is a candidate defect and
90+
neither is a reason to drop a target.
6491
11. Drop duplicate, assigned, prohibited, unreproducible or unsuitable targets.
6592
Record why and continue searching until N candidates pass. A dropped run
6693
costs no user decision. A failed candidate may be replaced after bounded
@@ -85,11 +112,20 @@ the command does not spawn a background agent or discover targets itself.
85112
exact local branch and final body. Run `handoff-check`. Keep all filings
86113
and upstream writes pending. For a self-sourced defect, prepare any required
87114
issue text alongside the PR and ask for approval of the ordered filings.
88-
15. `mailman hunt finish HUNT_ID` must exit 0. It counts only SEND decisions
115+
15. Refresh the aging evidence for every ready candidate together with
116+
`mailman hunt refresh HUNT_ID --owner TOKEN` immediately before finishing.
117+
Duplicate searches and claim reads expire in an hour, and refreshing them
118+
one at a time is how a hunt with two ready candidates reported zero.
119+
16. `mailman hunt finish HUNT_ID --owner TOKEN` must exit 0. It counts only SEND decisions
89120
with passing filing checks and generates the existing packet format.
90121
Inspect that generated packet visually. Never hand-write review HTML.
91122
Present the packet and ask for approval of the exact filings once.
92123

124+
Do not hold finished work back until the quota is met. `hunt status` writes a
125+
checkpoint page as soon as any candidate is ready. Show it. A candidate that is
126+
ready and invisible is indistinguishable to the user from no candidate at all,
127+
and that is what fifteen hours of silence looked like.
128+
93129
## What reaches the user
94130

95131
Only escalate unavailable user-controlled authentication, an explicit budget

tests/test_hunt.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
finish,
2020
load_hunt,
2121
next_action,
22+
refresh,
2223
restore_run,
2324
status,
2425
)
@@ -122,6 +123,77 @@ def test_no_checkpoint_is_written_when_nothing_is_ready(self):
122123
add_run(self.data_root, hunt, run.run_id)
123124
self.assertNotIn("checkpoint", status(self.data_root, hunt))
124125

126+
def test_refresh_renews_every_ready_candidate_in_one_batch(self):
127+
"""https://github.com/wolfgang-aura/Mailman/issues/69
128+
129+
Aging evidence is what turned two ready candidates into a reported
130+
zero. `hunt refresh` renews them together; `hunt finish` still decides.
131+
"""
132+
from datetime import timedelta
133+
134+
import mailman.hunt as hunt_module
135+
136+
hunt = self.new_hunt()
137+
directory = self.ready_run()
138+
add_run(self.data_root, hunt, directory.name)
139+
self.assertEqual(status(self.data_root, hunt)["ready"], 1)
140+
141+
stale = (datetime.now(UTC) - timedelta(hours=3)).isoformat()
142+
for name in ("duplicate-search.json", "claims.json"):
143+
payload = json.loads((directory / name).read_text(encoding="utf-8"))
144+
payload["searched_at" if "duplicate" in name else "collected_at"] = stale
145+
if "duplicate" in name:
146+
payload["query"] = "fixture defect"
147+
(directory / name).write_text(json.dumps(payload), encoding="utf-8")
148+
self.assertEqual(status(self.data_root, hunt)["ready"], 0)
149+
self.assertFalse(finish(self.data_root, hunt)["complete"])
150+
151+
now = datetime.now(UTC).isoformat()
152+
calls = []
153+
154+
def fake_duplicate_search(run_directory, *, repository, query, issue_number=None,
155+
symbols=(), **_):
156+
calls.append(("duplicate-search", query))
157+
record = {"success": True, "complete": True, "searched_at": now,
158+
"repository": "example/project", "query": query, "matches": [],
159+
"match_count": 0, "decided_by": "broad", "symbols": list(symbols)}
160+
(run_directory / "duplicate-search.json").write_text(
161+
json.dumps(record), encoding="utf-8")
162+
return record
163+
164+
def fake_read_claims(run_directory, **_):
165+
calls.append(("claims", run_directory.name))
166+
record = json.loads((run_directory / "claims.json").read_text(encoding="utf-8"))
167+
record.update(collected_at=now, success=True)
168+
(run_directory / "claims.json").write_text(json.dumps(record), encoding="utf-8")
169+
return record
170+
171+
import mailman.claims
172+
import mailman.submission
173+
original_search = mailman.submission.record_duplicate_search
174+
original_claims = mailman.claims.read_claims
175+
mailman.submission.record_duplicate_search = fake_duplicate_search
176+
mailman.claims.read_claims = fake_read_claims
177+
try:
178+
result = refresh(self.data_root, hunt)
179+
finally:
180+
mailman.submission.record_duplicate_search = original_search
181+
mailman.claims.read_claims = original_claims
182+
183+
self.assertEqual(result["ready_before_refresh"], 0)
184+
self.assertEqual(result["ready"], 1)
185+
self.assertEqual(len(result["refreshed"]), 1)
186+
self.assertEqual([kind for kind, _ in calls], ["duplicate-search", "claims"])
187+
self.assertTrue(finish(self.data_root, hunt)["complete"])
188+
189+
def test_refresh_leaves_a_ready_candidate_alone(self):
190+
hunt = self.new_hunt()
191+
directory = self.ready_run()
192+
add_run(self.data_root, hunt, directory.name)
193+
result = refresh(self.data_root, hunt)
194+
self.assertEqual(result["ready"], 1)
195+
self.assertEqual(result["refreshed"], [])
196+
125197
def test_a_second_coordinator_cannot_change_a_leased_hunt(self):
126198
"""https://github.com/wolfgang-aura/Mailman/issues/63"""
127199
hunt = self.new_hunt()

tests/test_prevention.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,3 +218,45 @@ def test_a_search_with_nothing_narrow_to_go_on_is_unchanged(self):
218218
)
219219
self.assertEqual(record["decided_by"], "broad")
220220
self.assertNotIn("narrow", record["methods"]["pr"])
221+
222+
223+
class VerificationPassthroughTests(unittest.TestCase):
224+
"""https://github.com/wolfgang-aura/Mailman/issues/70"""
225+
226+
def test_a_mailman_option_after_the_separator_is_refused(self):
227+
from mailman.cli import check_verification_command
228+
229+
for command in (
230+
["pytest", "-q", "--data-root", "runs"],
231+
["pytest", "--max-review-cycles=2"],
232+
["python", "-m", "pytest", "--reasoning-effort", "max"],
233+
):
234+
with self.subTest(command=command):
235+
with self.assertRaisesRegex(ValueError, "Mailman option"):
236+
check_verification_command(command)
237+
238+
def test_a_command_that_starts_with_an_option_is_refused(self):
239+
from mailman.cli import check_verification_command
240+
241+
with self.assertRaisesRegex(ValueError, "must be an executable"):
242+
check_verification_command(["-q", "tests"])
243+
244+
def test_a_real_verification_command_passes(self):
245+
from mailman.cli import check_verification_command
246+
247+
check_verification_command(
248+
["python", "-m", "pytest", "tests/test_show.py", "-q", "-p", "no:cacheprovider"]
249+
)
250+
check_verification_command([])
251+
252+
def test_the_cli_reports_it_rather_than_running_the_runner(self):
253+
from contextlib import redirect_stderr
254+
from io import StringIO
255+
256+
from mailman.cli import main
257+
258+
stderr = StringIO()
259+
with redirect_stderr(stderr):
260+
code = main(["build-prompts", "RUN", "--", "pytest", "--data-root", "x"])
261+
self.assertNotEqual(code, 0)
262+
self.assertIn("Mailman option", stderr.getvalue())

0 commit comments

Comments
 (0)