feat(agent-runtime): add getLatestRunId to resolve a thread's latest run#451
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-thread recent-run-id recording and retrieval: types and handler hook, OSSAgentStore best-effort metadata writes and lookup, AgentRuntime delegation returning ChangesRecent Run ID Tracking Feature
Sequence diagramsequenceDiagram
participant Client
participant AgentController
participant AgentRuntime
participant OSSAgentStore
participant Storage
Client->>AgentController: GET /threads/:id/latest-run
AgentController->>AgentRuntime: getLatestRunId(threadId)
AgentRuntime->>OSSAgentStore: getLatestRunId(threadId)
OSSAgentStore->>Storage: read threads/{id}/meta.json
Storage-->>OSSAgentStore: meta.json (may contain latestRunId)
OSSAgentStore-->>AgentRuntime: runId|null
AgentRuntime-->>AgentController: {threadId, runId}
AgentController-->>Client: HTTP 200 / 404
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
core/agent-runtime/src/AgentRuntime.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. core/agent-runtime/src/OSSAgentStore.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. core/agent-runtime/test/AgentRuntime.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to track and retrieve the most recent run ID associated with a thread by adding a lastRunId field to thread metadata and exposing a new getRecentRunId endpoint. Feedback suggests making the recordLastRunId write non-blocking to prevent adding network latency to the critical path of run creation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (threadId) { | ||
| await this.recordLastRunId(threadId, runId); | ||
| } |
There was a problem hiding this comment.
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 }![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
core/controller-decorator/test/AgentController.test.ts (3)
108-116:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
getRecentRunIdin context index test.The
methodsarray doesn't includegetRecentRunId, so the new method isn't being tested to verify it has no contextIndex set.📋 Proposed fix to include getRecentRunId in context index test
it('should not set contextIndex on any method', () => { - const methods = [ 'createThread', 'getThread', 'asyncRun', 'streamRun', 'syncRun', 'getRun', 'cancelRun' ]; + const methods = [ 'createThread', 'getThread', 'getRecentRunId', 'asyncRun', 'streamRun', 'syncRun', 'getRun', 'cancelRun' ]; for (const methodName of methods) {🤖 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/controller-decorator/test/AgentController.test.ts` around lines 108 - 116, Add the missing method name to the test: update the methods array in the 'context index' test to include "getRecentRunId" so MethodInfoUtil.getMethodContextIndex(AgentFooController, 'getRecentRunId') is also asserted to be undefined; this ensures AgentFooController's getRecentRunId is checked alongside createThread, getThread, asyncRun, streamRun, syncRun, getRun, and cancelRun.
147-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing test case for
getRecentRunIdstub error behavior.The
stubMethodsarray doesn't includegetRecentRunId, so there's no test verifying that the default stub throws the expected error message when called.📋 Proposed fix to test getRecentRunId stub behavior
const stubMethods = [ { name: 'createThread', args: [] }, { name: 'getThread', args: [ 'thread_1' ] }, + { name: 'getRecentRunId', args: [ 'thread_1' ] }, { name: 'asyncRun', args: [{ input: { messages: [] } }] }, { name: 'streamRun', args: [{ input: { messages: [] } }] }, { name: 'getRunStream', args: [ 'run_1', '0' ] }, { name: 'syncRun', args: [{ input: { messages: [] } }] }, { name: 'getRun', args: [ 'run_1' ] }, { name: 'cancelRun', args: [ 'run_1' ] }, ];🤖 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/controller-decorator/test/AgentController.test.ts` around lines 147 - 163, Add a test for the missing stub method getRecentRunId: update the stubMethods array in AgentController.test.ts to include { name: 'getRecentRunId', args: [ 'thread_1' ] } (or appropriate args) and ensure the for-loop creates an it(...) case asserting that calling instance.getRecentRunId(...args) rejects with /getRecentRunId not implemented/; reference the AgentFooController instantiation and the existing test pattern used for other stub methods so the new test mirrors those assertions.
132-145:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIncomplete test coverage for
getRecentRunIddefault stub.The test description says "all 8 route methods" but should now be 9. More importantly, the
routeMethodsarray on line 136 doesn't include'getRecentRunId', so the new method is not being tested to verify it has a default stub marked asAGENT_NOT_IMPLEMENTED.📋 Proposed fix to add getRecentRunId to test coverage
- 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', 'getRecentRunId', 'asyncRun', 'streamRun', 'getRunStream', 'syncRun', 'getRun', 'cancelRun' ]; for (const methodName of routeMethods) {🤖 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/controller-decorator/test/AgentController.test.ts` around lines 132 - 145, The test in AgentController.test.ts asserts that the controller prototype has default stubs for all route methods but the routeMethods array (used in the it block) omits 'getRecentRunId'; update the routeMethods array inside the test that iterates over AgentFooController.prototype to include 'getRecentRunId' so the test checks that proto['getRecentRunId'] is a function and that AgentInfoUtil.isNotImplemented(proto['getRecentRunId']) is true, matching the other route method assertions.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@core/controller-decorator/test/AgentController.test.ts`:
- Around line 108-116: Add the missing method name to the test: update the
methods array in the 'context index' test to include "getRecentRunId" so
MethodInfoUtil.getMethodContextIndex(AgentFooController, 'getRecentRunId') is
also asserted to be undefined; this ensures AgentFooController's getRecentRunId
is checked alongside createThread, getThread, asyncRun, streamRun, syncRun,
getRun, and cancelRun.
- Around line 147-163: Add a test for the missing stub method getRecentRunId:
update the stubMethods array in AgentController.test.ts to include { name:
'getRecentRunId', args: [ 'thread_1' ] } (or appropriate args) and ensure the
for-loop creates an it(...) case asserting that calling
instance.getRecentRunId(...args) rejects with /getRecentRunId not implemented/;
reference the AgentFooController instantiation and the existing test pattern
used for other stub methods so the new test mirrors those assertions.
- Around line 132-145: The test in AgentController.test.ts asserts that the
controller prototype has default stubs for all route methods but the
routeMethods array (used in the it block) omits 'getRecentRunId'; update the
routeMethods array inside the test that iterates over
AgentFooController.prototype to include 'getRecentRunId' so the test checks that
proto['getRecentRunId'] is a function and that
AgentInfoUtil.isNotImplemented(proto['getRecentRunId']) is true, matching the
other route method assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 641f13fa-997e-445f-bef5-92333d388b14
📒 Files selected for processing (9)
core/agent-runtime/src/AgentRuntime.tscore/agent-runtime/src/OSSAgentStore.tscore/agent-runtime/test/AgentRuntime.test.tscore/agent-runtime/test/OSSAgentStore.test.tscore/controller-decorator/src/decorator/agent/AgentController.tscore/controller-decorator/src/decorator/agent/AgentHandler.tscore/controller-decorator/test/AgentController.test.tscore/types/agent-runtime/AgentStore.tsplugin/controller/lib/AgentControllerObject.ts
jerryliang64
force-pushed
the
feat-recent-run-id
branch
2 times, most recently
from
14d5607 to
27a4223
Compare
|
Thanks for the review! Adopted the suggestion: |
jerryliang64
force-pushed
the
feat-recent-run-id
branch
from
27a4223 to
03e9449
Compare
|
@coderabbitai Adopted all 3 test-coverage findings in Note: the Gemini comment on |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✨ |
Adds a platform-level capability to look up the most recent run of a
thread without an external index, so consumers no longer need to persist
runId/seq on their side.
- AgentStore/ThreadRecord: new optional `latestRunId` on thread meta and a
`getLatestRunId(threadId)` store method.
- OSSAgentStore: `createRun` records `latestRunId` into the thread meta.json
(best-effort read-modify-write; never blocks run creation), and
`getLatestRunId` reads it back — null when the thread has no run yet
(incl. threads created before this field existed), AgentNotFoundError
when the thread itself is missing.
- AgentRuntime: `getLatestRunId(threadId) -> { threadId, runId }`.
- @AgentController: new route GET /api/v1/threads/:id/latest-run, wired
through the existing stub/delegate mechanism.
Backward compatible: `latestRunId` is optional, old thread meta parses fine
and degrades to a null latest-run.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jerryliang64
force-pushed
the
feat-recent-run-id
branch
from
03e9449 to
70a99da
Compare
jerryliang64
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@core/agent-runtime/test/OSSAgentStore.test.ts`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25207054-1d35-4c09-af34-8d42febf4f49
📒 Files selected for processing (9)
core/agent-runtime/src/AgentRuntime.tscore/agent-runtime/src/OSSAgentStore.tscore/agent-runtime/test/AgentRuntime.test.tscore/agent-runtime/test/OSSAgentStore.test.tscore/controller-decorator/src/decorator/agent/AgentController.tscore/controller-decorator/src/decorator/agent/AgentHandler.tscore/controller-decorator/test/AgentController.test.tscore/types/agent-runtime/AgentStore.tsplugin/controller/lib/AgentControllerObject.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- core/agent-runtime/test/AgentRuntime.test.ts
- core/types/agent-runtime/AgentStore.ts
| 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. | ||
| }); |
There was a problem hiding this comment.
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.
|
1 |
2 participants
What
Adds a platform-level capability to resolve a thread's most latest run without an external index, so
@AgentControllerconsumers no longer need to persistrunId/seqon their side.New endpoint:
GET /api/v1/threads/:id/latest-run→{ threadId, runId }.How
latestRunIdon thread meta, and agetLatestRunId(threadId): Promise<string | null>store method.createRunrecordslatestRunIdinto the threadmeta.jsonvia a best-effort read-modify-write (swallowed + warned on failure, so it never blocks run creation).getLatestRunIdreads it back.getLatestRunId(threadId) → { threadId, runId }.AGENT_METHOD_NAMES).Semantics
AgentNotFoundError(404), consistent withgetThread/getRunrunId: nullBackward compatibility
latestRunIdis optional; old threadmeta.jsonparses fine and degrades to a null latest-run. No migration needed.Consistency note
latestRunIdis a best-effort, weakly-consistent pointer: runs on a thread are normally sequential, but concurrentcreateRunon the same thread is last-write-wins, so it may momentarily point at a non-latest run. OnlylatestRunIdcan race — all other thread metadata is preserved.Tests
OSSAgentStore: record/read, multiple-run latest, metadata preservation, historical null, no-threadId, missing-thread best-effort, not-found throw.AgentRuntime: delegation + return shape + null + not-found.@AgentController: route table updated (count + route metadata).All affected packages pass
tsc; agent-runtime / controller-decorator / plugin-controller test suites green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests