Skip to content

fix(cli): stop inline skill-shell doc examples from triggering permission prompts - #12802

Merged
bagatao-anaconda merged 5 commits into
mainfrom
fix/skill-shell-inline-code-spans
Aug 3, 2026
Merged

fix(cli): stop inline skill-shell doc examples from triggering permission prompts#12802
bagatao-anaconda merged 5 commits into
mainfrom
fix/skill-shell-inline-code-spans

Conversation

@bagatao-anaconda

Copy link
Copy Markdown
Collaborator

Issue

No tracked issue; found and fixed while testing skill-shell command execution.

Context

Loading a skill whose SKILL.md documents the !cmdplaceholder syntax inline (e.g. !cmd ``, the standard CommonMark way to show a literal single backtick) triggered a real bash permission prompt for that documentation text. `SkillInject.render` only treated fenced code blocks as inert; it had no concept of inline code spans, so the built-in `kilo-config` skill's own docs false-positived on every load.

Implementation

Extended the inert-range check to also cover inline code spans of 2+ backticks: a single-backtick span can never contain a backtick, so a single-backtick pair nested inside a longer run is always documentation, never a live placeholder.

Pairing is scoped to one blank-line-delimited paragraph at a time (CommonMark spans can't cross a blank line) using a single-pass map, with a binary-searched fence lookup.

Both issues were caught in local review, reproduced, and fixed before this PR; regression tests cover each.

Screenshots / Video

N/A — no UI change (CLI-only classification logic).

How to Test

Manual/local verification

  • Loaded a skill with a genuine live !cmdplaceholder alongside an inline !cmd `` doc example; confirmed only the live command prompted/executed and the example stayed literal text.

Reviewer test steps

  1. From packages/opencode/, run bun test ./test/kilocode/skills/inject.test.ts ./test/tool/skill.test.ts — 22 tests pass, including the inline-example, cross-paragraph, and quadratic-scaling regressions.
  2. bun run typecheck — clean.
  3. In Agent Manager (or any session), load the built-in kilo-config skill and confirm no Bash permission card appears.

@bagatao-anaconda bagatao-anaconda self-assigned this Aug 3, 2026
}),
)

unix("does not let distant unrelated inline code spans merge into one inert range", () =>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about windows? Is this a unix only fix?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying fix is cross-platform. Only the tests that also exercise real process spawning (such as the one above) are unix-only.

Comment thread packages/opencode/src/kilocode/skills/inject.ts Outdated
Comment thread packages/opencode/test/kilocode/skills/inject.test.ts Outdated
Comment thread packages/opencode/src/kilocode/skills/inject.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/kilocode/skills/inject.ts 7 Comment rewrites unrelated to the fix drop rationale about Shell.acceptable parse alignment and the cleared timeout timer
Resolved since last review
  • packages/opencode/src/kilocode/skills/inject.ts - nested/overlapping inline-span ranges. pairs now consumes everything between an opener and its equal-length closer (i = j + 1) and chunks splits on both fences and blank lines, so spans are sorted and disjoint by construction, satisfying within's binary-search precondition. Covered by the new nested-span and fence-crossing regression tests.
  • packages/opencode/test/kilocode/skills/inject.test.ts - flake-prone 5s wall-clock assertion. Bound raised to 20s with a matching 30s bun test timeout, and the comment now states it guards complexity rather than latency.
Files Reviewed (2 files)
  • packages/opencode/src/kilocode/skills/inject.ts - 1 issue
  • packages/opencode/test/kilocode/skills/inject.test.ts

Fix these issues in Kilo Cloud

Previous Review Summary (commit 5bd420a)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 5bd420a)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/kilocode/skills/inject.ts 188 ranges can emit nested/overlapping spans, breaking within's binary-search precondition; a placeholder inside an outer code span can still be classified as live

SUGGESTION

File Line Issue
packages/opencode/test/kilocode/skills/inject.test.ts 456 5s wall-clock assertion is flake-prone on loaded CI runners
packages/opencode/src/kilocode/skills/inject.ts 7 Comment rewrites unrelated to the fix drop rationale about Shell.acceptable parse alignment and the cleared timeout timer
Files Reviewed (3 files)
  • packages/opencode/src/kilocode/skills/inject.ts - 2 issues
  • packages/opencode/test/kilocode/skills/inject.test.ts - 1 issue
  • .changeset/skill-shell-inline-docs.md

Fix these issues in Kilo Cloud


Reviewed by claude-opus-5 · Input: 30 · Output: 11.9K · Cached: 812.3K

Review guidance: REVIEW.md from base branch main

@bagatao-anaconda
bagatao-anaconda merged commit afdb194 into main Aug 3, 2026
31 checks passed
@bagatao-anaconda
bagatao-anaconda deleted the fix/skill-shell-inline-code-spans branch August 3, 2026 11:29
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…e-code-spans

fix(cli): stop inline skill-shell doc examples from triggering permission prompts
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.

2 participants