Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/state/kv.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,40 @@
import type { ISdk } from 'iii-sdk'

// Worker-wide invocationTimeoutMs is 180000ms (src/index.ts), sized for
// LLM-backed functions like mem::graph-extract's provider.compress() call,
// which legitimately needs that much slack for slow local models. Plain KV
// round-trips share no such excuse — a healthy state::get/state::set is a
// local file-backed read/write and should return in well under a second.
// Before this fix, every StateKV call inherited the full 180s worker default
// via iii-sdk's per-call `timeoutMs` override (unused, defaulting through),
// so a single stalled KV call (contention, a stuck file_based adapter) could
// silently block for up to 3 minutes with zero visibility. Functions that
// issue many sequential KV calls per invocation — mem::graph-extract does up
// to 19 — had no per-call signal to distinguish "this one call is stuck" from
// "the LLM is just slow," and the eventual failure surfaced as an opaque
// "Invocation timeout after 180000ms: mem::graph-extract" with no indication
// the actual stall was in a KV call, not the LLM call (see #1127). A short,
// KV-specific timeoutMs makes a stuck call fail fast and attributably
// (function_id + scope/key in the resulting error) instead of silently
// consuming the same 180s budget reserved for LLM work.
const KV_TIMEOUT_MS = 10_000

export class StateKV {
constructor(private sdk: ISdk) {}

async get<T = unknown>(scope: string, key: string): Promise<T | null> {
return this.sdk.trigger<{ scope: string; key: string }, T | null>({
function_id: 'state::get',
payload: { scope, key },
timeoutMs: KV_TIMEOUT_MS,
})
}

async set<T = unknown>(scope: string, key: string, value: T): Promise<T> {
return this.sdk.trigger<{ scope: string; key: string; value: T }, T>({
function_id: 'state::set',
payload: { scope, key, value },
timeoutMs: KV_TIMEOUT_MS,
})
}

Expand All @@ -28,20 +49,23 @@ export class StateKV {
>({
function_id: 'state::update',
payload: { scope, key, ops },
timeoutMs: KV_TIMEOUT_MS,
})
}

async delete(scope: string, key: string): Promise<void> {
return this.sdk.trigger<{ scope: string; key: string }, void>({
function_id: 'state::delete',
payload: { scope, key },
timeoutMs: KV_TIMEOUT_MS,
})
}

async list<T = unknown>(scope: string): Promise<T[]> {
return this.sdk.trigger<{ scope: string }, T[]>({
function_id: 'state::list',
payload: { scope },
timeoutMs: KV_TIMEOUT_MS,
})
}
}
53 changes: 53 additions & 0 deletions test/kv.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { describe, it, expect, vi } from "vitest";
import { StateKV } from "../src/state/kv.js";
import type { ISdk } from "iii-sdk";

function fakeSdk(trigger: ISdk["trigger"]): ISdk {
return { trigger } as unknown as ISdk;
}

describe("StateKV", () => {
it("passes a timeoutMs shorter than the 180s worker default on every call", async () => {
const trigger = vi.fn().mockResolvedValue(null);
const kv = new StateKV(fakeSdk(trigger));

await kv.get("scope", "key");
await kv.set("scope", "key", { a: 1 });
await kv.update("scope", "key", [{ type: "set", path: "/a", value: 1 }]);
await kv.delete("scope", "key");
await kv.list("scope");

expect(trigger).toHaveBeenCalledTimes(5);
for (const call of trigger.mock.calls) {
const request = call[0] as { timeoutMs?: number };
// The whole point of #1127's fix: KV calls must not silently inherit
// the 180s worker default sized for LLM-backed functions — a stuck
// KV call should fail fast, well before that ceiling. Pinned to the
// exact configured value (not just "some number under 180s") so a
// future change to KV_TIMEOUT_MS is a deliberate, visible diff here.
expect(request.timeoutMs).toBe(10_000);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

it("propagates a per-call timeout rejection to the caller (doesn't swallow it)", async () => {
const trigger = vi.fn().mockRejectedValue(new Error("timeout"));
const kv = new StateKV(fakeSdk(trigger));

await expect(kv.get("scope", "key")).rejects.toThrow("timeout");
});

it("forwards the correct function_id and payload alongside the timeout", async () => {
const trigger = vi.fn().mockResolvedValue(null);
const kv = new StateKV(fakeSdk(trigger));

await kv.get("mem:memories", "mem_123");

expect(trigger).toHaveBeenCalledWith(
expect.objectContaining({
function_id: "state::get",
payload: { scope: "mem:memories", key: "mem_123" },
timeoutMs: expect.any(Number),
}),
);
});
});