fix(coderd/x/chatd): keep task attempts alive while execute tools make progress - #27103
ibetitsmike wants to merge 10 commits into
Conversation
8cf446d to
26369ad
Compare
cfe86d1 to
7f46733
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
7f46733 to
3135ef7
Compare
ceb245f to
0bc4075
Compare
3135ef7 to
b84c305
Compare
0bc4075 to
6a29678
Compare
d2ee076 to
a272bf7
Compare
a8ec642 to
437fcd2
Compare
a272bf7 to
02a5363
Compare
437fcd2 to
8e72ddd
Compare
02a5363 to
57ecf81
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
57ecf81 to
d1d4dfa
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
d1d4dfa to
1aad417
Compare
2916ac1 to
0dcbd7e
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1aad417 to
e912b0b
Compare
0dcbd7e to
62ca73b
Compare
d331b9a to
e06273d
Compare
3bed2f2 to
6e7c6f8
Compare
e06273d to
6b59101
Compare
6e7c6f8 to
9d45193
Compare
6b59101 to
be5e13b
Compare
9d45193 to
b4bb971
Compare
115b230 to
0c310f5
Compare
b4bb971 to
81f3737
Compare
…e progress Task attempts previously died at a fixed 15 minute timeout even while an execute tool was actively polling a long-running process. The timeout is now an idle window that workspace tools reset after each successful agent round-trip, with a separate 24 hour absolute cap that is never reset. Streaming code never resets the watchdog, so the stream-silence guard semantics are unchanged. Part of CODAGT-757.
attemptIdleTimer duplicated chatloop's streamSilenceGuard: the same mutex, quartz timer, cancel cause, and settle-once lifecycle copied into a second concurrency-sensitive type. Extract the primitive into internal/watchdog and use it for both the stream-silence guard and the task attempt idle window, so timer race and shutdown fixes land in one place. The disarm-versus-fire race tests move to the shared package; quartz timer tags are unchanged.
…output The idle watchdog reset was documented as a general workspace-tool behavior, but only the execute and process_output tools kick it, after process-API round-trips. Narrow the architecture doc and the keepalive comment to that reality and record the scoping decision: quick tools finish well within the idle window and deliberately do not extend it.
…ful re-attach snapshot
81f3737 to
e817be7
Compare
|
Superseded by the recomposed CODAGT-757 stack: #27317, #27318, #27319, #27320, #27321, #27322, #27323, #27324, #27325. The combined final tree of the new stack is byte-identical to the reviewed heads of this stack (minus an accidental workflow orchestration file); the recomposition splits the work into reviewable pieces.
|
Problem
Chatd task retries respawn execute tool processes, leaving orphaned duplicates in workspaces (CODAGT-757). The earlier PRs in this stack make retries re-attach to the original process instead of starting a new one, but every task attempt still dies at a fixed 15 minute timeout. A legitimately long-running execute (the clamp allows up to 4 hours) can never finish within one attempt: the attempt times out mid-wait, the task retries, re-attaches, and times out again, burning retries on a command that is making progress.
What this PR changes
coderd/x/chatd/tasks.goas a resettable idle window, and adds a non-resettable 24 hour absolute cap (maxTaskAttemptDuration). Both cancel the attempt with the existing retryableerrTaskTimeout.chattool.WithAttemptKeepalive/chattool.KickAttemptKeepalive(newcoderd/x/chatd/chattool/keepalive.go) using an unexported typed context key. Kicking is a safe no-op when the keepalive is absent or the attempt already ended.taskAttemptContextinstalls the kick into the attempt context.StartProcessreturns (foreground and background), every successfulProcessOutputround insidewaitForProcess(including rounds where the process is still running), the successful re-attach snapshot, and theprocess_outputtool. Other workspace tools finish well within the idle window and deliberately do not extend it.coderd/x/chatd/ARCHITECTURE.md.wait_agent's model-suppliedtimeout_secondsto 15 minutes per call (maxSubagentWaitTimeout), so a sibling tool cannot ride a progressingexecute's keepalives for hours on one blocking call; await_agenttimeout stays a resumable non-failure. A singlecomputerwait action is likewise clamped to 30 seconds.streamSilenceGuardalready used intocoderd/x/chatd/internal/watchdogand backs both the stream-silence guard and the new idle window with it, so the concurrency-sensitive timer lifecycle exists once. Quartz timer tags are unchanged; the disarm-versus-fire race tests move to the shared package.What this PR does not change
Test plan
go test ./coderd/x/chatd/...(full chatd tree, passes)TestTaskAttemptContext_*tests withquartz.NewMock: kicks extend the attempt past the idle window, the 24h cap fires regardless of kicks, a no-kick attempt still dies at 15m, kicking after cancel or timeout is a no-op.process_outputtool kicks on success.internal/watchdogtests: fires with its cause, reset restarts the window, the disarm-versus-fire race cancels at most once, disarm preserves the context cause.make fmt,make lint,make gen: clean, no dirty diff.Stack
PR 3 of 3 for CODAGT-757: builds on #27102 (agent idempotency token), which builds on #27100 (execution ledger).