Skip to content

Commit e67307b

Browse files
Danatharclaudehivecommons-hive[bot]
authored
πŸ› [v6] contribute: never dispatch rework for a merged or closed PR (#17403)
* πŸ› [v6] contribute: never dispatch rework for a merged or closed PR #17385 merged at 13:13:58Z; at ~14:25Z the hub still handed Danathar's relay "fix-pr hive#17385", and the relay ran it twice. Hub: - Queue a nudged PR under owner/name, not the nudge's Display (the bare configured name on a hive that lists repos without the org). Resolve keys on owner/name, so the bare entry was never removed when the PR left stage 072, and the relay could not match the task's repo "hive" to an owner/name. - Before dispatching a queued fix, check its PR is still open (state cache first, else one PR read tagged hive:contribute_fix_dispatch). Merged or closed PRs leave the queue with a log line; an unreadable state holds the dispatch for this tick. fixqueue.Next previews what a dispatch would take so only those PRs are checked. - Send a boundary fix task once: the ready handler ships the returned message, so sendFixTask no longer pushes it as well. - Progress for an in-flight fix task (a boundary fix runs as the relay's primary task) is not a resume claim and is no longer revoked. Relay: - A fix-pr task_id the relay already runs (primary, parallel, preempting or deferred) is ignored on a second task_assign, with a log line naming where it runs; a fix that becomes the primary task says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1 Signed-off-by: Danathar <Danathar@users.noreply.github.com> * test(contribute): a retried boundary fix's progress is not revoked (utah-packages#972) projectbluefin/utah-packages#972, 2026-11-09: the relay refused fix-karibhou-1, the hub requeued it and served it at the next ready as fix-karibhou-2 on the same connection, and the relay's progress for the retry (repo empty, client_gen 1870) was answered with "no matching server-issued lease" and task_revoke at 14:50:21Z and 14:54:36Z. The handleTaskProgress change in this PR already covers it; this test drives the real fail -> requeue -> boundary dispatch path and sends progress without a repo, as the relay did, so the retried task id must be tracked on the connection and never revoked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1 Signed-off-by: Danathar <Danathar@users.noreply.github.com> * contribute: an unreadable PR state no longer keeps the relay idle When a queued fix's PR state could not be read, selectFixTask answered task_unavailable/fix_pr_in_flight, so the relay got no new issue either. That happens when GitHub is struggling or the API budget is low, the time the hive needs its contributors most. The fix now stays queued and is retried on the next ready, and the relay takes ordinary work meanwhile. Busy fix slots still answer task_unavailable as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1 Signed-off-by: Douglas Baggett <doug.baggett@gmail.com> --------- Signed-off-by: Danathar <Danathar@users.noreply.github.com> Signed-off-by: Douglas Baggett <doug.baggett@gmail.com> Co-authored-by: Danathar <Danathar@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: hivecommons-hive[bot] <280983584+hivecommons-hive[bot]@users.noreply.github.com>
1 parent 2b549c8 commit e67307b

11 files changed

Lines changed: 651 additions & 26 deletions

β€Žbin/contributor-relay.jsβ€Ž

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5007,12 +5007,25 @@ function escapeRegExp(s) {
50075007
return String(s).replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
50085008
}
50095009

5010+
// fixPRRunningWhere says where this relay is already running fix-pr task
5011+
// taskId, or null. One fix-pr task runs in exactly one place: a second
5012+
// task_assign with the same task_id (a hub that delivered it twice) must not
5013+
// start it again as the primary task or in a parallel session.
5014+
function fixPRRunningWhere(taskId) {
5015+
if (!taskId) return null;
5016+
if (currentTask && currentTask.task_id === taskId) return 'as the primary task';
5017+
if (parallelFixes.has(taskId)) return 'in a parallel fix session';
5018+
if (preemptingFix && preemptingFix.task_id === taskId) return 'as the fix preempting the primary task';
5019+
if (deferredFixes.some(d => d.task.task_id === taskId)) return 'deferred until the current task ends';
5020+
return null;
5021+
}
5022+
50105023
// handleFixPRInterrupt decides spawn / preempt / defer for a fix-pr task that
50115024
// arrived while currentTask is running.
50125025
function handleFixPRInterrupt(msg, hub) {
5013-
if (parallelFixes.has(msg.task_id) || deferredFixes.some(d => d.task.task_id === msg.task_id)
5014-
|| (preemptingFix && preemptingFix.task_id === msg.task_id)) {
5015-
console.log(`Ignoring duplicate fix-pr ${sanitizeHubText(msg.task_id)} β€” already handling it`);
5026+
const where = fixPRRunningWhere(msg.task_id);
5027+
if (where) {
5028+
console.log(`Ignoring duplicate fix-pr ${sanitizeHubText(msg.task_id)} β€” already running it ${where}`);
50165029
return;
50175030
}
50185031
noteTaskAttemptStart(hub, msg);
@@ -10016,6 +10029,10 @@ function handleMessage(data, hub) {
1001610029
handleFixPRInterrupt(msg, hub);
1001710030
break;
1001810031
}
10032+
if (msg.kind === FIX_PR_KIND && fixPRRunningWhere(msg.task_id)) {
10033+
console.log(`Ignoring duplicate fix-pr ${sanitizeHubText(msg.task_id)} β€” already running it ${fixPRRunningWhere(msg.task_id)}`);
10034+
break;
10035+
}
1001910036
if (currentTask) {
1002010037
console.log(`Rejecting task ${taskDisplay(msg)} from ${hub.url} β€” already working on ${taskDisplay(currentTask)}`);
1002110038
sendTo(hub, { type: 'task_failed', seq: nextSeq(), task_id: msg.task_id, reason: 'Already has active task' });
@@ -10108,6 +10125,7 @@ function handleMessage(data, hub) {
1010810125
writeRelayStatus();
1010910126
activeHubIndex = hubs.indexOf(hub);
1011010127
console.log(`Task assigned: ${sanitizeHubText(msg.kind)} ${sanitizeHubText(taskDisplay(msg))} β€” ${sanitizeHubText(msg.title)} (from ${hub.url})`);
10128+
if (msg.kind === FIX_PR_KIND) console.log(`fix-pr ${fixPRLabel(msg)}: running as the primary task (no task was running)`);
1011110129
if (msg.github_token) {
1011210130
injectGhToken(msg.github_token);
1011310131
tokenExpiresAt = msg.token_expires_at ? new Date(msg.token_expires_at).getTime() : null;
@@ -10871,6 +10889,7 @@ if (process.env.HIVE_RELAY_TEST_MODE === '1') {
1087110889
FIX_PR_DONE_MARKER,
1087210890
FIX_PR_FAILED_MARKER,
1087310891
fixPRState,
10892+
fixPRRunningWhere,
1087410893
scanPaneForFixMarkers,
1087510894
relaySupportsParallelFixes,
1087610895
blockingPromptKey,

β€Žbin/contributor-relay.test.jsβ€Ž

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15600,6 +15600,58 @@ test('#12436 headless primary: a preempt fix is deferred and runs before the nex
1560015600
} finally { console.log = log; teardown(relay); }
1560115601
});
1560215602

15603+
// hivecommons/hive#17385 (2026-11-09): the hub delivered one boundary fix-pr
15604+
// task twice. The relay started the first copy as its primary task and the
15605+
// second in a parallel fix session, so the same fix ran in two places.
15606+
test('#17385 a fix-pr task_id delivered twice runs once, as the primary task', () => {
15607+
const relay = loadRelay({ backend: 'claude', env: { HIVE_MAX_PARALLEL_FIXES: '1', HIVE_CONTRIBUTOR_QUOTA_GUARD: 'off' }, execFileResult: { defer: true } });
15608+
const log = console.log;
15609+
const lines = [];
15610+
console.log = (...a) => lines.push(a.join(' '));
15611+
try {
15612+
const hub = relay.getHubs()[0];
15613+
const sent = [];
15614+
hub.ws = { readyState: 1, send: p => sent.push(JSON.parse(p)) };
15615+
relay.setCurrentTask(null);
15616+
const boundary = FIX_PR_TASK('fix-danathar-1', { priority: 'queued', mode: 'boundary' });
15617+
relay.handleMessage(JSON.stringify(boundary), hub);
15618+
assert.strictEqual(relay.getCurrentTask() && relay.getCurrentTask().task_id, 'fix-danathar-1', 'the first copy becomes the primary task');
15619+
const spawnsBefore = relay.__execFileCalls.length;
15620+
relay.handleMessage(JSON.stringify(boundary), hub);
15621+
assert.deepStrictEqual(relay.fixPRState().parallel, [], 'the second copy must not start a parallel fix session');
15622+
assert.strictEqual(relay.__execFileCalls.length, spawnsBefore, 'no parallel child spawned for the duplicate');
15623+
assert.strictEqual(relay.getCurrentTask().task_id, 'fix-danathar-1');
15624+
assert.strictEqual(sent.filter(m => m.type === 'task_accepted' && m.task_id === 'fix-danathar-1').length, 1, `accepted once: ${JSON.stringify(sent)}`);
15625+
assert.ok(!sent.some(m => m.type === 'task_failed' && m.task_id === 'fix-danathar-1'), 'the duplicate is ignored, not failed back');
15626+
assert.ok(lines.some(l => /running as the primary task/.test(l)), `log names the path taken: ${lines.join('\n')}`);
15627+
assert.ok(lines.some(l => /Ignoring duplicate fix-pr fix-danathar-1 β€” already running it as the primary task/.test(l)), lines.join('\n'));
15628+
} finally { console.log = log; teardown(relay); }
15629+
});
15630+
15631+
test('#17385 a fix-pr task_id already in a parallel session is not also started as the primary task', () => {
15632+
const relay = loadRelay({ backend: 'claude', env: { HIVE_MAX_PARALLEL_FIXES: '1', HIVE_CONTRIBUTOR_QUOTA_GUARD: 'off' }, execFileResult: { defer: true } });
15633+
const log = console.log;
15634+
const lines = [];
15635+
console.log = (...a) => lines.push(a.join(' '));
15636+
try {
15637+
const hub = relay.getHubs()[0];
15638+
const sent = [];
15639+
hub.ws = { readyState: 1, send: p => sent.push(JSON.parse(p)) };
15640+
relay.setCurrentTask({ task_id: 't-primary', task_gen: 1, kind: 'issue', repo: 'foo/bar', number: 1, title: 'primary' });
15641+
relay.handleMessage(JSON.stringify(FIX_PR_TASK('fix-r')), hub);
15642+
assert.deepStrictEqual(relay.fixPRState().parallel, ['fix-r']);
15643+
// The primary ends while the fix still runs; the same task arrives again.
15644+
relay.setCurrentTask(null);
15645+
relay.handleMessage(JSON.stringify(FIX_PR_TASK('fix-r')), hub);
15646+
assert.strictEqual(relay.getCurrentTask(), null, 'the duplicate must not become the primary task');
15647+
assert.deepStrictEqual(relay.fixPRState().parallel, ['fix-r']);
15648+
assert.strictEqual(sent.filter(m => m.type === 'task_accepted' && m.task_id === 'fix-r').length, 1, JSON.stringify(sent));
15649+
assert.ok(lines.some(l => /Ignoring duplicate fix-pr fix-r β€” already running it in a parallel fix session/.test(l)), lines.join('\n'));
15650+
assert.strictEqual(relay.fixPRRunningWhere('fix-r'), 'in a parallel fix session');
15651+
assert.strictEqual(relay.fixPRRunningWhere('other'), null);
15652+
} finally { console.log = log; teardown(relay); }
15653+
});
15654+
1560315655
(async () => {
1560415656
for (const [name, fn] of only ? tests.filter(([n]) => n.includes(only)) : tests) {
1560515657
try {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- A contributor's relay is no longer sent to rework a PR that has already merged or closed, and one `fix-pr` task now runs once. On hivecommons/hive#17385 a stage-072 nudge queued a fix at 12:32Z, the PR merged at 13:13:58Z, and at about 14:25Z the hub still handed the relay `fix-pr hive#17385`, which the relay then ran twice: about 4.5 minutes lost, nothing pushed. Each link in that chain is fixed. The fix queue took the nudge's display name (`hive` on a hive whose repos are listed without the org), so the "left stage 072" resolve, which looks up `owner/name`, never found the entry, and the relay got a repo it could not match to its authorized list; queued fixes are now keyed and sent as `owner/name`. Right before a queued fix is handed out, the hub now checks the PR is still open (from the state cache when it holds the repo, else one PR read tagged `hive:contribute_fix_dispatch`); a merged or closed PR leaves the queue with a `fix queue: dropped a PR that is no longer open` log line, and if the state cannot be read the fix waits for the next tick while the relay takes ordinary work. A fix handed out at a task boundary was pushed to the relay twice (once by the dispatcher, once by the `ready` handler); it is now sent once, and the relay ignores a second `task_assign` for a `fix-pr` task it is already running (as its primary task, in a parallel session, preempting, or deferred) and logs where it runs. Progress reports from a boundary fix running as the relay's primary task are no longer answered with `task_revoke: no active lease for this task`, which killed the fix mid-run.

β€Žsrc/docs/contributor-relay.mdβ€Ž

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,9 +1757,12 @@ Set `HIVE_MAX_PARALLEL_FIXES=0` in `~/.config/hive/contributor.env` and restart
17571757
- `mode=preempt`, interactive pane: `task_accepted`, then the pane receives an `INTERRUPT (hive fix-pr, preempt mode)` prompt telling the agent to park the current issue (commit/stash WIP; the claim stays alive), fix the PR, print exactly `HIVE_FIX_PR_DONE <task_id>` (or `HIVE_FIX_PR_FAILED <task_id> <reason>`), then resume. The progress tick reads the marker off the pane and reports `task_complete`/`task_failed` for the fix's `task_id`. Only one fix preempts at a time; more are queued locally.
17581758
- `mode=preempt`, headless primary: there is no pane to interrupt, so the fix is deferred (`task_accepted` immediately) and runs before the relay's next `ready` β€” "fix what you broke before taking new work" holds on the client too.
17591759
- With no task running, a `fix-pr` is just the next task.
1760+
- One `fix-pr` task runs in one place. A second `task_assign` with a `task_id` the relay is already running β€” as its primary task, in a parallel session, as the preempting fix, or deferred β€” is ignored, and the log says where it is already running.
17601761

17611762
The hub records the mode it used in the nudge comment's marker (`mode=spawn|preempt|queued|comment`) and exposes queue depth / in-flight as `fix_queue` on the contributor profile API.
17621763

1764+
**Only open PRs are dispatched.** Right before a queued fix is handed out, the hub checks that its PR is still open: from the state cache when it holds the repo (free while the cached copy is inside its freshness bound, otherwise one re-read), else with one PR read tagged `hive:contribute_fix_dispatch`. A PR that has merged or closed leaves the queue with a `fix queue: dropped a PR that is no longer open` log line, and no relay is sent to rework it. When the state cannot be read, the fix is not dispatched that tick: it stays queued and the next one asks again, and a relay asking for its next task gets ordinary work meanwhile instead of waiting idle. Queued fixes carry the repo as `owner/name` even on a hive that lists its repos without the org, so the fix task's `repo` and the PR leaving stage 072 both match the queued entry.
1765+
17631766
**When the relay fails the task.** `task_failed` for a fix's `task_id` does not settle it as done: its PRs go back in the hub's queue (same place by age) and are not interrupted back in for 5 minutes, though the relay's next `ready` serves them at once. After three failed dispatches a PR leaves the fix queue and the stage engine's re-nudge / adoption path takes over. A dispatch the relay fails within one second of the nudge is not reported as delivered: the nudge comment reads `mode=queued`, says the relay could not take the task (quoting the relay's `message`/`reason`), and the stage log carries `relay_failed=true` (hivecommons/hive#14668).
17641767

17651768
#### What the operator sees (Operations β†’ Connected contributor agents)

β€Žsrc/pkg/dashboard/api_repo_item_takeover_test.goβ€Ž

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dashboard
22

33
import (
4+
"context"
45
"encoding/json"
56
"net/http"
67
"slices"
@@ -236,6 +237,9 @@ func TestRepoItemReroutePickupLapseFallsBackToAdoptNow(t *testing.T) {
236237
srv, store := takeoverServer(t, mock)
237238
hub := srv.contributeHub
238239
hub.fixDispatched = func(_ *ContributorConnection, _ WSMessage) {}
240+
// org/other#1 is a fixture the GitHub mock does not serve: report it
241+
// open so the dispatch check keeps it.
242+
hub.fixPRStateHook = func(context.Context, string, int) (fixPRState, string, error) { return fixPRStateOpen, "", nil }
239243
// bee2 is online but already holds a task: the reroute queues behind it.
240244
c := fixConn(hub, "bee2", fixCaps(capFixPRInterrupt), time.Now())
241245
c.mu.Lock()

β€Žsrc/pkg/dashboard/contribute_coverage_test.goβ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ func TestRoleCoverage_FixTaskDispatchEndsNoWorkRun(t *testing.T) {
603603
relay.mu.Unlock()
604604
hub.sendFixTask(relay, "alice", &fixqueue.Dispatch{ID: "fix-1", Mode: fixqueue.ModeBoundary, Items: []fixqueue.Item{
605605
{Repo: "myorg/repo1", Number: 7, URL: "https://github.com/myorg/repo1/pull/7", Title: "red PR"},
606-
}})
606+
}}, true)
607607
if got := coverageAt(hub, relay, later); !reflect.DeepEqual(got, scannerAlice) {
608608
t.Fatalf("relay working a fix task: RoleCoverage = %+v, want %+v", got, scannerAlice)
609609
}

0 commit comments

Comments
Β (0)