Skip to content

Conversation

@vanpelt
Copy link
Collaborator

@vanpelt vanpelt commented Dec 21, 2025

Summary

  • Add Siri App Intents for voice-controlled interaction with Claude Code agents
    • SendPromptIntent: "Send a prompt to Claude in Catnip"
    • CheckStatusIntent: "What's Claude working on in Catnip"
  • Add push notification infrastructure for async Claude responses
    • APNs integration via jose JWT signing
    • Device token storage in CodespaceStore
    • Background processing with waitUntil pattern
  • Add Siri endpoints to Cloudflare Worker
    • POST /v1/siri/prompt - queues prompt, returns immediately, sends push when done
    • GET /v1/siri/status - returns workspace status for voice feedback

Architecture

iOS App Intents → Cloudflare Worker → Codespace → Claude Code

APNs Push Notification → iOS

Test Plan

  • Worker builds: pnpm typecheck:worker
  • iOS builds: just build (from xcode directory)
  • Push notification received when testing on device
  • "Hey Siri, send a prompt to Claude in Catnip" works
  • "Hey Siri, what's Claude working on in Catnip" works
  • Tapping notification opens correct workspace

Configuration Required

APNs secrets must be configured in Cloudflare:

  • APNS_AUTH_KEY - .p8 file contents
  • APNS_KEY_ID - 10-character Key ID
  • APNS_TEAM_ID - Apple Team ID
  • APNS_BUNDLE_ID - com.wandb.catnip

🤖 Generated with Claude Code

@github-actions
Copy link

Code Review: Siri Integration with Push Notifications

Thank you for this well-structured PR! The Siri integration with push notifications is a great addition. Here's my feedback:


Strengths

  1. Clean Architecture: The flow from iOS App Intents → Cloudflare Worker → Codespace → Claude Code is well-designed and clearly documented
  2. Good Error Handling: Comprehensive user-facing error messages in all failure scenarios (no codespace, expired credentials, unreachable codespace, etc.)
  3. Proper Async Pattern: Using waitUntil for background processing is the correct approach for Cloudflare Workers
  4. Security: Using APNs JWT authentication with proper key management via environment variables
  5. User Experience: Immediate response to Siri with push notification when complete is good UX

🔴 Critical Issues

1. Polling Loop in Background Task Violates Worker Limits (worker/index.ts:582-622)

The processSiriPrompt function polls for up to 2 minutes (120 seconds) in a background task. Cloudflare Workers have hard limits:

  • CPU time limit: 50ms for free tier, 50-500ms for paid
  • Execution time: 30 seconds max with waitUntil, but CPU time is still limited

Problem: The polling loop with setTimeout calls will exceed CPU limits and likely fail.

Recommendation: Use a different architecture:

  • Option A: Use Cloudflare Durable Objects with WebSocket connections to get notified when Claude finishes
  • Option B: Use Cloudflare Queues to defer the polling work and retry
  • Option C: Have the codespace/container push a completion webhook back to the worker when Claude finishes
  • Option D: Accept the limitation and send the notification immediately with a generic "Claude is working on it" message

2. Device Token Not Retrieved in Background Processing (worker/index.ts:1257-1270)

In /v1/siri/prompt, the device token is stored but then immediately passed to processSiriPrompt. However, if the device token isn't provided in the initial request, the background task will have undefined and won't be able to send notifications.

Recommendation: In processSiriPrompt, if deviceToken is not provided, fetch it from storage first.


⚠️ High Priority Issues

3. No Timeout on Network Requests in Polling Loop (worker/index.ts:591-594)

The session status fetch inside the polling loop has no timeout. If the codespace becomes unresponsive, each poll could hang for a long time, compounding the CPU time issue.

Recommendation: Add a timeout signal like the status endpoint does (worker/index.ts:1313):

signal: AbortSignal.timeout(5000)

4. Incomplete Error Handling in Polling Loop (worker/index.ts:596-621)

If sessionResponse.ok is false, the code silently continues polling. After 2 minutes of failures, it sends a "Claude is still working" message, which is misleading.

Recommendation: Track consecutive failures and bail out if the endpoint is consistently unreachable.

5. Missing Input Validation (worker/index.ts:1246)

The prompt is not validated for length or content. Very long prompts could cause issues.

Recommendation: Add validation for prompt length (e.g., max 10000 characters).


🟡 Medium Priority Issues

6. Hardcoded 1-Second Delay (worker/index.ts:553)

This is wasteful in the worker environment and might not be necessary. Consider removing it or making it much shorter.

7. JSON Serialization Error Not Handled (xcode/catnip/Intents/SendPromptIntent.swift:46)

Using try? silently ignores errors. If serialization fails, the request body will be nil, leading to a confusing 400 error from the server.

8. No Rate Limiting on Siri Endpoints

Users could potentially spam Siri requests. Consider adding rate limiting per user.

9. Device Token Never Expires (worker/codespace-store.ts:279-282)

Device tokens have an updatedAt field but it's never checked. Old/invalid tokens should be cleaned up when APNs returns token errors.


🔵 Low Priority / Nice to Have

10. No Tests for New Functionality

The PR adds significant new code but no tests. Consider adding:

  • Unit tests for sendPushNotification in worker/apns.ts
  • Unit tests for processSiriPrompt (with mocked fetch)
  • Integration tests for the /v1/siri/prompt and /v1/siri/status endpoints
  • iOS unit tests for the App Intents

11. TypeScript Type Safety Could Be Improved

Several places use as type assertions that could fail at runtime. Consider using Zod or a similar library for runtime validation.

12. Workspace Selection Logic (worker/index.ts:537-539)

The code selects the "most recent workspace" by taking the first item from the workspaces array. This assumes the backend sorts by last accessed. Add a comment explaining this assumption or add explicit sorting logic.

13. APNs Production Endpoint Only (worker/apns.ts:44)

The code hardcodes the production endpoint. During development, you might want to use the sandbox endpoint. Make this configurable via environment variable.


🎯 Summary

This is a well-designed feature with good separation of concerns. However, the polling loop in the background worker task is a critical architectural issue that will likely cause failures in production due to Cloudflare Worker CPU limits.

Recommended Next Steps:

  1. ✅ Fix the polling loop architecture (critical)
  2. ✅ Fix device token retrieval in background task (critical)
  3. ✅ Add timeout to polling requests (high priority)
  4. ✅ Add error handling for consecutive failures (high priority)
  5. ✅ Add input validation (high priority)
  6. Consider adding tests
  7. Consider adding rate limiting

Great work overall! Let me know if you'd like help implementing any of these suggestions.

Claude and others added 6 commits December 20, 2025 20:53
Address PR review feedback with several improvements:

- Replace Worker polling loop with Cloudflare Queue-based approach
  to avoid violating Worker CPU time limits
- Fetch device token from storage if not provided in request
- Add AbortSignal.timeout to all fetch calls
- Add max poll count (20) and max time (90s) limits with backoff
- Add input validation for prompt length (max 10000 chars)
- Fix Swift JSON serialization error handling

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Match the iOS app behavior - use workspace.name (e.g., "feature-api-docs")
instead of workspace.path for the PTY session query parameter.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Increase SIRI_MAX_POLL_TIME_MS from 2 minutes to 5 minutes
- Remove redundant SIRI_MAX_POLL_COUNT check (time-based is sufficient)
- Log elapsed time instead of poll count on timeout

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add SIRI_POLL_INTERVAL_SECONDS constant (10 seconds) to control
the delay between queue polling invocations. Previously hardcoded
to 3-5 seconds in various places.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ties

Remove Siri App Intents feature while preserving APNs push notifications
and Live Activity support for codespace creation progress tracking.

Removed:
- Siri intent files (SendPromptIntent, CheckStatusIntent, AppShortcuts)
- /v1/siri/prompt and /v1/siri/status endpoints
- Siri prompt queue and polling logic
- Siri entitlement from app capabilities

Kept:
- APNs push notification infrastructure (worker/apns.ts)
- Live Activity registration and progress updates
- Device token storage for future notification use
- Production APNs environment in entitlements

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Disable runsForEachTargetApplicationUIConfiguration to prevent
  testLaunch from running for every UI configuration (was causing
  timeouts in CI)
- Move notification permission request from app launch to when user
  actually installs Catnip or creates a codespace (prevents dialog
  from blocking UI tests)
- Simplify CodespaceButtonTests to look for mock repository names
  instead of section headers (more reliable in accessibility tree)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants