Repository navigation
fix(inline-tools): gate slideControlTool exposure on presenter-mode sentinel (#1171) - #1175
bassilkhilo-ag2 wants to merge 1 commit into
Conversation
VasiliyRad
left a comment
There was a problem hiding this comment.
The gate itself is correct and minimal: PRESENTER_ACTIVE is checked once at module load via existsSync(join(WORKSPACE_DIR, 'state', 'presenter-mode.sentinel')), workspace-resolved, and the conditional spread ...(PRESENTER_ACTIVE ? [slideControlTool] : []) is applied consistently in both inlineTools and ownerOnlyTools. Structural test passes. The fix is the right minimal change.
One concern: the PR branch has 4 commits over main, and 3 of them are unrelated (d9a384c health-check multi-pid fix + its test, and two chore(log) commits). These should be on their own PRs or squashed out before merging — the health-check change is substantive enough to need its own review context. build_log.md is gitignored but still shows in the diff, likely force-added by a prior chore commit.
If you can rebase/squash this to just d94dac6 (the actual fix commit), it's good to approve. As-is I'd hold off on merging until the branch is cleaned up.
…entinel (sonichi#1171) Inline tools registered globally are visible to Gemini every turn. Without a gate, Gemini fires slide_control (and similar mode-specific tools) on greetings and fillers even when no presentation is active — producing spurious tool_call rows in voice sqlite and wasting tokens. Add PRESENTER_ACTIVE at module load, checked against $WORKSPACE/state/presenter-mode.sentinel. slideControlTool is spread into inlineTools and ownerOnlyTools only when the sentinel is present; otherwise the tool description is never shown to Gemini and can't be triggered spuriously. Trade-off (documented): toggling presenter-mode.sh mid-session won't hot-reload the tool list. The existing workflow (run presenter-mode.sh BEFORE starting the voice agent) already satisfies this contract. Structural tests in tests/inline-tools-presenter-gate.test.py verify the guard is present and the unconditional array entry is gone. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d94dac6 to
9f7ebf7
Compare
|
Rebased to just the fix commit |
|
Branch has been rebased — the 3 unrelated commits VasiliyRad flagged are now stripped. The PR is now a single commit:
Same state as the end of that review comment's suggestion. Ready for re-review / approval. |
Summary
PRESENTER_ACTIVEsentinel check at module load insrc/inline-tools.ts(closes voice-agent: copres_next_anchor fires on greetings — inline tools needing mode-gating should gate at exposure, not just effect #1171)slideControlToolis spread intoinlineToolsandownerOnlyToolsonly whenstate/presenter-mode.sentinelis presentslide_controldescription → can't fire it on greetings or filler utterancesexistsSync(join(WORKSPACE_DIR, 'state', 'presenter-mode.sentinel'))— workspace-resolved, not repo-relativeBackground
slide_controlwas unconditionally registered as a global inline tool. Gemini saw its description every turn and fired it spuriously on greetings/silence even when no presentation was active. The tool body had no intent guard, and the spurioustool_callentries landed in voice sqlite, wasting tokens and noising up observability.Susan's MBP bot (Maddy) filed this after observing 3 unprovoked
copres_next_anchorfirings in <40s on a greeting — the OSS-visible analog isslide_control.Trade-off
Toggling
presenter-mode.sh start Nmid-session won't hot-reload the tool list (voice-agent must restart). The existing workflow already runspresenter-mode.shbefore the voice agent, so the contract holds in normal use.Test plan
python3 tests/inline-tools-presenter-gate.test.py— PASSslide_controlabsent from tool listbash scripts/presenter-mode.sh start 60, restart voice agent →slide_controlpresentRelated
🤖 Generated with Claude Code