no-string-fallback-for-non-string-message: false positive on type-narrowed catch-param alias (safeoutputs_cli.cjs:48)
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 773
Description
### Summary
`no-string-fallback-for-non-string-message` flags a live, correct call site as if it were the "stringifies the wrong container" bug it targets, because its structural name-comparison doesn't recognize that the alternate's identifier is a locally-declared, type-narrowed alias of the tested chain's root object — not a genuinely different value.
### Live occurrence
`actions/setup/js/safeoutputs_cli.cjs:45-48` (`runSafeOutputsCLI`'s catch block):
```js
} catch (error) {
const err = /** @type {{message?: string, stderr?: string | Buffer}} */ error ?? {};
const stderr = typeof err.stderr === "string" ? err.stderr.trim() : Buffer.isBuffer(err.stderr) ? err.stderr.toString("utf8").trim() : "";
const message = typeof err.message === "string" ? err.message : String(error);
...
```
The rule's `getTypeofMessageCheckChainKey`/`memberChainKey` machinery resolves the tested chain to `"err.message"` and the alternate's argument to `"error"`. Since `"error" !== "err.message"` and `"error" !== "err"` (the container-guard exemption at lines 99-104 of `no-string-fallback-for-non-string-message.ts` also doesn't match, since there's no `typeof error === "object"` guard in the test), this trips `stringifiesContainerInsteadOfMessage`.
But `err` is declared as `const err = /** @type {...} */ error ?? {}` — a type-narrowing alias of the *same* catch-clause binding, not an unrelated container:
- When `error` is truthy (the overwhelmingly common case for a `childProcess.execFileSync` throw), `err === error` by reference, so `String(error)` and `String(err)` produce **identical** output — there is no information loss.
- When `error` is falsy (`null`/`undefined`), `err` becomes `{}` while `error` keeps the original falsy value. `String(error)` then yields `"null"`/`"undefined"` (informative), whereas `String(err)` would yield `"[object Object]"` (the exact failure mode this rule exists to prevent) — i.e. in the one case where the two diverge, the code as written is *more* correct than the rule's suggested fix.
So this is a false positive: the pattern `const alias = /** @type {...} */ original ?? fallback` followed by `typeof alias.message === "string" ? alias.message : String(original)` is not the "stringifies a different container" bug — it's an intentional, more-robust choice — but the rule can't distinguish it from a genuine mismatched-container bug because it only compares identifier/member-chain text, not aliasing relationships.
Confirmed no `eslint-disable` suppression exists near this line, and the rule's `.test.ts` has no case covering this alias shape — this is a live, currently-unaddressed defect in the rule as shipped.
### Suggested fix
When the alternate's chain key doesn't match the tested container, check whether the alternate identifier's declarator initializes it as ` ?? ` (or similar narrowing pattern) — if so, treat it as referring to the same underlying value and skip the report. Alternatively (simpler and sufficient to close this gap), special-case: if the alternate is a bare identifier `X` whose single declarator is `const X = Y ?? ` where `Y` is the tested chain's root object, exempt it.
### Acceptance criteria
- [ ] `const err = error ?? {}; ... typeof err.message === "string" ? err.message : String(error)` (and the `ternary-chain`/optional-chaining variants) no longer trigger `stringifiesContainerInsteadOfMessage`.
- [ ] Existing invalid-case tests (genuinely different, unrelated containers) still report.
- [ ] A regression test is added mirroring the exact `safeoutputs_cli.cjs:45-48` shape.
**References:**
- `eslint-factory/src/rules/no-string-fallback-for-non-string-message.ts`
- `actions/setup/js/safeoutputs_cli.cjs:45-48`
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
> Generated by [🤖 ESLint Refiner](https://github.com/github/gh-aw/actions/runs/34809465962) · claude · agent · 357.2 AIC · ⌖ 7.35 AIC · ⊞ 5.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Feslint-refiner%22&type=issues)
> - [x] expires on Sep 20, 2026, 9:37 PM UTC-08:00
Contributor guide
Research direction
Start with eslint-factory/src/rules/no-string-fallback-for-non-string-message.ts, especially getTypeofMessageCheckChainKey and memberChainKey, then inspect the rule's .test.ts cases. Reproduce the reported shape from actions/setup/js/safeoutputs_cli.cjs:45-48 and compare it with an unrelated-container case. Done means the alias cases, including the listed chain variants, are accepted while invalid cases still report and a regression test covers the live shape.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, typescript
- Domain
- testing-qa, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100