fix skill broker chatId + add per-skill auto_allow#22
Merged
Conversation
ollama-tier skills routed broker confirmation prompts to chatId=0 (a hardcoded stub with a stale "log correlation only" comment), so every confirm-tier tool call silently fail-closed to deny without the operator ever seeing a prompt. read the real chatId/auditId from skillToolCtx inside runSkillBareWithTools; both production callers already wrap us in the ALS. new auto_allow: bool frontmatter (default false) suppresses the interactive confirm prompt for every tool the skill body calls. appropriate for skills whose entire purpose IS a known write (e.g. /log -> notion). loop detector, hard-deny classifier, and cost cap all still gate. wired into both engines: - claude tiers: canUseTool swapped for skillDefaultAllowAll; PreToolUse hook unchanged. - ollama tier: executeToolCall confirm branch falls through to invoke; per-round confirm cap skipped (no prompts to stack). docs/USAGE.md updated.
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
1 participant
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
chatId: 0(hardcoded inrunSkillBareWithToolswith a stale "log correlation only" comment). Everyconfirm-tier tool call (e.g./log→notion_create_page) fail-closed to deny without the operator ever seeing a prompt. Now reads the realchatId/auditIdfromskillToolCtx(the AsyncLocalStorage both production callers already wrap us in).auto_allow: trueSKILL.md frontmatter (defaultfalse) suppresses the interactive Telegram-confirm prompt for every tool the skill body calls. Loop detector, hard-deny classifier, and cost cap all still gate — only the prompt is bypassed. Wired into both engines:runSkill):canUseToolswapped forskillDefaultAllowAll; PreToolUse hook unchanged.executeToolCall): confirm branch falls through to invoke; per-round confirm cap skipped (no prompts to stack).Why
Without the bug fix, no skill with default tier
ollamacould write to a confirm-tier integration through Telegram —notion_create_page,gmail_send_draft, etc. all denied silently. The fix is mandatory to ship/log.The
auto_allowflag is a UX win for skills whose entire purpose IS the operation. Re-prompting on everynotion_create_pageinside/logis friction without a safety benefit (the cost cap + loop detector still backstop). Opt-in, per-skill — no global toggle, no env var.Impact
auto_allowdefaults tofalse.auto_allowis a per-skill opt-in surfaced through SKILL.md, not a removal of the gate.Test plan
npm run typecheckpassesbun test— 753 pass (6 new: 4auto_allowparse cases, 2 executor bypass cases)auto_allow: trueto/logSKILL.md, invoke/log fooin Telegram; verify Notion entry appears with no promptauto_allow, re-invoke/log foo; verify confirm prompt now appears (regression check for the bug fix)