fix: make a volunteer claim mean the same thing on both paths (#6112) - #6149
Merged
Conversation
A human commenting "taking this" on an unassigned issue was resolved by two code paths that wrote OPPOSITE forge state. The deterministic issue-watcher (assignVolunteer) set the assignee, stamped `in-progress`, and left the `good first issue` / `help wanted` invitations up; the claim agent's Phase 1 handoff was instructed to do exactly the reverse. Whichever ran first decided what the issue looked like afterwards. Both now write one policy — assignee + `in-progress` + invitations retired — declared once as `volunteerClaimLabels()` in lib/dispatchLabels.js, which the watcher reads directly and the claim prompt renders its shell commands from (claim-issue v25). A volunteer claim is a claim: it is what the Issues tab hides on, and an issue somebody has taken must stop advertising itself to the next contributor. A failed or unverified handoff still writes nothing. `in-progress` also gained the releaser it was missing. It had three appliers and two releasers, so a volunteer who was assigned and never opened a PR matched the reconciler's STALLED state on every pass, forever. issueReconcile now classifies that case ABANDONED — a non-owner claim with no merged PR, no open PR, no claim branch, no agent, untouched for 14 days — and releases the marker and the stale assignee back to the queue with an explanatory comment. It is deliberately narrow: an unresolvable viewer identity, an unknown update time, or a claim held by this install never qualifies, so the release can never race a run that is live on a peer. Also fixes a latent 422 in the Layered Intelligence pause path: applyBlockingLabel is the only caller that applies its label and had no lazy `label create`, so the first pause on an install that had never filed an issue would have failed. Claude-Session: https://claude.ai/code/session_01SbKSP9rMnsqZj1iVSRfpYX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two code paths resolved the same event — a human commenting "taking this" on an unassigned issue — and they wrote opposite forge state.
issueWatcher.js#assignVolunteerset the assignee, stampedin-progress, and left thegood first issue/help wantedinvitations up; the claim agent's Phase 1 handoff was instructed to do exactly the reverse ("leave contributor-invitation labels intact, do NOT addin-progress"). Whichever ran first decided what the issue looked like afterwards.This takes option (A) from the issue — a volunteer claim IS a claim — and encodes it once.
One policy, one place.
volunteerClaimLabels()inserver/lib/dispatchLabels.jsstates it (in-progresson, both contributor invitations off), beside the existingformatContributorLabelReleaseCommands. The issue-watcher reads the label lists directly; the claim prompt renders its shell commands fromformatVolunteerClaimCommands(), the wayCONTRIBUTOR_RELEASE_GHis already interpolated — so the prompt and the deterministic path cannot drift again. The rendered handoff includes the lazylabel createbefore the--add-label(a fresh fork would otherwise 422 and silently drop the marker), and a failed or unverified handoff still writes nothing at all.The marker finally has a releaser.
in-progresshad three appliers and two releasers: a volunteer assigned but never opening a PR matchedissueReconcile.js#classifyIssue'sSTALLEDon every pass, indefinitely. The classifier gains a narrowerABANDONEDstate — a non-owner claim with no merged PR, no open PR, no claim branch, no active agent, untouched forFOREIGN_CLAIM_STALE_DAYS(14) — andreleaseAbandonedClaims()strips the label and the stale assignee with an explanatory comment, returning the issue to the queue. It is deliberately conservative:ghauth blip) meanshasForeignClaimisfalsefor everything — absent is never "everyone is a foreigner".ABANDONED— both flows that create a volunteer claim aregh-only, so there is no PortOS applier there to release.The release runs deterministically in the
issue-reconcilepre-step rather than through the coordinator agent (the facts already are the decision, so an LLM call would only re-derive them), and it runs before the "no zombies → park" return — a repo whose only stuck issues are abandoned claims has zero zombies on every pass.Also folded in (the issue's "Related" note):
layeredIntelligence/forgeFiler.js#applyBlockingLabelis the only path that appliesLI_BLOCKING_LABELand had no lazylabel create, so the first pause on an install that had never filed an issue would have hit the same latent 422. It now callsensureForgeLabelsfirst. The broader four-call-siteapplyForgeLabelconsolidation is not included — the four callers use three different exec conventions, so it is not the ~10 lines the issue estimated and would have doubled this diff.Distribution-model bookkeeping:
PROMPT_VERSIONS['claim-issue']24 → 25, the outgoing v24 body preserved inPREVIOUS_DEFAULT_PROMPTS, the integrity snapshot regenerated, andscripts/migrations/338-claim-issue-volunteer-claim-contract.jsupgrades un-customized stored prompts on existing installs.Test plan
cd server && npm test— 1920 files / 38765 tests passing.cd client && npm test— 834 files / 10246 tests passing (untouched by this change; run to confirm).server/lib/dispatchLabels.test.js— the policy shape, that it hands back a mutable copy rather than the frozen shared list, and the create → add → release command order for bothghandglab.server/services/issueWatcher.test.js— the full marker set on the fast path and on the label-create retry path, plus a new case proving a--remove-label422 (an issue carrying neither invitation, the common case) does not take the assignment down with it.server/services/issueReconcile.test.js—ABANDONEDvsSTALLEDacross the threshold, a null stale-day count, an owner-held claim, an unresolvable viewer login, and GitLab;releaseAbandonedClaimscomment/edit ordering, the comment-fails-release-still-happens path, the edit-fails-retry-next-pass path, and the no-write guards; plus a guard that the identity probe is skipped when nothing is assigned.server/services/cosTaskPreStepBlocks.test.js— the release is ordered after the null guard and before every parking/dispatch return.server/services/taskPromptDefaults.test.js— v25 renders the shared helper's exact commands, both halves of the old contract are gone, and the outgoing v24 body is preserved.scripts/migrations/338-...test.js— upgrades a stored v24 default in both schedule locations, leaves customized and already-current prompts alone.ollama, gemma-4-12B-coder, effort low): "No findings" on the source diff. The full diff exceeded the model's 32k context, so the review was run over the five source files only.Closes #6112
https://claude.ai/code/session_01SbKSP9rMnsqZj1iVSRfpYX