fix(plugins): bound the exit flush, and stop openclaw deleting a legacy account

Second review round. The first two are regressions from the first round.

The forced exit flush added fifteen seconds to host shutdown. Node re-emits
beforeExit whenever the handler schedules async work, so an unconditional
flush(true) looped until the five-attempt budget was spent, and against the real
3s delivery timeout that is 15s added to the shutdown of whatever editor or CLI
is hosting this. The backoff used to end that loop after one attempt; removing it
for the forced path removed the only thing bounding it. Measured at 15008ms, now
6001ms with a one-shot latch, and 12ms when delivery is healthy, which is the
only case most people ever see.

openclaw deleted a legacy account before it had anything to replace it with. An
install predating keyFingerprint has an email and no fingerprint, so the
comparison failed and clearResolvedAccount() ran immediately; if the re-resolve
then failed because the user was offline the email was gone from disk for good,
and the per-key latch was already set so nothing retried. Now cleared only when a
real fingerprint disagrees, which is the same trade the Python core makes and
documents: verify, and keep what you have until the verification succeeds. The
latch is released on a failed lookup so the next capture tries again.

The overflow test did not exercise the path it is named for: with only two
events captured the re-queue had an empty queue to merge into, so the slice on
the failure path never ran, and that is the half deciding which end gets dropped.
It now fills past the cap before flushing and asserts the exact surviving order.

core 36, openclaw 432, opencode 35, deepseek 47. Wire e2e 9 of 9 and identity
e2e 7 of 7 still pass against a real local server.

Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
This commit is contained in:
Saket Aryan
2026-09-17 19:43:25 +05:30
parent 44c6a07ddf
commit cbf97c6077
3 changed files with 53 additions and 5 deletions
@@ -95,6 +95,7 @@ export function createTelemetry(config: TelemetryConfig) {
let timer: ReturnType<typeof setInterval> | undefined;
let consecutiveFailures = 0;
let retryNotBefore = 0;
let exitFlushAttempted = false;
const flushThreshold = config.flushThreshold ?? 10;
const maxQueueSize = config.maxQueueSize ?? 100;
@@ -151,6 +152,14 @@ export function createTelemetry(config: TelemetryConfig) {
}
function beforeExit(): void {
// Once, and only once. Node re-emits beforeExit whenever the handler
// schedules more async work, so an unconditional forced flush looped until
// the attempt budget was spent: five attempts against a 3s delivery timeout
// is fifteen seconds added to the shutdown of whatever editor or CLI is
// hosting this. The backoff used to end that loop after one attempt, and
// removing it for the forced path removed the only thing bounding it.
if (exitFlushAttempted) return;
exitFlushAttempted = true;
void flush(true);
}
@@ -207,16 +207,19 @@ test("a full queue drops the new event and keeps the batch being retried", async
delivery: async () => { throw new Error("down"); },
});
// Fill past the cap BEFORE the flush, so the re-queue actually has to truncate.
// Capturing only two left the queue empty at re-queue time and the slice on the
// failure path never ran, which is the half that decides the direction.
telemetry.capture("a");
telemetry.capture("b");
await telemetry.flush();
telemetry.capture("c");
await telemetry.flush();
telemetry.capture("d");
telemetry.capture("e");
const events = telemetry.queueForTesting().map((e) => (e as any).event);
assert.ok(events.length <= 3, "queue grew past maxQueueSize");
assert.deepEqual(events.slice(0, 2), ["a", "b"], "the retried batch was evicted instead of the new events");
assert.equal(events.length, 3, "queue grew past maxQueueSize");
assert.deepEqual(events, ["a", "b", "c"], "the retried batch was evicted instead of the new events");
telemetry.resetForTesting();
});
@@ -319,3 +322,25 @@ test("a 2xx is a delivery", async () => {
telemetry.resetForTesting();
}
});
test("the exit flush is attempted once, not until the budget is spent", async () => {
// Node re-emits beforeExit whenever the handler schedules async work, so an
// unconditional forced flush looped until MAX_DELIVERY_ATTEMPTS. Against the
// real 3s delivery timeout that is fifteen seconds added to a host's shutdown.
let attempts = 0;
const telemetry = createTelemetry({
host: "h", source: "S", version: "1", distinctId: "d", flushThreshold: 1000,
delivery: async () => { attempts += 1; throw new Error("down"); },
});
telemetry.capture("a");
const handlers = process.listeners("beforeExit");
const ours = handlers[handlers.length - 1] as () => void;
ours();
ours();
ours();
await new Promise((resolve) => setTimeout(resolve, 20));
assert.equal(attempts, 1, `exit flush ran ${attempts} times`);
telemetry.resetForTesting();
});
+16 -2
View File
@@ -58,7 +58,15 @@ function distinctId(apiKey?: string): string {
// apiKey guard the comparison is `undefined === ""` for any call that
// simply omits the key, so a capture with no context wiped a perfectly
// good account out of openclaw.json.
if (apiKey) clearResolvedAccount();
//
// A row with an email and NO fingerprint is the legacy shape, from an
// install predating this field. Clearing it here deleted a real account
// before anything had replaced it, and if the re-resolve then failed
// because the user was offline the email was gone from disk for good. The
// Python core refuses the same trade: verify, and keep what you have until
// the verification succeeds. resolveEmail below overwrites both fields
// when it does, so there is nothing to clear first.
if (apiKey && auth.keyFingerprint) clearResolvedAccount();
}
} catch {
// Fall through to the API key or anonymous identity.
@@ -94,6 +102,11 @@ function resolveEmail(apiKey: string): void {
const fingerprint = keyFingerprint(apiKey);
if (resolutionAttemptedFor === fingerprint) return;
resolutionAttemptedFor = fingerprint;
const releaseLatch = () => {
// A failed lookup must not pin the fallback identity for the rest of the
// process. Released so the next capture tries again.
if (resolutionAttemptedFor === fingerprint) resolutionAttemptedFor = "";
};
fetch(`${getBaseUrl().replace(/\/+$/, "")}/v1/ping/`, {
method: "GET",
headers: { Authorization: `Token ${apiKey}`, "Content-Type": "application/json" },
@@ -110,7 +123,8 @@ function resolveEmail(apiKey: string): void {
}
})
.catch(() => {
// The API-key hash remains a stable fallback.
// The API-key hash remains a stable fallback, and the next capture retries.
releaseLatch();
});
}