Repository navigation
Phone: add conversation context to inbound calls - #28
Merged
Merged
Conversation
Root cause of hallucinated greetings: phone agent had zero context. Now reads conversation.log for recent history (reference only, not brought up unless asked). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
that referenced
this pull request
Apr 5, 2026
- dashboard.py: Add resolve() + is_relative_to() for notes read and delete endpoints (supplements existing regex guard) - agent-api.py: Remove unused mimetypes import (replaced by SAFE_TYPES allowlist in previous security fix) Addresses CodeQL alerts #28-31 (dashboard) and cleans up #19-23 (agent-api already has guards, but adding belt-and-suspenders). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks done
sonichi
pushed a commit
that referenced
this pull request
Apr 7, 2026
* Pass previous description to Gemini for natural continuation Screen descriptions now flow naturally instead of every sentence starting with "The screen displays...". When a previous description exists, the prompt says "Continue naturally — do NOT start with The screen displays or repeat the previous description." Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Revert "Pass previous description to Gemini for natural continuation" This reverts commit 1105ed89e6cc41a0de777bc3b1dff9d157589d23. * Make task watcher persistent (loop instead of one-shot) watch-tasks.sh now loops continuously instead of exiting after detecting one batch of tasks. This eliminates the recurring issue where the watcher dies between task detections, causing delayed task pickup (e.g. the Discord video send delay). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add proactive Discord DM support to bridge The bridge now scans results/proactive-*.txt and sends them to the owner's DM channel. Previously only task replies were sent back — proactive messages (like sending a video unprompted) were silently dropped because they had no matching pending_replies entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix 3 CodeQL security alerts - browser-tools.ts: Fix command injection in typeText — write AppleScript to temp file instead of inline shell interpolation - conversation-server.ts: Sanitize DTMF digits in TwiML Play (strip non-digit chars to prevent XML injection) - conversation-server.ts: Improve phone number masking in startup log (show only first 2 + last 2 digits) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Harden path traversal guards in dashboard and agent-api - dashboard.py: Add resolve() + is_relative_to() for notes read and delete endpoints (supplements existing regex guard) - agent-api.py: Remove unused mimetypes import (replaced by SAFE_TYPES allowlist in previous security fix) Addresses CodeQL alerts #28-31 (dashboard) and cleans up #19-23 (agent-api already has guards, but adding belt-and-suspenders). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add 3 new call log scanner detectors from transcript analysis New patterns detected across 263 calls: - reconnect_leak: 37 calls with "I'm back" after Gemini reconnect - repeated_summon: 3 calls where user asked to summon 3+ times - repeated_tab_switch: 7 calls where tab switching failed repeatedly Added 6 tests (20 total, was 14). Scanner now has 8 detectors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Expand security tests for CodeQL fixes (33 assertions, was 17) New test coverage for: - Dashboard note slug validation (7 tests: traversal, slash, null byte, injection) - Dashboard note path resolution via is_relative_to (2 tests) - DTMF digit sanitization for TwiML injection (5 tests) - TypeText AppleScript injection prevention (2 tests) Total test suite: 120 assertions across 4 suites, all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add test runner script (4 suites, 120 assertions) bash tests/run-all.sh runs all test suites: - Phone access control (49 assertions) - Browser tools / STT corrections (18 assertions) - Security sanitization (33 assertions) - Call log scanner (20 assertions) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix migration script: auto-detect memory dir + don't exit on optional paths - Memory dir now auto-detected from repo path slug (falls back to default ~/Desktop/sutando path) - Session dir also auto-detected - Replaced set -e with set -uo pipefail — many migration paths legitimately don't exist and shouldn't abort the script - Tested: successfully bundles .env, 22 memory files, claude config, session history, flywheel data into 3.2MB tarball Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add --summary flag to call scanner for quality trend tracking Shows daily breakdown: total calls, clean vs issues, issue rate, top patterns per day. Clear improvement trend visible: - Mar 28-30: 80-100% issue rate (duplicates) - Mar 31: 45% after PR #67 - Apr 2-3: 13-29% after PR #105 - Apr 4: 21% (remaining: confusion + tab switching) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add migration script tests (14 assertions, 5 suites total) Tests cover: - REPO_SLUG generation (no slashes, starts with dash) - Memory dir auto-detection from repo path - Session dir detection - Bundle creation (env, memory, MEMORY.md) - Script syntax validation (no set -e) Total: 134 assertions across 5 suites, all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add ngrok tunnel to health check (13 checks total) Verifies the ngrok HTTPS tunnel is active — required for inbound phone calls via Twilio. Shows tunnel URL in health output. Only checked when Twilio is configured in .env. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Twilio webhook URL verification script Checks if Twilio's phone number webhook matches the current ngrok tunnel. Stale webhooks are a common failure mode — inbound calls silently fail. Supports --fix flag to auto-update Twilio. Usage: bash src/check-webhook.sh [--fix] Exit: 0=match, 1=mismatch, 2=error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Twilio webhook verification to health check (15 checks) Runs check-webhook.sh as part of the health check — warns if Twilio's voice URL doesn't match the current ngrok tunnel. Stale webhooks cause inbound calls to silently fail. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix Python 3.9 compat in github-webhook.py Remove str | None return type annotation (requires Python 3.10+). Susan runs Python 3.9 — this would crash on import. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review: fix 2 blockers + 2 yellow items (Wenjix review) Blocker 1: check-webhook.sh Python injection — pass phone number via env var instead of interpolating into Python source string. Blocker 2: phone mask padEnd alignment — wrap entire masked string in parens before padEnd. Show last 4 digits (standard compromise). Yellow: discord-bridge proactive files — rename to .failed on send failure instead of deleting (preserves message for inspection). Yellow: typeText tmp file — add random suffix to prevent race, use try/finally to ensure cleanup on execSync failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove test files from PR (tests/ is gitignored, should stay local) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove non-security changes from PR Revert infra/tooling files to main — this PR should only contain security fixes with CodeQL citations. Non-security changes (webhook script, scanner detectors, health checks, migration, watcher, Discord DM, Python 3.9 fix) moved to local infra branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add CodeQL citation comments to all security fixes Each fix now has an inline code comment citing the specific CodeQL alert number and what vulnerability it addresses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
that referenced
this pull request
Apr 13, 2026
…35-36) Existing resolve() + is_relative_to() + regex guards in agent-api.py and dashboard.py properly block all path traversal attempts. CodeQL flags these as false positives because it can't track taint through the validation logic. POC: scripts/poc-codeql-path-injection.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
that referenced
this pull request
Apr 13, 2026
…28-31, #35-36) CodeQL couldn't track taint through _safe_id() function calls or conditional regex match checks. Restructured to use re.sub() inline before Path() constructors so taint is visibly broken in the same scope. - agent-api.py: inline re.sub in _safe_path(), /media/, /answer handlers - dashboard.py: re.sub + reassign slug before Path() in GET and DELETE No behavior change — same sanitization logic, restructured for static analysis. Includes POC: scripts/poc-codeql-path-injection.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks done
sonichi
pushed a commit
that referenced
this pull request
Apr 14, 2026
* fix: validate display param is numeric (CodeQL #33) Unsanitized display query param flowed into subprocess.run args. Add isdigit() check so only numeric values reach screencapture. Includes POC script: scripts/poc-codeql-33-cmd-injection.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: escape backslashes before quotes in AppleScript strings (CodeQL #42, #39) The .replace(/"/g, '\\"') only escaped quotes but not backslashes. A backslash before a quote (\\") would become \\" which closes the AppleScript string context. Fix: escape backslashes first, then quotes. Includes POC: scripts/poc-codeql-42-39-string-escaping.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: POC confirms path injection guards work (CodeQL #16-23, #28-31, #35-36) Existing resolve() + is_relative_to() + regex guards in agent-api.py and dashboard.py properly block all path traversal attempts. CodeQL flags these as false positives because it can't track taint through the validation logic. POC: scripts/poc-codeql-path-injection.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use hostname endswith instead of substring check (CodeQL #38) "ts.net" in url matches spoofed hostnames like evil-ts.net.attacker.com. Parse URL and check hostname endswith(".ts.net") instead. POC: scripts/poc-codeql-38-url-substring.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: reduce phone number exposure in logs (CodeQL #37) Extract masked phone to separate variable and reduce visible digits from 6 (2+4) to 4 (2+2). Breaks CodeQL taint flow from env var to console.log. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sonichi
pushed a commit
that referenced
this pull request
Apr 14, 2026
…28-31, #35-36) (#323) CodeQL couldn't track taint through _safe_id() function calls or conditional regex match checks. Restructured to use re.sub() inline before Path() constructors so taint is visibly broken in the same scope. - agent-api.py: inline re.sub in _safe_path(), /media/, /answer handlers - dashboard.py: re.sub + reassign slug before Path() in GET and DELETE No behavior change — same sanitization logic, restructured for static analysis. Includes POC: scripts/poc-codeql-path-injection.sh Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 20, 2026
sonichi
added a commit
that referenced
this pull request
Apr 20, 2026
…36) (#487) 6 open path-injection alerts (#28-31, #35-36) in dashboard.py's /notes/{slug} handler. The existing sanitization was strong (regex whitelist + .is_relative_to confinement), but CodeQL's taint analysis didn't recognize .is_relative_to as a sanitizer and kept flagging the string-interpolated path. Refactor: extract the sanitize+resolve logic into _resolve_note_path() and rebuild the filename via Path(...).name — a CodeQL-recognized sanitizer that strips any path components. Functionally a no-op after the whitelist (slug is already [\w-]+), but breaks the taint flow. Also removes the duplication between do_GET and do_DELETE — both now call the same helper and share the reject-on-invalid path. Verified edge cases: valid slugs accepted (`valid-slug`, `my_note123`), traversal/separators/empty rejected (`../etc/passwd`, `''`, `a b c`, `slug.md`, `hello/world`, `..`). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 21, 2026
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
Root cause of hallucinated greetings: phone agent had zero context. Now reads conversation.log for recent history (reference only, not brought up unless asked). Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
* Pass previous description to Gemini for natural continuation Screen descriptions now flow naturally instead of every sentence starting with "The screen displays...". When a previous description exists, the prompt says "Continue naturally — do NOT start with The screen displays or repeat the previous description." Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Revert "Pass previous description to Gemini for natural continuation" This reverts commit 1105ed89e6cc41a0de777bc3b1dff9d157589d23. * Make task watcher persistent (loop instead of one-shot) watch-tasks.sh now loops continuously instead of exiting after detecting one batch of tasks. This eliminates the recurring issue where the watcher dies between task detections, causing delayed task pickup (e.g. the Discord video send delay). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add proactive Discord DM support to bridge The bridge now scans results/proactive-*.txt and sends them to the owner's DM channel. Previously only task replies were sent back — proactive messages (like sending a video unprompted) were silently dropped because they had no matching pending_replies entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix 3 CodeQL security alerts - browser-tools.ts: Fix command injection in typeText — write AppleScript to temp file instead of inline shell interpolation - conversation-server.ts: Sanitize DTMF digits in TwiML Play (strip non-digit chars to prevent XML injection) - conversation-server.ts: Improve phone number masking in startup log (show only first 2 + last 2 digits) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Harden path traversal guards in dashboard and agent-api - dashboard.py: Add resolve() + is_relative_to() for notes read and delete endpoints (supplements existing regex guard) - agent-api.py: Remove unused mimetypes import (replaced by SAFE_TYPES allowlist in previous security fix) Addresses CodeQL alerts sonichi#28-31 (dashboard) and cleans up sonichi#19-23 (agent-api already has guards, but adding belt-and-suspenders). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add 3 new call log scanner detectors from transcript analysis New patterns detected across 263 calls: - reconnect_leak: 37 calls with "I'm back" after Gemini reconnect - repeated_summon: 3 calls where user asked to summon 3+ times - repeated_tab_switch: 7 calls where tab switching failed repeatedly Added 6 tests (20 total, was 14). Scanner now has 8 detectors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Expand security tests for CodeQL fixes (33 assertions, was 17) New test coverage for: - Dashboard note slug validation (7 tests: traversal, slash, null byte, injection) - Dashboard note path resolution via is_relative_to (2 tests) - DTMF digit sanitization for TwiML injection (5 tests) - TypeText AppleScript injection prevention (2 tests) Total test suite: 120 assertions across 4 suites, all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add test runner script (4 suites, 120 assertions) bash tests/run-all.sh runs all test suites: - Phone access control (49 assertions) - Browser tools / STT corrections (18 assertions) - Security sanitization (33 assertions) - Call log scanner (20 assertions) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix migration script: auto-detect memory dir + don't exit on optional paths - Memory dir now auto-detected from repo path slug (falls back to default ~/Desktop/sutando path) - Session dir also auto-detected - Replaced set -e with set -uo pipefail — many migration paths legitimately don't exist and shouldn't abort the script - Tested: successfully bundles .env, 22 memory files, claude config, session history, flywheel data into 3.2MB tarball Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add --summary flag to call scanner for quality trend tracking Shows daily breakdown: total calls, clean vs issues, issue rate, top patterns per day. Clear improvement trend visible: - Mar 28-30: 80-100% issue rate (duplicates) - Mar 31: 45% after PR sonichi#67 - Apr 2-3: 13-29% after PR sonichi#105 - Apr 4: 21% (remaining: confusion + tab switching) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add migration script tests (14 assertions, 5 suites total) Tests cover: - REPO_SLUG generation (no slashes, starts with dash) - Memory dir auto-detection from repo path - Session dir detection - Bundle creation (env, memory, MEMORY.md) - Script syntax validation (no set -e) Total: 134 assertions across 5 suites, all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add ngrok tunnel to health check (13 checks total) Verifies the ngrok HTTPS tunnel is active — required for inbound phone calls via Twilio. Shows tunnel URL in health output. Only checked when Twilio is configured in .env. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Twilio webhook URL verification script Checks if Twilio's phone number webhook matches the current ngrok tunnel. Stale webhooks are a common failure mode — inbound calls silently fail. Supports --fix flag to auto-update Twilio. Usage: bash src/check-webhook.sh [--fix] Exit: 0=match, 1=mismatch, 2=error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Twilio webhook verification to health check (15 checks) Runs check-webhook.sh as part of the health check — warns if Twilio's voice URL doesn't match the current ngrok tunnel. Stale webhooks cause inbound calls to silently fail. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix Python 3.9 compat in github-webhook.py Remove str | None return type annotation (requires Python 3.10+). Susan runs Python 3.9 — this would crash on import. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review: fix 2 blockers + 2 yellow items (Wenjix review) Blocker 1: check-webhook.sh Python injection — pass phone number via env var instead of interpolating into Python source string. Blocker 2: phone mask padEnd alignment — wrap entire masked string in parens before padEnd. Show last 4 digits (standard compromise). Yellow: discord-bridge proactive files — rename to .failed on send failure instead of deleting (preserves message for inspection). Yellow: typeText tmp file — add random suffix to prevent race, use try/finally to ensure cleanup on execSync failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove test files from PR (tests/ is gitignored, should stay local) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove non-security changes from PR Revert infra/tooling files to main — this PR should only contain security fixes with CodeQL citations. Non-security changes (webhook script, scanner detectors, health checks, migration, watcher, Discord DM, Python 3.9 fix) moved to local infra branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add CodeQL citation comments to all security fixes Each fix now has an inline code comment citing the specific CodeQL alert number and what vulnerability it addresses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…ichi#39, sonichi#42) (sonichi#320) * fix: validate display param is numeric (CodeQL sonichi#33) Unsanitized display query param flowed into subprocess.run args. Add isdigit() check so only numeric values reach screencapture. Includes POC script: scripts/poc-codeql-33-cmd-injection.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: escape backslashes before quotes in AppleScript strings (CodeQL sonichi#42, sonichi#39) The .replace(/"/g, '\\"') only escaped quotes but not backslashes. A backslash before a quote (\\") would become \\" which closes the AppleScript string context. Fix: escape backslashes first, then quotes. Includes POC: scripts/poc-codeql-42-39-string-escaping.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: POC confirms path injection guards work (CodeQL sonichi#16-23, sonichi#28-31, sonichi#35-36) Existing resolve() + is_relative_to() + regex guards in agent-api.py and dashboard.py properly block all path traversal attempts. CodeQL flags these as false positives because it can't track taint through the validation logic. POC: scripts/poc-codeql-path-injection.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use hostname endswith instead of substring check (CodeQL sonichi#38) "ts.net" in url matches spoofed hostnames like evil-ts.net.attacker.com. Parse URL and check hostname endswith(".ts.net") instead. POC: scripts/poc-codeql-38-url-substring.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: reduce phone number exposure in logs (CodeQL sonichi#37) Extract masked phone to separate variable and reduce visible digits from 6 (2+4) to 4 (2+2). Breaks CodeQL taint flow from env var to console.log. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
-23, sonichi#28-31, sonichi#35-36) (sonichi#323) CodeQL couldn't track taint through _safe_id() function calls or conditional regex match checks. Restructured to use re.sub() inline before Path() constructors so taint is visibly broken in the same scope. - agent-api.py: inline re.sub in _safe_path(), /media/, /answer handlers - dashboard.py: re.sub + reassign slug before Path() in GET and DELETE No behavior change — same sanitization logic, restructured for static analysis. Includes POC: scripts/poc-codeql-path-injection.sh Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…chi#28-36) (sonichi#487) 6 open path-injection alerts (sonichi#28-31, sonichi#35-36) in dashboard.py's /notes/{slug} handler. The existing sanitization was strong (regex whitelist + .is_relative_to confinement), but CodeQL's taint analysis didn't recognize .is_relative_to as a sanitizer and kept flagging the string-interpolated path. Refactor: extract the sanitize+resolve logic into _resolve_note_path() and rebuild the filename via Path(...).name — a CodeQL-recognized sanitizer that strips any path components. Functionally a no-op after the whitelist (slug is already [\w-]+), but breaks the taint flow. Also removes the duplication between do_GET and do_DELETE — both now call the same helper and share the reject-on-invalid path. Verified edge cases: valid slugs accepted (`valid-slug`, `my_note123`), traversal/separators/empty rejected (`../etc/passwd`, `''`, `a b c`, `slug.md`, `hello/world`, `..`). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bassilkhilo-ag2
added a commit
that referenced
this pull request
Sep 23, 2026
…a Gemini key is named, campaign work goes to campaign-runner Three owner-visible misroutes from user feedback (2026-09-17 to 09-20), each a wrong or missing sentence rather than a missing capability. 1. Sending email "still needs an app password" (#28 on the tracker). The gmail-write-guard hook only matches the claude.ai Gmail MCP tools; the Station Gmail connector (composio_find → composio_exec) is a different tool name it never touches and it works. But the guard's denial REASON led with the app-password IMAP/SMTP path, so a core that reached for the claude.ai tool was pointed at generating an app password by hand even when Gmail was connected in Settings → Integrations. The reason now routes to the Station connector first, requires a read-back of the sent message (recipient + subject) before reporting a send — the check the draft/send mismatch incidents in field report 05cb849a lacked — and names IMAP/SMTP only as the fallback when the Station tools are absent. docs/built-in-tools.md and hooks/README.md say the same. 2. Gemini image generation on a free-tier key (#5). A free-tier key has 0 image requests per day; Google answers 429 RESOURCE_EXHAUSTED, which generate.py collapsed into api_error "try again in a moment" — the wrong advice for a quota. A 429, or any RESOURCE_EXHAUSTED/quota body, is now its own `quota` outcome whose remedy says to enable billing on the key's project or use the managed key, and the message names which key ran out (the owner's own GEMINI_API_KEY vs the managed one) because resolve_key() already knew. 3. Outbound campaigns went to the bare Smartlead connector (#27), which can only create, read and delete a campaign shell; attaching the mailbox, uploading leads and writing the sequence were left to the owner by hand — while the installed campaign-runner skill does all of it. precheck_apps.json gains a `prefer_skill` field (smartlead → campaign-runner), listed first so its phrases beat Gmail's bare "email"; match_apps lets a longer phrase win over a shorter one it contains ("cold email campaign" names Smartlead, not Gmail); the precheck line reports `prefer_skill=… (installed|not installed)` and never proposes a Connect card for such an app; the skill text says to invoke the skill or offer to install it, never the bare connector. Tests: gmail-write-guard (routing + read-back + fallback order), image-generation-cli (429 → quota with the key named; quota body on a 400; a plain 400 stays api_error), connect-apps-precheck (phrase precedence; prefer_skill line with and without the skill installed). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bassilkhilo-ag2
added a commit
that referenced
this pull request
Sep 23, 2026
…a Gemini key is named, campaign work goes to campaign-runner Three owner-visible misroutes from user feedback (2026-09-17 to 09-20), each a wrong or missing sentence rather than a missing capability. 1. Sending email "still needs an app password" (#28 on the tracker). The gmail-write-guard hook only matches the claude.ai Gmail MCP tools; the Station Gmail connector (composio_find → composio_exec) is a different tool name it never touches and it works. But the guard's denial REASON led with the app-password IMAP/SMTP path, so a core that reached for the claude.ai tool was pointed at generating an app password by hand even when Gmail was connected in Settings → Integrations. The reason now routes to the Station connector first, requires a read-back of the sent message (recipient + subject) before reporting a send — the check the draft/send mismatch incidents in field report 05cb849a lacked — and names IMAP/SMTP only as the fallback when the Station tools are absent. docs/built-in-tools.md and hooks/README.md say the same. 2. Gemini image generation on a free-tier key (#5). A free-tier key has 0 image requests per day; Google answers 429 RESOURCE_EXHAUSTED, which generate.py collapsed into api_error "try again in a moment" — the wrong advice for a quota. A 429, or any RESOURCE_EXHAUSTED/quota body, is now its own `quota` outcome whose remedy says to enable billing on the key's project or use the managed key, and the message names which key ran out (the owner's own GEMINI_API_KEY vs the managed one) because resolve_key() already knew. 3. Outbound campaigns went to the bare Smartlead connector (#27), which can only create, read and delete a campaign shell; attaching the mailbox, uploading leads and writing the sequence were left to the owner by hand — while the installed campaign-runner skill does all of it. precheck_apps.json gains a `prefer_skill` field (smartlead → campaign-runner), listed first so its phrases beat Gmail's bare "email"; match_apps lets a longer phrase win over a shorter one it contains ("cold email campaign" names Smartlead, not Gmail); the precheck line reports `prefer_skill=… (installed|not installed)` and never proposes a Connect card for such an app; the skill text says to invoke the skill or offer to install it, never the bare connector. Tests: gmail-write-guard (routing + read-back + fallback order), image-generation-cli (429 → quota with the key named; quota body on a 400; a plain 400 stays api_error), connect-apps-precheck (phrase precedence; prefer_skill line with and without the skill installed). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bassilkhilo-ag2
added a commit
that referenced
this pull request
Sep 26, 2026
…a Gemini key is named, campaign work goes to campaign-runner (#4661) * fix(owner-facing): email routes to the Station connector, a zero-quota Gemini key is named, campaign work goes to campaign-runner Three owner-visible misroutes from user feedback (2026-09-17 to 09-20), each a wrong or missing sentence rather than a missing capability. 1. Sending email "still needs an app password" (#28 on the tracker). The gmail-write-guard hook only matches the claude.ai Gmail MCP tools; the Station Gmail connector (composio_find → composio_exec) is a different tool name it never touches and it works. But the guard's denial REASON led with the app-password IMAP/SMTP path, so a core that reached for the claude.ai tool was pointed at generating an app password by hand even when Gmail was connected in Settings → Integrations. The reason now routes to the Station connector first, requires a read-back of the sent message (recipient + subject) before reporting a send — the check the draft/send mismatch incidents in field report 05cb849a lacked — and names IMAP/SMTP only as the fallback when the Station tools are absent. docs/built-in-tools.md and hooks/README.md say the same. 2. Gemini image generation on a free-tier key (#5). A free-tier key has 0 image requests per day; Google answers 429 RESOURCE_EXHAUSTED, which generate.py collapsed into api_error "try again in a moment" — the wrong advice for a quota. A 429, or any RESOURCE_EXHAUSTED/quota body, is now its own `quota` outcome whose remedy says to enable billing on the key's project or use the managed key, and the message names which key ran out (the owner's own GEMINI_API_KEY vs the managed one) because resolve_key() already knew. 3. Outbound campaigns went to the bare Smartlead connector (#27), which can only create, read and delete a campaign shell; attaching the mailbox, uploading leads and writing the sequence were left to the owner by hand — while the installed campaign-runner skill does all of it. precheck_apps.json gains a `prefer_skill` field (smartlead → campaign-runner), listed first so its phrases beat Gmail's bare "email"; match_apps lets a longer phrase win over a shorter one it contains ("cold email campaign" names Smartlead, not Gmail); the precheck line reports `prefer_skill=… (installed|not installed)` and never proposes a Connect card for such an app; the skill text says to invoke the skill or offer to install it, never the bare connector. Tests: gmail-write-guard (routing + read-back + fallback order), image-generation-cli (429 → quota with the key named; quota body on a 400; a plain 400 stays api_error), connect-apps-precheck (phrase precedence; prefer_skill line with and without the skill installed). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(owner-facing): trim the guard-test comment, name the key in use, keep a rate-limited 429 retryable, route through the Claude-home helper Review findings on the previous revision, all addressed here. Blocker: `typecheck + node & shell tests` was red because tests/gmail-write-guard.test.py:60 carried a four-line comment block with incident history (prose-cap allows two lines). The comment now states the assertion only; the history stays in the PR body. The dated paragraphs in the gmail-write-guard.py module docstring and the classify_http_error docstring are trimmed the same way. skill_installed() in skills/connect-apps/hooks/connect-precheck.py re-implemented Claude-home resolution as CLAUDE_CONFIG_DIR -> ~/.claude, skipping the CLAUDE_HOME level of the canonical chain, so a setup that only sets $CLAUDE_HOME was told the skill is missing. It now resolves through util_paths.claude_home_path("skills") the way workspace_for() already resolves the workspace, with the stock ~/.claude only when no core tree is importable. Tests cover CLAUDE_HOME-only, CLAUDE_CONFIG_DIR outranking it, and the no-core-tree fallback. key_note("env") always named GEMINI_API_KEY, but the resolver and the fallback chain both accept GEMINI_VOICE_API_KEY for gemini-image, so a voice-key-only install was pointed at the wrong variable. The note now names whichever variable holds the key that was used. classify_http_error() treated every 429 (and any body containing "quota") as the free-tier zero-quota case, whose SKILL.md remedy is "enable billing, never retry". A paid key hitting the per-minute rate limit is also a 429 and needs the opposite advice. `quota` now requires plan-level wording (per day / free tier / limit 0 / billing) in the body; any other 429 stays `api_error` with its retry remedy. Tests use Google's free-tier per-day body and add a per-minute body that must stay retryable. match_apps() kept one keyword per app and then dropped a hit whose keyword sat inside another app's longer phrase, so "reply to the cold email from Bob in my inbox" lost Gmail although "inbox" names it. Every matched keyword now counts; an app is dropped only when all of its keywords are inside other apps' phrases. Test added for that sentence. Verification: python3 tests/gmail-write-guard.test.py (PASS), tests/image-generation-cli.test.py (29 OK), tests/connect-apps-precheck.test.py (26 OK); scripts/review-checks.sh --diff PASS; diff coverage of changed Python lines 100%; lint-workspace-resolution --diff clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(connect-precheck): suppress a shorter keyword by match span, not by keyword string `match_apps` was meant to keep the "email" inside Smartlead's "cold email campaign" from also naming Gmail. The previous head compared keyword *strings*: a keyword was dropped whenever it was a substring of any keyword another app had matched anywhere in the text. So Gmail vanished from "draft an email to Bob and check my email campaign stats" (two separate requests, no shared span), from "email me when the email sequence finishes", and from "send an email about the cold email campaign" — it survived only when it also matched a word that is not a substring of anything, such as "inbox". The PR's own test covered just that single-overlap case, which is why CI stayed green with the defect live. Now each match is kept as (keyword, span) — `pat.finditer(low)` over every occurrence, since `search` would stop at a first occurrence that happens to sit inside another app's phrase — and a match is discarded only when its span lies strictly inside another app's longer match at that position (containment with an unequal span; equal spans of two apps both stay). An app is reported when any of its matches survives, with the first surviving keyword in table order as before. Against the shipped precheck_apps.json: smartlead+gmail reply to the cold email from Bob in my inbox smartlead+gmail email me when the email sequence finishes smartlead+gmail send an email about the cold email campaign smartlead+gmail draft an email to Bob and check my email campaign stats smartlead+gmail set up a cold email campaign, then email Bob the results smartlead set up a Smartlead campaign smartlead cold email campaign The last row is the intended suppression: Gmail's only match is the "email" inside "cold email" / "cold email campaign", same position, so it stays quiet. The new test pins all seven texts plus the reported keyword for the mixed case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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
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.
Root cause fix for hallucinated greetings. Injects recent conversation.log context.