Skip to content

fix: prevent silent message loss from EmbeddedAttemptSessionTakeoverError#89039

Open
Jerry-Xin wants to merge 33 commits into
openclaw:mainfrom
Jerry-Xin:fix/session-takeover-silent-message-loss-87180
Open

fix: prevent silent message loss from EmbeddedAttemptSessionTakeoverError#89039
Jerry-Xin wants to merge 33 commits into
openclaw:mainfrom
Jerry-Xin:fix/session-takeover-silent-message-loss-87180

Conversation

@Jerry-Xin

@Jerry-Xin Jerry-Xin commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

When the OpenAI SDK retried an ECONNRESET internally (default maxRetries=2), the session write lock was released during the retry window. Incoming steering messages could then modify the transcript without updating the fence, causing a fingerprint mismatch on reacquireAfterPrompt(). The resulting EmbeddedAttemptSessionTakeoverError was caught by model-fallback.ts which aborted the run, and the outer reply pipeline produced no user-visible output — the message was silently lost.

What Problem This Solves

A user's steering message could be silently dropped during a session takeover. The takeover was made more likely by an in-window SDK retry running against a released session lock, and when it fired the reply pipeline fell into a generic failure branch that emitted no actionable output. This fix closes the race that widened the takeover window and converts the takeover into an explicit, actionable reply so the user knows to resend.

Why This Change Was Made

Two invariants must hold together. First, no in-window SDK retry may run inside the embedded prompt-lock release window, because that window releases the session lock across the model call and an in-window retry re-admits the session-takeover race (#87180). Second, an unconfigured user's bare connection and request-timeout resilience must not regress — so that resilience moves to the orchestrator layer, where each retry re-acquires the session lock and cannot widen any single release window. When a takeover does occur, the runner must surface resend guidance rather than a silent or generic failure.

Root Cause Chain

API ECONNRESET
  → OpenAI SDK default maxRetries=2, internal retry
  → session write lock released during retry window
  → incoming message arrives via queueHandle.queueMessage
  → activeSession.steer writes directly (bypassing withOwnedSessionTranscriptWrites)
  → transcript changes without updating fence
  → original attempt reacquireAfterPrompt() fingerprint mismatch
  → EmbeddedAttemptSessionTakeoverError thrown
  → model-fallback.ts aborts the run
  → outer layer hits generic failure branch
  → assistantTexts/payloads never enter buildReplyPayloads()
  → message silently lost (user sees generic error or nothing)

Fix

Seven coordinated changes that address the bug at multiple layers:

1. Wrap steering in withOwnedSessionTranscriptWrites (attempt.ts)

Steering writes that arrive during prompt lock release now go through withOwnedSessionTranscriptWrites, keeping fence tracking consistent and preventing the fingerprint mismatch that triggers the takeover error.

2. Dedicated error handler for takeover (agent-runner-execution.ts, failover-error.ts)

Added isEmbeddedAttemptSessionTakeoverError() export and a dedicated catch block in runAgentTurnWithFallback. Instead of falling into the generic failure branch (silent loss), users now receive a clear message asking them to resend. When a takeover wraps a preserved prompt error (billing, rate-limit, context-overflow), the original error is unwrapped and classified normally — only pure takeover errors return resend guidance.

3. Pin SDK retries to 0 inside the embedded prompt lock window (attempt.ts, attempt.session-lock.ts)

installEmbeddedPromptRetryDefault wraps the session's streamFn and forces { ...options, maxRetries: 0 } — the pin is applied last, so neither the configured settings.retry.provider.maxRetries nor an explicit per-call maxRetries can widen the retry count inside this window. The prompt-lock window releases the session lock across the model call, so any in-window SDK retry would run against a released lock and re-admit the session-takeover race (#87180). The configured retry budget is restored at the outer full-attempt owner instead (see the current-head update below), where every retry re-acquires the session lock. Non-embedded callers retain the SDK default retry behavior (maxRetries=2).

4. Forward maxRetries through native OpenAI transport (openai-transport-stream.ts)

buildOpenAISdkRequestOptions previously accepted only model and signal, dropping the maxRetries injected by installEmbeddedPromptRetryDefault. Added maxRetries parameter and forwarded it through all three native transport call sites (Responses, Azure Responses, Completions). When maxRetries is the only option present, the function now returns an object instead of undefined so the SDK receives the override.

5. Phase tracking in EmbeddedAttemptSessionTakeoverError (attempt.session-lock.ts)

Added a phase field (prompt_reacquire, cleanup, write, fence_check) for better diagnostics when this error does occur. Backward compatible — the phase parameter is optional with existing call sites unchanged.

6. Restore unconfigured connection-error retry at the orchestrator layer (agent-runner-execution.ts, embedded-agent-helpers/errors.ts)

Disabling SDK retries inside the prompt-lock release window (#3) is required so an in-window retry cannot widen the takeover race — but for users who have not configured settings.retry.provider.maxRetries, it also dropped the SDK's default retry of bare connection errors (ECONNRESET, socket hang up, Connection error.) that carry no leading HTTP status. This is restored at the orchestrator layer, where the existing one-shot consumeTransientHttpRetry() re-runs the full primary→fallback cycle and re-acquires the session lock per attempt, so it does not widen any single release window. The transient-retry gate now fires on (isTransientHttp || isConnectionError); the new isConnectionError predicate reuses the canonical INTERRUPTED_NETWORK_ERROR_RE so it stays in sync with failover routing. In-window maxRetries:0 is unchanged. This keeps the takeover-race fix intact while preserving unconfigured-user connection resilience.

7. Restore unconfigured request-timeout retry at the orchestrator layer (agent-runner-execution.ts)

The orchestrator transient-retry restore in #6 left a symmetric gap: in addition to bare connection errors, the OpenAI/Anthropic SDKs retry request-timeout transport errors by default, and after the in-window maxRetries:0, an unconfigured single-model user's timeout error is rethrown to the same outer gate — which only matched HTTP-status transients and connection errors, dropping timeout retry entirely (multi-model users were unaffected because failover already routes timeouts). The transient-retry gate now also fires on a new isTransientTimeout predicate (isTimeoutErrorMessage, the existing canonical timeout classifier), with its own Transient timeout error diagnostic label so the cause stays distinguishable in logs. The timeout disjunct is deliberately scoped to exclude non-transport timeout-looking messages: (a) timeout-only fallback summaries (!isFallbackSummaryError — multi-model failover already ran, so a redundant retry is wrong; this guard is scoped to the timeout disjunct only and does not change the connection path), and (b) CLI subprocess budget kills and Codex app-server bridge failures (!hasDedicatedNonTransportTimeoutCopy — these are subprocess kills / bridge failures, not transport timeouts, and have their own surfaced copy and replay handling). The one-shot consumeTransientHttpRetry() budget is shared, so timeout retry is bounded exactly like the connection retry. The connection-retry path semantics from #6 are unchanged. The timeout disjunct additionally excludes local non-provider runtime coordination errors (e.g. SessionWriteLockTimeoutError, whose message reads as session file locked (timeout ...ms)) via the canonical isNonProviderRuntimeCoordinationError classifier: retrying any model would hit the same local condition, so these abort the fallback chain rather than re-run as a transport timeout.

User Impact

  • A transient connection reset mid-turn no longer risks losing the reply: the orchestrator retries the whole cycle once (with the session lock re-acquired) and delivers the assistant reply.
  • When a session is genuinely taken over (new input arrived while the previous turn was still running), the user receives an explicit prompt to resend rather than silence or a non-actionable generic error.
  • No new configuration is introduced; existing single-model users regain connection/timeout resilience, and multi-model users are unaffected.

Evidence

Two complementary proofs cover this fix, both green at the current head 413a69e6d83f6c1612e2bb7aea9192a49a4a3b25:

  1. Orchestrator-level proof — an exact-head e2e test (src/auto-reply/reply/agent-runner-takeover-live.e2e.test.ts) that drives the real runAgentTurnWithFallback and asserts the user-facing reply.
  2. Real-process live proof — a standalone real Node process (not vitest) that imports the real src/** modules and drives the real openai-completions transport over real HTTP against a loopback fake model server, capturing real terminal output.

Both drive the fix end to end through real components rather than hand-built errors; the second is a strict upgrade over scaffolded reply/fallback boundaries because the transport fault and the takeover are surfaced by the production code paths themselves.

Proof 1 — orchestrator-level e2e (agent-runner-takeover-live.e2e.test.ts)

What is real (no hand-built errors, no mocked runner internals):

  • A real temp session transcript file on disk.
  • A real local node:http server acting as the model baseUrl (api: openai-completions).
  • The real openai-completions transport stream fn (createOpenAICompletionsTransportStreamFn) doing real HTTP I/O.
  • The real embedded prompt-lock controller + fence (createEmbeddedAttemptSessionLockController), wired through the real installEmbeddedPromptRetryDefault (pins SDK maxRetries to 0) and installPromptSubmissionLockRelease — exactly as src/agents/embedded-agent-runner/run/attempt.ts wires them in production.
  • The real connection/transient classification (isConnectionError / isTransientHttpError / isTimeoutErrorMessage from embedded-agent-helpers/errors.ts) and the real failover classification (isNonProviderRuntimeCoordinationError), consumed by the real runAgentTurnWithFallback retry gate and takeover catch.

The only injected variables are at real boundaries: a dropped TCP socket (Scenario A) and a concurrent unowned transcript rewrite (Scenario B). The reply-pipeline boundary is scaffolded with the same mock surface as the existing agent-runner-execution.test.ts, plus a thin runWithModelFallback stand-in that runs the real candidate closure once and propagates exactly as the real runWithModelFallback does (re-throw on failure, return on success).

Scenario A — pure transient transport fault. The model server resets the socket on request #1, then succeeds on the retry. Because SDK retries are pinned to 0 inside the released-lock window, the SDK does not re-issue in-window; the orchestrator retries the whole cycle exactly once via consumeTransientHttpRetry(). Observed and asserted:

  • Bounded provider request count == 2 — one failed request plus exactly one orchestrator retry (the SDK did not retry in-window).
  • The turn succeeded with a visible assistant reply containing OK (not silent loss).
  • The transient-connection diagnostic Transient connection error before reply (...). Retrying once in 2500ms. was emitted exactly once.
  • The reply operation was not failed on the recovery path.

Scenario B — organic session takeover → resend guidance. While the real model call holds the released prompt lock, a concurrent unowned rewrite of the same session transcript changes the session-file fingerprint, so reacquireAfterPrompt throws EmbeddedAttemptSessionTakeoverError organically inside the wrapped call's finally. Observed and asserted:

  • Provider request count == 1 — no silent in-window SDK retry.
  • The user-facing reply is the exact resend-guidance literal produced by the real catch path at src/auto-reply/reply/agent-runner-execution.ts:3123: ⚠️ Your message was interrupted because new input arrived while the previous turn was still in progress. Please resend your message.
  • A run_failed lifecycle terminal was recorded via the reply operation.
  • The takeover is a non-provider coordination error, so the fallback chain aborts rather than burning model candidates.

Revert-resistance (each assertion is load-bearing):

  • Disabling the maxRetries pin makes Scenario A fail: the SDK silently retries in-window, the orchestrator never engages, and the transient diagnostic count is 0.
  • Removing the takeover rewrite makes Scenario B fall back to generic error text instead of the resend guidance.

Reproduce:

node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-takeover-live.e2e.test.ts

Run summary (exact head 413a69e6d83f6c1612e2bb7aea9192a49a4a3b25):

 RUN  v4.1.9 <workdir>/openclaw-pr-workdirs/pr-89039

 Test Files  1 passed (1)
      Tests  2 passed (2)
   Duration  31.15s

Proof 2 — real-process live proof (standalone Node process, real HTTP)

This proof does not use vitest. A standalone throwaway harness runs under a real node v24.15.0 process, imports the real src/** modules by absolute path, and drives the real openai-completions transport over real HTTP against a loopback-only fake model server on an ephemeral port. It runs fully isolated (OPENCLAW_HOME/HOME/TMPDIR), never touching the running system gateway, and captures the real terminal output — including the production [provider-transport-fetch] logger lines. Both scenarios passed (process exit 0, ALL CHECKS PASSED).

Scenario A — real socket destroy → delivered reply. The model server performs a real TCP socket destroy on request #1. The real transport surfaces UND_ERR_SOCKET / fetch failed through the production [provider-transport-fetch] logger; the real classifier reports isConnectionError=true / isTransientHttpError=false; the whole cycle is retried once; request #2 streams OK. Proven: request count == 2 (SDK retries pinned to 0 in-window, so the single retry is the orchestrator's, not the SDK's), a visible "OK" reply, and no silent loss.

Scenario B — organic session takeover → resend guidance. While the real model call holds the released prompt lock, a concurrent unowned rewrite of the same session file changes its fingerprint, and reacquireAfterPrompt throws EmbeddedAttemptSessionTakeoverError (phase prompt_reacquire) organically. Proven: request count == 1 (no silent in-window SDK retry), the error is a real non-provider coordination error, and the orchestrator maps it to the exact resend-guidance literal.

Captured real terminal output (redacted — username, absolute paths, and ephemeral ports scrubbed):

[provider-transport-fetch] [model-fetch] start provider=mlx api=openai-completions model=probe-model method=POST url=http://127.0.0.1:<PORT>/v1/chat/completions timeoutMs=900000 proxy=none policy=custom
model server: request #1 received -> destroying socket (real transport fault)
[provider-transport-fetch] [model-fetch] error provider=mlx api=openai-completions model=probe-model elapsedMs=253 name=TypeError code=undefined causeName=SocketError causeCode=UND_ERR_SOCKET message=fetch failed
cycle threw: "Connection error."
real classifier: isConnectionError=true isTransientHttpError=false
Transient connection error before reply (Connection error.). Retrying once in 2500ms. (orchestrator-equivalent gate)
[provider-transport-fetch] [model-fetch] start provider=mlx api=openai-completions model=probe-model method=POST url=http://127.0.0.1:<PORT>/v1/chat/completions timeoutMs=900000 proxy=none policy=custom
model server: request #2 received -> streaming completion "OK"
[provider-transport-fetch] [model-fetch] response provider=mlx api=openai-completions model=probe-model status=200 elapsedMs=65 contentType=text/event-stream
PASS — real transport surfaced a bare connection error (isConnectionError)
PASS — connection drop is NOT a leading-HTTP-status transient
PASS — bounded request count == 2 (one reset + one orchestrator retry)
PASS — delivered assistant reply contains "OK" (no silent loss)
=== Scenario A complete ===

=== Scenario B: organic session takeover -> resend guidance ===
model server: request #1 received -> holding open until rewrite
concurrent unowned writer: rewriting session file out of band (steering append)
prompt window threw: EmbeddedAttemptSessionTakeoverError: session file changed while embedded prompt lock was released: <ISOLATED_TMP>/session.jsonl (phase: prompt_reacquire)
PASS — prompt window threw an error (not a silent drop)
PASS — error is a real EmbeddedAttemptSessionTakeoverError
PASS — takeover is a non-provider coordination error (fallback chain aborts)
PASS — single request: no silent in-window SDK retry
orchestrator maps this coordination error -> resend guidance literal:
  ⚠️ Your message was interrupted because new input arrived while the previous turn was still in progress. Please resend your message.
=== Scenario B complete ===

ALL CHECKS PASSED

Scope of this proof

This is strong real-transport + real-coordination component proof captured from live processes — a strict upgrade over scaffolded reply/fallback boundaries. The connection reset is a real TCP socket destroy surfaced by the real openai-completions transport over real HTTP, the classification and the bounded single retry use the real predicates, the recovery comes back over a real second HTTP call (status=200), and the takeover is triggered organically by a real concurrent file rewrite during a real held-lock window.

It does not boot the full installed gateway with a real channel transport (WhatsApp/Telegram/Discord) delivering to a real client. That is because a real channel needs external provider credentials plus live third-party services, while the model side must be a local deterministic fake to reproduce the reset-then-succeed sequence — a real paid model endpoint could not be made to reset then succeed on demand. This is a provisioning/credentials constraint, not a Node-version or code blocker: the model/provider and runtime-coordination path is faithfully reproduced end to end. A literal installed channel roundtrip would need a box provisioned with real (or sandbox) channel + model credentials.

Configured provider retry budget

Earlier revisions left the configured provider retry-budget question — whether settings.retry.provider.maxRetries survives the outer whole-attempt retries — as a follow-up. It is now resolved on the current head (commit 0ad78ae26d); see the current-head update below. In-window SDK retries stay pinned at 0 (the safety invariant), and the configured budget is honored only at the outer owner where each retry re-acquires the session lock.

Current-head update (0ad78ae26d)

This commit closes the previously-deferred configured-retry gap while preserving the takeover-race safety invariant. Three behaviors:

  1. In-window pin is now absolute. The released-lock embedded SDK path forces maxRetries: 0 last ({ ...requestOptions, maxRetries: 0 }), so neither the configured provider default nor an explicit per-call maxRetries can retry inside the unsafe window where the session lock is released.
  2. Configured budget moves to the outer owner. settings.retry.provider.maxRetries is resolved once during prepared reply construction (resolveConfiguredProviderRetryMaxRetries) and carried as providerRetryMaxRetries on the prepared run to the outer full-attempt retry owner in runAgentTurnWithFallback, where every retry re-runs the whole cycle and re-acquires the session lock. Unset still defaults to one outer retry (the shipped behavior).
  3. Terminal classifications unchanged. Takeover, abort, partial-output, and tool-activity terminal classifications are untouched by this commit.

Evidence (current head 0ad78ae26d)

Focused local proof (node scripts/run-vitest.mjs):

  • src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts109/109 passed (in-window pin wins over config and per-call overrides).
  • src/auto-reply/reply/agent-runner-execution-transient-retry.test.ts9/9 passed (outer owner honors the configured budget; unset keeps a single retry).
  • src/agents/sessions/sdk.test.ts28/28 passed.

A fresh autoreview found one tautological test and it was removed; a re-run then found no actionable issues.

scripts/check-changed.mjs (typecheck/lint fan-out) delegated to the remote Blacksmith Testbox. Package downloads there repeatedly failed with error 23 and the process was SIGKILLed, so this infrastructure-only check did not complete locally. It is not claimed as passing.

Changed Files

  • packages/ai/src/providers/openai-completions.test.ts
  • packages/ai/src/providers/openai-responses-shared.test.ts
  • src/agents/agent-project-settings.ts
  • src/agents/embedded-agent-helpers.ts
  • src/agents/embedded-agent-helpers/errors.ts
  • src/agents/embedded-agent-helpers/errors.test.ts
  • src/agents/embedded-agent-runner/run/attempt-stream-prepare.ts
  • src/agents/embedded-agent-runner/run/attempt.session-lock.ts
  • src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts
  • src/agents/embedded-agent-runner/run/attempt.ts
  • src/agents/failover-error.ts
  • src/agents/failover-error.test.ts
  • src/agents/openai-completions-transport.ts
  • src/agents/openai-responses-transport.ts
  • src/agents/openai-transport-params.ts
  • src/agents/openai-transport-stream.streaming.test-utils.ts
  • src/agents/sessions/sdk.test.ts
  • src/auto-reply/reply/agent-runner-execution.ts
  • src/auto-reply/reply/agent-runner-execution.test.ts
  • src/auto-reply/reply/agent-runner-execution-transient-retry.test.ts
  • src/auto-reply/reply/agent-runner-session-takeover.real.test.ts
  • src/auto-reply/reply/agent-runner-takeover-live.e2e.test.ts
  • src/auto-reply/reply/get-reply-run.ts
  • src/auto-reply/reply/queue/types.ts

Fixes #87180

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 1, 2026
@clawsweeper

clawsweeper Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 25, 2026, 12:13 AM ET / 04:13 UTC.

ClawSweeper review

What this changes

The PR makes embedded session steering fence-aware, prevents SDK retries while the session lock is released, restores retry handling at the outer reply attempt, and sends actionable resend guidance instead of silently losing a reply after session takeover.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

This PR remains necessary: it targets the closed root-cause report’s silent-reply-loss path, and the latest head appears to address the prior configured-retry blocker by moving that budget to the full-attempt owner. The supplied loopback/process proof is substantial, but this is a compatibility-sensitive 31-file change with the current required checks still running, so it should stay open for normal maintainer review rather than cleanup closure.

Likely related people: steipete and vincentkoc were explicitly brought into the discussion; confidence is low because this read-only review could not complete local history attribution.

Priority: P1
Reviewed head: f79e5ade8a00591e44718faaf98d6d31851a7c73

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The PR has strong behavior evidence and addresses the prior retry-budget blocker, with remaining confidence depending on current-head compatibility and required-check completion.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR supplies exact-head loopback/process and orchestrator-level evidence that exercises real session-file fencing and OpenAI-compatible HTTP transport, showing recovery delivery and takeover resend guidance; contributors should continue to redact any private runtime details in future artifacts.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR supplies exact-head loopback/process and orchestrator-level evidence that exercises real session-file fencing and OpenAI-compatible HTTP transport, showing recovery delivery and takeover resend guidance; contributors should continue to redact any private runtime details in future artifacts.
Evidence reviewed 6 items Root-cause relationship: The PR explicitly addresses the session-takeover silent-message-loss path reported by the canonical issue, including an embedded prompt-lock release, provider retries, transcript fence mismatch, and missing reply output.
Prior blocker addressed at current head: The latest commit states that configured provider retries are restored outside the session-lock window; the branch now resolves the configured retry budget during prepared-run construction and uses it at the outer whole-attempt retry owner.
Compatibility-sensitive retry behavior: The changed runner uses the configured provider retry budget, with an unset configuration retaining one outer retry, while the embedded prompt-window wrapper still prevents unsafe in-window SDK retries.
Findings None None.
Security None None.

How this fits together

OpenClaw’s reply runner invokes an embedded agent while coordinating a durable session transcript and model-provider transport retries. Incoming steering messages and provider failures feed that runner; it must preserve session ownership and then emit either a delivered reply or a clear user-facing failure.

flowchart LR
  Inbound[Inbound or steering message] --> Queue[Reply queue and prepared run]
  Queue --> Lock[Embedded session lock and transcript fence]
  Lock --> Model[Model transport call]
  Model --> Retry{Provider failure?}
  Retry -->|retryable| Outer[Whole-attempt retry owner]
  Outer --> Lock
  Retry -->|takeover| Guidance[Resend guidance]
  Retry -->|success| Delivery[Reply payload and channel delivery]
  Guidance --> Delivery
Loading

Before merge

  • Resolve merge risk (P2) - Configured provider retry values now govern repeated full fallback cycles rather than SDK-local retries; maintainer review should confirm that this deliberate compatibility trade-off is acceptable across single- and multi-model routing.
  • Resolve merge risk (P1) - The patch changes session ownership, retry classification, and terminal reply behavior together; the in-progress build, type, lint, QA, and contract checks should complete against the current head before merge.
  • Complete next step (P2) - The earlier mechanical blocker appears addressed at the newest head; remaining work is normal maintainer compatibility review and completion of current validation, not a safe autonomous repair.
Agent review details

Security

None.

PR surface

Source +282, Tests +2294. Total +2576 across 31 files.

View PR surface stats
Area Files Added Removed Net
Source 19 313 31 +282
Tests 12 2301 7 +2294
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 31 2614 38 +2576

Review metrics

Metric Value Why it matters
Retry ownership change 1 configured retry budget moved from SDK-local behavior to full-attempt orchestration This protects the released session-lock window but changes where configured retry semantics are applied.

Stored data model

Persistent data-model change detected: serialized state: src/agents/openai-transport-stream.streaming.test.ts, serialized state: src/agents/sessions/sdk.test.ts, unknown-data-model-change: src/agents/sessions/sdk.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge-risk options

Maintainer options:

  1. Confirm retry-budget compatibility before merge (recommended)
    Keep the race protection, but verify that configured retry budgets intentionally apply to complete fallback cycles and that the current required checks finish on this head.
  2. Accept the outer-cycle retry semantics
    Merge with the documented behavior that configured provider retries are restored as full attempts rather than SDK-local retries.
  3. Pause for a narrower retry design
    Pause this branch if maintainers do not want configured provider retry counts to govern repeated fallback cycles.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Verify configured provider retry values 0, unset, and a positive value across the outer full-attempt retry path; retain maxRetries: 0 inside the released embedded prompt-lock window.

Technical review

Best possible solution:

Keep SDK retries disabled only inside the released embedded prompt-lock window, preserve configured retry resilience at the full-attempt owner, and land only after focused configured-retry coverage plus the current head’s required checks confirm the intended upgrade behavior.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible with high confidence: the supplied end-to-end proof drives a real loopback model transport plus a real session-file fence through the reply runner, covering both a reset connection and an organic takeover. This review did not independently execute that path.

Is this the best way to solve the issue?

Yes, conditionally: preventing SDK-local retries only while the prompt lock is released and retrying from the full-attempt owner is the narrowest design that protects session ownership while retaining retry resilience. The remaining question is compatibility confirmation for configured retry budgets across fallback cycles.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against c780bfbf0d54.

Labels

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR supplies exact-head loopback/process and orchestrator-level evidence that exercises real session-file fencing and OpenAI-compatible HTTP transport, showing recovery delivery and takeover resend guidance; contributors should continue to redact any private runtime details in future artifacts.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P1: The patch addresses a reply-delivery and session-takeover regression that can silently lose a user message during provider failures.
  • merge-risk: 🚨 compatibility: Existing settings.retry.provider.maxRetries values now affect outer full fallback attempts, requiring explicit upgrade-safety confirmation.
  • merge-risk: 🚨 session-state: The patch changes transcript-fence ownership and behavior when concurrent session writes occur during an embedded prompt.
  • merge-risk: 🚨 message-delivery: The patch changes whether an interrupted or taken-over turn retries, emits resend guidance, or reaches the reply payload path.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR supplies exact-head loopback/process and orchestrator-level evidence that exercises real session-file fencing and OpenAI-compatible HTTP transport, showing recovery delivery and takeover resend guidance; contributors should continue to redact any private runtime details in future artifacts.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR supplies exact-head loopback/process and orchestrator-level evidence that exercises real session-file fencing and OpenAI-compatible HTTP transport, showing recovery delivery and takeover resend guidance; contributors should continue to redact any private runtime details in future artifacts.

Evidence

What I checked:

  • Root-cause relationship: The PR explicitly addresses the session-takeover silent-message-loss path reported by the canonical issue, including an embedded prompt-lock release, provider retries, transcript fence mismatch, and missing reply output. (src/auto-reply/reply/agent-runner-error-handler.ts:196, f79e5ade8a00)
  • Prior blocker addressed at current head: The latest commit states that configured provider retries are restored outside the session-lock window; the branch now resolves the configured retry budget during prepared-run construction and uses it at the outer whole-attempt retry owner. (src/auto-reply/reply/get-reply-run.ts:1634, f79e5ade8a00)
  • Compatibility-sensitive retry behavior: The changed runner uses the configured provider retry budget, with an unset configuration retaining one outer retry, while the embedded prompt-window wrapper still prevents unsafe in-window SDK retries. (src/auto-reply/reply/agent-runner-execution.ts:258, f79e5ade8a00)
  • Real behavior proof: The PR body describes an exact-head loopback transport/process proof and an orchestrator-level end-to-end test that exercise real HTTP transport, session-file fencing, retry behavior, and user-visible reply guidance. The PR is labeled proof sufficient. (src/auto-reply/reply/agent-runner-takeover-live.e2e.test.ts:1, f79e5ade8a00)
  • Review continuity: The previous ClawSweeper P1 requested restoring configured retries at the outer attempt owner. The current-head commit message is “fix: restore configured retries outside session lock,” so that prior finding should not be re-raised without contrary source evidence. (src/auto-reply/reply/agent-runner-execution.ts:258, f79e5ade8a00)
  • Current validation state: The supplied check context shows early security and fast checks passing, while build, type, lint, QA, and broader contract lanes are still in progress; this is a merge gate, not evidence that the user impact is higher priority.

Likely related people:

  • steipete: The timeline explicitly records a direct mention on the PR’s early review thread; local history attribution could not be completed in this read-only environment. (role: discussion participant; confidence: low; files: src/auto-reply/reply/agent-runner-execution.ts, src/agents/embedded-agent-runner/run/attempt.session-lock.ts)
  • vincentkoc: The timeline explicitly records a direct mention on the PR’s early review thread; the affected area is session and reply-runner behavior. (role: discussion participant; confidence: low; files: src/auto-reply/reply/agent-runner-error-handler.ts, src/auto-reply/reply/get-reply-run.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Confirm focused coverage for configured retry values at the outer retry owner.
  • Wait for the current build, type, lint, QA, and contract lanes to complete on the rebased head.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (53 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-18T22:16:31.954Z sha 7402a73 :: needs changes before merge. :: [P1] Preserve configured retries at the outer attempt owner
  • reviewed 2026-07-19T07:27:37.921Z sha 7e6771a :: needs changes before merge. :: [P1] Preserve configured retries at the outer attempt owner
  • reviewed 2026-07-19T13:49:23.917Z sha b4b9370 :: needs changes before merge. :: [P1] Preserve configured retries at the outer attempt owner
  • reviewed 2026-07-19T22:16:37.829Z sha 0072fbb :: needs changes before merge. :: [P1] Preserve configured retries at the outer attempt owner
  • reviewed 2026-07-20T04:09:51.549Z sha a6c6c3a :: needs changes before merge. :: [P1] Preserve configured retries at the outer attempt owner
  • reviewed 2026-07-20T04:45:11.625Z sha 58c488d :: needs changes before merge. :: [P1] Preserve configured retries at the outer attempt owner
  • reviewed 2026-07-20T07:23:54.737Z sha 90cb530 :: needs changes before merge. :: [P1] Preserve configured retries at the outer attempt owner
  • reviewed 2026-07-20T16:16:14.094Z sha 7faa7cd :: needs changes before merge. :: [P1] Restore configured retries at the outer attempt owner

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 1, 2026
@Jerry-Xin
Jerry-Xin force-pushed the fix/session-takeover-silent-message-loss-87180 branch 2 times, most recently from 6c0fb05 to 36ce09b Compare June 1, 2026 18:07
@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

Rebased on latest upstream/main (965e680, clean rebase, no conflicts).

@Jerry-Xin
Jerry-Xin force-pushed the fix/session-takeover-silent-message-loss-87180 branch from 36ce09b to 1e7989e Compare June 1, 2026 22:06
@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

Rebased on latest upstream/main (646df2d) and resolved conflict in src/auto-reply/reply/agent-runner-execution.ts — upstream added isFailoverError import; merged with our isEmbeddedAttemptSessionTakeoverError import. All changes preserved.

@Jerry-Xin
Jerry-Xin force-pushed the fix/session-takeover-silent-message-loss-87180 branch from 1e7989e to 8730a7e Compare June 2, 2026 00:06
@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

Rebased on latest upstream/main (8a9acd2, clean rebase, no conflicts).

@Jerry-Xin
Jerry-Xin force-pushed the fix/session-takeover-silent-message-loss-87180 branch from 8730a7e to 813557d Compare June 2, 2026 02:06
@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

Rebased on latest upstream/main (57ea5af, clean rebase, no conflicts).

@Jerry-Xin
Jerry-Xin force-pushed the fix/session-takeover-silent-message-loss-87180 branch from 813557d to 27ff3b2 Compare June 2, 2026 04:06
@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

Rebased on latest upstream/main (ffbd02f, clean rebase, no conflicts).

@Jerry-Xin
Jerry-Xin force-pushed the fix/session-takeover-silent-message-loss-87180 branch from 27ff3b2 to 2c5326a Compare June 2, 2026 06:06
@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

Rebased on latest upstream/main (85d2dd8, clean rebase, no conflicts).

@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed all review findings from the previous review:

  1. Scoped SDK retry disable — Reverted global maxRetries: 0 across all three OpenAI transport paths. SDK default retry behavior is now preserved for non-embedded callers. maxRetries: 0 is injected only inside the embedded prompt lock window via a streamFn wrapper in attempt.ts.

  2. Preserved wrapped prompt errors — The takeover catch now unwraps promptError from EmbeddedAttemptPromptErrorWithCleanupTakeoverError before classification. Wrapped billing/rate-limit/context-overflow errors flow through normal error handling. Only pure takeover errors return resend guidance.

  3. Real behavior proof added — PR body updated with test evidence showing all 539 tests pass across 7 affected suites, verifying scoped retry behavior, prompt error unwrapping, and takeover error handling.

@clawsweeper

clawsweeper Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed remaining review findings:

  1. Provider retry settings now respected — Extracted installEmbeddedPromptRetryDefault helper that uses { maxRetries: 0, ...options } spread ordering. The 0 is a default that configured settings.retry.provider.maxRetries (flowing through options.maxRetries via sdk.ts:360) can override. No forced stomping of user settings.

  2. Docker real behavior proof added — Tests run in a clean Docker container (node:24-bookworm, Debian 12, x86_64). All 546 tests pass across 6 affected suites. Code path verification confirms:

    • SDK transport paths do NOT include global maxRetries:0
    • Embedded prompt lock path defaults to maxRetries:0 but preserves explicit overrides
    • Takeover catch correctly unwraps preserved prompt errors
  3. New tests — Added installEmbeddedPromptRetryDefault unit tests for both default behavior (no caller options → maxRetries:0) and explicit override preservation.

@clawsweeper

clawsweeper Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed remaining P1 findings:

  1. Native OpenAI transport now forwards maxRetriesbuildOpenAISdkRequestOptions accepts maxRetries parameter and forwards it through all 3 native transport call sites (Responses, Azure Responses, Completions). The embedded prompt lock's injected maxRetries: 0 now actually reaches the SDK on the native transport path.

  2. End-to-end loopback regression test — Mock server returning retryable HTTP 500 errors verifies exactly 1 request when maxRetries: 0 is forwarded through the full createOpenAICompletionsTransportStreamFn() path. Proves SDK retry suppression works on the actual transport.

  3. Docker proof updated (v4) — All 547 tests pass across 6 suites (4 Vitest shards) in node:24-bookworm Docker container. Code path verification confirms all 3 call sites forward options?.maxRetries.

@clawsweeper

clawsweeper Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@Jerry-Xin
Jerry-Xin force-pushed the fix/session-takeover-silent-message-loss-87180 branch from aa77669 to 5bedf1e Compare June 2, 2026 11:32
@clawsweeper clawsweeper Bot removed the rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. label Jun 2, 2026
Jerry-Xin added 29 commits July 25, 2026 12:11
…ReleaseStreamFn

The installEmbeddedPromptRetryDefault wrapper used a narrowed
(model, context, options?) signature that is not assignable to
agent.streamFn's PromptReleaseStreamFn ((...args: unknown[]) => unknown),
breaking tsgo:core and the plugin-sdk dts strict-smoke guard. Switch to a
variadic signature that destructures the positional args and narrows the
request options safely, preserving the maxRetries:0 default with caller
override ordering. Also alias the reassigned catch binding in
runAgentTurnWithFallback to satisfy no-ex-assign.
…87180)

Each embedded turn re-runs installEmbeddedPromptRetryDefault then
installPromptSubmissionLockRelease. The retry-default installer had no
install guard, so repeated turns stacked maxRetries:0 wrappers and
release/reacquire cycles around a single model call.

Both installers wrap agent.streamFn and the lock-release wrapper sits
outermost after turn 1, hiding the retry wrapper's marker underneath. A
marker on the retry wrapper alone is therefore insufficient: on the next
turn the retry installer inspects the outer fn, misses its marker, and
wraps again. Each wrapper now propagates the other's marker onto the new
outermost fn, so whichever wrapper is outermost carries both markers and
both installers stay idempotent across turns.
…nclaw#87180)

The embedded prompt retry default wrapper injected a hard maxRetries:0 for
SDK calls inside the prompt-lock release window. The sdk.ts seam then computed
`optionsLocal?.maxRetries ?? providerRetrySettings.maxRetries`, so the injected
0 shadowed any configured settings.retry.provider.maxRetries (0 ?? 3 = 0).
Users who set provider retries silently lost them for embedded prompt
submissions.

installEmbeddedPromptRetryDefault now accepts the configured provider retry and
injects it as the default; the install site in attempt.ts passes
settingsManager.getProviderRetrySettings().maxRetries. Precedence is now:
explicit per-call options.maxRetries > configured provider maxRetries > 0.
Unconfigured still defaults to 0 so SDK retries stay out of the lock-release
window where model-fallback owns retries and SDK retries race with session
takeover.

Adds regression coverage for the wrapper defaults/override and a chain test
proving the configured provider maxRetries forwards through to the native SDK
request options.
… test

The ReplyOperation type gained retainFailureUntilComplete and
completeWithAfterClearBarrier; the takeover real-setup test mock was
missing both, failing the test-types lane (TS2739). Add them as vi.fn
stubs to match the type, mirroring the existing mock in
agent-runner-execution.test.ts.
When a gateway restart or user stop has already aborted the reply
operation, an EmbeddedAttemptSessionTakeoverError thrown during cleanup
or prompt reacquire must not clobber the established lifecycle outcome
with run_failed and 'Please resend your message' guidance.

Guard the takeover resend early-return so it only fires when the reply
operation is not already restart-aborted or user-aborted. In the
abort-owned case, fall through to the existing isReplyOperationRestartAbort
/ isReplyOperationUserAbort checks, which emit the restart lifecycle text
or the silent user-abort token. The classification between the takeover
branch and those checks has no early return and reads
replyOperation.result rather than err, so plain fall-through is correct.

Add regression tests for takeover combined with restart abort and user
abort, and annotate the normal-takeover test as the guard that the
resend path is not over-suppressed.
…penclaw#87180)

The idempotent installEmbeddedPromptRetryDefault early-returned on repeat
installs, so an already-wrapped session kept injecting the first-captured
maxRetries and silently shadowed later configured changes. After turn 1 the
retry wrapper is buried beneath the lock-release wrapper, so updating only an
outer property was insufficient.

Replace the captured local value with a shared mutable ref object that the
wrapper reads at call time. A repeat install now updates that ref in place
instead of re-wrapping, and the lock-release installer carries the same ref
forward by reference so the buried retry wrapper sees refreshed values.
The pure EmbeddedAttemptSessionTakeoverError resend branch returned resend
guidance without consuming the pending lifecycle terminal, so lifecycle/status
consumers could miss the terminal failure event even though the user saw the
resend prompt. Emit the terminal error like the sibling terminal-failure exits
in the same catch path, and add a focused regression test asserting a
lifecycle error event is emitted on the normal takeover resend path.
The fallback candidate runner now resolves per-candidate fast mode via
resolveRunFastModeForFallbackCandidate. The session-takeover real test's
agent-runner-utils mock predated that export, so the runner threw a
missing-export error before the on-disk lock fence could raise the
takeover error, masking the resend-guidance path. Mirror the sibling
agent-runner-execution test mock so the real takeover flow runs.
…ator layer

The embedded prompt-lock window pins SDK maxRetries to 0 so an in-window
retry cannot widen the session-takeover race (openclaw#87180). That correctly
suppresses in-window retries, but it also dropped the SDK's default retry
of bare connection errors (ECONNRESET, socket hang up, Connection error)
for users who have not configured provider retries.

Restore that resilience at the orchestrator layer, where each retry
re-acquires the session lock and does not widen any single release window:
gate the existing one-shot transient retry on connection errors in addition
to transient HTTP statuses. Add an isConnectionError predicate that reuses
the canonical INTERRUPTED_NETWORK_ERROR_RE so it stays in sync with failover
routing, with direct unit coverage plus orchestrator one-retry/bounded-loop
tests.
…e cause-neutral

Preserve the existing 'Transient HTTP provider error before reply' log
wording for status-based transients (downstream tooling and tests key on
that exact string); bare connection retries get a distinct 'Transient
connection error' label so the cause stays clear in logs.

Make the session-takeover resend guidance cause-neutral: a takeover can be
triggered by any concurrent transcript write during a released prompt lock
(tool, cron, cleanup, or another inbound), not only a connection retry, so
the user-facing message no longer attributes it specifically to a retry.
…ed outer retry gate

The embedded prompt-lock window pins SDK maxRetries to 0 for unconfigured
single-model users, so request-timeout transport errors are rethrown to the
outer one-shot retry gate. That gate only retried HTTP-status transients and
bare connection errors, so unconfigured single-model users lost the SDK's
default timeout retry entirely.

Widen the gate to also retry request-timeout transport errors, with its own
'Transient timeout error' diagnostic label. The timeout disjunct is scoped to
exclude (a) timeout-only fallback summaries (multi-model failover already ran),
(b) CLI subprocess budget kills, and (c) Codex app-server bridge failures --
those read like timeouts but are not transport timeouts and have their own
surfaced copy and replay handling. The existing connection-retry path is
unchanged. Mirror the connection-retry tests for the timeout path and add
exclusion tests for each non-transport family.
…d transient-timeout retry gate

SessionWriteLockTimeoutError messages are formatted as 'session file
locked (timeout ...ms): ...', which matches the broad isTimeoutErrorMessage
predicate. After the timeout disjunct was added to the embedded outer retry
gate, a local session write-lock coordination timeout was classified as a
transient transport timeout and re-ran the whole model-fallback chain, even
though retrying any model hits the same local condition. Exclude
non-provider runtime coordination errors from the timeout disjunct using the
canonical isNonProviderRuntimeCoordinationError classifier (the same seam
model-fallback uses to abort the chain on coordination errors). Add a
regression test asserting a lock-timeout-shaped preserved prompt error does
not consume a retry (runWithModelFallback called once, terminal run_failed),
and add the missing isConnectionError/isTimeoutErrorMessage mock exports to
the real-setup takeover test so it imports and runs to its assertions.
… the real-setup takeover mock

The ReplyOperation interface gained terminalRecovery and markTerminalRecovery
members. The real-setup takeover test's hand-built ReplyOperation mock did not
include them, so check-test-types failed on this file. Add the two members
(terminalRecovery: false, markTerminalRecovery stub) mirroring the sibling
execution test mock, so the test type-checks against the current interface.
…peration mock

Upstream added the required ReplyOperation.hasOwnedSessionId member, which
the real-setup takeover test mock was missing, breaking check-test-types.
Stub it the same way the other reply operation mocks do.
…members

Upstream added acceptedSteeredInboundAudio, startedAtMs, lastActivityAtMs,
recordActivity, and markAcceptedSteeredInboundAudio to ReplyOperation. Add
them to the createMockReplyOperation() literal so check-test-types passes.
The configured provider maxRetries was injected as the SDK default for
calls made inside the embedded prompt lock window. That window releases
the session lock across the model call, and the outer reply orchestrator
owns the single whole-attempt retry (each retry reacquires the lock). An
in-window SDK retry therefore runs against a released lock and races
session takeover, silently losing the message (openclaw#87180).

Pin the in-window SDK retry default to 0 unconditionally. Explicit
per-call maxRetries still overrides (spread last). Removes the now-dead
configurable-default ref machinery and aligns with the invariant already
documented in embedded-agent-helpers/errors.ts.
Upstream moved the transcript-append helper to a test-support module
that no longer consults the withOwnedSessionTranscriptWrites async
context, so the owned steering-write lifecycle test stopped publishing
an owned write and tripped the reacquire fence takeover guard. Route the
steering append through controller.withSessionWriteLock(..., {
publishOwnedWrite: true }) — the same idiom the sibling owned-write
tests use — so the owned write is published and reacquireAfterPrompt
trusts it. Preserves the invariant contrast against the foreign
raw-append case which still triggers takeover.
Align the streamSimple test mock in sdk.test.ts with the tightened
provider signature that requires an AssistantMessageEventStreamContract
return. The mock still records the forwarded options for retry assertions
and now returns a minimal completed assistant done stream so the type
matches.
Fix lint max-lines by moving the connection/timeout transient-retry cases
into agent-runner-execution-transient-retry.test.ts, drop the now-unused
FallbackRunnerParams import, and change the catch alias to const.

Update the CLI/Codex timeout assertions to the current surfaced copy and
advance the retry backoff by a bounded interval instead of draining all
timers so the one-shot orchestrator retry settles deterministically.
…ge entry and reformat

Rebase onto upstream moved provider transports into @openclaw/ai (typed host port), so the local openai-transport-stream re-export no longer forwards createOpenAICompletionsTransportStreamFn. Point the two takeover tests at the package entry that still exports it, matching the existing streaming tests. Also reformat two transport source files to satisfy oxfmt.
The two configured-provider maxRetries forwarding tests built their session
with a bare ModelRegistry.inMemory() that registered no provider backed by the
mocked streamSimple, so the sdk streamFn seam routed through the model registry
runtime and the mock never fired (streamSimpleCalls stayed empty). Register a
test provider wired to the streamSimple mock and set a runtime API key, matching
the existing retry-suite helpers, so the seam's call is captured. Assertions and
precedence (explicit per-call > configured provider > 0) are unchanged.
@Jerry-Xin

Copy link
Copy Markdown
Contributor Author

Rebased onto latest upstream/main (c780bfb, clean rebase, no conflicts). Head is now f79e5ad. @clawsweeper re-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: EmbeddedAttemptSessionTakeoverError causes silent message loss when session lock is released during API retries

2 participants