Repository navigation
Conversation
sonichi
left a comment
There was a problem hiding this comment.
Review — WIP: Desktop widget + PTT + task cancellation
Great additions. Here's my assessment:
Widget (widget.swift) ✅
- SwiftUI floating panel, always-on-top, all-spaces — solid UX
- Core status polling (reads core-status.json) — integrates with the status system we built today
- Task display, conversation view, text input — all useful
- 1346 lines but well-structured
Task cancellation ✅
- /task/cancel API endpoint — fills a gap (only had inline cancel before)
- Cancel buttons in widget + web client
- Proper result polling stop on cancel
PTT (Push-to-talk)
- Owner suggests PTT may be unnecessary for v1 — hotkey connect + voice "bye" disconnect is simpler
- Could defer to v2 as optional mode
- Carbon Events + Accessibility fallback adds complexity
Web client improvements ✅
- Auto-connect overlay — good UX
- Better mic error messages — addresses real user pain
- setMuteState refactor — enables remote control from widget
Utility scripts ✅
- kill-all.sh, clean-state.sh — useful maintenance tools
Suggestions:
- Consider making PTT optional/deferrrable
- The widget auto-build in startup.sh needs Xcode CLT — add a check
- Port 9901 (control server) should be in health-check.py
Overall: strong PR, widget is a differentiator. Merge when TODOs are resolved.
Addresses reviewer suggestion to include the widget/PTT control server in the health check alongside other optional services. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Reviewed the diff — the features look solid (task cancel, PTT, widget, kill-all/clean-state scripts). However, the PR has merge conflicts against current main (startup.sh, voice-agent.ts, web-client.ts, .gitignore have all changed significantly since March 29). Note: the native Sutando menu bar app (PR #74-76) now handles some of what the widget covers (global hotkeys, voice toggle, mute). The widget adds PTT and a floating panel which are complementary, but the startup.sh and voice-agent.ts changes will need careful rebasing to avoid conflicts with the new inline tools, tab aliases, and stand-identity.json loading. @ncrispino could you rebase against main when you get a chance? |
|
@sonichi I think we can just close this for now, I can put more targeted features against main when I have time. |
"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: 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>
…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>
Summary
Adds a native macOS desktop widget and several cross-cutting features.
Desktop widget (
widget.swift)SUTANDO_PTT_KEY)startup.sh(gracefully skips if Xcode CLT not installed)Push-to-talk (PTT)
Task cancellation
/task/cancelendpoint in agent-apiWeb client improvements
setMuteStaterefactor — widget/PTT can control mute remotelyVoice agent
voice-conversation.jsonfor widget conversation displayUtility scripts
kill-all.sh— reliably stop all Sutando processesclean-state.sh— remove stale task/result filesReview feedback addressed
eventNotHandledErrwhen event not matchedkill-all.sh: killwatch-tasksinstead offswatchOpen
Test plan
swiftc -o sutando-widget src/widget.swift -framework Cocoa -framework SwiftUI -framework CoreGraphics -O🤖 Generated with Claude Code