-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix(coding-agent): serialize post-compaction continuation on run settlement #1817
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
Changes from all commits
40fac3f
980d22d
4d9aa20
8e57ca7
6fd14f2
7369191
c38d47e
a7e26e6
8ba6695
3881a53
8891bfa
ee720c8
0bb6e44
1ed6add
cf8fda3
b4ad907
d0f9438
bf98b51
b6b98ef
3836e61
4d11b97
0f70c78
0338d32
f34b713
f504c64
0519888
ce60aac
a8e4997
7e4f95a
4a67ea2
5c9cb24
93bb548
8449cb2
3162143
34f36d9
d9e1a86
1ff8125
d4d6e52
304ef0e
e3aefdd
938b0a7
8bacbec
b18f772
29e994a
6eb14d8
1cb0f22
384a702
c148b4d
e7b00bc
3c7ffc1
5ad43a1
20c2482
35d4433
ed169d3
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 |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed graceful Python kernel disposal so timed-out final snapshots are cancelled before teardown. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Removed the delay before continuing sessions after compaction. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed remote agent messages being delivered twice when the daemon request timed out or the response was lost: the message is now sent exactly once per call, and post-send failures surface as errors instead of triggering a resend. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Allowed long-running RPC commands and agent turns to complete without fixed client timeouts. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed queued-message editing so duplicate prompts always target the selected queue entry. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed supervised session renames failing after the supervisor approved an available name. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -830,11 +830,12 @@ function createAgentMessageDeferred(): AgentMessageDeferred { | |
|
|
||
| /** One-shot settlement for a scheduled post-compaction continuation; a settled failure is never re-exposed to later waiters. */ | ||
| interface PostCompactionContinuationSettlement extends AgentMessageDeferred { | ||
| continueAfterSessionInput: boolean; | ||
| settled: boolean; | ||
| } | ||
|
|
||
| function createPostCompactionContinuationSettlement(): PostCompactionContinuationSettlement { | ||
| return { ...createAgentMessageDeferred(), settled: false }; | ||
| return { ...createAgentMessageDeferred(), continueAfterSessionInput: false, settled: false }; | ||
| } | ||
|
|
||
| export interface ModelCycleResult { | ||
|
|
@@ -1198,7 +1199,6 @@ export class AgentSession { | |
| private _compactAutoRefinePending = false; | ||
| private _turnIntervalAutoRefinePending = false; | ||
| private _postCompactionContinuationScheduled = false; | ||
| private _postCompactionContinuationTimer: ReturnType<typeof setTimeout> | undefined; | ||
| private _postCompactionContinuationSettlement: PostCompactionContinuationSettlement | undefined; | ||
| private _postCompactionContinuationMessages: AgentMessage[] = []; | ||
| private _scheduledPostCompactionContinuationMessages: AgentMessage[] = []; | ||
|
|
@@ -7311,6 +7311,7 @@ export class AgentSession { | |
| throw new Error("Cannot compact without aborting while the agent is running."); | ||
| } | ||
| const hadPostCompactionContinue = this._postCompactionContinuationScheduled; | ||
| const continueAfterSessionInput = this._postCompactionContinuationSettlement?.continueAfterSessionInput ?? false; | ||
| this._disconnectFromAgent(); | ||
| if (!options.skipAbort) await this.abort(); | ||
| let didCompact = false; | ||
|
|
@@ -7379,7 +7380,7 @@ export class AgentSession { | |
| if (didCompact) { | ||
| this._discardPendingAutoRefine({ cancelPostCompactionContinue: true }); | ||
| if (hadPostCompactionContinue) { | ||
| this._schedulePostCompactionContinue(); | ||
| this._schedulePostCompactionContinue(continueAfterSessionInput); | ||
| } | ||
| // Queued agent or session-owned inputs resume the loop; defer refine | ||
| // behind them instead of interleaving it before their turns. | ||
|
|
@@ -7498,7 +7499,7 @@ export class AgentSession { | |
| } | ||
|
|
||
| private _settlePostCompactionContinue(error?: Error): void { | ||
| if (!error && (this._postCompactionContinuationScheduled || this._postCompactionContinuationTimer)) return; | ||
| if (!error && this._postCompactionContinuationScheduled) return; | ||
| const settlement = this._postCompactionContinuationSettlement; | ||
| if (!settlement || settlement.settled) return; | ||
| settlement.settled = true; | ||
|
|
@@ -7508,10 +7509,6 @@ export class AgentSession { | |
| } | ||
|
|
||
| private _cancelPostCompactionContinue(): void { | ||
| if (this._postCompactionContinuationTimer) { | ||
| clearTimeout(this._postCompactionContinuationTimer); | ||
| this._postCompactionContinuationTimer = undefined; | ||
| } | ||
| this._postCompactionContinuationScheduled = false; | ||
| this._scheduledPostCompactionContinuationMessages = []; | ||
| this._settlePostCompactionContinue(); | ||
|
|
@@ -7607,75 +7604,124 @@ export class AgentSession { | |
| this._scheduleAutoRefine("compact"); | ||
| } | ||
|
|
||
| private _schedulePostCompactionContinue(): void { | ||
| if (this._postCompactionContinuationScheduled) { | ||
| return; | ||
| } | ||
| private _schedulePostCompactionContinue(continueAfterSessionInput = false): void { | ||
| if (!this._postCompactionContinuationSettlement || this._postCompactionContinuationSettlement.settled) { | ||
| this._postCompactionContinuationSettlement = createPostCompactionContinuationSettlement(); | ||
| } | ||
| const settlement = this._postCompactionContinuationSettlement; | ||
| settlement.continueAfterSessionInput ||= continueAfterSessionInput; | ||
| if (this._postCompactionContinuationScheduled) { | ||
| return; | ||
| } | ||
| this._postCompactionContinuationScheduled = true; | ||
| this._scheduledPostCompactionContinuationMessages = [...this._postCompactionContinuationMessages]; | ||
| this._postCompactionContinuationTimer = setTimeout(() => { | ||
| this._postCompactionContinuationTimer = undefined; | ||
| void this._runScheduledPostCompactionContinue() | ||
| .catch(() => undefined) | ||
| .finally(() => this._settlePostCompactionContinue()); | ||
| }, 100); | ||
| void this._runScheduledPostCompactionContinue(settlement) | ||
| .catch(() => undefined) | ||
| .finally(() => { | ||
| if (this._postCompactionContinuationSettlement === settlement) { | ||
| this._settlePostCompactionContinue(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private _sessionOwnsScheduledContinuations(continuationMessages: AgentMessage[]): boolean { | ||
| return continuationMessages.some((message) => this._postCompactionContinuationMessages.includes(message)); | ||
| } | ||
|
|
||
| private async _runScheduledPostCompactionContinue(): Promise<void> { | ||
| await this._waitForRefineIdle(); | ||
| if (!this._postCompactionContinuationScheduled) { | ||
| return; | ||
| } | ||
| if (this.isStreaming || this.isCompacting || this.isRetrying || this._queuedWorkPauses.size > 0) { | ||
| this._postCompactionContinuationScheduled = false; | ||
| this._schedulePostCompactionContinue(); | ||
| return; | ||
| private async _waitForQueuedWorkResume(settlement: PostCompactionContinuationSettlement): Promise<void> { | ||
| while (this._queuedWorkPauses.size > 0 && this._postCompactionContinuationSettlement === settlement) { | ||
| let resume = () => {}; | ||
| const resumed = new Promise<void>((resolve) => { | ||
| resume = resolve; | ||
| this._sessionInputCheckpointWaiters.add(resolve); | ||
| }); | ||
| try { | ||
| await Promise.race([resumed, settlement.promise]); | ||
| } finally { | ||
| this._sessionInputCheckpointWaiters.delete(resume); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const continuationMessages = [...this._scheduledPostCompactionContinuationMessages]; | ||
| if (continuationMessages.length > 0 && !this._sessionOwnsScheduledContinuations(continuationMessages)) { | ||
| this._cancelPostCompactionContinue(); | ||
| this._scheduleAutoRefineAfterAgentEnd(); | ||
| return; | ||
| } | ||
| // An empty queue is not idle while the scheduler still owns active work. | ||
| if (this.unfinishedActionCount > 0 || this._sessionInputPumpRequested) { | ||
| this._scheduleSessionInputPump(); | ||
| await this._sessionInputPump; | ||
| if (this._postCompactionContinuationScheduled) { | ||
| this._postCompactionContinuationScheduled = false; | ||
| const shouldReschedule = | ||
| continuationMessages.length === 0 | ||
| ? this.unfinishedActionCount > 0 | ||
| : this._sessionOwnsScheduledContinuations(continuationMessages); | ||
| if (shouldReschedule) { | ||
| this._schedulePostCompactionContinue(); | ||
| } else { | ||
| this._scheduledPostCompactionContinuationMessages = []; | ||
| private async _runScheduledPostCompactionContinue(settlement: PostCompactionContinuationSettlement): Promise<void> { | ||
| while (this._postCompactionContinuationScheduled && this._postCompactionContinuationSettlement === settlement) { | ||
| await this.agent.waitForIdle(); | ||
| await this.waitForRetry(); | ||
| await this._waitForRefineIdle(); | ||
| await this._waitForQueuedWorkResume(settlement); | ||
| const compactionOperation = this._compactionOperation; | ||
| if (compactionOperation) { | ||
| await Promise.race([compactionOperation, settlement.promise]); | ||
| continue; | ||
| } | ||
|
|
||
| const commitFence = await this._acquireSessionActionCommitFence(); | ||
| let continuation: Promise<void> | undefined; | ||
| let continuationMessages: AgentMessage[] = []; | ||
| let waitForSessionInput = false; | ||
| try { | ||
| await this.agent.waitForIdle(); | ||
| if ( | ||
| !this._postCompactionContinuationScheduled || | ||
| this._postCompactionContinuationSettlement !== settlement | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| if (this._queuedWorkPauses.size > 0 || this._compactionOperation) { | ||
| continue; | ||
| } | ||
|
|
||
| continuationMessages = [...this._scheduledPostCompactionContinuationMessages]; | ||
| if (continuationMessages.length > 0 && !this._sessionOwnsScheduledContinuations(continuationMessages)) { | ||
| this._cancelPostCompactionContinue(); | ||
| this._scheduleAutoRefineAfterAgentEnd(); | ||
| return; | ||
| } | ||
| if (this.unfinishedActionCount > 0 || this._sessionInputPumpRequested) { | ||
| this._scheduleSessionInputPump(); | ||
| waitForSessionInput = true; | ||
| } else { | ||
| this._postCompactionContinuationScheduled = false; | ||
| continuation = this.agent.continue(); | ||
| } | ||
|
Comment on lines
+7685
to
7687
Contributor
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. 🟠 High Messages from A second } else {
this._postCompactionContinuationScheduled = false;
+ await this._waitForRefineIdle();
continuation = this.agent.continue();
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||
| } finally { | ||
| commitFence.release(); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| this._postCompactionContinuationScheduled = false; | ||
| try { | ||
| await this.agent.continue(); | ||
| this._forgetConsumedPostCompactionContinuations(continuationMessages); | ||
| } catch (error) { | ||
| const code = error instanceof AgentContinueError ? error.code : undefined; | ||
| if (code === "busy") { | ||
| this._schedulePostCompactionContinue(); | ||
| } else if (code !== "nothing-to-continue") { | ||
| // "nothing-to-continue" means the turn already completed; anything else must reject headless idle waiters. | ||
| this._settlePostCompactionContinue(this._asError(error)); | ||
| if (waitForSessionInput) { | ||
| await this.waitForIdle(); | ||
| if (this._postCompactionContinuationSettlement !== settlement) return; | ||
| const shouldContinue = | ||
| (settlement.continueAfterSessionInput && continuationMessages.length === 0) || | ||
| this._sessionOwnsScheduledContinuations(continuationMessages); | ||
| if (shouldContinue) { | ||
| this._scheduledPostCompactionContinuationMessages = [...this._postCompactionContinuationMessages]; | ||
| continue; | ||
| } | ||
| this._postCompactionContinuationScheduled = false; | ||
| this._scheduledPostCompactionContinuationMessages = []; | ||
| this._scheduleAutoRefineAfterAgentEnd(); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await continuation; | ||
| this._forgetConsumedPostCompactionContinuations(continuationMessages); | ||
| return; | ||
| } catch (error) { | ||
| const code = error instanceof AgentContinueError ? error.code : undefined; | ||
| if (code === "busy") { | ||
| if (this._postCompactionContinuationSettlement === settlement) { | ||
| this._postCompactionContinuationScheduled = true; | ||
| this._scheduledPostCompactionContinuationMessages = [...this._postCompactionContinuationMessages]; | ||
| } | ||
| continue; | ||
| } | ||
| if (code !== "nothing-to-continue" && this._postCompactionContinuationSettlement === settlement) { | ||
| this._settlePostCompactionContinue(this._asError(error)); | ||
| } | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -8489,7 +8535,7 @@ export class AgentSession { | |
| (reason === "requested" || reason === "threshold") && | ||
| (shouldContinueAfterCompaction || this.agent.hasQueuedMessages() || this.hasPendingSessionWork) | ||
| ) { | ||
| this._schedulePostCompactionContinue(); | ||
| this._schedulePostCompactionContinue(shouldContinueAfterCompaction); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -8541,13 +8587,13 @@ export class AgentSession { | |
| this.agent.state.messages = messages.slice(0, -1); | ||
| } | ||
|
|
||
| this._schedulePostCompactionContinue(); | ||
| this._schedulePostCompactionContinue(true); | ||
| this._scheduleAutoRefineAfterCompaction(willContinueAfterCompaction); | ||
| return true; | ||
| } else if (shouldContinueAfterCompaction || hasQueuedMessages) { | ||
| // Compaction can intentionally stop a tool loop between turns. | ||
| // Queued follow-up/steering/custom messages can also be waiting. | ||
| this._schedulePostCompactionContinue(); | ||
| this._schedulePostCompactionContinue(shouldContinueAfterCompaction); | ||
| this._scheduleAutoRefineAfterCompaction(willContinueAfterCompaction); | ||
| } else { | ||
| this._scheduleAutoRefineAfterCompaction(willContinueAfterCompaction); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,6 @@ import { | |
| parseDiffDisplay, | ||
| parseSentAgentMessage, | ||
| raceStartupWithAbort, | ||
| SNAPSHOT_DISPOSE_TIMEOUT_MS, | ||
| SNAPSHOT_EXECUTION_TIMEOUT_MS, | ||
| } from "./shared.js"; | ||
| import { | ||
|
|
@@ -1100,19 +1099,21 @@ export class ReplKernelManager { | |
| } | ||
| } | ||
|
|
||
| /** Best-effort final snapshot before a graceful dispose, bounded by a timeout. */ | ||
| private async flushSnapshotForDispose(): Promise<void> { | ||
| if (!this.options.snapshot || !this.isRunning) return; | ||
| const pendingExecutions = this.executionQueue; | ||
|
Contributor
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. 🟠 High
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||
| if (this.activeExecution) void this.interrupt().catch(() => undefined); | ||
| let timeout: ReturnType<typeof globalThis.setTimeout> | undefined; | ||
| const guard = new Promise<void>((resolve) => { | ||
| timeout = globalThis.setTimeout(resolve, SNAPSHOT_DISPOSE_TIMEOUT_MS); | ||
| if (timeout && typeof timeout === "object" && "unref" in timeout) timeout.unref(); | ||
| }); | ||
| try { | ||
| await Promise.race([this.snapshotState().then(() => undefined), guard]); | ||
| } finally { | ||
| if (timeout) clearTimeout(timeout); | ||
| } | ||
| const queueSettled = await Promise.race([ | ||
| pendingExecutions.then(() => true), | ||
| new Promise<false>((resolve) => { | ||
| timeout = globalThis.setTimeout(() => resolve(false), SNAPSHOT_EXECUTION_TIMEOUT_MS); | ||
| timeout.unref?.(); | ||
| }), | ||
| ]); | ||
| if (timeout) globalThis.clearTimeout(timeout); | ||
| if (!queueSettled) return; | ||
| await this.captureSnapshot({ executionTimeoutMs: SNAPSHOT_EXECUTION_TIMEOUT_MS }); | ||
| } | ||
|
|
||
| /** Graceful cleanup. Waits briefly for in-flight host request handlers before killing the child. */ | ||
|
|
||
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.
Continuation identity reused across runners
High Severity
_schedulePostCompactionContinuereuses an unsettled settlement whenever_postCompactionContinuationScheduledis already false, and that flag is cleared beforeagent.continue()finishes. A later schedule (including auto-compaction during that same run) therefore starts a second runner on the same settlement, so identity checks cannot tell the stale runner from the new one. The first runner can still forget continuation messages or reject the shared settlement, which drops the follow-up turn or fails headless idle waiters.Additional Locations (2)
packages/coding-agent/src/core/agent-session.ts#L7683-L7724packages/coding-agent/src/core/agent-session.ts#L7500-L7508Reviewed by Cursor Bugbot for commit ed169d3. Configure here.