Hacktoberfest 2026：メンテナが10月に向けて印を付けた、オープンで初心者向けの issue。 Hacktoberfest の issue を見る

util/config: assertOptionalStringArray '// istanbul ignore else' hides untested 'array contains non-strings' branch

オープン 初心者向け
  • 何か問題がありますか？
#68 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
2/5
見積もり時間
1〜3時間
初心者へのやさしさ
72/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
静か
技術スタック
typescript
領域
testing-qa

調査の方向性

src/prompt-generators/util/config.ts の assertOptionalStringArray から始め、次に load-cli-config.test.ts の関連するケースを確認します。対象を絞ったテストとカバレッジを実行し、文字列ではない項目を含む配列のケースを追加して、正当な理由のない ignore ディレクティブを使わずに、バリデーターとカバレッジレポートでその分岐が実行されることを確認します。

索引モデルが issue の本文から書いたものです。

説明

bug S4

Observed behavior

assertOptionalStringArray in src/prompt-generators/util/config.ts (lines 76-90) has an // istanbul ignore else directive immediately above its validation check:

export function assertOptionalStringArray(
  value: Record<string, unknown>,
  key: string,
  field: string,
): void {
  if (!(key in value)) {
    return;
  }

  const array = value[key];
  // istanbul ignore else
  if (!Array.isArray(array) || array.some(item => typeof item !== 'string')) {
    throw new Error(`${field} must be an array of strings`);
  }
}

git blame traces the directive to commit 8f4d7d6 ("Push coverage to 100% using ignores"). The condition is a two-clause OR:

  1. !Array.isArray(array) - covered by tests that pass a plain string for an array field (e.g. load-cli-config.test.ts:503, :720 pass components: 'DOM: Workers' and labels: 'bug').
  2. array.some(item => typeof item !== 'string') - I cannot find a single test in the repo that passes an array containing a non-string item to a field validated by assertOptionalStringArray. Searching for tests using bugStatus, keywords, components, bugFields, bugSeverity, labels, and excludePatterns only turns up valid string arrays and "not even an array" failure cases.

The // istanbul ignore else is on the if statement, so strictly it suppresses the implicit "else" (no-throw) branch, not the OR's branch coverage. But it sits exactly where a branch is uncovered. Combined with the commit message, this strongly suggests the directive is silencing a real coverage gap: the array.some(...) arm of the OR has no test.

The runtime consequence is small but real: a regression that broke the per-item string check (for example, changing typeof item !== 'string' to typeof item === 'string', or accidentally removing the .some call) would not be caught by any existing test, and coverage would still report 100%.

Expected behavior

Either:

  • Remove the // istanbul ignore else directive and add a test that calls assertOptionalStringArray (or one of its callers, like normalizeBugzillaTaskConfig with bugStatus: ['OPEN', 42]) with an array containing a non-string item, so both arms of the OR are covered by an actual assertion, or
  • If the success path is the one that needs the suppression (because v8 is flagging it), explicitly justify that in a comment so future readers do not assume the directive is masking a different gap.

Minimal reproduction

import { assertOptionalStringArray } from './prompt-generators/util/config.js';

// Tests in the repo cover:
assertOptionalStringArray({ foo: 'bar' }, 'foo', 'test.foo'); // throws (first OR arm)
assertOptionalStringArray({ foo: ['a', 'b'] }, 'foo', 'test.foo'); // does not throw (else of if)

// Not covered anywhere:
assertOptionalStringArray({ foo: ['a', 42] }, 'foo', 'test.foo'); // throws (second OR arm)

Suggested fix

Remove the // istanbul ignore else line and add a unit test (or extend load-cli-config.test.ts) that exercises an array with at least one non-string element, e.g.:

it('should reject Bugzilla bugStatus arrays containing non-strings', async () => {
  await expect(
    normalizeCliConfig(
      {
        name: 'test',
        agent: 'claude-sdk',
        promptGenerator: [
          'bugzilla',
          {
            search: { bugStatus: ['OPEN', 42] },
            promptTemplate: 'Review {{id}}',
          } as unknown as BugzillaTask,
        ],
      },
      join(configDir, 'config.json'),
    ),
  ).rejects.toThrow('bugzilla.search.bugStatus must be an array of strings');
});

Severity

S4 - cleanup with low behavioral risk. The validator works correctly today; the issue is that a future regression in the per-item type check would slip past both tests and the 100% coverage gate.

主要言語
TypeScript
スター
2
フォーク
1
30日以内にマージされた PR はありません

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

joewalker/loop-the-loop のほかの issue

joewalker/loop-the-loop の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。