Add Agent Plugins: voice window for Claude Code and pi - #247
Open
Discountrobot wants to merge 6 commits into
Open
Discountrobot wants to merge 6 commits into
Discountrobot wants to merge 6 commits into
Conversation
Opt-in voice window that lets a coding agent reach you when it needs you, wherever you are. When Claude Code or pi finishes a turn, asks a multiple-choice question, or requests a permission, a floating card appears; you answer by voice, typing, or tapping an option and the reply lands back in the exact session that asked. Off by default, enabled per agent in Settings -> Agent Plugins. - Two agents behind a single AgentIntegrationsClient registry, so the rest of the app stays agnostic and adding another is one provider. - In-band replies via a hook-relayed response file: a reply can't land in the wrong window or silently fail after a card's hook has timed out. - Concurrent sessions queue one card at a time with a project-avatar selector; each card shows the project name and current git branch. - Card appears passively without stealing keyboard focus; any Enter sends, Escape dismisses and stops playback. - Optional on-device read-aloud via Kokoro TTS, with a selectable voice and a distinct voice per concurrent project. Stays under the macOS App Sandbox: Hex never writes to ~/.claude or ~/.pi. It generates the hook/extension plus an installer inside its own container and surfaces a one-time copy-paste command to register it; a thin stub execs the real hook from the container. The unsandboxed hook does the privileged reads (last assistant message, project GitHub owner) and passes them in. Closes #2
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds Agent Plugins settings, bridge clients, prompt parsing, speech support, and the agent window response flow. ChangesAgent Plugins
Sequence Diagram(s)Agent hook round trip sequenceDiagram
participant ClaudePluginClientLive
participant HexAppDelegate
participant AgentFeature
participant AgentView
participant AgentHookResponder
ClaudePluginClientLive->>HexAppDelegate: open hex://agent-update
HexAppDelegate->>AgentFeature: send .show(payload)
AgentFeature->>AgentView: render prompt queue
AgentView->>AgentFeature: send reply or permission action
AgentFeature->>AgentHookResponder: respond(payloadPath:json:)
AgentHookResponder->>ClaudePluginClientLive: write payload.response
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Hex/Features/Transcription/TranscriptionFeature.swift (1)
304-323: 🩺 Stability & Availability | 🟠 MajorEnsure speech synthesis stops before recording begins.
Effect.mergeexecutes child effects concurrently, meaningspeechSynthesizer.stop()andrecording.startRecording()can run in parallel. This reintroduces the race condition where the microphone might capture the agent's voice before it stops.Replace
.mergewith.concatenateto enforce sequential execution, or move thestop()call inside the recording.runblock to guarantee it completes first:Corrected approach
- return .merge( - .cancel(id: CancelID.recordingCleanup), - .run { _ in await speechSynthesizer.stop() }, - .run { [sleepManagement, preventSleep = state.hexSettings.preventSystemSleep] _ in + return .merge( + .cancel(id: CancelID.recordingCleanup), + .concatenate( + // Silence TTS before opening the mic + .run { _ in await speechSynthesizer.stop() }, + .run { [sleepManagement, preventSleep = state.hexSettings.preventSystemSleep] _ in soundEffect.play(.startRecording) @@ - } - .cancellable(id: CancelID.recordingStart, cancelInFlight: true) + } + .cancellable(id: CancelID.recordingStart, cancelInFlight: true) + ) )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 304 - 323, The recording setup in TranscriptionFeature is starting speech stop and recording in parallel because Effect.merge runs the child effects concurrently. Update the logic around the recording start sequence so speechSynthesizer.stop() completes before recording.startRecording() is invoked, either by switching this effect chain to sequential composition with .concatenate or by moving the stop call into the same .run block before the recording begins. Keep the existing CancelID.recordingStart and sleep-management behavior intact while ensuring the stop/start ordering is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Hex/App/HexAppDelegate.swift`:
- Around line 160-164: The logging in handleHexURL(_:) is exposing sensitive URL
contents publicly; update the initial appLogger.notice call so only the
host/event is logged with public privacy, and keep the full url.absoluteString
private or remove it from the log entirely. Preserve the existing guard on
url.host == "agent-update", but change the received-URL message to avoid
emitting cwd, transcript, payload, or metadata in public logs, following the
same privacy annotations used elsewhere in HexAppDelegate.
In `@Hex/Clients/AgentTranscriptClient.swift`:
- Around line 54-90: The hook file reads in AgentTranscriptClient’s latestPrompt
and readJSON currently trust payloadPath and transcriptPath directly, which can
escape the expected agent directory. Update the path handling to resolve both
inputs against the allow-listed hooks/transcripts base directory, canonicalize
any symlinks, and verify the final resolved path stays inside that base before
calling Data(contentsOf:) or String(contentsOf:). Keep the existing latestPrompt
flow and readJSON helper, but add the path validation at the boundary where
those raw paths are consumed.
- Around line 125-135: The lastAssistantText helper is skipping the most recent
user boundary when the newest assistant messages have no text blocks, which can
cause it to return stale text from an older turn. Update lastAssistantText(_:),
using parseLine and textBlocks, so it always stops at the first user event
encountered while scanning backward and returns an empty string if no assistant
text exists in the latest turn. Ensure the break condition does not depend on
collected having content.
In `@Hex/Clients/ClaudePluginClient.swift`:
- Around line 166-168: The hook-group filtering in ClaudePluginClient is
removing entire matcher groups whenever one hook command matches Hex’s stub,
which deletes unrelated user hooks. Update the install/uninstall flow in
ClaudePluginClient, including the logic around groups and uninstallScript(), to
filter only the Hex stub hook via a shared without_hex_stub helper while
preserving any other hooks in the same group, and only drop a group when it
becomes empty.
In `@Hex/Clients/PiPluginClient.swift`:
- Around line 301-310: The destructive-command detection in isDangerousBash is
missing common recursive rm flag variants, so expand the DANGEROUS_BASH pattern
used by isDangerousBash to catch combined and split forms such as rm -fr and
similar permutations. Update the regex in the DANGEROUS_BASH array so bash
commands passed through PiPluginClient are flagged consistently before
execution.
In `@Hex/Clients/SpeechSynthesizerClient.swift`:
- Around line 122-125: The shared initialization path in readyManager(progress:)
drops progress updates for later callers because it returns managerTask.value
without notifying the new callback. Update
SpeechSynthesizerClient.readyManager(progress:) so that when managerTask already
exists, the passed progress handler still receives completion (at minimum call
progress?(1) after awaiting the shared task), or otherwise fan out the shared
initialization progress to every caller. Keep the fix centered on
readyManager(progress:) and the prepareKokoro call path that depends on it.
In `@Hex/Features/Agent/AgentFeature.swift`:
- Around line 102-107: The AgentFeature.avatarURL(owner:) helper currently
returns a remote github.com image URL that AgentView loads via AsyncImage, which
should be removed for the on-device path. Update the avatar handling in
AgentFeature and any call sites in AgentView to use a local deterministic avatar
or folder glyph based on the owner string instead of fetching from GitHub,
unless there is an explicit user opt-in for remote avatars.
- Around line 589-590: The permission prompt text in AgentFeature’s permission
case is hardcoded to “Claude wants…”, which makes the read-aloud copy
agent-specific. Update the string returned from the `.permission(permission)`
branch in `AgentFeature` to use generic agent-agnostic wording that references
the tool without naming Claude, so the spoken prompt works across multiple
agents.
- Around line 207-215: The UserPromptSubmit handling in AgentFeature’s request
reducer only removes the queued card and may still leave an active auto-send
countdown that can fire after advance(&state). Update the request-removal path
so any pending send effect tied to the current request is cancelled when
terminal input supersedes it, using the same request identity in
AgentFeature/advance flow to stop a stale .send from targeting the next visible
request.
In `@Hex/Features/Agent/AgentHookResponder.swift`:
- Around line 25-39: `AgentHookResponder.respond(payloadPath:json:)` currently
trusts a path derived from `hex://agent-update` and uses it for reads/writes, so
confine it to the agent rendezvous directory before touching
`<payload>.response`. Add validation that the resolved `payloadPath` stays under
`<container>/agent/io/` and matches the expected `hook.*.json` shape, and only
then proceed with `JSONSerialization.data(withJSONObject:)` and
`Data.write(...)`; otherwise log and abort without creating the response file.
Apply the same safeguard anywhere else the `payloadPath` is consumed in the
related response flow.
In `@Hex/Features/Agent/AgentView.swift`:
- Around line 250-253: The runtime permission/timeout card copy is hardcoded to
“Claude,” which makes the multi-agent UI misleading for non-Claude agents.
Update the affected text in AgentView’s permissionView and the corresponding
timeout card to use agent-agnostic wording, ideally by deriving the agent name
from the current agent/context instead of embedding “Claude” directly in the
string literals.
In `@Hex/Features/Settings/SettingsFeature.swift`:
- Around line 636-649: The Kokoro prepare flow in SettingsFeature’s
prepareKokoro logic can leave state stuck because queued .kokoroPrepareProgress
events may arrive after .kokoroPrepared has already completed. Update the
.kokoroPrepareProgress handling to only apply progress for the active prepare
cycle, or ignore late callbacks once the operation has finished, and make sure
the completion path in the prepareKokoro effect clears/invalidates any in-flight
progress state so later calls do not trip the “already downloading” guard.
- Around line 611-615: The Kokoro warm-up flow is auto-triggering a voice
preview even when .prepareKokoro was started only to preload for
.setAgentDistinctSessionVoices. Update SettingsFeature’s handling of
.prepareKokoro and .kokoroPrepared(success:) to track whether the request came
from a preview action versus a warm-up/preload, and only send .previewAgentVoice
for preview-originated prepares. Use the existing state/actions around
.setAgentDistinctSessionVoices and .prepareKokoro to store and check this flag
so enabling Distinct Voice per Project does not speak the sample unexpectedly.
- Line 643: The Kokoro download failure log is exposing potentially sensitive
user-specific path data via error.localizedDescription. Update the error
handling in SettingsFeature’s Kokoro download flow to use HexLog instead of
settingsLogger, and mark the dynamic error details as private in the log message
so any transcript/path information is not publicly emitted.
In `@Hex/Info.plist`:
- Around line 11-21: The custom URL scheme registered in CFBundleURLTypes is too
generic, which can cause agent callback routing to be intercepted by another
installed app or build. Update the URL scheme in Hex/Info.plist to a
bundle-specific namespaced value, and then update the bridge/client code that
emits the agent callback URL (for example, the code that currently opens
hex://agent-update) to use the same new scheme consistently.
In `@Hex/Views/AgentPanel.swift`:
- Around line 89-93: The origin clamping in the panel positioning logic can
still push the card off-screen when the card is larger than the visible frame,
because the min/max bounds invert in the current setFrameOrigin path. Update the
origin calculation in AgentPanel’s placement code to handle oversized frames
explicitly by computing the available area first and clamping against a valid
range, falling back to centering or pinning within the visible frame when
frame.width or frame.height exceeds the padded bounds. Keep the fix localized to
the origin.x/origin.y clamp logic so the setFrameOrigin behavior remains
consistent for normal-sized panels.
---
Outside diff comments:
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 304-323: The recording setup in TranscriptionFeature is starting
speech stop and recording in parallel because Effect.merge runs the child
effects concurrently. Update the logic around the recording start sequence so
speechSynthesizer.stop() completes before recording.startRecording() is invoked,
either by switching this effect chain to sequential composition with
.concatenate or by moving the stop call into the same .run block before the
recording begins. Keep the existing CancelID.recordingStart and sleep-management
behavior intact while ensuring the stop/start ordering is enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7870e78d-a890-4d35-b38c-0e4cfd1d5768
⛔ Files ignored due to path filters (3)
Hex/Assets.xcassets/IntegrationClaude.imageset/claude.pngis excluded by!**/*.pngHex/Assets.xcassets/IntegrationCodex.imageset/openai.pngis excluded by!**/*.pngHex/Assets.xcassets/IntegrationPi.imageset/pi.pngis excluded by!**/*.png
📒 Files selected for processing (22)
.changeset/agent-plugins.mdHex.xcodeproj/project.pbxprojHex/App/HexAppDelegate.swiftHex/Assets.xcassets/IntegrationClaude.imageset/Contents.jsonHex/Assets.xcassets/IntegrationCodex.imageset/Contents.jsonHex/Assets.xcassets/IntegrationPi.imageset/Contents.jsonHex/Clients/AgentIntegrationsClient.swiftHex/Clients/AgentTranscriptClient.swiftHex/Clients/ClaudePluginClient.swiftHex/Clients/PiPluginClient.swiftHex/Clients/SpeechSynthesizerClient.swiftHex/Features/Agent/AgentFeature.swiftHex/Features/Agent/AgentHookResponder.swiftHex/Features/Agent/AgentView.swiftHex/Features/Agent/SpokenText.swiftHex/Features/App/AppFeature.swiftHex/Features/Settings/AgentPluginsSectionView.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Transcription/TranscriptionFeature.swiftHex/Info.plistHex/Views/AgentPanel.swiftHexCore/Sources/HexCore/Settings/HexSettings.swift
Comment on lines
+160
to
+164
| private func handleHexURL(_ url: URL) { | ||
| appLogger.notice("Received hex URL: \(url.absoluteString, privacy: .public)") | ||
| guard url.host == "agent-update" else { | ||
| appLogger.notice("Ignoring unknown hex URL host: \(url.host ?? "nil", privacy: .public)") | ||
| return |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log the full agent URL as public.
Line 161 exposes cwd, transcript, and payload paths, plus project metadata, to public logs. Log only the host/event publicly and keep the full URL private or omit it. As per coding guidelines, “use privacy annotations ... for sensitive data like transcript text or file paths.”
Proposed logging change
- appLogger.notice("Received hex URL: \(url.absoluteString, privacy: .public)")
+ appLogger.notice("Received hex URL host=\(url.host ?? "nil", privacy: .public)")
+ appLogger.debug("Received full hex URL: \(url.absoluteString, privacy: .private)")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private func handleHexURL(_ url: URL) { | |
| appLogger.notice("Received hex URL: \(url.absoluteString, privacy: .public)") | |
| guard url.host == "agent-update" else { | |
| appLogger.notice("Ignoring unknown hex URL host: \(url.host ?? "nil", privacy: .public)") | |
| return | |
| private func handleHexURL(_ url: URL) { | |
| appLogger.notice("Received hex URL host=\(url.host ?? "nil", privacy: .public)") | |
| appLogger.debug("Received full hex URL: \(url.absoluteString, privacy: .private)") | |
| guard url.host == "agent-update" else { | |
| appLogger.notice("Ignoring unknown hex URL host: \(url.host ?? "nil", privacy: .public)") | |
| return |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/App/HexAppDelegate.swift` around lines 160 - 164, The logging in
handleHexURL(_:) is exposing sensitive URL contents publicly; update the initial
appLogger.notice call so only the host/event is logged with public privacy, and
keep the full url.absoluteString private or remove it from the log entirely.
Preserve the existing guard on url.host == "agent-update", but change the
received-URL message to avoid emitting cwd, transcript, payload, or metadata in
public logs, following the same privacy annotations used elsewhere in
HexAppDelegate.
Source: Coding guidelines
Comment on lines
+54
to
+90
| .init(latestPrompt: { payloadPath, transcriptPath in | ||
| // 1) Authoritative: the hook payload holds the CURRENT event + tool_input. | ||
| if let hook = readJSON(payloadPath) { | ||
| let event = hook["hook_event_name"] as? String ?? "" | ||
| let toolName = hook["tool_name"] as? String ?? "" | ||
| let input = hook["tool_input"] as? [String: Any] | ||
|
|
||
| if toolName == "AskUserQuestion", let input, let q = question(from: input) { | ||
| return .question(q) | ||
| } | ||
| if event == "PermissionRequest" { | ||
| return .permission(AgentPermission(tool: toolName, summary: permissionSummary(toolName, input))) | ||
| } | ||
| // Stop payloads carry the final text directly — no transcript parse needed. | ||
| if let last = hook["last_assistant_message"] as? String, !last.isEmpty { | ||
| return .message(last) | ||
| } | ||
| } | ||
|
|
||
| // 2) Fallback (Stop / Notification): the last assistant text from the transcript. | ||
| if let transcriptPath { | ||
| let expanded = (transcriptPath as NSString).expandingTildeInPath | ||
| if let contents = try? String(contentsOf: URL(fileURLWithPath: expanded), encoding: .utf8) { | ||
| let lines = contents.split(separator: "\n", omittingEmptySubsequences: true) | ||
| return .message(lastAssistantText(lines)) | ||
| } | ||
| } | ||
| return .message("") | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| private func readJSON(_ path: String?) -> [String: Any]? { | ||
| guard let path, !path.isEmpty else { return nil } | ||
| let expanded = (path as NSString).expandingTildeInPath | ||
| guard let data = try? Data(contentsOf: URL(fileURLWithPath: expanded)) else { return nil } | ||
| return (try? JSONSerialization.jsonObject(with: data)) as? [String: Any] |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Confine hook file reads to the expected agent directory.
payloadPath and transcriptPath are consumed as raw filesystem paths and passed straight into Data(contentsOf:) / String(contentsOf:). Those values cross the hex://agent-update integration boundary, so a buggy or malicious hook can point Hex at arbitrary readable files. Resolve against the allow-listed hooks/transcripts base directory, canonicalize symlinks, and reject anything that escapes it before reading.
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 75-75: A file is read from a path built from runtime/request input via FileManager.contents(atPath:), Data(contentsOf:), or String(contentsOfFile:). An attacker can supply '../' sequences or absolute paths to read files outside the intended directory (path traversal). Validate and canonicalize the path, reject '..' components, and confine reads to an allow-listed base directory (e.g. resolve with URL(fileURLWithPath:relativeTo:) and verify the resolved path is still inside the base) before reading.
Context: String(contentsOf: URL(fileURLWithPath: expanded), encoding: .utf8)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(path-traversal-file-read-request-input-swift)
[error] 88-88: A file is read from a path built from runtime/request input via FileManager.contents(atPath:), Data(contentsOf:), or String(contentsOfFile:). An attacker can supply '../' sequences or absolute paths to read files outside the intended directory (path traversal). Validate and canonicalize the path, reject '..' components, and confine reads to an allow-listed base directory (e.g. resolve with URL(fileURLWithPath:relativeTo:) and verify the resolved path is still inside the base) before reading.
Context: Data(contentsOf: URL(fileURLWithPath: expanded))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(path-traversal-file-read-request-input-swift)
🪛 SwiftLint (0.64.0)
[Warning] 54-54: Trailing closure syntax should be used whenever possible
(trailing_closure)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Clients/AgentTranscriptClient.swift` around lines 54 - 90, The hook file
reads in AgentTranscriptClient’s latestPrompt and readJSON currently trust
payloadPath and transcriptPath directly, which can escape the expected agent
directory. Update the path handling to resolve both inputs against the
allow-listed hooks/transcripts base directory, canonicalize any symlinks, and
verify the final resolved path stays inside that base before calling
Data(contentsOf:) or String(contentsOf:). Keep the existing latestPrompt flow
and readJSON helper, but add the path validation at the boundary where those raw
paths are consumed.
Source: Linters/SAST tools
Comment on lines
+125
to
+135
| private func lastAssistantText(_ lines: [Substring]) -> String { | ||
| var collected: [String] = [] | ||
| for line in lines.reversed() { | ||
| guard let obj = parseLine(line), let type = obj["type"] as? String else { continue } | ||
| if type == "assistant" { | ||
| if let text = textBlocks(obj), !text.isEmpty { collected.append(text) } | ||
| } else if type == "user", !collected.isEmpty { | ||
| break | ||
| } | ||
| } | ||
| return collected.reversed().joined(separator: "\n").trimmingCharacters(in: .whitespacesAndNewlines) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't walk past the latest user turn on empty assistant content.
If the newest assistant entries are tool-only or otherwise have no text blocks, collected stays empty, so Line 131 skips the delimiting user turn and the loop can return an older assistant reply from a previous turn. This fallback should stop at the first user event in the latest turn and return empty instead of surfacing stale text.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Clients/AgentTranscriptClient.swift` around lines 125 - 135, The
lastAssistantText helper is skipping the most recent user boundary when the
newest assistant messages have no text blocks, which can cause it to return
stale text from an older turn. Update lastAssistantText(_:), using parseLine and
textBlocks, so it always stops at the first user event encountered while
scanning backward and returns an empty string if no assistant text exists in the
latest turn. Ensure the break condition does not depend on collected having
content.
Comment on lines
+301
to
+310
| const DANGEROUS_BASH = [ | ||
| /\brm\s+(-rf?|--recursive)/i, | ||
| /\bsudo\b/i, | ||
| /\b(chmod|chown)\b.*777/i, | ||
| ]; | ||
|
|
||
| function isDangerousBash(toolName: string, input: any): boolean { | ||
| if (toolName !== "bash") return false; | ||
| const cmd = (input?.command ?? "") as string; | ||
| return DANGEROUS_BASH.some((p) => p.test(cmd)); |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Catch common destructive rm flag variants.
Line 302 misses rm -fr ... and split/combined variants, so pi can run a destructive recursive remove without showing the permission card.
Proposed regex tightening
const DANGEROUS_BASH = [
- /\brm\s+(-rf?|--recursive)/i,
+ /\brm\b(?=[^;&|\n]*\s-(?:[A-Za-z]*r|-[^\s]*recursive))/i,
/\bsudo\b/i,
/\b(chmod|chown)\b.*777/i,
];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Clients/PiPluginClient.swift` around lines 301 - 310, The
destructive-command detection in isDangerousBash is missing common recursive rm
flag variants, so expand the DANGEROUS_BASH pattern used by isDangerousBash to
catch combined and split forms such as rm -fr and similar permutations. Update
the regex in the DANGEROUS_BASH array so bash commands passed through
PiPluginClient are flagged consistently before execution.
Comment on lines
+611
to
+615
| case let .setAgentDistinctSessionVoices(enabled): | ||
| state.$hexSettings.withLock { $0.agentDistinctSessionVoices = enabled } | ||
| // Warm the Kokoro model so the extra voices are ready the first time a second | ||
| // project speaks (one model covers every voice, so this preloads them all). | ||
| return enabled && !state.kokoroReady ? .send(.prepareKokoro) : .none |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't auto-preview after a warm-up-only Kokoro prepare.
.setAgentDistinctSessionVoices uses .prepareKokoro just to preload the model, but .kokoroPrepared(success:) always follows with .previewAgentVoice. Turning on “Distinct Voice per Project” will therefore read the sample aloud even though the user never asked for a preview. Track whether the prepare was initiated by preview vs. preload and only auto-preview for the former.
Also applies to: 652-659
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Features/Settings/SettingsFeature.swift` around lines 611 - 615, The
Kokoro warm-up flow is auto-triggering a voice preview even when .prepareKokoro
was started only to preload for .setAgentDistinctSessionVoices. Update
SettingsFeature’s handling of .prepareKokoro and .kokoroPrepared(success:) to
track whether the request came from a preview action versus a warm-up/preload,
and only send .previewAgentVoice for preview-originated prepares. Use the
existing state/actions around .setAgentDistinctSessionVoices and .prepareKokoro
to store and check this flag so enabling Distinct Voice per Project does not
speak the sample unexpectedly.
Comment on lines
+636
to
+649
| return .run { send in | ||
| do { | ||
| try await speechSynthesizer.prepareKokoro { progress in | ||
| Task { await send(.kokoroPrepareProgress(progress)) } | ||
| } | ||
| await send(.kokoroPrepared(success: true)) | ||
| } catch { | ||
| settingsLogger.error("Kokoro model download failed: \(error.localizedDescription)") | ||
| await send(.kokoroPrepared(success: false)) | ||
| } | ||
| } | ||
|
|
||
| case let .kokoroPrepareProgress(progress): | ||
| state.kokoroDownloadProgress = progress |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Late progress callbacks can leave Kokoro stuck in a fake downloading state.
Each progress event is forwarded through Task { await send(...) }, so a queued .kokoroPrepareProgress can arrive after .kokoroPrepared. On a failed prepare, that can set kokoroDownloadProgress back to non-nil, and later .prepareKokoro calls will hit the “already downloading” guard forever. Tie progress events to the active prepare cycle, or ignore them once completion has been handled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Features/Settings/SettingsFeature.swift` around lines 636 - 649, The
Kokoro prepare flow in SettingsFeature’s prepareKokoro logic can leave state
stuck because queued .kokoroPrepareProgress events may arrive after
.kokoroPrepared has already completed. Update the .kokoroPrepareProgress
handling to only apply progress for the active prepare cycle, or ignore late
callbacks once the operation has finished, and make sure the completion path in
the prepareKokoro effect clears/invalidates any in-flight progress state so
later calls do not trip the “already downloading” guard.
| } | ||
| await send(.kokoroPrepared(success: true)) | ||
| } catch { | ||
| settingsLogger.error("Kokoro model download failed: \(error.localizedDescription)") |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Mark the Kokoro failure text as private.
error.localizedDescription can include user-specific paths from the on-device model install flow, so it should not be logged publicly. As per coding guidelines, use the unified logging helper HexLog with privacy annotations for sensitive data like transcript text or file paths.
Suggested fix
- settingsLogger.error("Kokoro model download failed: \(error.localizedDescription)")
+ settingsLogger.error("Kokoro model download failed: \(error.localizedDescription, privacy: .private)")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| settingsLogger.error("Kokoro model download failed: \(error.localizedDescription)") | |
| settingsLogger.error("Kokoro model download failed: \(error.localizedDescription, privacy: .private)") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Features/Settings/SettingsFeature.swift` at line 643, The Kokoro download
failure log is exposing potentially sensitive user-specific path data via
error.localizedDescription. Update the error handling in SettingsFeature’s
Kokoro download flow to use HexLog instead of settingsLogger, and mark the
dynamic error details as private in the log message so any transcript/path
information is not publicly emitted.
Source: Coding guidelines
Comment on lines
+11
to
+21
| <key>CFBundleURLTypes</key> | ||
| <array> | ||
| <dict> | ||
| <key>CFBundleURLName</key> | ||
| <string>com.kitlangton.Hex.agent</string> | ||
| <key>CFBundleURLSchemes</key> | ||
| <array> | ||
| <string>hex</string> | ||
| </array> | ||
| </dict> | ||
| </array> |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Use a namespaced URL scheme instead of hex.
Line 18 registers a very generic custom scheme for the agent callback path. Since the bridges open hex://agent-update, any other installed app — including a separate Hex Debug/Release build — can win that scheme registration and receive the payload first. That makes reply delivery nondeterministic and can leak agent prompt/session data across apps. Use a bundle-specific scheme and update the bridge clients to emit that scheme instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Info.plist` around lines 11 - 21, The custom URL scheme registered in
CFBundleURLTypes is too generic, which can cause agent callback routing to be
intercepted by another installed app or build. Update the URL scheme in
Hex/Info.plist to a bundle-specific namespaced value, and then update the
bridge/client code that emits the agent callback URL (for example, the code that
currently opens hex://agent-update) to use the same new scheme consistently.
Comment on lines
+89
to
+93
| var origin = NSPoint(x: center.x - frame.width / 2, y: center.y - frame.height / 2) | ||
| // Safety net: never let any part of the card fall off-screen (tiny displays / tall card). | ||
| origin.x = min(max(origin.x, vf.minX + 8), vf.maxX - frame.width - 8) | ||
| origin.y = min(max(origin.y, vf.minY + 8), vf.maxY - frame.height - 8) | ||
| setFrameOrigin(origin) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle panels larger than the visible frame when clamping.
When frame.width or frame.height exceeds the visible frame minus padding, vf.maxX - frame.width - 8 becomes smaller than vf.minX + 8, so the current clamp can still push the card off-screen.
Proposed clamp fix
var origin = NSPoint(x: center.x - frame.width / 2, y: center.y - frame.height / 2)
// Safety net: never let any part of the card fall off-screen (tiny displays / tall card).
- origin.x = min(max(origin.x, vf.minX + 8), vf.maxX - frame.width - 8)
- origin.y = min(max(origin.y, vf.minY + 8), vf.maxY - frame.height - 8)
+ let minX = vf.minX + 8
+ let maxX = max(minX, vf.maxX - frame.width - 8)
+ let minY = vf.minY + 8
+ let maxY = max(minY, vf.maxY - frame.height - 8)
+ origin.x = min(max(origin.x, minX), maxX)
+ origin.y = min(max(origin.y, minY), maxY)
setFrameOrigin(origin)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var origin = NSPoint(x: center.x - frame.width / 2, y: center.y - frame.height / 2) | |
| // Safety net: never let any part of the card fall off-screen (tiny displays / tall card). | |
| origin.x = min(max(origin.x, vf.minX + 8), vf.maxX - frame.width - 8) | |
| origin.y = min(max(origin.y, vf.minY + 8), vf.maxY - frame.height - 8) | |
| setFrameOrigin(origin) | |
| var origin = NSPoint(x: center.x - frame.width / 2, y: center.y - frame.height / 2) | |
| // Safety net: never let any part of the card fall off-screen (tiny displays / tall card). | |
| let minX = vf.minX + 8 | |
| let maxX = max(minX, vf.maxX - frame.width - 8) | |
| let minY = vf.minY + 8 | |
| let maxY = max(minY, vf.maxY - frame.height - 8) | |
| origin.x = min(max(origin.x, minX), maxX) | |
| origin.y = min(max(origin.y, minY), maxY) | |
| setFrameOrigin(origin) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Views/AgentPanel.swift` around lines 89 - 93, The origin clamping in the
panel positioning logic can still push the card off-screen when the card is
larger than the visible frame, because the min/max bounds invert in the current
setFrameOrigin path. Update the origin calculation in AgentPanel’s placement
code to handle oversized frames explicitly by computing the available area first
and clamping against a valid range, falling back to centering or pinning within
the visible frame when frame.width or frame.height exceeds the padded bounds.
Keep the fix localized to the origin.x/origin.y clamp logic so the
setFrameOrigin behavior remains consistent for normal-sized panels.
Add a per-card compress/expand toggle next to the mute button that switches read-aloud, and the card's displayed text, between the full reply and a short condensed summary. The summary is generated by the Claude Code hook only while read-aloud is on: on Stop it runs `claude --safe-mode -p --model haiku` headlessly to condense the assistant's last message and folds the result into the payload as hex_condensed. --safe-mode disables hooks/MCP/plugins (no recursion, clean run) while preserving OAuth auth; a HEX_AGENT_SUMMARY env guard at the top of the hook is a second layer against recursion. Generation is gated on a new read-aloud sentinel synced from agentSpeakOutput, so no model call happens when read-aloud is off.
Reword the condensation prompt the Stop hook sends to headless Claude so the summary surfaces what the user needs to pay attention to (key outcome, blockers, risks, surprises, and any question) rather than a neutral recap, and drop the explicit one-or-two-sentence word cap in favor of the 'spoken heads-up' framing.
…nches New agent cards inherit the remembered condensed read-aloud setting, and toggling a card's condensed button updates the stored default, so the choice survives later prompts and app launches.
The Stop hook shells out to headless Claude to condense the reply for the condensed read-aloud, which costs tokens on every turn. Until now that ran whenever read-aloud was on, even when the user preferred the full reply, so the full-reply mode still paid for a summary nobody heard. Gate the read-aloud summary sentinel on the condensed preference too: turning condensed off removes the sentinel, so the hook spends no model call and the voice window reads the full reply. The condensed toggle is now shown whenever read-aloud is on (not only when a summary already exists) so it works as a real on/off switch, and flipping it re-syncs the sentinel immediately.
Toggling condensed on the card can never retroactively condense the reply already on screen: the summary is generated by the agent Stop hook before the card appears, so a card produced while condensed was off has nothing to switch to. Gating generation on the per-card toggle therefore made enabling condensed silently apply only from the next turn. Split the concern instead. A new global "Condensed Read-Aloud" setting gates whether the hook generates a summary at all (the token cost). With it on, every turn is summarized, so the per-card toggle flips between summary and full reply instantly. With it off, nothing is generated and replies are read in full for free. The per-card toggle reverts to a pure live switch and only shows when the global setting is on.
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.
Adds Agent Plugins — an opt-in voice window that lets a coding agent reach you when it needs you, wherever you are. When Claude Code or pi finishes a turn, asks a multiple-choice question, or requests a permission, a small floating card appears. You answer by speaking, typing, or tapping an option, and the reply lands back in the exact session that asked. It's off by default and turned on per agent in Settings → Agent Plugins.
It handles several agents at once: when more than one project is blocked, the cards queue and a header selector of project avatars (the repo's GitHub owner) lets you switch between the ones waiting — each card showing the project name and its current git branch.
It hooks into the moments an agent genuinely needs you — finishing a turn, asking a multiple-choice question, or requesting a permission — and renders each as the right kind of card. A multiple-choice question (Claude Code's AskUserQuestion, pi's
ask_usertool) becomes tappable options you can pick by voice or click:What you get
kokoro-ane, follows once it's supported.)Sandboxing
Hex runs under the App Sandbox and generates the hook/extension plus an installer inside its own container. Settings → Agent Plugins shows a one-time copy-paste command you run yourself to register it; a thin stub in the agent's config dir
execs the real hook from the container, so Hex keeps the hook current on launch without you re-running anything. The privileged reads the app can't do — the agent's last message, the project's GitHub owner from git — are done by the unsandboxed hook and passed in. The IPC rendezvous lives in the container, so it works under local ad-hoc signing with no App Group (an App Group is the planned form for the eventual upstream PR).Summary by CodeRabbit
hex://agent-updatedeep link handling.