You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
gate(eligibility): hold PRs against a priority issue until its window opens (#9738) (#9847)
* gate(eligibility): hold PRs against a priority issue until its window opens (#9738)
Priority issues carry the highest payout, so assignment fairness matters most there -- and first-come
pickup is only fair if everyone can SEE the issue before anyone can act on it. A PR opened moments
after the label lands means the window between "issue becomes valuable" and "issue is claimed" was
effectively zero for everyone else watching the repo.
A PR closing a `gittensor:priority` issue is now gate-eligible only once the label has been publicly
present for `gate.priorityEligibilityWindow` minutes (default 30, per repo, `0` disables it). A PR
arriving inside the window is NOT rejected: it is HELD with a neutral comment naming the moment it
becomes eligible, and proceeds normally once the window elapses. No penalty beyond waiting.
Two decisions worth stating:
- The clock is anchored to the EARLIEST labeling event, not the most recent. The spec requires that
re-applying the label not reset the window for already-open PRs; an earliest anchor gives that to
everyone and makes "when does this issue open for work" a single knowable instant nothing can move.
- Every unknown FAILS OPEN -- an unreadable timestamp, an absent label, a GraphQL error, a missing
token. Holding a contributor's PR on a fact we could not read is a penalty for our own gap.
The hold reuses the existing merge-hold rail (`heldForManualReview`), so it can never close a PR.
Merge holds become DATA rather than a widening list of booleans. `MERGE_HOLD_INPUTS` is the one
table; the input type (`Record<MergeHoldInput, boolean>`), the `heldForManualReview` fold, and both
test fixtures all derive from it. Adding a hold was three independent edits that could each be
forgotten -- declaring one and not folding it into the decision compiled fine. It is now one entry,
and omitting the wiring is a compile error at the single call site.
`normalizeOptionalNonNegativeInteger` exists because `0` is meaningful here (it turns the rule off)
and the positive-integer normalizer would have discarded it as invalid and silently applied the
default -- an operator's explicit "off" becoming "on".
100% statements and branches on both changed source files.
* test(eligibility): cover the priority-window wiring, and fix the config round-trip it exposed
The GraphQL label-timestamp read had no tests at all: every fail-open path, the
earliest-labeling anchor, and the query shape (first:, not last: -- using last:
would return the most RECENT labeling, which is exactly the value re-applying the
label could use to push a contributor's window back) are now pinned.
Writing the manifest test surfaced a real gap: gateConfigToJson never emitted
priorityEligibilityWindow, so the setting did not round-trip.
resolvePriorityEligibilityHold now DEFAULTS fetchLabeledAt to the real GraphQL read
rather than returning undefined when none is injected -- its doc comment already
claimed that default existed. The production caller passes facts only, and a new
test proves the default really reaches GitHub instead of every case passing on an
injected fake.
resolvePriorityTypeLabel replaces the ad-hoc `typeLabels?.priority ?? DEFAULT`
each caller spelled for itself. The label-author rule (#9737) and this window act on
the same label; two spellings could disagree about a repo that renamed it.
* test(eligibility): close the last coverage gaps without suppressions
Codecov counts a /* v8 ignore */ line as unhit, so both suppressions are gone rather
than carried:
DEFAULT_PRIORITY_LABEL names the built-in value, so resolvePriorityTypeLabel returns
it directly. PrTypeLabelSet is an open Record, which made DEFAULT_TYPE_LABELS.priority
read as possibly-undefined and forced a fallback branch nothing could ever take.
The .catch on resolvePriorityEligibilityHold is removed: that function catches its own
GitHub read and cannot reject, so the guard was unreachable code that read as
tested-and-fine while never running.
The remaining branch is closed by an end-to-end test instead: a clean, approved, green
PR that would MERGE is held because the priority issue it closes only became public a
minute ago -- held, never closed.
* test(engine): behaviour-test the priority label and window in the ENGINE's own suite
packages/loopover-engine/src/** is credited by TWO Codecov uploads: the unflagged
backend one from the root vitest run, and an "engine" flag fed by the package's own
node:test suite (codecov.yml's own note: a PR touching engine source should get
credit from the suite that actually behaviour-tests it). These two surfaces were
only covered by the root suite, so the engine flag reported one uncovered statement
per file -- which is where codecov/patch's missing 2 lines came from. A full
unsharded local run (25,512 tests) confirms the backend side is already at 100%.
resolvePriorityTypeLabel is exported from the engine index so its own suite can
reach it, and MERGE_HOLD_INPUTS drops an export nothing outside its file used.
0 commit comments