eslint-factory: recognize JSDoc @param {typeof import('@actions/core')} as core-alias signal

[Copilot]

8 ESLint rules sharing core-method-resolve.ts were blind to DI-style @actions/core parameters with non-canonical names (e.g., coreArg) because isCoreAliasIdentifier only matched against CORE_ALIASES = {"core", "coreObj"}. Any function like async function f(coreArg, ctx) was invisible to all affected rules regardless of its JSDoc type annotation.

Changes

• core-method-resolve.ts

  • Added JSDOC_CORE_PARAM_RE — matches @param {typeof import('@actions/core')} <name> in block comments (single- and double-quote variants).
  • Added hasJSDocCoreParamAnnotation(functionNode, paramName, sourceCode) — checks JSDoc before the function node and its parent VariableDeclaration (for function expressions).
  • Added isJSDocCoreParamInScope(identifier, sourceCode) — resolves an identifier through scope and checks for the annotation.
  • isCoreAliasIdentifier — Parameter branch now has two gates: CORE_ALIASES fast path, then JSDoc slow path. Existing exact-name semantics are fully preserved.
  • isDestructuredCoreMethodIdentifier — accepts destructuring from JSDoc-annotated core parameters (e.g., const { setFailed } = coreArg).

• Tests — new it() groups in require-return-after-core-setfailed.test.ts and no-core-setoutput-non-string.test.ts covering:

  • Valid: corpus case coreArg.setFailed(msg); throw … recognized as compliant (not a false positive).
  • Invalid: missing control-transfer after coreArg.setFailed(…) is flagged with suggestion.
  • Negative: un-annotated coreArg without JSDoc remains unrecognized (false-positive guard).

Before / after

/**
 * @param {typeof import('@actions/core')} coreArg - GitHub Actions core library
 */
async function validateContextVariables(coreArg, ctx) {
  coreArg.setFailed(errorMessage);
  throw new Error(errorMessage);   // ← now recognized as compliant by require-return-after-core-setfailed
}

Parameters without the JSDoc type annotation continue to be ignored — broad prefix/regex matching is intentionally avoided.

• Fixes eslint-factory: core-method-resolve.ts CORE_ALIASES gap makes 8 sibling rules blind to DI-style core parameter names #49489

[github-actions]

PR Triage

• Category: bug | Risk: low | Priority: low
• Score: 17/100 (impact 10 + urgency 5 + quality 2)
• Recommended action: defer
• Batch: wip-drafts

Note: Draft, no diff yet (still generating)

Generated by 🔧 PR Triage Agent · auto · 48.9 AIC · ⌖ 6.24 AIC · ⊞ 7.9K · ◷

[Copilot]

Pull request overview

Extends shared ESLint core-method resolution to recognize JSDoc-typed dependency-injection parameters.

Changes:

• Adds JSDoc-based core parameter detection.
• Adds direct-call coverage for two ESLint rules.
• Regenerates an unrelated workflow lock file.

Show a summary per file

File	Description
core-method-resolve.ts	Adds JSDoc-aware alias resolution.
require-return-after-core-setfailed.test.ts	Tests annotated setFailed calls.
no-core-setoutput-non-string.test.ts	Tests annotated setOutput calls.
daily-byok-ollama-test.lock.yml	Adds generated model and pricing configuration.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

[Copilot]

Fixed in 9cfa1fa — changed JSDOC_CORE_PARAM_RE to use ([$\w]+) so identifiers containing $ (e.g. $core) are captured correctly.

[Copilot]

Fixed in 9cfa1fa — the variable branch in isCoreAliasIdentifier now checks || isJSDocCoreParamInScope(declarator.init as TSESTree.Identifier, sourceCode) so const c = coreArg is recognized when coreArg is JSDoc-annotated.

[Copilot]

Added in 9cfa1fa and 60e26fa — both test files now include valid/invalid cases for const { setFailed } = coreArg (CJS) and const { setOutput } = coreArg (CJS), exercising the destructuring branch directly.

[Copilot]

This lock file change cannot be reverted without breaking check-workflow-drift. The .md source is unchanged in this PR, but make recompile now generates the extended content (fallback pricing + model-routing policy) due to updates in the compilation tooling. The check-workflow-drift gate requires lock files to always match what compilation produces. The lock file stays in sync with the compiler output.

[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. ✅

[github-actions]

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

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

[github-actions]

The JSDoc-based slow path is well-structured. Global regex lastIndex is correctly reset before each exec loop. def.node for Parameter defs correctly points to the enclosing function, so getCommentsBefore finds the JSDoc. Arrow-function assignments are handled by walking up to VariableDeclarator.parent. Test coverage is thorough: valid and invalid cases for both affected rules, including the double-quote variant and the un-annotated-parameter must-not-match case.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 20.3 AIC · ⌖ 8.88 AIC · ⊞ 5.3K

[github-actions]

Skills-Based Review

Applied /diagnosing-bugs and /tdd — requesting changes on correctness gaps and missing test coverage.

Key Themes and Highlights

Key Themes

• Regex \w+ excludes $ in param names — JSDOC_CORE_PARAM_RE won't match $core or _core-style parameters (already flagged inline at line 9).
• Destructuring path has no test coverage — isDestructuredCoreMethodIdentifier was extended to accept destructuring from JSDoc-annotated params, but neither test file exercises this path (flagged inline).
• Reassignment blind spot undocumented — const c = coreArg; c.setOutput(...) silently escapes all rules because scope lookup only follows Parameter defs, not variable assignments. A negative test and a note in the PR description would set expectations (flagged inline).
• VariableDeclarator JSDoc path untested — const f = function(coreArg) {} with JSDoc on the const is code-path present but has no test case.

Positive Highlights

• Dual-gate strategy (fast path / slow path) is clean and well-documented
• lastIndex = 0 reset before each exec on the shared g-flag regex prevents stateful bugs
• Negative tests (un-annotated coreArg stays invisible) are a strong regression guard
• Both single- and double-quote JSDoc variants are covered

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

[github-actions]

Verdict: Request changes — solid, well-tested fix but with real false-negative gaps and an unexplained unrelated file change.

💡 Themes and highlights

The core logic (two-gate isCoreAliasIdentifier, scope-based isJSDocCoreParamInScope) is sound and well-documented, and the new test coverage for the DI + JSDoc pattern is genuinely good (valid/invalid/negative cases across two rule test suites).

However:

• Missed export function/export async function case — a pattern already used in this codebase (main() entrypoints) — because getCommentsBefore is only checked against the function node and its VariableDeclaration parent, never an ExportNamedDeclaration/ExportDefaultDeclaration wrapper. This silently defeats the annotation for a real, existing usage pattern.
• The \w+ parameter-name regex excludes $ and other legal identifier characters, narrowing the very guarantee this PR is trying to widen (flagged by a prior reviewer as well).
• The destructuring branch (isDestructuredCoreMethodIdentifier) gained JSDoc-alias support but has no direct test coverage exercising it (also flagged previously).
• The bundled .github/workflows/daily-byok-ollama-test.lock.yml diff (fallback pricing + model routing policy) is unrelated to the stated ESLint fix and unexplained in the PR description — this should be reverted from this PR or justified separately (also flagged previously).

The export-wrapped function gap is a new, concrete finding that should be fixed before merge since it silently reintroduces the exact false-negative class this PR sets out to close.

🔎 Code quality review by PR Code Quality Reviewer · auto · 58.9 AIC · ⌖ 9.31 AIC · ⊞ 7.8K
_{Comment /review to run again}

[github-actions]

JSDoc lookup misses a real, used pattern in this codebase: export function/export async function (e.g. main).

💡 Details

hasJSDocCoreParamAnnotation only checks comments before functionNode itself, and, when the parent is a VariableDeclarator, before the enclosing VariableDeclaration. It never checks a preceding ExportNamedDeclaration/ExportDefaultDeclaration wrapper. For:

/**
 * `@param` {typeof import('`@actions/core`')} coreArg
 */
export async function main(coreArg) { coreArg.setFailed("x"); }

sourceCode.getCommentsBefore(functionNode) returns an empty array because the JSDoc precedes the ExportNamedDeclaration, not the FunctionDeclaration node — so the annotation is silently ignored and the rule falls through to false-negative behavior. This pattern is already used elsewhere in this codebase (e.g. export async function main() fixtures in no-core-error-then-process-exitcode.test.ts), so this is a concrete gap, not a hypothetical one. Fix: also check functionNode.parent when it is an ExportNamedDeclaration/ExportDefaultDeclaration.

[Copilot]

Fixed in 60e26fa — hasJSDocCoreParamAnnotation now also pushes functionNode.parent onto nodesToCheck when the parent is an ExportNamedDeclaration or ExportDefaultDeclaration, so JSDoc before export async function main(coreArg) is correctly recognized. ESM valid and invalid test cases were added to both test files.

[github-actions]

The \w+ parameter-name matcher silently rejects valid JS identifiers containing $ or Unicode letters.

💡 Details

JSDOC_CORE_PARAM_RE captures the JSDoc param name with (\w+), which is [A-Za-z0-9_]. Real, legal JS identifiers like $core, core$, or coréArg won't match, so a correctly-annotated function using such a name is silently treated as un-annotated — a false negative with no test coverage or fallback warning. Given the whole point of this PR is closing false-negative gaps for non-canonical parameter names, this narrows the fix's own guarantee. Consider [$\w]+ at minimum, or a broader identifier-safe character class.

[Copilot]

Fixed in 9cfa1fa — JSDOC_CORE_PARAM_RE now captures ([$\w]+) instead of (\w+) so identifiers with $ (e.g. $core) match correctly.

[github-actions]

This generated workflow change (fallback pricing + full model-routing policy) is unrelated to the described ESLint fix and is bundled without explanation.

💡 Details

The PR description is entirely about core-method-resolve.ts JSDoc recognition, yet this lock file diff adds defaultAiCreditsPricing and a large models routing table to the daily BYOK Ollama test's firewall/proxy config. This is almost certainly a stale/regenerated artifact from running make recompile against a newer source template rather than an intentional part of this fix. Bundling unrelated generated changes into a focused PR makes it harder to review, bisect, and roll back independently — if this config change is wrong or has unintended effects on credit limits/model routing, it will be attributed to the wrong PR. Recommend reverting this file's changes here and letting it land in its own separate PR alongside whatever change actually motivates it (or confirming intentionally that this drift is expected).

[Copilot]

This lock file change cannot be reverted without breaking check-workflow-drift. The .md source is unchanged in this PR, but make recompile now generates the extended content due to updates in the compilation tooling. Keeping the lock file in sync with compilation is required by the tooling gate.

done

[pelikhan]

@copilot run pr-finisher skill

[Copilot]

@copilot run pr-finisher skill

All review feedback addressed (commits 9cfa1fa and 60e26fa):

• Regex $ support — JSDOC_CORE_PARAM_RE uses [$\w]+ to capture identifiers like $core.
• Variable alias branch — isCoreAliasIdentifier now calls isJSDocCoreParamInScope for the const c = coreArg pattern.
• Destructuring test coverage — valid/invalid const { setFailed } = coreArg and const { setOutput } = coreArg cases added to both test files.
• export async function wrapper — hasJSDocCoreParamAnnotation now checks the ExportNamedDeclaration/ExportDefaultDeclaration parent so JSDoc before exported functions is recognized. ESM valid/invalid test groups added.
• Lock file — cannot be reverted; make recompile generates the extended content from the updated compilation tooling and check-workflow-drift requires it to stay in sync. The .md source is unchanged in this PR.

Local validation: make agent-report-progress-no-test passes (zero lint errors, all lock files in sync). All 56 affected unit tests pass. CI re-trigger needed before merge.

[gh-aw-bot]

@copilot quick triage for this PR:

• There are no failing checks in the compact candidate set, but the PR still has unresolved review feedback.
• Please address the newest blocking review items first:
  1. Matt Pocock Skills Reviewer — broaden the JSDoc param-name matcher beyond \w+ so legal names like $core remain supported; review summary: https://github.com/github/gh-aw/actions/runs/30689563950
  2. PR Code Quality Reviewer — cover/export-wrap JSDoc on export function / export async function and add direct tests for destructuring and VariableDeclarator JSDoc paths; review summary: https://github.com/github/gh-aw/actions/runs/30689563972
  3. Revert or justify the unrelated .github/workflows/daily-byok-ollama-test.lock.yml change if it is still part of the branch diff.
• After that, run the pr-finisher skill and summarize what changed.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 24.4 AIC · ⌖ 8.51 AIC · ⊞ 8.1K · ◷
_{Comment /souschef to run again}

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.84.2