iframe-proxy

github-actions · 2026-10-18T03:41:03Z

Adds gh-aw-custom/no-split-filter-trim-without-trim-map (warn).

Pattern: x.split(",").filter(v => v.trim()) drops blank entries but keeps surrounding whitespace on the rest ("a, b" gives " b"). Newline separators are excluded to limit false positives.

Evidence in actions/setup/js (lint output): check_permissions_utils.cjs (GH_AW_REQUIRED_ROLES, GH_AW_ALLOWED_BOTS), redact_secrets.cjs (secret names, 2 sites), extra_empty_commit.cjs. Env-list values with spaces would silently fail matching.

Validation: npm run build, rule unit tests, and npm run lint:setup-js run; the rule reports the sites above. No files outside eslint-factory changed.

Generated by ESLint Miner · copilot · auto · 31.9 AIC · ⌖ 0.655 AIC · ⊞ 9K · ◷

expires on Oct 24, 2026, 7:40 PM UTC-08:00

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-10-18T06:24:36Z

github-actions · 2026-10-18T06:24:37Z

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Design Decision Gate 🏗️

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Design Decision Gate 🏗️ · opus55 · 11.2 AIC · ⌖ 36.2 AIC · ◇ 17.1 AIC · ⊞ 1.9K · ◷

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-10-18T06:24:40Z

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · 5.68 AIC · ⌖ 5.36 AIC · ⊞ 14.6K · ◷

Generated by Ponytail Reviewer for #69831

github-actions · 2026-10-18T06:24:44Z

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-10-18T06:24:45Z

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

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-10-18T06:29:40Z

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)

reviewed_at: 2026-10-18T06:26:06.166+00:00
review_event: REQUEST_CHANGES
top_themes:
  - optional-chain false negative in split matcher
  - overbroad binary-expression matching / false positives
files_reviewed:
  - eslint-factory/eslint.config.cjs
  - eslint-factory/src/index.ts
  - eslint-factory/src/rules/no-split-filter-trim-without-trim-map.test.ts
  - eslint-factory/src/rules/no-split-filter-trim-without-trim-map.ts
comment_count: 2

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 59.5 AIC · ⌖ 5.97 AIC · ⊞ 12.7K · ◷
_{Comment /review to run again}

github-actions

Request changes

This rule has correctness gaps in the matcher itself: it currently misses optional-chained split() receivers and it also over-matches unrelated binary comparisons, so it can both under-report and over-report the pattern it claims to enforce.

Themes

- optional chaining is not unwrapped before the .split(...) check

BinaryExpression matching is too loose for a rule that is supposed to detect blank-entry truthiness filters

Static review only; I reviewed the capped diff and a few adjacent rule implementations, but did not run the rule locally.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 59.5 AIC · ⌖ 5.97 AIC · ⊞ 12.7K
_{Comment /review to run again}

github-actions · 2026-10-18T06:29:42Z

+        const param = predicate.params[0].name;
+
+        let source: TSESTree.Node = callee.object;
+        if (source.type !== "CallExpression" || source.callee.type !== "MemberExpression" || source.callee.computed || source.callee.property.type !== "Identifier" || source.callee.property.name !== "split") return;


This misses optional-chained split() receivers, so one of the rule's advertised target shapes can slip through unflagged and the current test suite gives false confidence.
💡 Why this is a real correctness gap
In @typescript-eslint/Espree, x?.split(",") is wrapped in a ChainExpression, so callee.object is not always a bare CallExpression. This branch returns early before you ever inspect the inner .split(...), which means code like process.env.X?.split(",").filter(p => p.trim()) can be missed even though the new test marks it invalid. Please unwrap ChainExpression before the source.type !== "CallExpression" check (same pattern other rules here already use), then keep traversing the inner call.

Handled in 36be333: unwrap ChainExpression before checking the split call and retain the optional-chain regression case.

github-actions · 2026-10-18T06:29:42Z

+  if (isTrimCallOn(node, paramName)) return true;
+  if (node.type === "UnaryExpression" && node.operator === "!") return isTrimmedTruthiness(node.argument, paramName);
+  if (node.type === "MemberExpression" && !node.computed && node.property.type === "Identifier" && node.property.name === "length") return isTrimCallOn(node.object, paramName);
+  if (node.type === "BinaryExpression") return isTrimmedTruthiness(node.left, paramName) || isTrimmedTruthiness(node.right, paramName);


The BinaryExpression fallback is broad enough to flag legitimate comparisons, which turns this from a focused bug detector into a noisy false-positive generator.
💡 Why the current match is too loose
isTrimmedTruthiness() treats any binary expression with x.trim() on either side as a hit, so predicates like parts.split(",").filter(p => p.trim() !== p) or filter(p => p.trim().length === min) would be reported even though they are not using trim() as a blank-entry truthiness check. That creates merge-blocking lint noise on valid filtering logic. Please narrow the binary handling to the specific truthiness shapes you actually want (x.trim().length > 0, x.trim() !== "", etc.) and add a valid test that preserves an intentional comparison.

Handled in 36be333: matching is limited to nonempty trim truthiness and explicit empty-string/positive-length comparisons; unrelated comparisons now have regression coverage.

github-actions

Skills-Based Review 🧠

Applied /tdd — the new rule itself is well-tested and reasonably scoped (newline exclusion, truthiness-shape matching), but the PR is missing a required README update that an existing repo-level test enforces.

📝 Triage

pr-triage classified this as a new_feature adding a custom ESLint rule with comprehensive unit tests; recommended skills /tdd and /grill-with-docs.

📋 Key Themes & Highlights

Key Themes

Missing README entry (likely CI-breaking): eslint-factory/src/index-readme-parity.test.ts asserts every rule exported from index.ts has a matching ### \rule-name`heading inREADME.md. This PR adds no-split-filter-trim-without-trim-maptoindex.tsbut not toREADME.md`, so that parity test should fail as written.
Minor detection gap: the rule only matches ArrowFunctionExpression predicates, missing function (v) { ... } callbacks with the identical bug. No current site in actions/setup/js uses that form, so it is low severity, but worth a follow-up test case for completeness.

Positive Highlights

✅ Clear, well-justified exclusion of newline separators with an inline comment explaining the rationale.
✅ isTrimmedTruthiness correctly recurses through !x.trim(), .length checks, and binary expressions — good anticipation of real-world variants.
✅ Test suite covers both direct and chained usages (.trim().length > 0, block-statement return), with real evidence cited from actions/setup/js.

Requesting changes primarily to add the README documentation entry so the parity test passes; the predicate-shape gap is a nice-to-have follow-up.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 59.1 AIC · ⌖ 13.9 AIC · ⊞ 10.1K
_{Comment /matt to run again}

github-actions · 2026-10-18T06:33:46Z

    "no-single-char-string-replace": noSingleCharStringReplaceRule,
    "no-unguarded-async-timer-callback": noUnguardedAsyncTimerCallbackRule,
    "no-async-foreach-callback": noAsyncForEachCallbackRule,
+    "no-split-filter-trim-without-trim-map": noSplitFilterTrimWithoutTrimMapRule,


[/tdd] This PR registers a new rule but does not add a matching ### no-split-filter-trim-without-trim-map`` section to eslint-factory/README.md. The repo has an automated parity check (`src/index-readme-parity.test.ts`) that asserts every exported rule name has a matching README heading — this PR as-is would fail that test.

💡 Details

The PR description claims npm run build, rule unit tests, and npm run lint:setup-js were run and passed, but doesn't mention the full vitest suite. index-readme-parity.test.ts does a structural diff between the rules object in index.ts and ### rule`` headings in README.md (sorted-array equality), so a missing entry fails loudly.

Add a row to the rules table and a ### no-split-filter-trim-without-trim-map`` section (following the existing no-map-parseint-callback pattern) describing the pattern and fix.

@copilot please address this.

Documented the rule in the linked Rules table and with a supported-pattern/safe-replacement section in 36be333; the README parity test passes.

github-actions · 2026-10-18T06:33:47Z

+        const callee = node.callee;
+        if (callee.type !== "MemberExpression" || callee.computed || callee.property.type !== "Identifier" || callee.property.name !== "filter") return;
+        const predicate = node.arguments[0];
+        if (!predicate || predicate.type !== "ArrowFunctionExpression" || predicate.params.length !== 1 || predicate.params[0].type !== "Identifier") return;


[/tdd] The predicate-shape check only accepts ArrowFunctionExpression; a .filter(function (v) { return v.trim(); }) callback with the same bug is silently skipped. No current site in actions/setup/js uses this form, so it's low-severity, but the rule's own message ("Filtering split() output...") implies general coverage.

💡 Suggested fix

if (!predicate || (predicate.type !== "ArrowFunctionExpression" && predicate.type !== "FunctionExpression") || predicate.params.length !== 1 || predicate.params[0].type !== "Identifier") return;

Add a corresponding invalid case to the test file, mirroring the existing no-async-foreach-callback rule which already handles both function shapes.

@copilot please address this.

Handled in 36be333: both arrow and function-expression filter callbacks are supported, with a function-expression regression case.

Copilot

🟡 Changes recommended

False positives can prompt behavior-changing rewrites, and missing README documentation fails the existing parity check.

3 open issues

What changed in this PR

Adds a warning-level rule to gh-aw’s custom ESLint plugin for split/filter chains that test trimmed values but retain surrounding whitespace.

Changes:

Detects trim-based predicates, excluding newline separators.
Adds rule tests and documentation-URL validation.
Registers and enables the rule as a warning.

File	Description
`eslint-factory/src/rules/no-split-filter-trim-without-trim-map.ts`	Implements pattern matching and diagnostics.
`eslint-factory/src/rules/no-split-filter-trim-without-trim-map.test.ts`	Tests accepted and reported patterns and the documentation URL.
`eslint-factory/src/index.ts`	Registers the new rule.
`eslint-factory/eslint.config.cjs`	Enables warning-level diagnostics.

_{Review effort: Max}

Copilot · 2026-10-18T07:25:23Z

+  if (node.type === "UnaryExpression" && node.operator === "!") return isTrimmedTruthiness(node.argument, paramName);
+  if (node.type === "MemberExpression" && !node.computed && node.property.type === "Identifier" && node.property.name === "length") return isTrimCallOn(node.object, paramName);
+  if (node.type === "BinaryExpression") return isTrimmedTruthiness(node.left, paramName) || isTrimmedTruthiness(node.right, paramName);


Handled in 36be333: the matcher now restricts negation to even counts and comparisons to explicit nonempty checks. The cited counterexamples are valid regression cases.

Copilot · 2026-10-18T07:25:22Z

+        const expression = body.type === "BlockStatement" ? (body.body.length === 1 && body.body[0].type === "ReturnStatement" ? body.body[0].argument : null) : body;
+        if (!expression || !isTrimmedTruthiness(expression, param)) return;
+
+        context.report({ node, messageId: "filterTrimKeepsWhitespace", data: { param } });


Handled in 36be333: the rule skips a directly following synchronous trim map, covered by expression-body and single-return-block mapper tests.

Copilot · 2026-10-18T07:25:22Z

    "no-single-char-string-replace": noSingleCharStringReplaceRule,
    "no-unguarded-async-timer-callback": noUnguardedAsyncTimerCallbackRule,
    "no-async-foreach-callback": noAsyncForEachCallbackRule,
+    "no-split-filter-trim-without-trim-map": noSplitFilterTrimWithoutTrimMapRule,


Added the linked Rules-table row and backtick-quoted heading with the supported pattern and safe replacement in 36be333; README parity tests pass.

gh-aw-bot · 2026-10-18T07:03:51Z

@copilot address the following outstanding work in one pass:

Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
Review (eslint-factory/src/rules/no-split-filter-trim-without-trim-map.ts:23): Recursing through every ! and binary expression also reports !v.trim(), v.trim().length === 0, v.trim() === "admin", and v === v.trim(). These select blanks, a specific value, or already-unpadded strings rather than merely removing blank entries. The recommended .map(v => v.trim()).filter(Boolean) changes which entries survive. Restrict matching to even negations and explicit nonempty-string or length comparisons, and add valid regression cases for these counterexamples. - [eslint-miner] eslint-factory: add no-split-filter-trim-without-trim-map rule #69831 (comment)
Review (eslint-factory/src/rules/no-split-filter-trim-without-trim-map.ts:60): The visitor only checks the filter's input, so s.split(",").filter(v => v.trim()).map(v => v.trim()) still receives this warning. That chain already produces trimmed strings and is equivalent to the recommended rewrite. Skip reporting when a synchronous trim-map follows directly, and add valid tests for both expression-body and single-return-block mappers. - [eslint-miner] eslint-factory: add no-split-filter-trim-without-trim-map rule #69831 (comment)
Review (eslint-factory/src/index.ts:145): Registering this rule without documenting it makes src/index-readme-parity.test.ts:23–24 fail: the plugin now exports 69 rules, but eslint-factory/README.md has only 68 rule headings. It also leaves the new rule's documentation URL without a target anchor. Add a linked Rules-table row and a ### heading with the backtick-quoted rule name, explaining the supported pattern and safe replacement. - [eslint-miner] eslint-factory: add no-split-filter-trim-without-trim-map rule #69831 (comment)

Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI.

Sous-chef head: 7d1da05
Sous-chef work: 759601d2b14ceab1ae5235acaaa74e24e88e7ad2573a50928f44e346a07b4abb 8b36383459a96e2540a427c87c6e9b4a00bd5a81021040ec41a88df840e76bb3 989f69eebc3b98881bfe9e2d0618fb1e13d4b4f917b953f49de4bca60eed55a2
Sous-chef state: 44db7526ebc404d0f210816897fbe6f2fa1f651f5e96f4a9f717c8c432e44fba

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · 3.69 AIC · ⌖ 7.61 AIC · ⊞ 1.1K · ◷
_{Comment /souschef to run again}

…lter-trim-20261018-6b26026061e0b8ab Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

…map rule (#69831) Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

eslint-factory: add no-split-filter-trim-without-trim-map rule

7d1da05

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

github-actions Bot mentioned this pull request Oct 18, 2026

[Contribution Check Report] Contribution Check — 2026-10-18 #69811

Closed

pelikhan mentioned this pull request Oct 18, 2026

fix: include Work and Claim references in safe-output footers #69865

Merged

pelikhan marked this pull request as ready for review October 18, 2026 06:23

Copilot AI max review requested due to automatic review settings October 18, 2026 06:23

Copilot started reviewing on behalf of pelikhan October 18, 2026 06:24

View session

This was referenced Oct 18, 2026

fix(workflows): report PR content integrity blockers accurately #69866

Merged

Integrity limitation: PR diff/file content is always labeled none, blocking approved-integrity review workflows github/gh-aw-mcpg#15160

Closed

github-actions Bot commented Oct 18, 2026

View reviewed changes

Copilot AI reviewed Oct 18, 2026

View reviewed changes

github-actions Bot mentioned this pull request Oct 18, 2026

[pr-sous-chef] 🍳 PR Sous Chef Run Summary #69815

Closed

Copilot started work on behalf of gh-aw-bot October 18, 2026 07:04

View session

Copilot AI and others added 2 commits October 18, 2026 07:10

Merge remote-tracking branch 'origin/main' into eslint-miner-split-fi…

6887c92

…lter-trim-20261018-6b26026061e0b8ab Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Fix split filter trim rule review feedback

36be333

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot finished work on behalf of gh-aw-bot October 18, 2026 07:26

Copilot AI requested a review from gh-aw-bot October 18, 2026 07:26

github-actions Bot mentioned this pull request Oct 18, 2026

[pr-sous-chef] PR Sous Chef Report: PR #69813 nudged for implementation #69909

Closed

pelikhan merged commit 0af0e45 into main Oct 18, 2026
2 checks passed

Copilot AI pushed a commit that referenced this pull request Oct 18, 2026

[eslint-miner] eslint-factory: add no-split-filter-trim-without-trim-…

9b8b644

…map rule (#69831) Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

pelikhan mentioned this pull request Oct 18, 2026

Fix work queue delivery transport continuity #70016

Merged

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

github-actions Bot commented Oct 18, 2026

Uh oh!

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

Uh oh!

Test Quality Sentinel

Uh oh!

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

Uh oh!

Design Decision Gate 🏗️

Uh oh!

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

Uh oh!

Ponytail Reviewer

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

github-actions Bot commented Oct 18, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Request changes

Uh oh!

github-actions Bot Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 18, 2026

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 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

gh-aw-bot commented Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

github-actions Bot commented Oct 18, 2026 •

edited

Loading

github-actions Bot commented Oct 18, 2026 •

edited

Loading

github-actions Bot commented Oct 18, 2026 •

edited

Loading

github-actions Bot commented Oct 18, 2026 •

edited

Loading

github-actions Bot commented Oct 18, 2026 •

edited

Loading