feat(core): add ConfigTool for programmatic config read/write#2911
Draft
feat(core): add ConfigTool for programmatic config read/write#2911
Conversation
Allow the Agent to read and write configuration settings via a new ConfigTool, eliminating the need for users to manually run /model or /settings commands during multi-phase tasks. Phase 1 supports the "model" setting only. GET is auto-allowed (read-only), SET requires user confirmation. A curated allowlist in supported-config-settings.ts ensures only approved settings are accessible. - Add ConfigSettingDescriptor allowlist with "model" descriptor - Add ConfigTool + ConfigToolInvocation (GET/SET with permission control) - Register CONFIG in tool-names.ts and config.ts - 12 unit tests covering validation, GET/SET, permissions, errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📋 Review SummaryThis PR introduces a 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
- Wrap descriptor.read() in try-catch in getConfirmationDetails - Sanitize non-Error exceptions in write function - Add debug logging for getAvailableModels failure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Thanks for the review. Addressed the actionable items in d30de21:
Other notes:
|
…ull assertion - Use Object.hasOwn() instead of `in` operator in isSupported() to prevent prototype chain keys (toString, __proto__) from bypassing the allowlist - Reject empty/whitespace-only values in SET validation - Replace value! non-null assertion with defensive null check in execute() - Add 4 new tests: empty string, whitespace, toString, __proto__ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new core ConfigTool that lets agents programmatically read/write selected configuration settings (Phase 1: model) with a whitelist-based safety boundary and confirmation for writes.
Changes:
- Add
configas a registered core tool (ToolNames.CONFIG/ToolDisplayNames.CONFIG) and wire it into core tool registration. - Implement
ConfigTool+ an allowlisted settings registry (SUPPORTED_CONFIG_SETTINGS) with GET (auto-allow) and SET (ask). - Add unit tests covering validation, GET/SET behavior, permissions, confirmation details, and error handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/tools/tool-names.ts | Adds the new config tool name and display name constants. |
| packages/core/src/tools/supported-config-settings.ts | Introduces a whitelist/descriptor registry for settings accessible via ConfigTool (Phase 1: model). |
| packages/core/src/tools/config-tool.ts | Implements ConfigTool and its invocation logic (GET/SET, confirmation details, model options listing). |
| packages/core/src/tools/config-tool.test.ts | Adds vitest coverage for validation, permissions, execution results, and confirmation prompts. |
| packages/core/src/config/config.ts | Registers ConfigTool in the core tool registry during config initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The Config(action:setting) rule format would not be matched by PermissionManager since it has no alias/specifier mapping registered. Remove permissionRules and set hideAlwaysAllow=true so config changes always require per-invocation confirmation in Phase 1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
- getDescriptor() now uses Object.hasOwn() to match isSupported(), preventing prototype chain keys from returning non-descriptor values - Add 'config' to PermissionManager.CORE_TOOLS so it follows the same registry-level enable/disable semantics as other core tools Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Return error with ToolErrorType.EXECUTION_FAILED on SET failure and unknown setting, so coreToolScheduler correctly classifies failures - Restrict ConfigSettingDescriptor.type to 'string' for Phase 1 since the descriptor API is string-only (boolean coercion deferred to Phase 2) - Add error assertion in SET failure test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Agent 无法程序化读写配置。当复杂任务需要在不同阶段使用不同模型时（大模型分析 → 小模型生成模板 → 大模型审查），用户必须手动执行
/model切换，打断工作流。本 PR 新增 ConfigTool，让 Agent 通过
config GET/SET自主读写配置设置。Before
/model命令After
config SET model qwen3-coder自动切换模型'ask'权限），GET 自动放行SUPPORTED_CONFIG_SETTINGS添加条目Behavior matrix
config GET modelconfig SET model <id>config GET <unknown>config SET model <invalid>Key changes
tools/supported-config-settings.tsmodeltools/config-tool.tstools/config-tool.test.tstools/tool-names.tsCONFIG常量config/config.tsPhase 1 scope
只支持
model设置。后续 Phase 2 可通过 callback 机制桥接 CLI 层设置（theme、language、approvalMode 等），每个新设置只需在白名单添加约 10 行。Test plan
🤖 Generated with Claude Code