Repository navigation
fix: make agent run timeout configurable #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7428ba8
cbc081b
0283142
0c76d3d
56b207e
64944a7
f5a9588
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| import { Store } from './store.js'; | ||
| import { DEFAULT_WORKER_LEASE_MS, Store } from './store.js'; | ||
| import type { Result, Memory } from '../shared/types.js'; | ||
| import type { Claim } from './store.js'; | ||
| import { research, type Config } from './research.js'; | ||
| import { DEFAULT_AGENT_RUN_TIMEOUT_MS } from './platform-config.js'; | ||
| export class Runner { | ||
| private timer?: ReturnType<typeof setInterval>; | ||
| private active = new Map<string, AbortController>(); | ||
|
|
@@ -14,6 +15,7 @@ export class Runner { | |
| signal: AbortSignal, | ||
| progress: (text: string) => void, | ||
| ) => Promise<Result>, | ||
| private runTimeoutMs = DEFAULT_AGENT_RUN_TIMEOUT_MS, | ||
| ) {} | ||
| start() { | ||
| if (!this.timer) { | ||
|
|
@@ -53,7 +55,11 @@ export class Runner { | |
| } | ||
| private async runTick() { | ||
| if (this.active.size) return; | ||
| const claim = this.store.claim(); | ||
| // Leave time to persist the outcome after the configured run timeout. | ||
| const claim = this.store.claim( | ||
| Date.now(), | ||
| Math.max(DEFAULT_WORKER_LEASE_MS, this.runTimeoutMs * 2), | ||
| ); | ||
| if (!claim) return; | ||
| const controller = new AbortController(); | ||
| this.active.set(claim.id, controller); | ||
|
|
@@ -68,9 +74,11 @@ export class Runner { | |
| const timeout = setTimeout( | ||
| () => | ||
| controller.abort( | ||
| new Error('Research exceeded the 90 second time limit.'), | ||
| new Error( | ||
| `Research exceeded the ${this.runTimeoutMs / 1000} second time limit.`, | ||
| ), | ||
| ), | ||
| 90_000, | ||
| this.runTimeoutMs, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Keep the task lease valid for the configured run duration. With AGENT_RUN_TIMEOUT_MS=240000, another worker polling the same database interrupts the active run at 180 seconds, before the configured timeout. A two-Runner/two-Store fixture reproduces “Worker lease expired” at 180001ms and no result. The task is held interrupted, not automatically run twice. Update lease duration or renewal together with the timeout and cover this interaction. |
||
| ); | ||
| try { | ||
| const settings = this.store.settings(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Forward AGENT_RUN_TIMEOUT_MS through the app service in compose.yml. Setting the documented option in .env currently leaves Docker deployments at the 90-second default because the variable never reaches the container. docker compose config confirms the app environment omits the variable even when set to 240000.