[eslint-miner] feat(eslint): add no-core-exportvariable-non-string rule by github-actions[bot] · Pull Request #45075 · github/gh-aw

github-actions · 2026-07-12T09:33:04Z

Summary

Adds a new ESLint rule no-core-exportvariable-non-string to the gh-aw-custom ESLint plugin. The rule warns when core.exportVariable(name, value) is called with a non-string value argument, preventing silent implicit coercions (e.g., null stringified as "null", true as "true") that can cause unexpected behaviour in downstream GitHub Actions steps that consume the exported environment variable.

Motivation

@actions/core's exportVariable writes a value to $GITHUB_ENV. When callers pass numeric literals, booleans, null, undefined, or .length expressions, JavaScript silently coerces the value to a string. The coercion result may not match the caller's intent and can be misread by downstream steps. Requiring an explicit String(...) call or template literal makes the coercion visible and intentional.

Changes

`eslint-factory/src/rules/no-core-exportvariable-non-string.ts` (new, 122 lines)

Implements the rule using @typescript-eslint/utils (ESLintUtils.RuleCreator).
Targets five low-false-positive value forms: numeric literals, boolean literals, null, undefined, and .length member accesses.
Rule type: "problem"; auto-fix suggestions via hasSuggestions: true.
- null/undefined: two suggestions — replace with "" or wrap with String(...).
- All other flagged forms: one suggestion — wrap with String(...).
Matches both core.exportVariable(...) and core["exportVariable"](...) call forms; ignores callee objects other than exactly core.

`eslint-factory/src/rules/no-core-exportvariable-non-string.test.ts` (new, 172 lines)

Vitest + ESLint RuleTester suite.
Valid cases: string literals, template literals, String(...), .toString(), arbitrary variable references, non-core receiver objects.
Invalid cases: numeric literals (0, 42), boolean literals (true, false), null, undefined, .length member access, computed core["exportVariable"] with a numeric argument.
Asserts correct messageId values and exact suggestion fix outputs for each invalid case.

`eslint-factory/src/index.ts`

Imports and registers noCoreExportVariableNonStringRule under key "no-core-exportvariable-non-string" in the plugin rules map.

`eslint-factory/eslint.config.cjs`

Enables "gh-aw-custom/no-core-exportvariable-non-string" at severity "warn" in the default plugin config.

Design Decisions

Detection is limited to syntactically unambiguous non-string forms to keep false-positive rate low without requiring type information.
Computed .length via bracket notation (expr["length"]) is excluded to further reduce false positives.
Rule is registered as "warn" rather than "error" to allow gradual adoption across the codebase.

Generated by PR Description Updater for #45075 · 41.3 AIC · ⌖ 5.63 AIC · ⊞ 4.7K · ◷

Add a new ESLint rule that flags non-string values passed to core.exportVariable() — numbers, booleans, null, undefined, or .length — that silently produce unexpected strings (e.g. 'null', 'true') when the exported env var is read by downstream steps. This mirrors the existing no-core-setoutput-non-string rule and closes a preventive gap: all current exportVariable calls in actions/setup/js already use strings, but the rule guards against future regressions and matches the codebase convention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-13T06:31:29Z

✅ PR Code Quality Reviewer completed the code quality review.

github-actions · 2026-07-13T06:31:36Z

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

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

github-actions · 2026-07-13T06:31:42Z

✅ Test Quality Sentinel completed test quality analysis.

github-actions · 2026-07-13T06:31:50Z

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

Copilot

Pull request overview

Adds an ESLint rule requiring explicit string values for core.exportVariable() calls.

Changes:

Implements and registers the new rule.
Adds valid, invalid, and suggestion tests.
Enables the rule as a warning for setup JavaScript.

Show a summary per file

File	Description
`eslint-factory/src/rules/no-core-exportvariable-non-string.ts`	Implements detection and suggestions.
`eslint-factory/src/rules/no-core-exportvariable-non-string.test.ts`	Tests rule behavior.
`eslint-factory/src/index.ts`	Registers the rule.
`eslint-factory/eslint.config.cjs`	Enables the rule.

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: 5
Review effort level: Medium

+    schema: [],
+    messages: {
+      nonStringValue:
+        "The exportVariable value {{valueText}} is a {{kind}}. Implicit coercion may produce unexpected strings such as 'null' or 'true' when the environment variable is read by downstream steps. Use an explicit string conversion and choose the suggestion that matches the intended output semantics.",


+              messageId: "wrapWithString" as const,
+              data: { valueText },
+              fix(fixer: TSESLint.RuleFixer) {
+                return fixer.replaceText(valueArg, `String(${valueText})`);
+              },


+  if (node.type === AST_NODE_TYPES.Literal) {
+    if (typeof node.value === "number") return NUMERIC_LITERAL_KIND;
+    if (typeof node.value === "boolean") return BOOLEAN_LITERAL_KIND;
+    if (node.value === null) return NULL_KIND;


+  if (node.type === AST_NODE_TYPES.Identifier && node.name === "undefined") {
+    return UNDEFINED_KIND;


+    hasSuggestions: true,
+    docs: {
+      description:
+        "Require core.exportVariable value arguments to be explicit strings; passing numbers, booleans, null, undefined, or .length can silently produce unexpected string representations (e.g. 'null', 'true') in downstream GitHub Actions steps that read the exported environment variable. Detects only calls in the form core.exportVariable(name, value).",


github-actions

Review: `no-core-exportvariable-non-string`

Clean, correct implementation mirroring the existing no-core-setoutput-non-string rule. Logic, tests (10/10), and registration all look good.

One non-blocking suggestion (see inline comment): nonStringKind and the five kind constants are duplicated verbatim from the sibling rule file. Extracting them to a shared rule-utils.ts would eliminate the duplication and prevent drift when new targeted forms are added in future.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 19.2 AIC · ⌖ 4.37 AIC · ⊞ 4.8K

github-actions · 2026-07-13T06:36:50Z

+ *   - Identifier `undefined`
+ *   - .length member access: commonly numeric in practice
+ */
+const NUMERIC_LITERAL_KIND = "numeric literal" as const;


The nonStringKind helper function and the five kind-constant declarations (NUMERIC_LITERAL_KIND, BOOLEAN_LITERAL_KIND, NULL_KIND, UNDEFINED_KIND, LENGTH_KIND) are duplicated verbatim between no-core-exportvariable-non-string.ts and no-core-setoutput-non-string.ts. Consider extracting them to a shared rule-utils.ts module so future changes (e.g., adding a new targeted form) only need to be made once.

github-actions

Solid, well-scoped addition. One non-blocking observation on the argument-count guard (commented inline).

### Review themes

No blocking issues found. The rule is a faithful mirror of the existing no-core-setoutput-non-string sibling with appropriate targeted patterns, clear messages, actionable suggestions, and thorough test coverage (valid + invalid + both suggestion variants for null/undefined).

One low-priority note (commented inline):

The arguments.length !== 2 early-return silently skips spread-argument calls — not a real risk given the fixed API signature, but deserves a one-line comment for future maintainers.

Hardcoded core object name: consistent with the sibling rule's design; aliased imports will not be caught, but this is a documented limitation of syntax-only rules without type information and is not a regression.

🔎 Code quality review by PR Code Quality Reviewer · 26.3 AIC · ⌖ 4.43 AIC · ⊞ 5.4K
_{Comment /review to run again}

github-actions · 2026-07-13T06:37:50Z

+        context.report({
+          node,
+          messageId: "nonStringValue",
+          data: { kind, valueText },


Silent skip on spread/rest arguments could hide violations: node.arguments.length !== 2 silently ignores core.exportVariable(...args) with no diagnostic.

💡 Suggested fix

Low practical risk since the API is always (name, val), but the intent should be documented:

// Spread-argument calls (e.g. core.exportVariable(...args)) are intentionally excluded — // the rule targets only literal call sites where the value type is statically known. if (node.arguments.length !== 2) return;

This mirrors the same guard in the sibling no-core-setoutput-non-string rule, so consistency is preserved.

github-actions · 2026-07-13T06:38:01Z

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 86/100 — Excellent

Analyzed 9 test(s): 8 design, 1 implementation, 0 violation(s).

📊 Metrics (9 tests)

Metric	Value
Analyzed	9 (Go: 0, JS/TS: 9)
✅ Design	8 (89%)
⚠️ Implementation	1 (11%)
Edge/error coverage	6 (67%)
Duplicate clusters	0
Inflation	No (172/122 = 1.41:1)
🚨 Violations	0

Test	File	Classification	Issues
uses the correct docs URL	no-core-exportvariable-non-string.test.ts:13	implementation_test	Checks internal metadata (docs URL); low behavioral value
valid: string literal values are accepted	:17	design_test	✅ Behavioral contract
valid: non-core.exportVariable calls not flagged	:32	design_test	✅ Behavioral contract
valid: computed string-literal exportVariable	:39	design_test	✅ Behavioral contract
invalid: numeric literal value is flagged	:46	design_test	✅ Error + suggestion verified
invalid: boolean literal value is flagged	:72	design_test	✅ Error + suggestion verified
invalid: null value is flagged	:98	design_test	✅ Edge case (null/undefined path)
invalid: undefined identifier is flagged	:118	design_test	✅ Edge case (null/undefined path)
invalid: .length member access is flagged	:138	design_test	✅ Edge case
invalid: computed exportVariable with numeric	:155	design_test	✅ Behavioral contract

⚠️ Flagged Tests (1)

uses the correct docs URL (no-core-exportvariable-non-string.test.ts:13) — implementation_test. Verifies a hardcoded URL string in rule metadata rather than observable lint behavior. If deleted, no real behavioral regression would escape. Consider removing or combining with a more meaningful assertion if the URL format ever changes.

Verdict

✅ Passed. 11% implementation tests (threshold: 30%). The test suite uses ESLint's RuleTester to enforce behavioral contracts across valid/invalid cases, including edge cases for null, undefined, .length, and computed property access. Suggestion outputs are also verified.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🧪 Test quality analysis by Test Quality Sentinel · 26.4 AIC · ⌖ 14.6 AIC · ⊞ 6.8K · ◷
_{Comment /review to run again}

github-actions

✅ Test Quality Sentinel: 86/100. 11% implementation tests (threshold: 30%).

github-actions

Skills-Based Review 🧠

Applied /codebase-design and /tdd — requesting changes on two issues.

📋 Key Themes & Highlights

Issues

Code duplication: nonStringKind and all its constants are a verbatim copy from no-core-setoutput-non-string.ts. A shared utility module removes a future maintenance hazard.
Arity guard too strict: arguments.length !== 2 silently misses core.exportVariable('X', 42, extra). Change to < 2 and this edge case is also covered by a new test.

Positive Highlights

✅ Excellent test coverage for the targeted forms (numeric, boolean, null, undefined, .length)
✅ Two-suggestion design for null/undefined (empty string vs. String()) is thoughtful UX
✅ Clear and rationale-driven PR description with coercion examples
✅ Zero baseline warnings confirms the rule is not noisy
✅ Rule structure mirrors the existing pattern cleanly

Minor (non-blocking)

The sibling rule no-core-setoutput-non-string is documented in eslint-factory/README.md at line 83. This new rule should get a matching entry.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 34.1 AIC · ⌖ 4.7 AIC · ⊞ 6.6K
_{Comment /matt to run again}

github-actions · 2026-07-13T06:38:49Z

+
+function nonStringKind(node: TSESTree.Node): NonStringKind | null {
+  if (node.type === AST_NODE_TYPES.Literal) {
+    if (typeof node.value === "number") return NUMERIC_LITERAL_KIND;


[/codebase-design] nonStringKind and its constants are copied verbatim from no-core-setoutput-non-string.ts — any future bug fix to detection logic must be applied in two places.

💡 Suggested fix

Extract the shared detection logic to a utility module:

// eslint-factory/src/rules/shared/non-string-kind.ts export const NUMERIC_LITERAL_KIND = 'numeric literal' as const; // ... other constants ... export type NonStringKind = ...; export function nonStringKind(node: TSESTree.Node): NonStringKind | null { ... }

Both rules then import from that module. This follows the deep-module pattern: one seam for detection logic, two thin wrappers for rule-specific reporting.

@copilot please address this.

github-actions · 2026-07-13T06:38:50Z

+        if (callee.type !== AST_NODE_TYPES.MemberExpression) return;
+
+        // Object must be `core` (the @actions/core import convention in actions/setup/js)
+        if (callee.object.type !== AST_NODE_TYPES.Identifier || callee.object.name !== "core") return;


[/codebase-design] The arity guard node.arguments.length !== 2 silently skips a call like core.exportVariable('X', 42, extra) — a non-string value goes undetected if extra arguments are present.

💡 Suggested fix

Guard on minimum arity (< 2) and look at only the second argument regardless of extra arguments:

if (node.arguments.length < 2) return;

This matches how the real @actions/core API works and makes the rule robust to minor call-site variations.

@copilot please address this.

github-actions · 2026-07-13T06:38:50Z

+            },
+          ],
+        },
+      ],


[/tdd] No test for the arity guard edge case: core.exportVariable('X', 42, extra) with three arguments. If the guard is ever loosened (or changed to < 2), there's no regression test to verify detection still works with extra args.

💡 Suggested test

it('invalid: numeric literal with extra arguments is still flagged', () => { cjsRuleTester.run('no-core-exportvariable-non-string', noCoreExportVariableNonStringRule, { valid: [], invalid: [ { code: `core.exportVariable("MY_COUNT", 42, extra);`, errors: [{ messageId: 'nonStringValue' }], }, ], }); });

@copilot please address this.

github-actions · 2026-07-13T16:48:25Z

🎉 This pull request is included in a new release.

Release: v0.82.9

github-actions Bot added automation cookie Issue Monster Loves Cookies! eslint labels Jul 12, 2026

github-actions Bot mentioned this pull request Jul 12, 2026

[Contribution Check Report] Contribution Check — 2026-07-12 #45028

Closed

pelikhan marked this pull request as ready for review July 13, 2026 06:30

Copilot AI review requested due to automatic review settings July 13, 2026 06:30

Copilot started reviewing on behalf of pelikhan July 13, 2026 06:31 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

github-actions Bot commented Jul 13, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jul 13, 2026

[aw] No-Op Runs #43499

Open

github-actions Bot commented Jul 13, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jul 13, 2026

[pr-sous-chef] Run report — 4 nudged, 0 skipped #45140

Closed

pelikhan merged commit 2364b94 into main Jul 13, 2026
46 checks passed

pelikhan deleted the eslint-miner/no-core-exportvariable-non-string-dc088cb9cbd884e7 branch July 13, 2026 14:32

		if (node.type === AST_NODE_TYPES.Identifier && node.name === "undefined") {
		return UNDEFINED_KIND;

Uh oh!

Conversation

github-actions Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

eslint-factory/src/rules/no-core-exportvariable-non-string.ts (new, 122 lines)

eslint-factory/src/rules/no-core-exportvariable-non-string.test.ts (new, 172 lines)

eslint-factory/src/index.ts

eslint-factory/eslint.config.cjs

Design Decisions

Uh oh!

github-actions Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review: no-core-exportvariable-non-string

Uh oh!

github-actions Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 13, 2026

🧪 Test Quality Sentinel Report

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Issues

Positive Highlights

Minor (non-blocking)

Uh oh!

github-actions Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jul 12, 2026 •

edited

Loading

`eslint-factory/src/rules/no-core-exportvariable-non-string.ts` (new, 122 lines)

`eslint-factory/src/rules/no-core-exportvariable-non-string.test.ts` (new, 172 lines)

`eslint-factory/src/index.ts`

`eslint-factory/eslint.config.cjs`

github-actions Bot commented Jul 13, 2026 •

edited

Loading

github-actions Bot commented Jul 13, 2026 •

edited

Loading

github-actions Bot commented Jul 13, 2026 •

edited

Loading

github-actions Bot commented Jul 13, 2026 •

edited

Loading

Review: `no-core-exportvariable-non-string`