Skip to content

fix: enforce skill deny pattern filtering#21881

Closed
saurav-shakya wants to merge 3 commits intoanomalyco:devfrom
saurav-shakya:fix/issue-21793-skill-permission-pattern
Closed

fix: enforce skill deny pattern filtering#21881
saurav-shakya wants to merge 3 commits intoanomalyco:devfrom
saurav-shakya:fix/issue-21793-skill-permission-pattern

Conversation

@saurav-shakya
Copy link
Copy Markdown

@saurav-shakya saurav-shakya commented Apr 10, 2026

Issue for this PR

Closes #21793

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

This fixes a bug where permission.skill pattern rules (for example lark-*: deny) were not fully enforced in skill exposure.

Changes made:

  • Removed static skill listing from the base skill tool definition to avoid leaking denied skills.
  • Kept skill listing in the registry path that already uses agent-aware filtering.
  • Ensured the system prompt skips the skills section when no skills are available after permission filtering.
  • Added a regression test that denies lark-* and verifies only allowed skills appear.

Why this works:

  • Skill visibility is now sourced from permission-aware paths, so denied patterns are filtered before model exposure.
  • The regression test covers the reported case and prevents reintroduction.

How did you verify your code works?

I verified by:

  • Reviewing the diff to ensure only skill exposure and related prompt behavior were changed.
  • Adding a regression test for denied skill pattern filtering.

Screenshots / recordings

N/A (non-UI change)

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

…rflow

Two fixes in this commit:

### 1. Render <think> tags as reasoning blocks at display layer

Models like GLM-5 embed reasoning in raw <think>...</think> tags within text parts.
Previously these were displayed as raw HTML to the user.

**Approach:** Parse and render <think> / <thinking> tags at the display layer (TUI, Web UI, CLI)
rather than stripping them from the message storage. This preserves the full model output for
multi-turn context while giving users clean, styled reasoning blocks.

**Changes:**
- packages/util/src/think.ts -- Shared utility: stripThinkTags(), splitThinkBlocks() with regex matching both <think> and <thinking> variants
- packages/opencode/src/util/format.ts -- Same utilities for the core package
- packages/opencode/src/cli/cmd/tui/routes/session/index.tsx -- TextPart renders think blocks with dim/italic/bordered styling matching ReasoningPart
- packages/ui/src/components/message-part.tsx -- TextPartDisplay renders think blocks in <div data-component="reasoning-part">
- packages/opencode/src/cli/cmd/run.ts -- CLI strips think tags from output, shows reasoning with --thinking flag

### 2. Fix MCP MaxListeners overflow with many servers

When 7+ stdio-based MCP servers are configured, the default Node.js EventEmitter limit of ~10 is exceeded.
Each StdioClientTransport spawns a child process and adds listeners to its stdin/stdout/stderr pipes.
The MCP protocol layer also sends many concurrent requests (getPrompt, listTools, etc.) that each add a 'drain' listener.

**Fix:** Dynamically calculate the needed listener limit based on the number of configured local MCP servers
and increase both EventEmitter.defaultMaxListeners and process stream limits before connecting.

**Changes:**
- packages/opencode/src/mcp/index.ts -- Count local MCP servers, set EventEmitter.defaultMaxListeners and process stream limits proportionally

## Related Issues
- Fixes anomalyco#16903 (GLM-5 thinking output pollution)
- Fixes anomalyco#11439 (think tag rendering regression from removal of extractReasoningMiddleware)
- Related: anomalyco#4033, anomalyco#7779
@github-actions github-actions bot added needs:compliance This means the issue will auto-close after 2 hours. and removed needs:compliance This means the issue will auto-close after 2 hours. labels Apr 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

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.

bug: permission.skill pattern rules are not fully enforced in skill exposure flow

2 participants