Skip to content

Commit ad7b16f

Browse files
TaylorWatsonTaylor Watsonengels74
authored
fix: resolve Allow/Deny buttons not triggering CLI approval (#106)
* fix: resolve Allow/Deny buttons not triggering CLI approval (farouqaldori#98) Four fixes addressing the permission approval flow not reaching the CLI: 1. Generate fallback UUID when tool_use_id is missing from PermissionRequest events, preventing silent socket closure that left UI buttons unable to send approval back to the CLI. 2. Increase socket read window from 200ms to 2s with early exit on complete JSON, preventing truncated reads on large tool_input payloads. 3. Flush stdout in Python hook script to ensure hookSpecificOutput JSON reaches Claude Code without buffering delays. 4. Guard against race condition where Stop/SessionEnd events clean up permissions before the user has time to respond (2s grace period). Also includes enhanced diagnostic logging (Swift .warning level for failures, conditional stderr logging in Python via CLAUDE_ISLAND_DEBUG=1) and fixes for @inline(always) -> @inline(__always) and .signaled -> .unhandledException build compatibility issues. Closes farouqaldori#98 * fix: resolve CI failures before code review (PR #106) - Revert TerminationStatus case from `.unhandledException` to `.signaled` to match swift-subprocess 0.4 API (CI resolves latest compatible version) - Bump swift-subprocess minimum version constraint from 0.2.0 to 0.4.0 to ensure consistent API resolution locally and in CI * fix: address code review comments (PR #106, iteration 1) - readClientData: remove 10ms quiet-period early break and require complete-JSON validation (looksLikeCompleteJSON) before exiting the poll loop; widen poll slice to 100ms within the existing 2s cap. Prevents truncation of multi-packet hook payloads that could reintroduce the "Processing…" hang from farouqaldori#98. - sendPermissionResponse + sendPermissionResponseBySession: introduce writeAllBytes helper that loops until all bytes are sent, retries on EINTR, and waits for POLLOUT on EAGAIN/EWOULDBLOCK within a 2s deadline. permissionFailureHandler is now only invoked on truly unrecoverable writes, eliminating spurious failures on partial writes to the non-blocking socket. * fix: address code review comments (PR #106, iteration 2) - claude-island-state.py send_event: replace single sock.recv(4096) with an EOF-accumulating loop so permission responses that span multiple kernel reads (normal on SOCK_STREAM; guaranteed when the `reason` field pushes the JSON past 4KB) are decoded correctly. The Swift side (writeAllBytes + close) uses EOF as the terminator, so recv-until-zero is the authoritative contract. Eliminates the silent fallback to "ask" that caused intermittent Allow/Deny loss. * fix: address code review comments (PR #106, iteration 3) - claude-island-state.py send_event: include UnicodeDecodeError in the except tuple around response.decode()/json.loads. Without it, a mid-multibyte UTF-8 truncation would crash the hook process and skip the intended None → "{}"/"ask" fallback in handle_permission_response. * fix: address code review comments (PR #106, iteration 4) - HookSocketServer.acceptConnection: update stale "200ms" comment to reflect readClientData's current 2s cap, so the accept-queue head-of-line risk is accurately documented. * fix: address code review comments (PR #106, iteration 5) - cleanupPendingPermissions: schedule a follow-up cleanup for entries skipped under the <2s age guard (via queue.asyncAfter) so Stop events on brand-new permissions no longer leave the Python hook blocked on its 300s recv. Retry atomically removes the entry only if the toolUseID still maps to the same sessionID, guarding against session reuse and double-close races. - readClientData poll loop: explicitly handle POLLHUP/POLLERR/POLLNVAL to exit promptly on peer close or invalid fd; previously the loop spun until the 2s cap when poll returned >0 without POLLIN. POLLIN is still checked first so pending data is drained before exit. - writeAllBytes: assign EIO to finalErrno when write() returns 0 (POSIX doesn't guarantee errno is meaningful in that case), so logs and diagnostics show a stable code instead of a stale/0 value. Move the errno capture for negative returns into the n<0 branch where it's defined to be valid. * fix: address code review comments (PR #106, iteration 6) - readClientData: retry read() on EINTR instead of treating it as a fatal error. Symmetric with the existing poll() EINTR handling; prevents signal-induced truncation of multi-packet JSON payloads. - cleanupPendingPermissionsRetry: stop invoking permissionFailureHandler. Stop hook events are authoritative for session phase (drive to .idle via determinePhase); letting the deferred retry fire the handler ~2s later could override .idle back to .waitingForApproval via SessionStore.processSocketFailure's findNextPendingTool. Both cleanup paths (main + retry) are now silent FD-close, consistent with Stop owning phase management. The handler still fires from the genuine failure sites: handlePermissionSocketDisconnect, cleanupTimedOutPermission, and sendPermissionResponse* write-failure branches. * fix: address code review comments (PR #106, iteration 7) - cleanupPendingPermissions main loop: call markPermissionResponded alongside the pendingPermissions removal, matching all other removal sites (cleanupPendingPermissionsRetry, cleanupSpecificPermission, handlePermissionSocketDisconnect, sendPermissionResponse*). Late approve/deny responses for a Stop-cancelled session now classify as .alreadyResponded (quiet) instead of .notFound (warning), and dedupe behavior is consistent across all paths. * fix: address code review comments (PR #106, iteration 8) - claude-island-state.py send_event: make the debug log path resilient to malformed bytes by using response.decode(errors='replace') in the _log f-string (which is evaluated eagerly regardless of DEBUG flag). Pass response bytes directly to json.loads, which handles UTF-8/16/32 BOM detection itself, eliminating the redundant strict decode. Debug mode can no longer perturb functional parsing. The except tuple with UnicodeDecodeError is retained as a defensive net. --------- Co-authored-by: Taylor Watson <taylor.watson@soti.net> Co-authored-by: engels74 <141435164+engels74@users.noreply.github.com>
1 parent 2e05a9f commit ad7b16f

5 files changed

Lines changed: 245 additions & 67 deletions

File tree

ClaudeIsland.xcodeproj/project.pbxproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@
386386
repositoryURL = "https://github.com/swiftlang/swift-subprocess";
387387
requirement = {
388388
kind = upToNextMajorVersion;
389-
minimumVersion = 0.2.0;
389+
minimumVersion = 0.4.0;
390390
};
391391
};
392392
FD33FD712EDF32D7002A6548 /* XCRemoteSwiftPackageReference "swift-markdown" */ = {

ClaudeIsland/Core/NotchGeometry.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ struct NotchGeometry: Sendable {
3838
}
3939

4040
/// Check if a point is in the notch area (with padding for easier interaction)
41-
@inline(always)
41+
@inline(__always)
4242
func isPointInNotch(_ point: CGPoint) -> Bool {
4343
self.notchScreenRect.insetBy(dx: -10, dy: -5).contains(point)
4444
}
@@ -49,7 +49,7 @@ struct NotchGeometry: Sendable {
4949
}
5050

5151
/// Check if a point is outside the opened panel (for closing)
52-
@inline(always)
52+
@inline(__always)
5353
func isPointOutsidePanel(_ point: CGPoint, size: CGSize) -> Bool {
5454
!self.openedScreenRect(for: size).contains(point)
5555
}

ClaudeIsland/Models/SessionPhase.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ nonisolated enum SessionPhase: Sendable {
9797
// swiftlint:disable attributes
9898

9999
/// Whether this phase indicates the session needs user attention
100-
@inline(always) var needsAttention: Bool {
100+
@inline(__always) var needsAttention: Bool {
101101
switch self {
102102
case .waitingForApproval,
103103
.waitingForInput:
@@ -108,7 +108,7 @@ nonisolated enum SessionPhase: Sendable {
108108
}
109109

110110
/// Whether this phase indicates active processing
111-
@inline(always) var isActive: Bool {
111+
@inline(__always) var isActive: Bool {
112112
switch self {
113113
case .processing,
114114
.compacting:
@@ -119,7 +119,7 @@ nonisolated enum SessionPhase: Sendable {
119119
}
120120

121121
/// Whether this is a waitingForApproval phase
122-
@inline(always) nonisolated var isWaitingForApproval: Bool {
122+
@inline(__always) nonisolated var isWaitingForApproval: Bool {
123123
if case .waitingForApproval = self {
124124
return true
125125
}

ClaudeIsland/Resources/claude-island-state.py

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@
3838
from pathlib import Path
3939
from typing import NotRequired, TypedDict, TypeIs, cast
4040

41+
_DEBUG = os.environ.get("CLAUDE_ISLAND_DEBUG") == "1"
42+
43+
44+
def _log(msg: str, /) -> None:
45+
"""Log to stderr when CLAUDE_ISLAND_DEBUG=1. Does not affect stdout JSON."""
46+
if _DEBUG:
47+
print(f"[claude-island] {msg}", file=sys.stderr, flush=True)
48+
4149

4250
# TypedDict definitions for JSON structures
4351
class HookEventData(TypedDict, total=False):
@@ -349,15 +357,37 @@ def send_event(state: SessionState, /) -> PermissionResponse | None:
349357
with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
350358
sock.settimeout(CONNECT_TIMEOUT_SECONDS)
351359
sock.connect(str(SOCKET_PATH))
352-
sock.sendall(json.dumps(state.to_dict()).encode())
360+
_log(f"connected to {SOCKET_PATH}")
361+
payload = json.dumps(state.to_dict()).encode()
362+
sock.sendall(payload)
363+
_log(
364+
f"sent {len(payload)} bytes (event={state.event}, status={state.status})"
365+
)
353366
if state.status == "waiting_for_approval":
354367
sock.settimeout(PERMISSION_RECV_TIMEOUT_SECONDS)
355-
if response := sock.recv(4096):
356-
parsed = cast(object, json.loads(response.decode()))
368+
_log("waiting for permission response...")
369+
# Accumulate bytes until the Swift side closes the socket (EOF).
370+
# SOCK_STREAM may split the JSON payload across multiple recv()
371+
# calls, and longer `reason` strings can exceed a single 4KB
372+
# chunk. Swift's sendPermissionResponse always close(2)s the
373+
# client fd after writing, so EOF is the authoritative terminator.
374+
chunks: list[bytes] = []
375+
while chunk := sock.recv(4096):
376+
chunks.append(chunk)
377+
response = b"".join(chunks)
378+
if response:
379+
_log(
380+
f"received {len(response)} bytes: {response.decode(errors='replace')}"
381+
)
382+
parsed = cast(object, json.loads(response))
357383
if is_permission_response(parsed):
358384
return parsed
385+
_log(f"response failed validation: {parsed}")
386+
else:
387+
_log("empty response (socket closed)")
359388
return None
360-
except OSError, json.JSONDecodeError:
389+
except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc:
390+
_log(f"error: {exc}")
361391
return None
362392

363393

@@ -461,7 +491,7 @@ def handle_permission_response(response: PermissionResponse | None, /) -> None:
461491
"""
462492
if not response:
463493
# No response or "ask" - let Claude Code show its normal UI
464-
print("{}")
494+
print("{}", flush=True)
465495
return
466496

467497
decision = response.get("decision", "ask")
@@ -475,7 +505,8 @@ def handle_permission_response(response: PermissionResponse | None, /) -> None:
475505
"decision": {"behavior": "allow"},
476506
}
477507
}
478-
print(json.dumps(output))
508+
_log(f"allow -> {json.dumps(output)}")
509+
print(json.dumps(output), flush=True)
479510
sys.exit(0)
480511

481512
case "deny":
@@ -488,12 +519,14 @@ def handle_permission_response(response: PermissionResponse | None, /) -> None:
488519
},
489520
}
490521
}
491-
print(json.dumps(output))
522+
_log(f"deny -> {json.dumps(output)}")
523+
print(json.dumps(output), flush=True)
492524
sys.exit(0)
493525

494526
case _decision:
495527
# "ask" or unknown - let Claude Code show its normal UI
496-
print("{}")
528+
_log(f"unknown decision: {_decision}")
529+
print("{}", flush=True)
497530

498531

499532
def main() -> None:
@@ -514,9 +547,13 @@ def main() -> None:
514547
# Determine status early (pure computation, no I/O)
515548
status, extras = determine_status(event, data)
516549

550+
_log(
551+
f"event={event} session={session_id[:8]} status={status} has_tool_use_id={'tool_use_id' in extras}"
552+
)
553+
517554
# Skip certain events (e.g. stale PreToolUse registration)
518555
if status == "skip":
519-
print("{}")
556+
print("{}", flush=True)
520557
sys.exit(0)
521558

522559
# Resolve PID, TTY, build state
@@ -545,7 +582,7 @@ def main() -> None:
545582
if status == "waiting_for_approval":
546583
handle_permission_response(response)
547584
else:
548-
print("{}")
585+
print("{}", flush=True)
549586

550587

551588
if __name__ == "__main__":

0 commit comments

Comments
 (0)