fix(openclaw): make keyFingerprint actually persist, and serialize flushes

Three findings from @kartik-mem0 on this PR. The first two mean the fingerprint
gate this PR added never worked, and the first would have broken the plugin.

keyFingerprint was not in ALLOWED_KEYS, and assertAllowedKeys throws on an
unknown key. So the first successful lookup wrote a config that every later load
of the plugin rejected outright. Added there and to the manifest schema, with a
round-trip test that parses a config carrying the field.

readPluginAuth builds its result field by field and did not include
keyFingerprint, so the comparison always ran against undefined, the resolved
email was never used again, and telemetry fell back to the API key hash for every
event. That is worse than the defect this PR set out to fix, which at least used
the email. Returned now, with a test against the real reader.

Both were invisible to the telemetry tests because those mock the config module,
and the mock returned a field the real reader drops. That is the actual lesson
here, so the new tests live in config-file.test.ts and config.test.ts against the
real implementations. Confirmed both fail without the fixes.

flush() also had no in-flight guard. Two overlapping flushes each detach the
queue and each prepend their own batch back on failure, so the later batch landed
in front of the earlier one and the truncation then dropped the OLDER events
first, inverting the priority the failure path exists to establish. Serialized
with a guard; a second caller returns and the queue waits for the next flush.
Regression test asserts one delivery in flight and the backlog order preserved.

core 37, openclaw 435, opencode 35, deepseek 47. Wire e2e 9 of 9, exit cost
unchanged at 6s failing and 12ms healthy.

Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb
This commit is contained in:
Saket Aryan
2026-09-17 21:59:03 +05:30
parent 7900a5d8d1
commit 8fabc87f6e
7 changed files with 98 additions and 0 deletions
@@ -101,6 +101,7 @@ export function createTelemetry(config: TelemetryConfig) {
let consecutiveFailures = 0;
let retryNotBefore = 0;
let exitFlushAttempted = false;
let flushing = false;
const flushThreshold = config.flushThreshold ?? 10;
const maxQueueSize = config.maxQueueSize ?? 100;
@@ -121,6 +122,12 @@ export function createTelemetry(config: TelemetryConfig) {
});
async function flush(force = false): Promise<void> {
// One at a time. Two overlapping flushes each detach the queue and each
// prepend their own batch back on failure, so the later batch lands in front
// of the earlier one and the truncation then drops the OLDER events first,
// inverting the priority the failure path exists to establish. A second
// caller returns immediately; the queue waits for the next flush.
if (flushing) return;
if (!queue.length) return;
// `force` skips the cooldown. beforeExit is the last chance this process
// gets, and gating it on the same backoff meant that after any failure the
@@ -129,6 +136,7 @@ export function createTelemetry(config: TelemetryConfig) {
if (!force && Date.now() < retryNotBefore) return;
const batch = queue;
queue = [];
flushing = true;
try {
await deliver(batch);
consecutiveFailures = 0;
@@ -155,6 +163,8 @@ export function createTelemetry(config: TelemetryConfig) {
// retry exists to save.
queue = [...batch, ...queue].slice(0, maxQueueSize);
retryNotBefore = Date.now() + Math.min(2 ** consecutiveFailures * 1_000, RETRY_BACKOFF_CEILING_MS);
} finally {
flushing = false;
}
}
@@ -344,3 +344,28 @@ test("the exit flush is attempted once, not until the budget is spent", async ()
assert.equal(attempts, 1, `exit flush ran ${attempts} times`);
telemetry.resetForTesting();
});
test("overlapping flushes do not reorder the backlog behind newer events", async () => {
// Each flush detaches the queue and prepends its own batch back on failure, so
// two in flight at once put the LATER batch in front of the earlier one. The
// truncation then drops the older events first, inverting the priority the
// failure path exists to establish.
let release: (() => void)[] = [];
const telemetry = createTelemetry({
host: "h", source: "S", version: "1", distinctId: "d", flushThreshold: 1000,
delivery: () => new Promise((_resolve, reject) => { release.push(() => reject(new Error("down"))); }),
});
telemetry.capture("first");
const a = telemetry.flush();
telemetry.capture("second");
const b = telemetry.flush();
release.forEach((fn) => fn());
await Promise.all([a, b]);
const events = telemetry.queueForTesting().map((e) => (e as any).event);
assert.equal(release.length, 1, "a second delivery started while one was in flight");
assert.deepEqual(events, ["first", "second"], `backlog reordered: ${events.join(",")}`);
telemetry.resetForTesting();
});
+3
View File
@@ -137,6 +137,9 @@ export function readPluginAuth(): PluginAuthConfig {
autoCapture: cfg.autoCapture as boolean | undefined,
topK: cfg.topK as number | undefined,
anonymousTelemetryId: cfg.anonymousTelemetryId as string | undefined,
// Without this the reader silently drops it, every fingerprint comparison
// fails against undefined, and the resolved email is never used again.
keyFingerprint: cfg.keyFingerprint as string | undefined,
};
}
+1
View File
@@ -148,6 +148,7 @@ const ALLOWED_KEYS = [
"mode",
"apiKey",
"anonymousTelemetryId",
"keyFingerprint",
"baseUrl",
"userId",
"userEmail",
@@ -209,6 +209,10 @@
"type": "string",
"description": "Persistent anonymous telemetry identifier"
},
"keyFingerprint": {
"type": "string",
"description": "Digest of the API key userEmail was resolved for. Set automatically; a mismatch re-resolves the account."
},
"oss": {
"type": "object",
"properties": {
@@ -237,3 +237,43 @@ describe("getBaseUrl", () => {
expect(getBaseUrl()).toBe(DEFAULT_BASE_URL);
});
});
// ---------------------------------------------------------------------------
// keyFingerprint round trip
// ---------------------------------------------------------------------------
describe("keyFingerprint survives a write and read", () => {
it("readPluginAuth returns a persisted keyFingerprint", () => {
// It did not. readPluginAuth builds its result field by field, and this one
// was missing, so every fingerprint comparison ran against undefined, the
// resolved email was never used again, and telemetry silently fell back to
// the API key hash. The telemetry tests could not see it because they mock
// this module and their mock returned the field the real reader dropped.
setConfigFile({
plugins: {
entries: {
"openclaw-mem0": {
config: {
apiKey: "m0-test",
userEmail: "person@example.com",
keyFingerprint: "0123456789abcdef",
},
},
},
},
});
expect(readPluginAuth().keyFingerprint).toBe("0123456789abcdef");
});
it("writePluginAuth persists it where readPluginAuth looks", () => {
setConfigFile({ plugins: { entries: { "openclaw-mem0": { config: {} } } } });
writePluginAuth({ userEmail: "person@example.com", keyFingerprint: "abc123" });
const written = JSON.parse(mockWriteText.mock.calls.at(-1)![1] as string);
const cfg = written.plugins.entries["openclaw-mem0"].config;
expect(cfg.keyFingerprint).toBe("abc123");
expect(cfg.userEmail).toBe("person@example.com");
});
});
@@ -485,3 +485,18 @@ describe("mem0ConfigSchema.parse() — apiKey edge cases", () => {
expect(cfg.needsSetup).toBe(false);
});
});
describe("telemetry fingerprint round trip", () => {
it("a config carrying keyFingerprint is accepted by the real schema", () => {
// What writePluginAuth persists after a successful lookup. It was not in
// ALLOWED_KEYS, and assertAllowedKeys throws, so the first successful
// resolve wrote a config that broke every subsequent load of the plugin.
const persisted = {
apiKey: "m0-test",
userEmail: "person@example.com",
keyFingerprint: "0123456789abcdef",
};
expect(() => mem0ConfigSchema.parse(persisted)).not.toThrow();
});
});