[dead-code] chore: remove dead functions — 1 function removed#34674
Conversation
Remove hasBashWildcardInTools which is unreachable from any production binary entry point as identified by the deadcode static analyzer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🧪 Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. PR #34674 is a dead-code removal that only deletes the TestHasBashWildcardInTools test function (along with its corresponding production function hasBashWildcardInTools). No new or changed tests to analyze. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #34674 does not have the 'implementation' label and has 0 new lines in default business logic directories (threshold: 100). |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Pull request overview
Removes a Go helper and its associated unit test that were identified as unreachable by the deadcode static analyzer, reducing maintenance surface in the workflow tooling layer.
Changes:
- Deleted
hasBashWildcardInToolsfrompkg/workflow/claude_tools.go. - Removed
TestHasBashWildcardInToolsfrompkg/workflow/claude_engine_tools_test.go. - Confirmed there are no remaining references to
hasBashWildcardInToolsin the Go codebase.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/claude_tools.go |
Removes the unused hasBashWildcardInTools helper. |
pkg/workflow/claude_engine_tools_test.go |
Removes the unit test that only covered the deleted helper. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Dead code removal is correct — one documentation gap remains
The removal is valid: hasBashWildcardInTools was already decoupled from the permission-mode selection path in a prior commit (the claude_engine_test.go test names — "bash wildcard * no longer forces bypassPermissions" etc. — make this explicit). Deleting both the function and its test is the right cleanup.
Documentation gap (not blocking — already merged): AGENTS.md line 721 still reads:
hasBashWildcardInTools()inclaude_tools.godetermines which mode is selected — changes here affect the security boundary
This is now stale. A follow-up commit should update that section to describe the actual current mechanism (engine.permission-mode override field + stripClaudePermissionModeArgs) so future contributors do not search for a function that no longer exists.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 818.3K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /zoom-out — clean dead-code removal; one documentation note.
📋 Key Themes & Highlights
Key Themes
- AGENTS.md is now stale: The security-model section ("Claude Engine Tool Enforcement Security Model") still references
hasBashWildcardInTools()as the function that determines which permission mode is selected, and describesbypassPermissionsas being auto-selected forbash: "*"workflows — but neither of those things is true anymore. The currentclaude_engine.godefaults toacceptEditsand only switches toautoviaisEditToolExplicitlyDisabled, never tobypassPermissionsautomatically. AGENTS.md lines 715, 721 should be updated to reflect the current behavior. - The removal itself is correct:
deadcodeconfirmed no live call site existed;go buildandgo vetpass; the 9-case test suite is appropriately retired alongside the function it covered.
Positive Highlights
- ✅ Pure deletion — zero additions, no risk of introducing new bugs
- ✅ Build and vet verified before opening the PR
- ✅ Well-documented PR description explaining before/after dead-function count
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.2M
[dead-code] chore: remove dead functions — 1 function removed
Summary
Removes the unused
hasBashWildcardInToolshelper function frompkg/workflow/claude_tools.goand its corresponding test suite frompkg/workflow/claude_engine_tools_test.go. No call sites remain; this is a pure dead-code cleanup with no behaviour change.Changes by file
pkg/workflow/claude_tools.gopkg/workflow/claude_engine_tools_test.goDetailed file changes
pkg/workflow/claude_tools.go— deleted functionhasBashWildcardInTools(tools map[string]any) bool.bash: true,bash: nil, andbash: ["*"]/bash: [":*"].pkg/workflow/claude_engine_tools_test.go— deleted testsTestHasBashWildcardInToolsand all its table-driven sub-cases that exercised the deleted helper.Risk assessment
Agentic analysis hints
dead_code_label:hasBashWildcardInToolsaffected_packages:pkg/workflowdeleted_symbols:hasBashWildcardInToolsdeleted_tests:TestHasBashWildcardInToolscommit:b136dc47apr_category: dead-code / chorerequires_migration: falserequires_config_change: falsetouches_security_logic: true (detection only, not enforcement)