Skip to content
Merged
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
11 changes: 11 additions & 0 deletions core/agent-runtime/src/AgentRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,17 @@ export class AgentRuntime {
return RunBuilder.fromRecord(run).snapshot();
}

/**
* Resolve the most recent run created on a thread. Returns `{ runId: null }`
* when the thread exists but has no recorded run (e.g. threads created
* before run tracking, or with no runs yet). Throws AgentNotFoundError when
* the thread does not exist.
*/
async getLatestRunId(threadId: string): Promise<{ threadId: string; runId: string | null }> {
const runId = await this.store.getLatestRunId(threadId);
return { threadId, runId };
}

/**
* Cancel a running task. The call blocks until either (a) the executor
* reports its session is safely committed to storage, and the task has
Expand Down
66 changes: 66 additions & 0 deletions core/agent-runtime/src/OSSAgentStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,66 @@ export class OSSAgentStore implements AgentStore {
createdAt: nowUnix(),
};
await this.client.put(this.runKey(runId), JSON.stringify(record));
if (threadId) {
// Fire-and-forget: keep the latestRunId pointer write off the run-creation
// hot path (it would otherwise add a GET+PUT roundtrip to every run
// start). Tracked in pendingIndexWrites — same as writeThreadActivityIndex
// — so destroy() drains it before shutdown. recordLatestRunId never rejects
// (it logs and swallows), so no unhandled rejection can leak here.
const tracked: Promise<void> = this.recordLatestRunId(threadId, runId)
.finally(() => {
this.pendingIndexWrites.delete(tracked);
});
this.pendingIndexWrites.add(tracked);
}
Comment on lines +221 to +232

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Awaiting recordLastRunId synchronously blocks the run creation process, adding two extra network roundtrips (one GET and one PUT to the object storage) to the critical path of createRun. Since the client already receives the runId in the response of the run creation call (or via the run_created event in streaming), they do not need to immediately query getRecentRunId to find it.\n\nWe can make this write non-blocking (running in the background) and track it using the existing pendingIndexWrites set so that destroy() still safely awaits it before shutdown. This significantly improves the latency of starting a run.

    if (threadId) {\n      const p = this.recordLastRunId(threadId, runId)\n        .finally(() => {\n          this.pendingIndexWrites.delete(p);\n        });\n      this.pendingIndexWrites.add(p);\n    }

return record;
}

/**
* Record `runId` as the thread's most recent run by merging `latestRunId`
* into the thread metadata (a read-modify-write on `meta.json` that
* preserves all existing fields).
*
* Runs in the background (scheduled fire-and-forget by `createRun` and
* tracked in `pendingIndexWrites`) so it never adds latency to run creation;
* `awaitPendingWrites()` / `destroy()` drain it.
*
* Best-effort: a missing thread meta or any storage error is logged and
* swallowed. When it is skipped, `getLatestRunId` simply degrades to
* returning the previously recorded value (or `null`).
*
* Weak consistency: because the write is asynchronous and an unconditional
* read-modify-write with no compare-and-swap, `getLatestRunId` is eventually
* consistent — a read racing a just-created run may briefly see the prior
* value, and concurrent runs on the same thread are last-writer-wins. Only
* the `latestRunId` field can race; every writer carries the same
* `id/object/metadata/createdAt`, so no other thread metadata is lost.
*/
private async recordLatestRunId(threadId: string, runId: string): Promise<void> {
try {
const metaData = await this.client.get(this.threadMetaKey(threadId));
if (!metaData) {
this.logger.warn(
'[OSSAgentStore] skip latestRunId write: thread meta not found threadId=%s runId=%s',
threadId,
runId,
);
return;
}
const meta = JSON.parse(metaData) as ThreadMetadata;
meta.latestRunId = runId;
await this.client.put(this.threadMetaKey(threadId), JSON.stringify(meta));
} catch (err: unknown) {
const errForLog: Error = err instanceof Error ? err : new Error(String(err));
this.logger.warn(
'[OSSAgentStore] failed to record latestRunId threadId=%s runId=%s',
threadId,
runId,
errForLog,
);
}
}

async getRun(runId: string): Promise<RunRecord> {
const data = await this.client.get(this.runKey(runId));
if (!data) {
Expand All @@ -229,6 +286,15 @@ export class OSSAgentStore implements AgentStore {
return JSON.parse(data) as RunRecord;
}

async getLatestRunId(threadId: string): Promise<string | null> {
const metaData = await this.client.get(this.threadMetaKey(threadId));
if (!metaData) {
throw new AgentNotFoundError(`Thread ${threadId} not found`);
}
const meta = JSON.parse(metaData) as ThreadMetadata;
return meta.latestRunId ?? null;
}

async updateRun(runId: string, updates: Partial<RunRecord>): Promise<void> {
const run = await this.getRun(runId);
const safeUpdates = { ...updates };
Expand Down
25 changes: 25 additions & 0 deletions core/agent-runtime/test/AgentRuntime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,31 @@ describe('test/AgentRuntime.test.ts', () => {
});
});

describe('getLatestRunId', () => {
it('should resolve the most recent run id for a thread', async () => {
const created = await runtime.createThread();
const run = await runtime.syncRun({
threadId: created.id,
input: { messages: [{ role: 'user', content: 'Hi' }] },
});

// latestRunId is recorded in the background; drain before asserting.
await store.awaitPendingWrites();
const result = await runtime.getLatestRunId(created.id);
assert.deepStrictEqual(result, { threadId: created.id, runId: run.id });
});

it('should return runId null for a thread with no run', async () => {
const created = await runtime.createThread();
const result = await runtime.getLatestRunId(created.id);
assert.deepStrictEqual(result, { threadId: created.id, runId: null });
});

it('should reject for a non-existent thread', async () => {
await assert.rejects(() => runtime.getLatestRunId('thread_non_existent'));
});
});

describe('cancelRun', () => {
it('should cancel a run', async () => {
executor.execRun = createSlowExecRun([
Expand Down
56 changes: 56 additions & 0 deletions core/agent-runtime/test/OSSAgentStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,62 @@ describe('test/OSSAgentStore.test.ts', () => {
});
});

describe('recent run id', () => {
it('should record latestRunId on the thread when createRun has a threadId', async () => {
const thread = await store.createThread();
const run = await store.createRun([{ role: 'user', content: 'Hello' }], thread.id);
// latestRunId is written in the background; drain before asserting.
await store.awaitPendingWrites();
assert.equal(await store.getLatestRunId(thread.id), run.id);
});

it('should point getLatestRunId at the most recent run', async () => {
const thread = await store.createThread();
await store.createRun([{ role: 'user', content: 'first' }], thread.id);
const second = await store.createRun([{ role: 'user', content: 'second' }], thread.id);
await store.awaitPendingWrites();
assert.equal(await store.getLatestRunId(thread.id), second.id);
});

it('should preserve existing thread metadata when recording latestRunId', async () => {
const thread = await store.createThread({ origin: 'audit', agentName: 'foo' });
const run = await store.createRun([{ role: 'user', content: 'Hello' }], thread.id);
await store.awaitPendingWrites();
const fetched = await store.getThread(thread.id);
assert.deepEqual(fetched.metadata, { origin: 'audit', agentName: 'foo' });
assert.equal(await store.getLatestRunId(thread.id), run.id);
});

it('should return null for a thread that exists but has no run (historical data)', async () => {
const thread = await store.createThread();
assert.equal(await store.getLatestRunId(thread.id), null);
});

it('should not record latestRunId when createRun has no threadId', async () => {
const run = await store.createRun([{ role: 'user', content: 'Hello' }]);
assert(run.id.startsWith('run_'));
// No thread to query; the run simply has no latest-run pointer anywhere.
});
Comment on lines +264 to +268

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a real assertion in this test case.

Line 264’s test name claims pointer behavior, but Lines 265-268 only assert run id format. It won’t fail if pointer logic regresses. Please assert an observable invariant (for example, create a thread first, run without threadId, then assert that thread’s latest run id is still null).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/agent-runtime/test/OSSAgentStore.test.ts` around lines 264 - 268, The
test "should not record latestRunId when createRun has no threadId" currently
only checks run.id; update it to create a thread first (use store.createThread),
then call store.createRun without a threadId, keep the existing assertion that
run.id startsWith('run_'), then retrieve the original thread (e.g.,
store.getThread or store.getThreadById) and assert that the thread's latestRunId
(thread.latestRunId) is still null/undefined to ensure no pointer was recorded.


it('should not fail run creation when the threadId does not exist', async () => {
// Best-effort pointer write: a missing thread meta is swallowed.
const run = await store.createRun([{ role: 'user', content: 'Hello' }], 'thread_missing');
assert(run.id.startsWith('run_'));
});

it('should throw AgentNotFoundError from getLatestRunId for a non-existent thread', async () => {
await assert.rejects(
() => store.getLatestRunId('thread_non_existent'),
(err: unknown) => {
assert(err instanceof AgentNotFoundError);
assert.equal(err.status, 404);
assert.match(err.message, /Thread thread_non_existent not found/);
return true;
},
);
});
});

describe('init / destroy', () => {
it('should call client init when present', async () => {
let initCalled = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ const AGENT_ROUTES: AgentRouteDefinition[] = [
path: '/threads/:id',
params: [{ index: 0, type: 'pathParam', name: 'id' }],
},
{
methodName: 'getLatestRunId',
httpMethod: HTTPMethodEnum.GET,
path: '/threads/:id/latest-run',
params: [{ index: 0, type: 'pathParam', name: 'id' }],
},
{
methodName: 'asyncRun',
httpMethod: HTTPMethodEnum.POST,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export interface AgentHandler {
isSessionCommitted?(msg: AgentMessage, history: AgentMessage[]): boolean | Promise<boolean>;
createThread?(): Promise<ThreadObject>;
getThread?(threadId: string): Promise<ThreadObjectWithMessages>;
getLatestRunId?(threadId: string): Promise<{ threadId: string; runId: string | null }>;
asyncRun?(input: CreateRunInput): Promise<RunObject>;
streamRun?(input: CreateRunInput): Promise<void>;
syncRun?(input: CreateRunInput): Promise<RunObject>;
Expand Down
19 changes: 13 additions & 6 deletions core/controller-decorator/test/AgentController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('core/controller-decorator/test/AgentController.test.ts', () => {
const methodRoutes = [
{ methodName: 'createThread', httpMethod: HTTPMethodEnum.POST, path: '/threads' },
{ methodName: 'getThread', httpMethod: HTTPMethodEnum.GET, path: '/threads/:id' },
{ methodName: 'getLatestRunId', httpMethod: HTTPMethodEnum.GET, path: '/threads/:id/latest-run' },
{ methodName: 'asyncRun', httpMethod: HTTPMethodEnum.POST, path: '/runs' },
{ methodName: 'streamRun', httpMethod: HTTPMethodEnum.POST, path: '/runs/stream' },
{ methodName: 'syncRun', httpMethod: HTTPMethodEnum.POST, path: '/runs/wait' },
Expand Down Expand Up @@ -106,7 +107,7 @@ describe('core/controller-decorator/test/AgentController.test.ts', () => {

describe('context index', () => {
it('should not set contextIndex on any method', () => {
const methods = [ 'createThread', 'getThread', 'asyncRun', 'streamRun', 'syncRun', 'getRun', 'cancelRun' ];
const methods = [ 'createThread', 'getThread', 'getLatestRunId', 'asyncRun', 'streamRun', 'syncRun', 'getRun', 'cancelRun' ];
for (const methodName of methods) {
const contextIndex = MethodInfoUtil.getMethodContextIndex(AgentFooController, methodName);
assert.strictEqual(contextIndex, undefined, `${methodName} should not have contextIndex`);
Expand All @@ -128,11 +129,11 @@ describe('core/controller-decorator/test/AgentController.test.ts', () => {
});

describe('default implementations', () => {
it('should inject default stubs for all 8 route methods', () => {
it('should inject default stubs for all 9 route methods', () => {
// AgentFooController only implements execRun (smart defaults pattern)
// All 8 route methods should have stub defaults that throw
// All 9 route methods should have stub defaults that throw
const proto = AgentFooController.prototype as any;
const routeMethods = [ 'createThread', 'getThread', 'asyncRun', 'streamRun', 'getRunStream', 'syncRun', 'getRun', 'cancelRun' ];
const routeMethods = [ 'createThread', 'getThread', 'getLatestRunId', 'asyncRun', 'streamRun', 'getRunStream', 'syncRun', 'getRun', 'cancelRun' ];
for (const methodName of routeMethods) {
assert(typeof proto[methodName] === 'function', `${methodName} should be a function`);
assert.strictEqual(
Expand All @@ -146,6 +147,7 @@ describe('core/controller-decorator/test/AgentController.test.ts', () => {
const stubMethods = [
{ name: 'createThread', args: [] },
{ name: 'getThread', args: [ 'thread_1' ] },
{ name: 'getLatestRunId', args: [ 'thread_1' ] },
{ name: 'asyncRun', args: [{ input: { messages: [] } }] },
{ name: 'streamRun', args: [{ input: { messages: [] } }] },
{ name: 'getRunStream', args: [ 'run_1', '0' ] },
Expand All @@ -163,10 +165,10 @@ describe('core/controller-decorator/test/AgentController.test.ts', () => {
});

describe('HTTPControllerMetaBuilder integration', () => {
it('should build metadata with 8 HTTPMethodMeta entries', () => {
it('should build metadata with 9 HTTPMethodMeta entries', () => {
const meta = ControllerMetaBuilderFactory.build(AgentFooController, ControllerType.HTTP) as HTTPControllerMeta;
assert(meta);
assert.strictEqual(meta.methods.length, 8);
assert.strictEqual(meta.methods.length, 9);
assert.strictEqual(meta.path, '/api/v1');
});

Expand All @@ -183,6 +185,11 @@ describe('core/controller-decorator/test/AgentController.test.ts', () => {
assert.strictEqual(getThread.method, HTTPMethodEnum.GET);
assert.deepStrictEqual(getThread.paramMap, new Map([[ 0, new PathParamMeta('id') ]]));

const getLatestRunId = meta.methods.find(m => m.name === 'getLatestRunId')!;
assert.strictEqual(getLatestRunId.path, '/threads/:id/latest-run');
assert.strictEqual(getLatestRunId.method, HTTPMethodEnum.GET);
assert.deepStrictEqual(getLatestRunId.paramMap, new Map([[ 0, new PathParamMeta('id') ]]));

const asyncRun = meta.methods.find(m => m.name === 'asyncRun')!;
assert.strictEqual(asyncRun.path, '/runs');
assert.strictEqual(asyncRun.method, HTTPMethodEnum.POST);
Expand Down
15 changes: 15 additions & 0 deletions core/types/agent-runtime/AgentStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ export interface ThreadRecord {
messages: AgentMessage[];
metadata: Record<string, unknown>;
createdAt: number; // Unix seconds
/**
* Id of the most recently created run on this thread, maintained by
* `createRun` as a best-effort pointer so callers can resolve a thread's
* latest run without an external index. Absent on threads that have no
* runs yet, and on threads created before this field existed — a missing
* value must be treated as "no recent run".
*/
latestRunId?: string;
}

export interface RunRecord {
Expand Down Expand Up @@ -77,5 +85,12 @@ export interface AgentStore {
metadata?: Record<string, unknown>,
): Promise<RunRecord>;
getRun(runId: string): Promise<RunRecord>;
/**
* Id of the most recent run created on the thread, or `null` when the
* thread exists but has no recorded run (including threads created before
* run tracking existed). Throws AgentNotFoundError when the thread itself
* does not exist.
*/
getLatestRunId(threadId: string): Promise<string | null>;
updateRun(runId: string, updates: Partial<RunRecord>): Promise<void>;
}
10 changes: 9 additions & 1 deletion plugin/controller/lib/AgentControllerObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,19 @@ import type { EggLogger } from 'egg';
import { AgentControllerProto } from './AgentControllerProto';

/** Method names that can be delegated to AgentRuntime. */
type AgentMethodName = 'createThread' | 'getThread' | 'asyncRun' | 'syncRun' | 'getRun' | 'cancelRun';
type AgentMethodName =
| 'createThread'
| 'getThread'
| 'getLatestRunId'
| 'asyncRun'
| 'syncRun'
| 'getRun'
| 'cancelRun';

const AGENT_METHOD_NAMES: AgentMethodName[] = [
'createThread',
'getThread',
'getLatestRunId',
'asyncRun',
'syncRun',
'getRun',
Expand Down
Loading