[Feat] Resolve bare issue references (#234, ENG-512) against configured repositories - #722
Merged
Merged
Conversation
Contributor
|
1 issue outstanding. See task
Reviewed e627d99 |
Member
Author
|
Added the fake-timer test for the shared 8s deadline — and it caught a real (pre-existing, since #693) bug: |
Base automatically changed from
feature/external-issue-provider-registry
to
develop
July 23, 2026 16:49
daniel-lxs
marked this pull request as draft
July 23, 2026 17:08
The routing precheck already extracts references like #234, acme/web#42, or ENG-512 into externalReference, but nothing acted on it when the message carried no pasted URL. When the precheck asks for a lookup and URL parsing finds nothing, the reference now resolves deterministically against the deployment's own configuration: repo-qualified numbers must match a configured repository, bare numbers fan out across configured repositories only when there are at most three (labeled per repo so the informed re-route can disambiguate), and Linear-style identifiers fetch directly. Bare references normalize to canonical URLs and reuse the provider registry, so fetch-attempt construction stays single-source. The model still never picks fetch targets: it proposes reference text, code validates against a closed candidate set and executes under the existing shared 8s fail-open deadline. Pasted URLs take precedence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…xpires A new fake-timer test for the shared 8s deadline exposed that withExternalLookupTimeout received an already-invoked promise, so an exhausted deadline still fired the next MCP call and merely ignored its result (pre-existing since #693). The helper now takes a thunk and checks the remaining budget before dispatching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
daniel-lxs
force-pushed
the
feature/router-bare-issue-references
branch
from
July 24, 2026 14:10
e89da82 to
3049854
Compare
Per review: on deployments with many configured repositories, a bare issue number now fans out across the first five (environment order) rather than resolving to nothing. Non-existent issue numbers drop out at fetch time, so only real matches reach the informed re-route. Smarter candidate selection (routing memory) is a future follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Draft until reviewed — do not merge without Dan's call. Rebased onto
developafter #718 and #728 landed; now two commits: bare-reference resolution + a deadline-dispatch fix its new test caught.What changed
Closes the last gap in the lookup gate: the routing precheck already extracts references like
#234,acme/web#42, orENG-512into itsexternalReferencefield, but nothing acted on it when the message had no pasted URL. Now, when the precheck returnsneedsExternalLookup=trueand URL parsing finds nothing,gatherExternalIssueContextresolves the reference deterministically:ENG-512-style identifiers → Linearget_issuedirectly.owner/repo#N→ fetched only if that repo is in the deployment's configuredrepositoryNames(case-insensitive match, configured casing wins).#N/N→ fans out across the first 5 configured repositories (environment order; issue numbers that don't exist in a repo drop out at fetch time); each result is labeled per repo ([acme/web#234]) so the informed re-route call disambiguates. Smarter candidate selection (routing memory) is a planned follow-up.Bare references normalize to canonical URLs and run through #718's registry, so fetch-attempt construction (incl. GitHub's
issue_read→get_issuefallback) stays single-source. Pasted URLs always take precedence over the precheck reference.Also in this PR (found by its own test)
A fake-timer test for the shared 8s deadline exposed a pre-existing bug from #693:
withExternalLookupTimeoutreceived an already-invoked promise, so an exhausted deadline still dispatched the next MCP call and merely never awaited it. The helper now takes a thunk and checks the remaining budget before invoking.Security posture unchanged
The model proposes reference text only; fetch targets come from a closed candidate set (the deployment's own configured repos / the connected Linear workspace), execution stays in server code under the shared 8s fail-open deadline. No model changes — same routing model as develop.
Tests
105 router tests green: five bare-reference cases (bare Linear id, multi-repo fan-out with per-repo labels, repo-qualified validation incl. unconfigured-repo rejection, seven-repo fan-out capped at five, URL precedence), the deadline-sharing fake-timer test, and an end-to-end
Check issue #234→ fetch →phase: 'mcp'case.pnpm knip,tsc --noEmit,format:checkclean.🤖 Generated with Claude Code