feat(sdk): self-healing Signal session reset + idle daemon watchdog + bypass mode#257
Conversation
Add resetSession across the SDK (EncryptionContext -> TinyPlaceClient -> Agent) to drop a peer's persisted Double Ratchet session so the next send re-runs X3DH from a fresh pre-key bundle. Agent.resetSession resolves the recipient the same way sendMessage does, so the correct cryptoId-keyed session is cleared for a @handle/base64/cryptoId address. Wrap the daemon's outbound send with reset+retry: on a session-class error (relay 400 / ratchet / decrypt / prekey / contact), reset the session and retry once so daemon replies self-heal after a crash/restart mid-handshake. Also pass --dangerously-skip-permissions to the claude provider so daemon workers run unattended in bypass mode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the daemon's per-task timeout from a fixed wall-clock kill into an idle watchdog: the child CLI is reaped only after --task-timeout-ms with NO new provider event. `armIdle` re-arms on every parsed event, so a long-but- active run (streaming tool calls / output) keeps going while a genuinely wedged one is killed ~timeout after it goes silent. Default stays 600000ms. Also update the claude headless-argv test for the --dangerously-skip- permissions flag added with bypass mode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@sanil-23 is attempting to deploy a commit to the Vezures Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR adds Signal session reset APIs with daemon retry handling, and changes provider execution to use event-based idle watchdogs. It also threads an opt-in Claude permission-bypass flag through daemon task execution and validates its argument behavior. ChangesSignal session recovery
Provider execution watchdog
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)Signal session recoverysequenceDiagram
participant Daemon
participant Agent
participant TinyPlaceClient
participant EncryptionContext
Daemon->>Agent: sendMessage(recipient, message)
Agent-->>Daemon: session-related error
Daemon->>Agent: resetSession(recipient)
Agent->>TinyPlaceClient: resetSession(cryptoId)
TinyPlaceClient->>EncryptionContext: resetSession(cryptoId)
EncryptionContext-->>TinyPlaceClient: remove persisted session
Daemon->>Agent: retry sendMessage(recipient, message)
Provider idle watchdogsequenceDiagram
participant runProviderAttempt
participant ProviderProcess
participant EventMapper
runProviderAttempt->>ProviderProcess: spawn with provider arguments
runProviderAttempt->>runProviderAttempt: start idle watchdog
ProviderProcess-->>EventMapper: parsed semantic event
EventMapper->>runProviderAttempt: report progress
runProviderAttempt->>runProviderAttempt: re-arm watchdog
runProviderAttempt->>ProviderProcess: SIGKILL after sustained silence
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b92bba2f17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "--output-format", | ||
| "stream-json", | ||
| "--verbose", | ||
| "--dangerously-skip-permissions", |
There was a problem hiding this comment.
This unconditionally runs every Claude-backed daemon task with permission prompts bypassed; the Claude Code CLI reference documents --dangerously-skip-permissions as skipping permission prompts / bypassPermissions. Because tinyplace daemon publishes itself as a worker, auto-accepts inbound contacts, and feeds DM task text into the provider in the configured workspace, any accepted remote peer can now trigger file writes or shell commands without operator approval. Please make this an explicit daemon flag/env opt-in or keep Claude's normal permission mode by default.
Useful? React with 👍 / 👎.
| }; | ||
| const status = e?.status ?? e?.statusCode; | ||
| const text = `${e?.code ?? ""} ${e?.message ?? ""}`.toLowerCase(); | ||
| return status === 400 || /session|ratchet|decrypt|prekey|contact/.test(text); |
There was a problem hiding this comment.
This classifies the relay's not-yet-contact 400 as recoverable, but the retry at the send site happens immediately before the contact state has changed, so it fails the same way. Since MessagesApi.send encrypts before the HTTP PUT and SignalSession.encrypt stores the fresh X3DH session before that PUT can fail, the failed retry leaves an undelivered session behind; the next send after the contact is accepted will skip the pre-key bundle and be undecryptable. Please only retry explicit stale-ratchet errors, or reset again when the retry fails.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/typescript/src/cli/daemon/providers.ts (1)
155-164: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAvoid
--dangerously-skip-permissionsunless the daemon is fully sandboxedsdk/typescript/src/cli/daemon/providers.ts:155-164spawns Claude directly with the caller’scwdandenv, and the daemon path shows no isolation boundary here. That makes a remote prompt able to drive unrestricted file, shell, and network actions if the process can reach host resources.🤖 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 `@sdk/typescript/src/cli/daemon/providers.ts` around lines 155 - 164, Remove --dangerously-skip-permissions from the Claude argument list in the provider command construction for case "claude". Preserve the remaining Claude flags, extra arguments, and prompt ordering, and only reintroduce that option if this daemon path explicitly establishes a full sandbox boundary.
🧹 Nitpick comments (2)
sdk/typescript/src/cli/daemon/providers.ts (1)
260-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the idle-watchdog into a helper.
The
Promiseexecutor now bundles spawn wiring, buffering, line consumption, and the watchdog state machine in one ~135-line closure, well past the repo's 20-line function guideline. PullingarmIdle/cleanup/the timer state into a small factory (e.g.createIdleWatchdog(timeoutMs, onIdle)) would isolate the new logic and make it independently testable.As per coding guidelines, "Limit functions to a maximum of 20 lines of code; extract longer logic into separate helper functions."
🤖 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 `@sdk/typescript/src/cli/daemon/providers.ts` around lines 260 - 395, Extract the idle watchdog state from the Promise executor into a focused helper such as createIdleWatchdog(timeoutMs, onIdle), including timer state, re-arming, cleanup, and settled-state handling. Update the executor around armIdle and cleanup to use this helper while preserving the existing timeout error and child SIGKILL behavior. Keep the helper independently testable and ensure the affected functions comply with the 20-line guideline.Source: Coding guidelines
sdk/typescript/src/agent/agent.ts (1)
299-302: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd consistent error handling to the new async reset wrappers.
All three new async methods let resolution or store failures escape without a try/catch boundary. Catch at the appropriate layer, preserve the original cause, and add recipient/address context without swallowing the failure.
sdk/typescript/src/agent/agent.ts#L299-L302: wrap recipient resolution and client delegation with contextual error handling.sdk/typescript/src/messaging/encryption.ts#L134-L136: wrap persistence failures with the affected address.sdk/typescript/src/client.ts#L259-L261: preserve contextual errors from the encryption delegate.As per coding guidelines, all async functions must have proper error handling with try-catch blocks.
🤖 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 `@sdk/typescript/src/agent/agent.ts` around lines 299 - 302, Add try-catch handling to the async reset wrappers: in sdk/typescript/src/agent/agent.ts lines 299-302, wrap resolveRecipientKey and client.resetSession with recipient context while preserving the original cause; in sdk/typescript/src/messaging/encryption.ts lines 134-136, catch persistence failures and add the affected address without swallowing them; in sdk/typescript/src/client.ts lines 259-261, catch failures from the encryption delegate and preserve their existing contextual error information.Source: Coding guidelines
🤖 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 `@sdk/typescript/src/cli/daemon.ts`:
- Around line 179-191: Update the session-error recovery in send to await
agent.resetSession(to) without swallowing failures. Only call
agent.sendMessage(to, body) again after resetSession completes successfully;
propagate any reset failure and preserve the existing behavior for non-session
errors.
- Around line 282-298: Update isSessionError to use the existing
errorCode(error) and classifyError(error) helpers for structured
session/decrypt/prekey recovery cases, including the SDK’s pre-key variant.
Remove broad message-regex matching and do not treat every status 400 as
recoverable; retain status-based recovery only for the explicitly documented
relay condition.
In `@sdk/typescript/src/cli/daemon/providers.ts`:
- Around line 282-308: Add a separate wall-clock deadline for the provider run
alongside armIdle, using options.timeoutMs as the maximum total duration
regardless of parsed activity. Ensure the deadline calls finishError and
terminates the child, while preserving the existing idle timer’s re-arming
behavior and clearing both timers in cleanup.
---
Outside diff comments:
In `@sdk/typescript/src/cli/daemon/providers.ts`:
- Around line 155-164: Remove --dangerously-skip-permissions from the Claude
argument list in the provider command construction for case "claude". Preserve
the remaining Claude flags, extra arguments, and prompt ordering, and only
reintroduce that option if this daemon path explicitly establishes a full
sandbox boundary.
---
Nitpick comments:
In `@sdk/typescript/src/agent/agent.ts`:
- Around line 299-302: Add try-catch handling to the async reset wrappers: in
sdk/typescript/src/agent/agent.ts lines 299-302, wrap resolveRecipientKey and
client.resetSession with recipient context while preserving the original cause;
in sdk/typescript/src/messaging/encryption.ts lines 134-136, catch persistence
failures and add the affected address without swallowing them; in
sdk/typescript/src/client.ts lines 259-261, catch failures from the encryption
delegate and preserve their existing contextual error information.
In `@sdk/typescript/src/cli/daemon/providers.ts`:
- Around line 260-395: Extract the idle watchdog state from the Promise executor
into a focused helper such as createIdleWatchdog(timeoutMs, onIdle), including
timer state, re-arming, cleanup, and settled-state handling. Update the executor
around armIdle and cleanup to use this helper while preserving the existing
timeout error and child SIGKILL behavior. Keep the helper independently testable
and ensure the affected functions comply with the 20-line guideline.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3288a5ff-c536-4b55-a2c0-744efb84432e
📒 Files selected for processing (6)
sdk/typescript/src/agent/agent.tssdk/typescript/src/cli/daemon.tssdk/typescript/src/cli/daemon/providers.tssdk/typescript/src/client.tssdk/typescript/src/messaging/encryption.tssdk/typescript/tests/daemon.test.ts
|
|
||
| /** | ||
| * A send failure that a fresh Signal session can recover from: the relay's 400 on | ||
| * a not-yet-contact / stale ratchet, or an explicit session/decrypt/prekey fault. | ||
| * Distinguished from transport/auth errors, which resetting would not fix. | ||
| */ | ||
| function isSessionError(error: unknown): boolean { | ||
| const e = error as { | ||
| status?: number; | ||
| statusCode?: number; | ||
| code?: string; | ||
| message?: string; | ||
| }; | ||
| const status = e?.status ?? e?.statusCode; | ||
| const text = `${e?.code ?? ""} ${e?.message ?? ""}`.toLowerCase(); | ||
| return status === 400 || /session|ratchet|decrypt|prekey|contact/.test(text); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'FILE LIST\n'
git ls-files sdk/typescript/src/cli/daemon.ts sdk/typescript/src | sed -n '1,120p'
printf '\nOUTLINE daemon.ts\n'
ast-grep outline sdk/typescript/src/cli/daemon.ts --view expanded
printf '\nSEARCH classifyError/errorCode\n'
rg -n "classifyError|errorCode" sdk/typescript/src sdk/typescript -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'
printf '\nRELEVANT LINES daemon.ts\n'
sed -n '250,320p' sdk/typescript/src/cli/daemon.tsRepository: tinyhumansai/tiny.place
Length of output: 10351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '250,320p' sdk/typescript/src/cli/daemon.ts
echo '---'
rg -n "classifyError|errorCode" sdk/typescript/src sdk/typescript -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Repository: tinyhumansai/tiny.place
Length of output: 5403
🌐 Web query:
"errorCode classifyError" "tinyplace" sdk typescript
💡 Result:
In the tiny.place TypeScript SDK, classifyError and errorCode are components of a structured error taxonomy introduced to make the SDK more agent-friendly [1][2]. These tools allow agents to branch based on stable, machine-readable error codes rather than parsing free-text messages [1][2]. Key aspects of this error taxonomy include: Stable Error Codes: The system maps errors to a fixed set of machine-readable codes, including: payment_required, auth_invalid, handle_taken, not_found, rate_limited, validation, no_signer, transient, server, graphql, and unknown [1][2]. TinyPlaceError: This central error class self-classifies using code, hint, and retryable getters [1][2]. It also includes a toJSON() method so that structured error context is preserved when the error is serialized (e.g., via JSON.stringify) [1][2]. Recovery-Oriented Design: Each error code is paired with a one-sentence recovery hint and a boolean retryable flag, providing agents with actionable context for error handling and decision-making [1][2]. This classification logic is located in src/errors.ts within the TypeScript SDK [1][2]. It is designed to be structural and can classify both thrown TinyPlaceError instances and duck-typed error objects often encountered in CLI or other integration workflows [2]. Documentation for this contract can typically be found in the repository's SKILL.md or updated README files [1][2].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'SEARCH isSessionError usages\n'
rg -n "isSessionError\(" sdk/typescript/src/cli/daemon.ts sdk/typescript/src -g '*.ts'
printf '\nERROR TAXONOMY excerpt\n'
sed -n '1,220p' sdk/typescript/src/errors.ts
printf '\nDAEMON context around isSessionError usage\n'
sed -n '120,260p' sdk/typescript/src/cli/daemon.tsRepository: tinyhumansai/tiny.place
Length of output: 13935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "isSessionError\(" sdk/typescript/src/cli/daemon.ts sdk/typescript/src -g '*.ts' -g '*.tsx'
printf '\n--- errors.ts ---\n'
sed -n '1,220p' sdk/typescript/src/errors.ts
printf '\n--- daemon.ts around usage ---\n'
sed -n '180,320p' sdk/typescript/src/cli/daemon.tsRepository: tinyhumansai/tiny.place
Length of output: 13071
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'SEARCH session/ratchet/decrypt/prekey in SDK\n'
rg -n "session|ratchet|decrypt|prekey|pre-key|pre key|contact" sdk/typescript/src sdk/typescript/tests -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'
printf '\nSEND MESSAGE IMPLEMENTATION\n'
rg -n "sendMessage\(" sdk/typescript/src -g '*.ts'
printf '\nCLI daemon call sites around sendMessage/resetSession\n'
sed -n '150,210p' sdk/typescript/src/cli/daemon.tsRepository: tinyhumansai/tiny.place
Length of output: 50380
Classify session-recovery with exact allowlisted conditions. status === 400 can mark unrelated bad requests as recoverable, and the regex still matches arbitrary text while missing pre-key variants used elsewhere in the SDK. Use errorCode(error)/classifyError(error) for the structured cases and keep the session-recovery allowlist narrow; only fall back to status when the relay contract guarantees it.
🤖 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 `@sdk/typescript/src/cli/daemon.ts` around lines 282 - 298, Update
isSessionError to use the existing errorCode(error) and classifyError(error)
helpers for structured session/decrypt/prekey recovery cases, including the
SDK’s pre-key variant. Remove broad message-regex matching and do not treat
every status 400 as recoverable; retain status-based recovery only for the
explicitly documented relay condition.
Source: Coding guidelines
| // Idle watchdog, not a hard cap: kill the child only after `timeoutMs` with NO | ||
| // new event. `armIdle` re-arms on every parsed event (see consumeLine), so a | ||
| // long-but-active task (streaming tool calls / output) keeps running while a | ||
| // genuinely wedged one is reaped. Armed once at start to cover a child that | ||
| // never emits anything. | ||
| let idleTimer: ReturnType<typeof setTimeout> | undefined; | ||
| function armIdle(): void { | ||
| if (settled) return; | ||
| if (idleTimer) clearTimeout(idleTimer); | ||
| idleTimer = setTimeout(() => { | ||
| finishError( | ||
| new Error( | ||
| `${options.provider} task idle for ${options.timeoutMs}ms (no events)`, | ||
| ), | ||
| ); | ||
| child.kill("SIGKILL"); | ||
| }, options.timeoutMs); | ||
| } | ||
| armIdle(); | ||
|
|
||
| const onAbort = (): void => { | ||
| child.kill("SIGTERM"); | ||
| }; | ||
| options.signal?.addEventListener("abort", onAbort, { once: true }); | ||
|
|
||
| function cleanup(): void { | ||
| clearTimeout(timer); | ||
| if (idleTimer) clearTimeout(idleTimer); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether daemon.ts enforces an overall/hard deadline in addition to the idle timeout,
# and confirm timeoutMs typing/defaulting before it reaches runProviderAttempt.
rg -n 'timeoutMs' sdk/typescript/src/cli/daemon.ts sdk/typescript/src/cli/daemon/providers.ts -C3Repository: tinyhumansai/tiny.place
Length of output: 2171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- files ---'
git ls-files 'sdk/typescript/src/cli/daemon.ts' 'sdk/typescript/src/cli/daemon/providers.ts'
echo '--- outline providers.ts ---'
ast-grep outline sdk/typescript/src/cli/daemon/providers.ts --view expanded || true
echo '--- outline daemon.ts ---'
ast-grep outline sdk/typescript/src/cli/daemon.ts --view expanded || true
echo '--- search runProviderAttempt and timeoutMs wiring ---'
rg -n 'runProviderAttempt|timeoutMs|timeout' sdk/typescript/src/cli/daemon.ts sdk/typescript/src/cli/daemon/providers.ts -C 2Repository: tinyhumansai/tiny.place
Length of output: 5425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the key call path in smaller slices if the previous search is inconclusive.
sed -n '1,260p' sdk/typescript/src/cli/daemon.ts
printf '\n--- providers slice ---\n'
sed -n '1,220p' sdk/typescript/src/cli/daemon/providers.tsRepository: tinyhumansai/tiny.place
Length of output: 16606
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any wall-clock / overall deadline or attempt timeout in the daemon CLI path.
rg -n 'deadline|hard cap|hard timeout|overall timeout|wall-?clock|AbortSignal.timeout|setTimeout\(|timeoutMs|runProviderAttempt|attempt timeout|watchdog' sdk/typescript/src/cli -C 2
printf '\n--- daemon.ts around provider invocation ---\n'
python3 - <<'PY'
from pathlib import Path
p = Path('sdk/typescript/src/cli/daemon.ts')
text = p.read_text()
for needle in ['runProviderAttempt', 'timeoutMs', 'AbortSignal', 'setTimeout', 'deadline']:
idx = text.find(needle)
print(f'\n## {needle}: {idx}')
if idx != -1:
start = max(0, text.rfind('\n', 0, idx-400))
end = text.find('\n', idx+500)
print(text[start:end])
PYRepository: tinyhumansai/tiny.place
Length of output: 20460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline sdk/typescript/src/cli/daemon/runtime.ts --view expanded || true
printf '\n--- runtime references to taskTimeoutMs / timeout ---\n'
rg -n 'taskTimeoutMs|timeoutMs|deadline|AbortSignal|setTimeout|runProviderTask' sdk/typescript/src/cli/daemon/runtime.ts sdk/typescript/src/cli/daemon/providers.ts -C 3Repository: tinyhumansai/tiny.place
Length of output: 7551
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the runtime that consumes taskTimeoutMs to see whether there is any
# secondary wall-clock deadline beyond the provider idle watchdog.
git ls-files 'sdk/typescript/src/cli/daemon/runtime.ts'
ast-grep outline sdk/typescript/src/cli/daemon/runtime.ts --view expanded || true
printf '\n--- runtime search ---\n'
rg -n 'taskTimeoutMs|timeoutMs|deadline|AbortSignal|setTimeout|runProviderTask|provider task' sdk/typescript/src/cli/daemon/runtime.ts -C 3Repository: tinyhumansai/tiny.place
Length of output: 2537
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant runtime slice if it exists.
sed -n '1,260p' sdk/typescript/src/cli/daemon/runtime.tsRepository: tinyhumansai/tiny.place
Length of output: 9445
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('sdk/typescript/src/cli/daemon/runtime.ts')
print(p.exists())
if p.exists():
text = p.read_text()
for needle in ['taskTimeoutMs', 'timeoutMs', 'runProviderTask', 'deadline', 'setTimeout']:
idx = text.find(needle)
print(f'{needle}: {idx}')
PYRepository: tinyhumansai/tiny.place
Length of output: 251
Add a wall-clock cap for active provider runs
taskTimeoutMs is only used as an inactivity watchdog here, and daemon.ts passes that same value straight through with no separate deadline. A provider that keeps emitting parseable progress can still hold a worker indefinitely. --task-timeout-ms is already forced to a positive integer, so undefined/0 aren’t concerns.
🤖 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 `@sdk/typescript/src/cli/daemon/providers.ts` around lines 282 - 308, Add a
separate wall-clock deadline for the provider run alongside armIdle, using
options.timeoutMs as the maximum total duration regardless of parsed activity.
Ensure the deadline calls finishError and terminates the child, while preserving
the existing idle timer’s re-arming behavior and clearing both timers in
cleanup.
…r, propagate reset - Security (P1): `--dangerously-skip-permissions` is now opt-in behind a `--dangerously-skip-permissions` daemon flag (threaded through DaemonRuntime → RunTaskOptions → buildRunArgs), default OFF. A daemon auto-accepts contacts and runs remote task text, so bypassing approval must be an explicit operator choice, not the default. - Narrow isSessionError to explicit stale-ratchet/decrypt/prekey/session faults; stop treating a not-yet-contact 400 (or a bare HTTP status) as recoverable — resetting won't accept the contact and the failed retry would leave an undelivered X3DH session, making the next send undecryptable. - Do not swallow a failed resetSession: propagate it instead of retrying over an uncleared ratchet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the review feedback in
Validation: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/typescript/src/cli/daemon.ts (1)
184-198: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd explicit
Promise<void>return types to both callbacks.
sendand the nestedlock(async ...)callback should be annotated so the send contract stays explicit.🤖 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 `@sdk/typescript/src/cli/daemon.ts` around lines 184 - 198, Update the send callback and the nested lock callback in the send flow to explicitly declare Promise<void> return types. Preserve the existing session-error reset and retry behavior in both callbacks.Source: Coding guidelines
🤖 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 `@sdk/typescript/src/cli/daemon.ts`:
- Line 298: Rename the local variable e in the error-handling code to
errorDetails or errorInfo, and update all references within its scope while
preserving the existing behavior.
---
Outside diff comments:
In `@sdk/typescript/src/cli/daemon.ts`:
- Around line 184-198: Update the send callback and the nested lock callback in
the send flow to explicitly declare Promise<void> return types. Preserve the
existing session-error reset and retry behavior in both callbacks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8ed3800-5cee-4d6e-998b-d91df6fe9a72
📒 Files selected for processing (4)
sdk/typescript/src/cli/daemon.tssdk/typescript/src/cli/daemon/providers.tssdk/typescript/src/cli/daemon/runtime.tssdk/typescript/tests/daemon.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/typescript/src/cli/daemon/providers.ts
| * or a bare HTTP status, which says nothing about the ratchet. | ||
| */ | ||
| function isSessionError(error: unknown): boolean { | ||
| const e = error as { code?: string; message?: string }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the abbreviated e local.
Use errorDetails or errorInfo to satisfy the repository’s abbreviation rule.
As per coding guidelines, avoid abbreviations except for the explicitly listed names.
🤖 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 `@sdk/typescript/src/cli/daemon.ts` at line 298, Rename the local variable e in
the error-handling code to errorDetails or errorInfo, and update all references
within its scope while preserving the existing behavior.
Source: Coding guidelines
|
@coderabbitai the P1 (opt-in bypass), P2 (session classifier), and the swallowed-reset findings were all addressed in 60b6fc3. Please re-review / resolve. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
cd /repo 2>/dev/null || true
gh pr view 257 --repo tinyhumansai/tiny.place --json commits --jq '.commits[].oid' 2>/dev/null
echo "---"
gh api repos/tinyhumansai/tiny.place/commits/60b6fc3a -q '.files[].filename' 2>/dev/null
echo "---diff---"
gh api repos/tinyhumansai/tiny.place/commits/60b6fc3a -q '.files[] | .filename + "\n" + .patch' 2>/dev/nullLength of output: 6202 All three findings are confirmed fixed in commit
Nice, precise fixes — resolving the thread. [resolve] |
|
Two remaining threads — both handled, no further code change:
|
2 participants
Summary
Three related resilience improvements to the SDK + headless daemon, surfaced while running the medulla Lab TUI against a
tinyplace daemonfleet.1.
Agent.resetSession(recipient)— recover a poisoned Signal ratchetAdds a
resetSessionpassthrough (EncryptionContext→TinyPlaceClient→Agent) that drops a peer's persisted Double Ratchet session on the live in-client store instance, so the next send re-runs X3DH from a fresh pre-key bundle.Agent.resetSessionresolves the recipient the same waysendMessagedoes, so the correct cryptoId-keyed session is cleared even for a@handle/base64 address.Motivation: after a crash/restart mid-handshake (or the relay's 400 on a not-yet-contact), a send can wedge on a desynced ratchet with no recovery path. This is the primitive to self-heal it (reset + retry once).
2. Daemon: idle (no-event) watchdog instead of a hard cap
--task-timeout-mswas a fixed wall-clock kill — it reaped long-but-active tasks (e.g. a coding agent streaming tool calls for >10 min) just as readily as wedged ones. It's now an idle timeout: the child CLI is killed only after--task-timeout-mswith no new provider event;armIdlere-arms on every parsed event. A task lives as long as it makes progress; a genuinely silent/wedged one is reaped after the timeout. Default unchanged (600000ms).3. Bypass mode for the claude provider
Adds
--dangerously-skip-permissionsto the claude headless argv so daemon workers run unattended.Changes
src/messaging/encryption.ts,src/client.ts,src/agent/agent.ts—resetSessionpassthrough.src/cli/daemon/providers.ts— idle watchdog (armIdle) +--dangerously-skip-permissions.src/cli/daemon.ts— doc the--task-timeout-msidle semantics.tests/daemon.test.ts— updated the claude argv assertion for the bypass flag.Validation
pnpm build(tsc) clean;pnpm test— 524 passed, 1 skipped.🤖 Generated with Claude Code
Summary by CodeRabbit