fix(review): stop the deterministic type-label mislabel from #5233's broken closure check#5407
Conversation
…broken closure check
fetchLinkedIssueClosedByPullRequest read GitHub's REST /issues/{n}/timeline looking
for a source.issue field on "closed" events, but that field never appears there --
only on cross-referenced events. Confirmed against three live production issues
(commit_id null, no source key at all). This made the check always fail, turning
a rare race into a 100%-reproducing failure on every "Closes #N" merge. Replaced
with GraphQL's Issue.timelineItems -> ClosedEvent.closer, verified empirically
against live issues.
Also fixes two related gaps found during the same investigation:
- resolvePrTypeLabel picked the exclusive bug/feature label by config array
order (bug always won when both matched an issue's labels) instead of
declared precedence. Now the LAST-configured exclusive match wins; operators
declare exclusive mappings in ascending precedence order. Updated the two
bundled example configs, which still described the old first-wins rule.
- maybeReReviewOnLinkedIssueChange only checked isConvergenceRepoAllowed,
unlike the periodic sweep, which also falls back to
isAgentConfigured(settings.autonomy). Aligned the two gates, short-circuited
so the common allowlisted case never pays for the extra settings fetch.
Closes #5385
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
1 similar comment
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 17bb4a8 | Commit Preview URL Branch Preview URL |
Jul 12 2026, 05:03 PM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5407 +/- ##
==========================================
- Coverage 94.45% 94.41% -0.04%
==========================================
Files 552 552
Lines 44310 44317 +7
Branches 14659 14663 +4
==========================================
- Hits 41851 41841 -10
- Misses 1784 1801 +17
Partials 675 675
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 18:59:19 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 7 non-blocking
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Summary
fetchLinkedIssueClosedByPullRequestread GitHub's REST/issues/{n}/timelinelooking for asource.issuefield onclosed-type events, but that field structurally never appears there (confirmed against three live production examples) — only oncross-referencedevents. This made the check always returnnot_closed_by_pull_request, even for a completely legitimate same-PR close, turning every "Closes #N" merge into a guaranteed (not just occasional) title-heuristic mislabel. Replaced it with GraphQL'sIssue.timelineItems -> ClosedEvent.closerfield, which correctly and directly identifies the closing PR — verified empirically against three live issues before writing the fix.resolvePrTypeLabel: when a linked issue (or several disagreeing linked issues) carried bothgittensor:bugandgittensor:feature, the exclusive-mapping resolution picked whichever was declared FIRST in.gittensory.yml'smappings:array — currentlybug, listed beforefeature— so bug always won regardless of value, backwards from feature's higher scoring weight. Now the LAST-declared exclusive match wins, and operators are documented to declare exclusive mappings in ascending precedence order (matching the existing bug→feature→priority default order, so no config change is needed for the three gated repos). Updated.gittensory.yml.exampleand its byte-identical twinconfig/examples/gittensory.full.yml, which still described the old first-wins rule.maybeReReviewOnLinkedIssueChange(wakes a PR when its linked issue is relabeled) only checkedisConvergenceRepoAllowed, unlike the periodic sweep (sweepRepoRegate), which also falls back toisAgentConfigured(settings.autonomy). A repo with real acting autonomy configured but outside theGITTENSORY_REVIEW_REPOSenv allowlist could see a stale type label / hard-rule verdict indefinitely. Aligned the two gates, short-circuited so the common (already-allowlisted) case never pays for the extra settings fetch.Closes #5385
Root cause detail
See #5385 for the full investigation, including three confirmed-live mislabeled PRs found in the ~40 minutes after this morning's #5233 deploy (#5360, #5363, #5371 — all
gittensor:feature→gittensor:bug) and one that additionally lost itsgittensor:priorityreward multiplier entirely (#5359).Changes
src/github/backfill.ts:fetchLinkedIssueClosedByPullRequestrewritten to query GraphQL'sClosedEvent.closerinstead of REST's/issues/{n}/timeline. Removed the now-deadtimelineEventClosesIssueFromPullRequestREST-shape helper.src/settings/pr-type-label.ts(+ byte-identical mirror inpackages/gittensory-engine/src/settings/pr-type-label.ts): exclusive-mapping resolution now lets the LAST match win instead of the first.src/queue/processors.ts:maybeReReviewOnLinkedIssueChange's gate now falls back toisAgentConfigured(settings.autonomy), short-circuited to avoid the extra fetch in the common allowlisted case..gittensory.yml.example/config/examples/gittensory.full.yml: updated the exclusive-mapping precedence doc comment to match the new last-wins behavior.test/unit/linked-issue-label-propagation-fetch.test.tsandtest/unit/queue-5.test.tsfrom GitHub's REAL GraphQL response shape (not an assumed one — the exact gap that let fix(review): verify linked issue closure source #5233 ship broken). Added a dedicatedtest/unit/backfill.test.tssuite directly covering every branch of the rewrittenfetchLinkedIssueClosedByPullRequest(no token, malformed repo, GraphQL top-level errors array, malformed nodes, network failure, matching/non-matching/null/Commit closer) — several of these are real, confirmed-live GitHub response shapes, not hypotheticals. Updated the twopr-type-label.test.ts/pr-type-label-engine.test.tstests that encoded the old first-match-wins precedence. Added two newqueue.test.tsregression tests for the autonomy-fallback gate (wakes via fallback when unlisted-but-configured; stays dormant when genuinely inert).Test plan
npm run typecheck— cleannpm run test:ci(full local gate, unsharded coverage) — see CIpr-type-label.test.ts,pr-type-label-engine.test.ts,linked-issue-label-propagation-fetch.test.ts,queue.test.ts,queue-5.test.ts