fix(eslint): resolve identifier-bound route arguments in no-github-request-interpolated-route

[Copilot]

no-github-request-interpolated-route only inspected inline expressions, so extracting a dynamic route into a local variable silently bypassed the rule. The sibling rules (no-exec-interpolated-command, no-child-process-interpolated-command) already handle this via resolveWriteOnceInitializerChain — this PR applies the same fix.

Changes

• no-github-request-interpolated-route.ts: Import and apply resolveWriteOnceInitializerChain from command-initializer-utils to the first argument before classification. The reported error node remains firstArg for correct source location; only the route-kind check uses the resolved expression.
• no-github-request-interpolated-route.test.ts: Remove the now-incorrect "variable indirection is not resolved" valid case; add tests for the three new behaviors from the acceptance criteria.

Before / after

// Previously unflagged — now correctly reported as interpolatedRoute
function f(owner, repo) {
  const route = `GET /repos/${owner}/${repo}`;
  github.request(route, {});  // ← was silently missed
}

// Static ternary remains a true negative (matches create_project.cjs idiom)
function f(cond) {
  const route = cond ? "GET /a" : "GET /b";
  github.request(route, {});  // ← still not flagged
}

// Reassigned binding is not resolved (write-once semantics preserved)
function f(owner, suffix) {
  let route = `GET /repos/${owner}`;
  route = suffix;
  github.request(route, {});  // ← still not flagged
}

• Fixes no-github-request-interpolated-route: rule never scope-resolves the route argument — identifier-bound dynamic routes bypass dete [Content truncated due to length] #49913

[github-actions]

Thanks for this fix! 🚀 The ESLint rule enhancement to resolve identifier-bound route arguments is well-focused and brings no-github-request-interpolated-route in line with the existing patterns in no-exec-interpolated-command and no-child-process-interpolated-command.

This PR looks ready for review:

• ✅ Comprehensive test coverage with three new scenarios (identifier resolution, static ternary true-negative, write-once reassignment guard)
• ✅ Clear before/after examples documenting the fix
• ✅ Follows the established write-once initializer chain resolution pattern
• ✅ Directly addresses issue no-github-request-interpolated-route: rule never scope-resolves the route argument — identifier-bound dynamic routes bypass dete [Content truncated due to length] #49913

The surgical focus on a single rule with complete test suite and documentation sets a strong example for future fixes in the linter tooling.

Generated by ✅ Contribution Check · auto · 48.9 AIC · ⌖ 4.98 AIC · ⊞ 8.8K · ◷

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[Copilot]

🟢 Ready to approve

The focused implementation matches existing sibling-rule semantics and satisfies the linked issue’s acceptance criteria.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Resolves identifier-bound GitHub request routes before interpolation checks, closing the ESLint rule bypass described in #49913.

Changes:

• Reuses write-once initializer-chain resolution for route arguments.
• Adds coverage for dynamic, static ternary, and reassigned bindings.

File summaries

File	Description
eslint-factory/src/rules/no-github-request-interpolated-route.ts	Resolves safe identifier initializers before route classification.
eslint-factory/src/rules/no-github-request-interpolated-route.test.ts	Verifies the new behavior and false-positive guards.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR does not have the implementation label and has 0 new lines of code in business logic directories (threshold: 100).

[github-actions]

The identifier resolution logic is clean and well-tested. The resolveWriteOnceInitializerChain correctly scopes resolution to function boundaries, handles cycles, and rejects reassigned bindings. Error reporting is anchored to the original call-site firstArg node. No blocking issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 12.4 AIC · ⌖ 9.25 AIC · ⊞ 5.4K

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — changes are correct and well-tested. Approving.

📋 Summary

What the PR does

Applies resolveWriteOnceInitializerChain to the first argument of github.request() before route-kind classification, mirroring the pattern already used in sibling rules.

Positive highlights

• ✅ Root cause addressed (identifier resolution), not just symptom
• ✅ SpreadElement guard added to keep the cast to TSESTree.Expression safe
• ✅ Previously-incorrect "valid" test removed — good hygiene
• ✅ Three acceptance-criteria scenarios covered by new tests
• ✅ Error node stays as firstArg so source locations remain correct

Minor observations (non-blocking)

• The static-ternary test only covers the all-literal case. A mixed-branch test (one literal, one template) would strengthen coverage but is out of scope here.
• No regression test for the SpreadElement guard path — fine as a follow-up.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 16.2 AIC · ⌖ 7.93 AIC · ⊞ 7.1K
_{Comment /matt to run again}

[github-actions]

Review: no blocking issues

This is a clean, minimal fix that mirrors the exact pattern already used in no-exec-interpolated-command and no-child-process-interpolated-command (same SpreadElement guard, same resolveWriteOnceInitializerChain call signature). The reported node correctly stays firstArg (preserving source location) while only the classification logic uses the resolved expression.

What I checked

• Correctness: resolveWriteOnceInitializerChain walks identifier chains only while unvisited (cycle-safe via a seen set) and stops at the first non-Identifier or unresolved initializer; reassignment is correctly excluded, confirmed by the new reassignment test.
• Guard ordering: the SpreadElement early-return is added before resolving, consistent with sibling rules, avoiding a bad cast.
• Test coverage: new tests cover template-literal indirection, string-concat indirection, static-ternary true-negative, and reassignment true-negative.
• Limitation: could not execute the test suite locally (no node_modules, offline); verified by code reading and comparison with already-merged sibling rules.

No correctness, security, or maintainability concerns found in the diff.

🔎 Code quality review by PR Code Quality Reviewer · auto · 30.2 AIC · ⌖ 5.21 AIC · ⊞ 7.9K
_{Comment /review to run again}