Skip to content

fix: remove false allowedTools/disallowedTools warning and correct tool permission docs (#212) [v6] - #214

Merged
ben-vargas merged 1 commit into
ai-sdk-v6from
fix/allowed-disallowed-tools-212-v6
Oct 25, 2026
Merged

ben-vargas merged 1 commit into
ai-sdk-v6from
fix/allowed-disallowed-tools-212-v6

Conversation

@ben-vargas

Copy link
Copy Markdown
Owner

Refs #212

What changed

  • Warning removed: validateSettings no longer warns "Both allowedTools and disallowedTools are specified. Only allowedTools will be used." That was never true. The provider forwards both lists unchanged, and the CLI applies both, with matching deny rules taking precedence. Runtime forwarding is unchanged.

  • Tests: src/validation.test.ts pins that both lists are accepted with no warning, whether set as top-level settings, through sdkOptions, or mixed. src/claude-code-language-model.test.ts pins that both lists are forwarded unchanged to the SDK in each of those three configurations.

  • Docs: src/types.ts JSDoc and example, a new short README section, docs/ai-sdk-v5/GUIDE.md and the historical docs/ai-sdk-v4/GUIDE.md (Tool Management sections and tables), and examples/README.md now say:

    • the two lists can be combined, and matching deny rules take precedence;
    • allowedTools only pre-approves tools (no prompt); it is not an allowlist;
    • tools restricts which built-in tools exist (tools: [] = none), and MCP tools are controlled separately.

    The guide tables also no longer say an empty list sends an empty flag; empty lists aren't passed at all. The rewritten Tool Management examples now use defaultSettings. The v5 guide notes that a scoped Bash deny rule matches the command as written.

  • Example: examples/tool-management.ts uses tools: [] for the "no tools" case, and adds a combined case (allowedTools: ['Bash','Read'] + disallowedTools: ['Read']) that shows deny wins.

  • CHANGELOG entry under Unreleased → Fixed. No version bump.

Commands run

npm run lint, npm run typecheck, npm test (1156 tests passed), npm run format:check, npm run build, git diff --check. All passed (lint has 2 pre-existing warnings in claude-code-language-model.ts).

I ran examples/tool-management.ts live from a scratch cwd outside the repo, with CLAUDE_CODE_DISABLE_AUTO_MEMORY=1 set through the provider env setting:

  • With tools: [], Claude made no tool calls and reported having no tools.
  • With the combined lists, Read was unavailable and only Bash was called. The file contents came back.

Independent review (Codex gpt-6-astra, xhigh)

  • Round 1: CHANGES REQUESTED. Three findings, all fixed:
    • the guide examples didn't wrap settings in defaultSettings;
    • a README comment said Bash was pre-approved when only Bash(git log:*) was;
    • comments overstated what the scoped rule Bash(rm:*) protects.
  • Round 2: SIGN-OFF on both lines. No blocking findings and no nits.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T04:49:41.937118Z af3c95e PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Claude finished @ben-vargas's task in 13s —— View job


Review of PR #214

Verdict: looks good. I found no blocking issues. I reviewed the src/, test and example diffs closely. I only skimmed the markdown docs, and I did not run the test suite myself.

Code (src/validation.ts)

  • Removing the false "Only allowedTools will be used" warning is correct. It is a pure deletion with no behavior change, because the provider already forwards both lists.

Tests

  • validation.test.ts covers top-level, sdkOptions and mixed configurations. Asserting warnings equals [] pins the absence of any warning, not just the old one.
  • claude-code-language-model.test.ts verifies forwarding in the same three configurations. The sdkOptions case sets conflicting top-level values (Glob/Write) and checks that the sdkOptions values win. That also pins the override precedence.
  • Nit: settings as any is acceptable in tests under this repo's ESLint rules.

Docs and JSDoc

  • The allowedTools JSDoc now says it pre-approves tools and is not an allowlist. The disallowedTools JSDoc says deny wins. Both match the CLI semantics described in the PR.
  • The types.ts example comment says "Bash stays pre-approved except for calls that match Bash(rm:*)". That is accurate.
  • The wording on tools: [] versus allowedTools: [] is useful. It corrects the old claim that an empty allowedTools blocks every tool.

Example (examples/tool-management.ts)

  • The collect helper and the new combined case 6 demonstrate "deny wins" concretely.
  • The temp directory created with mkdtempSync is never cleaned up. That is harmless for an example, but you could add rmSync in a finally.
  • Case 7 still catches errors with a loose try/catch, consistent with the other cases.

Minor suggestions (optional)

  • Consider a short CHANGELOG note that anyone who relied on the old warning text, for example by matching on it, will no longer see it. It is unlikely to matter.
  • Your docs say an empty list isn't passed at all. A one-line test that allowedTools: [] is not forwarded would guard that claim. I didn't verify whether one already exists.

No security or performance concerns. This change only removes a warning and edits docs, tests and an example.

@ben-vargas
ben-vargas merged commit 0165160 into ai-sdk-v6 Oct 25, 2026
2 checks passed
@ben-vargas
ben-vargas deleted the fix/allowed-disallowed-tools-212-v6 branch October 25, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant