fix(rpc): do not block startup on undetermined Contacts auth - #187
fix(rpc): do not block startup on undetermined Contacts auth#187SebTardif wants to merge 4 commits into
Conversation
|
Codex review: needs changes before merge. Reviewed July 26, 2026, 9:23 PM ET / July 27, 2026, 01:23 UTC. ClawSweeper reviewWhat this changesThis PR makes Merge readinessKeep this PR open as the active candidate fix for the headless RPC startup hang described by #186. The scoped noninteractive policy preserves the interactive Contacts prompt path that the earlier review required; the remaining concrete cleanup is removing the release-owned Priority: P1 Review scores
Verification
How this fits together
flowchart LR
A[RPC stdin and stdout] --> B[RPC startup]
B --> C{Is stdin a TTY?}
C -->|Interactive| D[Request Contacts access if needed]
C -->|Headless| E[Use unavailable Contacts resolver]
D --> F[JSON-RPC server]
E --> F
F --> G[Chat history and send methods]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Land the scoped headless fail-open policy with its two-mode tests after removing the Do we have a high-confidence way to reproduce the issue? No current-main live reproduction was run in this review. The supplied report and PR body provide a concrete macOS non-TTY reproduction path, and the PR head directly targets that startup policy branch. Is this the best way to solve the issue? Yes, with one release-process cleanup. Limiting the non-blocking resolver to noninteractive stdin is the narrowest approach because it fixes LaunchAgent and pipe startup while retaining interactive Contacts authorization behavior. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d12312276d20. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
|
@clawsweeper re-review Added live headless |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
The ClawSweeper review raises a fair point: making Two options:
@cemendes, since you introduced the Contacts policy split in #136, could you weigh in on which approach fits the intended design? Happy to implement whichever direction you prefer. |
|
@clawsweeper re-review Scoped fail-open Contacts policy to noninteractive stdin (isatty). Interactive TTY keeps requestIfNeeded. Policy unit tests added for both modes. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
imsg rpc used ContactResolver.create() with the default requestIfNeeded policy, so a headless LaunchAgent could hang forever waiting on CNContactStore.requestAccess before the JSON-RPC server started. Use skipIfNotDetermined (same as chats/history) so RPC starts with optional name enrichment disabled until Contacts is already authorized. Closes openclaw#186 Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
ClawSweeper P1: unconditional skipIfNotDetermined changed the RPC authorization contract for interactive callers. Keep requestIfNeeded when stdin is a TTY; use skipIfNotDetermined only for headless pipes/LaunchAgents. Adds pure policy tests for both modes. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
3600199 to
6e36348
Compare
Unconditional import Darwin breaks linux-read-core (no Darwin module). Match SecurePath and use canImport Darwin/Glibc for isatty/STDIN_FILENO. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
Linux CI regression fixed: unconditional
|
Update (2026-07-26)
Scoped Contacts fail-open to noninteractive stdin only (ClawSweeper P1):
stdinnot a TTY →.skipIfNotDetermined(headless LaunchAgent/pipe).requestIfNeeded(preserve prompt path)Tests:
RpcCommandContactsPolicyTests(headless + interactive).What Problem This Solves
imsg rpcinitializes Contacts before starting the JSON-RPC server:When Contacts authorization is still
.notDetermined,requestIfNeededawaitsCNContactStore.requestAccess. In a headless Aqua-domain LaunchAgent that prompt can remain pending indefinitely. The process stays alive but never accepts JSON-RPC, so methods such aschats.list,messages.history, andsend.richhang before the server starts.Issue #186 reproduces this on macOS with imsg 0.12.3+ and the same call path on current
main.Evidence
Prior art (same repo)
ContactsAccessPolicy.skipIfNotDeterminedand applied it toimsg chatsandimsg historyso CLI list/read commands fail open.imsg rpcwas left on the default whenRpcCommandwas later extracted (commitb5b7464, 2026-07-11).ContactResolver.create(accessPolicy: .skipIfNotDetermined)already returnsNoOpContactResolver(contactsUnavailable: true)without callingrequestAccess.Fix
Wire RPC startup to the same fail-open policy:
Interactive CLI send paths keep the default prompt-if-needed behavior. Authorized installations still load contacts normally on RPC start (existing
ContactResolverauthorized branch; unchanged).Live headless transcript (after fix)
Environment: macOS 26.5.2, Contacts authorization notDetermined (
CNContactStore.authorizationStatus(for: .contacts) = 0), built binary at commitf46cec8, non-TTY subprocess (LaunchAgent-equivalent headless stdio). Fixture Messages DB via--dbso startup reachesContactResolver.create(real Messages path would also pass FDA when granted).Server accepted two methods in under 50ms while Contacts remained undetermined (no prompt hang). Participant handles left as fixture placeholders (
+15555550100).Real behavior proof
Behavior or issue addressed: Headless
imsg rpcmust start the JSON-RPC server without blocking on an undetermined Contacts permission prompt (hang for LaunchAgents / non-interactive hosts).Real environment tested: macOS 26.5.2 (Build 25F84), arm64, Contacts authorization status
notDetermined(raw 0) on the host. Patched binaryf46cec8at/tmp/imsg-186/bin/imsg. Headless non-TTY stdio subprocess (equivalent to LaunchAgent / gateway child). Fixture chat.db so FDA is not required and startup still runsContactResolver.create.Exact steps or command run after this patch:
Evidence after fix: terminal output from the headless RPC session above:
chats.listandmessages.historyboth returnedresultobjects with exit 0 in 0.039s while Contacts stayed undetermined.Observed result after fix: RPC no longer waits on
requestAccesswhen Contacts is undetermined. The server reachesRPCServer.run(), accepts methods, and returns JSON-RPC results. Name enrichment remains optional until Contacts is already authorized (fixture used chatdisplay_name/ handles only).What was not tested: Live authorized-Contacts path on this host (authorization is notDetermined, not authorized). Existing
ContactResolverauthorized branch is unchanged; interactive CLI send paths still use.requestIfNeeded. Real LaunchAgent plist install was not used; proof used an equivalent non-TTY stdio child process.Notes
0.13.2 - Unreleased/ JSON-RPCRpcCommandto arun(..., contactResolverFactory:)entry point consistent withChatsCommandfor testability