fix(review): derive linked-issue fact-fetch from the same fresh body parse as the overflow check - #8477
Conversation
…parse as the overflow check `resolveLinkedIssueHardRule` computed the overflow check from a fresh parse of the PR's current `body` (`extractLinkedIssueNumbersWithOverflow`) but ran its per-issue fact-fetch loop against a SEPARATE, caller-supplied `args.linkedIssues` array — a field synced by an earlier body parse. If a contributor edits their PR description to add a new closing reference between that sync and this evaluation, the overflow check (fresh body) and the fact-fetch loop (stale list) disagree about which issues are "currently linked", and the newly-added issue is silently never evaluated against the configured hard rules until a later re-sync. That is a real edit-driven detection-bypass window for a module whose whole purpose is "evaluate every currently-linked issue's facts against the hard rules." Fix: parse the body once and drive BOTH the overflow check and the fact-fetch loop (and the empty early-out) from that single `extractLinkedIssueNumbersWithOverflow` result, so the two can never disagree. Direction chosen (per the issue's second option): keep the `linkedIssues` parameter for call-site compatibility but stop reading it — a surgical change that leaves `src/queue/processors.ts` untouched; the parameter is documented as retained-but-not-the-source-of-truth. Add a regression test: a body linking JSONbored#9 with a stale `linkedIssues: [1]` now evaluates JSONbored#9's facts (previously it fetched only JSONbored#1, 404'd, and reported the wrong issue). Existing tests that relied on `linkedIssues` driving the fetch were updated to put the same references in `body` (their intent is unchanged); all still pass. Closes JSONbored#8354
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8477 +/- ##
==========================================
- Coverage 92.50% 89.60% -2.90%
==========================================
Files 791 98 -693
Lines 79328 22764 -56564
Branches 23959 3896 -20063
==========================================
- Hits 73379 20398 -52981
+ Misses 4810 2187 -2623
+ Partials 1139 179 -960
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 15:23:19 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver 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://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Closes #8354
Summary
src/review/linked-issue-hard-rules.ts'sresolveLinkedIssueHardRulecomputed the overflow check from a fresh parse of the PR's currentbody(extractLinkedIssueNumbersWithOverflow(args.body ?? "", …).overflow) but ran its per-issue fact-fetch loop against a separate, caller-suppliedargs.linkedIssuesarray — a field populated by an earlier body parse. The two could disagree: if a contributor edits their PR description to add a new closing reference between the lastpr.linkedIssuessync and this evaluation, the overflow check (fresh body) passes while the fact-fetch loop (stale list) never sees — and so never evaluates — the newly-added issue against the configured hard rules until a later re-sync. That's a genuine edit-driven detection-bypass window for a module whose whole job is "evaluate every currently-linked issue's facts against the hard rules."Fix: parse the body once and drive both the overflow check and the fact-fetch loop (and the
numbers.length === 0early-out) from that single result, so they can never disagree about which issues are currently linked.Direction chosen — the issue's second option: keep the
linkedIssuesparameter for call-site compatibility but stop reading it, deriving the fetch list from the fresh parse instead. This keeps the change surgical (src/queue/processors.ts's sole call site is untouched); the parameter is documented in-code as retained-but-not-the-source-of-truth so it can't be mistaken for live input.Regression test: a
bodylinking#9with a stalelinkedIssues: [1]now evaluates#9's facts and blocks on its ineligibility — whereas before the fix it fetched only#1, 404'd, and reported a "could not be found" violation for the wrong issue. This test fails on the old code and passes on the new.Existing tests that relied on
linkedIssuesdriving the fetch were updated to place the same references inbody(their intent is unchanged); all pass.Closes #8354
Scope
type(scope): short summaryConventional Commit format.linked-issue-hard-rules.tsplus its unit test; no unrelated backend/UI/MCP/docs/dep changes.CONTRIBUTING.md; does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #8354.Validation
git diff --check— clean.npm run actionlint— N/A: no workflow / composite-action changes.npm run typecheck— green.npm run test:coveragelocally —test/unit/linked-issue-hard-rules.test.ts(70 tests, incl. the new regression) and the end-to-endtest/unit/queue.test.ts+test/unit/agent-approval-queue.test.ts(260 tests) all pass.codecov/patch: the changed lines/branches inlinked-issue-hard-rules.tsare 100% covered (verified via lcov — the only uncovered branch in the file is at line 134, pre-existing and outside this diff); both arms of the overflow check and the empty early-out are exercised.npm run test:workers— N/A: no worker code changed.npm run build:mcp/npm run test:mcp-pack— N/A: no MCP changes.npm run ui:openapi:check/npm run ui:lint/npm run ui:typecheck/npm run ui:build— N/A: no UI, API, or OpenAPI surface changed.npm audit --audit-level=moderate— not run locally (sandbox audit endpoint returns a lockfile 400); no dependency changes, so it cannot affect the audit. CI runs it against a clean install.If any required check was skipped, explain why:
Safety
CHANGELOG.mdnot edited (not a release-prep PR).Notes
bodyandlinkedIssuesalready agree (the common case) — the fix only changes the outcome in the divergence window the issue describes, which is exactly what the regression test pins. The full end-to-end queue/processor suites confirm no other behavior shifted.