-
Notifications
You must be signed in to change notification settings - Fork 121
fix(acp): recover when compact is interrupted #275
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
a8b4ffe
387007a
76c4824
b4f999f
5e2845a
546fb73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| +14 −0 | src/CodexAcpServer.ts | |
| +9 −2 | src/CodexCommands.ts | |
| +35 −0 | src/__tests__/CodexACPAgent/CodexAcpClient.test.ts |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,3 +13,34 @@ export const isSessionContextCompacting = ( | |
| } | ||
| return false; | ||
| }; | ||
|
|
||
| export type CanStopAgentOptions = { | ||
| isContextCompacting: boolean; | ||
| isSessionActive: boolean; | ||
| activeAssistantTurnId: string | null; | ||
| isGoalActive: boolean; | ||
| canPauseGoal: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Whether the session Stop control should be exposed. | ||
| * | ||
| * The compaction branch is gated on a cancellable assistant turn: when a | ||
| * pending/in-progress compaction marker remains in history but its assistant | ||
| * entry is already finished (restart, interrupted notification stream), | ||
| * `isContextCompacting` is true while `activeAssistantTurnId` is null. In that | ||
| * state Stop would be shown but `handleStop` rejects the click as | ||
| * `missing_active_turn`, leaving an idle session with a permanently | ||
| * nonfunctional Stop button. Only expose Stop during compaction when there is | ||
| * a turn to cancel. | ||
| */ | ||
| export const canStopAgentEnabled = ({ | ||
| isContextCompacting, | ||
| isSessionActive, | ||
| activeAssistantTurnId, | ||
| isGoalActive, | ||
| canPauseGoal, | ||
| }: CanStopAgentOptions): boolean => | ||
| (isContextCompacting && activeAssistantTurnId != null) || | ||
|
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.
After a daemon restart or interrupted stream leaves both an in-progress compaction marker and an unfinished assistant entry in durable history, this condition still exposes Stop even though nothing is cancellable. Fresh evidence beyond the prior comment is that Useful? React with 👍 / 👎.
Contributor
Author
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. Acknowledged. The fix gates the compaction Stop branch on a non-null activeAssistantTurnId, but as noted that ID is history-derived and may not be live-cancellable after a daemon restart or interrupted stream. Finalizing stale history at cancellation time would require resolving the user turn ID in cancelSession and routing it through markDispatchCancelled, which is a backend change beyond this PR's frontend scope. Happy to take it on as a follow-up if the maintainers agree the live-cancellability signal or stale-finalization path is the right direction. |
||
| (isSessionActive && activeAssistantTurnId != null) || | ||
| (isGoalActive && canPauseGoal); | ||
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.
When the stale compaction belongs to an Operation target turn, this history write publishes a terminal assistant entry before
finalizeCancelledTurnrecords the canceled user status.LodyOperationCoordinatorchecks for cancellation first but then treats any terminal assistant as succeeded (apps/cli/src/orchestration/operation-coordinator.ts:538-547), so its history watcher can durably finish the operation as successful during this window, and the later cancellation cannot undo that completion. Preserve the existing cancellation-first ordering by recording the owning user turn's cancellation before settingfinished/endedAt, or make the updates atomic.Useful? React with 👍 / 👎.