Skip to content

Commit 98f6ec5

Browse files
bot_apkdevin-ai-integration[bot]
authored andcommitted
fix: address 6 review comments on PR openclaw#47719
1. [P1] Treat remap failures as resume failures — if replaceSubagentRunAfterSteer returns false, do NOT clear abortedLastRun, increment failed count. 2. [P2] Count scan-level exceptions as retryable failures — set result.failed > 0 in the outer catch block so scheduleOrphanRecovery retry logic triggers. 3. [P2] Persist resumed-session dedupe across recovery retries — accept resumedSessionKeys as a parameter; scheduleOrphanRecovery lifts the Set to its own scope and passes it through retries. 4. [Greptile] Use typed config accessors instead of raw structural cast for TLS check in lifecycle.ts. 5. [Greptile] Forward gateway.reload.deferralTimeoutMs to deferGatewayRestartUntilIdle in scheduleGatewaySigusr1Restart so user-configured value is not silently ignored. 6. [Greptile] Same as #4 — already addressed by the typed config fix. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent c780b6a commit 98f6ec5

File tree

3 files changed

+24
-6
lines changed

3 files changed

+24
-6
lines changed

src/agents/subagent-orphan-recovery.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,16 @@ async function resumeOrphanedSession(params: {
100100
},
101101
timeoutMs: 10_000,
102102
});
103-
replaceSubagentRunAfterSteer({
103+
const remapped = replaceSubagentRunAfterSteer({
104104
previousRunId: params.originalRunId,
105105
nextRunId: result.runId,
106106
});
107+
if (!remapped) {
108+
log.warn(
109+
`resumed orphaned session ${params.sessionKey} but remap failed (old run already removed); treating as failure`,
110+
);
111+
return false;
112+
}
107113
log.info(`resumed orphaned session: ${params.sessionKey}`);
108114
return true;
109115
} catch (err) {
@@ -125,9 +131,11 @@ async function resumeOrphanedSession(params: {
125131
*/
126132
export async function recoverOrphanedSubagentSessions(params: {
127133
getActiveRuns: () => Map<string, SubagentRunRecord>;
134+
/** Persisted across retries so already-resumed sessions are not resumed again. */
135+
resumedSessionKeys?: Set<string>;
128136
}): Promise<{ recovered: number; failed: number; skipped: number }> {
129137
const result = { recovered: 0, failed: 0, skipped: 0 };
130-
const resumedSessionKeys = new Set<string>();
138+
const resumedSessionKeys = params.resumedSessionKeys ?? new Set<string>();
131139
const configChangePattern = /openclaw\.json|openclaw gateway restart|config\.patch/i;
132140

133141
try {
@@ -236,6 +244,10 @@ export async function recoverOrphanedSubagentSessions(params: {
236244
}
237245
} catch (err) {
238246
log.warn(`orphan recovery scan failed: ${String(err)}`);
247+
// Ensure retry logic fires for scan-level exceptions.
248+
if (result.failed === 0) {
249+
result.failed = 1;
250+
}
239251
}
240252

241253
if (result.recovered > 0 || result.failed > 0) {
@@ -265,9 +277,11 @@ export function scheduleOrphanRecovery(params: {
265277
const initialDelay = params.delayMs ?? DEFAULT_RECOVERY_DELAY_MS;
266278
const maxRetries = params.maxRetries ?? MAX_RECOVERY_RETRIES;
267279

280+
const resumedSessionKeys = new Set<string>();
281+
268282
const attemptRecovery = (attempt: number, delay: number) => {
269283
setTimeout(() => {
270-
void recoverOrphanedSubagentSessions(params)
284+
void recoverOrphanedSubagentSessions({ ...params, resumedSessionKeys })
271285
.then((result) => {
272286
if (result.failed > 0 && attempt < maxRetries) {
273287
const nextDelay = delay * RETRY_BACKOFF_MULTIPLIER;

src/cli/daemon-cli/lifecycle.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ function resolveGatewayPortFallback(): Promise<number> {
5151

5252
async function assertUnmanagedGatewayRestartEnabled(port: number): Promise<void> {
5353
const cfg = await readBestEffortConfig().catch(() => undefined);
54-
const tlsEnabled = !!(cfg as { gateway?: { tls?: { enabled?: unknown } } } | undefined)?.gateway
55-
?.tls?.enabled;
54+
const tlsEnabled = !!cfg?.gateway?.tls?.enabled;
5655
const scheme = tlsEnabled ? "wss" : "ws";
5756
const probe = await probeGateway({
5857
url: `${scheme}://127.0.0.1:${port}`,

src/infra/restart.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { spawnSync } from "node:child_process";
22
import os from "node:os";
33
import path from "node:path";
4+
import { loadConfig } from "../config/config.js";
45
import {
56
resolveGatewayLaunchAgentLabel,
67
resolveGatewaySystemdServiceName,
@@ -476,7 +477,11 @@ export function scheduleGatewaySigusr1Restart(opts?: {
476477
emitGatewayRestart();
477478
return;
478479
}
479-
deferGatewayRestartUntilIdle({ getPendingCount: pendingCheck });
480+
const cfg = loadConfig();
481+
deferGatewayRestartUntilIdle({
482+
getPendingCount: pendingCheck,
483+
maxWaitMs: cfg.gateway?.reload?.deferralTimeoutMs,
484+
});
480485
},
481486
Math.max(0, requestedDueAt - nowMs),
482487
);

0 commit comments

Comments
 (0)