Skip to content

refactor: extract normalizeDisabledToolList shared helper (mode B follow-up to #4319) #4329

@doudouOUC

Description

@doudouOUC

Tracked as follow-up to #4319 (F1 acp-bridge package self-sufficiency) review thread by @wenshao. F1 declined the helper extraction itself because F1's mandate is mechanical lift + targeted fixes; a new shared util sits outside that scope.

Context

The boot path (packages/cli/src/config/config.ts's disabledTools array population around the tools.disabled settings read) and the MCP restart path (packages/cli/src/acp-integration/acpAgent.ts post-restartMcpServer settings refresh) both perform the same 4-step normalization on tools.disabled:

  1. typeof string filter (drop non-string entries)
  2. .trim() (handle hand-edited whitespace)
  3. Skip empties after trim
  4. Dedupe via Set

Currently the two implementations are copy-pasted. Any future enhancement (case-folding, Unicode normalization, plugin-name aliasing, etc.) requires editing both — and the recent F1 fold-in commit b78de2719 already exposed this drift when the restart path needed the same normalization the boot path was already doing.

Proposal

Extract a single helper:

// packages/cli/src/config/normalizeDisabledTools.ts  (or core/src/utils/...)

/**
 * Normalize a raw `tools.disabled` settings array into a Set of
 * tool names. Boot path + MCP restart path call this so both
 * agree byte-for-byte on what counts as "disabled" — without that
 * agreement, `ToolRegistry.has(tool.name)` exact-match check
 * silently re-registers tools whose disabled-name had whitespace.
 *
 * Behavior:
 *   1. Reject non-array input — return empty set.
 *   2. Reject non-string entries — skip individually.
 *   3. Trim each entry.
 *   4. Skip empty-after-trim.
 *   5. Dedupe (preserves first occurrence order if needed; Set
 *      drops it but callers don't depend on order today).
 */
export function normalizeDisabledToolList(raw: unknown): Set<string>;

Replace both inline implementations with a single call. Add unit tests for the helper covering:

  • Non-array input (object, number, undefined, null) → empty Set
  • Mixed-type array → only valid strings retained
  • Whitespace entries (' ', '\n', '\t') → dropped
  • Duplicates (different orderings, whitespace variants like 'Foo' vs ' Foo ') → deduped
  • BOM / zero-width chars in tool names → policy decision

Out of scope (probably)

  • Case-folding tool names (changes external contract)
  • Unicode normalization (String.prototype.normalize) — could land as a separate decision
  • Workspace-wide tools.allowed parallel — different feature

Refs

🤖 Generated with Qwen Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions