diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index bfe084b63..f10a7207e 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -1723,10 +1723,8 @@ async function executeToolCalls( const checkSteering = async (): Promise => { // `signal` (external/user abort) is checked separately from the internal // abort controllers: once the run is externally aborted it is unwinding - // and the interrupt would be redundant. Once an IRC/steering interrupt has - // already fired, do not poll again — especially not through a consuming - // legacy steering getter. - if (!shouldInterruptImmediately || interruptState.triggered || signal?.aborted) { + // and the interrupt would be redundant. + if (!shouldInterruptImmediately || signal?.aborted) { return; } // Mid-batch steering detection must be non-consuming. If a direct @@ -1739,12 +1737,16 @@ async function executeToolCalls( if (steeringQueued) { // User steering upgrades an in-flight IRC interrupt: it aborts the // shared signal so foreground tools stop as they do for a user Esc. + // Idempotent — a second steer poll after the abort is a no-op. if (!steeringAbortController.signal.aborted) { interruptState.triggered = true; steeringAbortController.abort(); } return; } + // IRC only fires once: a peer interrupt already recorded on interruptState + // must not re-abort, and (unlike steering above) never re-consume a queue. + if (interruptState.triggered) return; if (hasIrcInterrupts && (await hasIrcInterrupts())) { // Peer IRC only aborts interruptible waits: a foreground bash / write // mid-execution keeps running so we never leave partial side effects. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 49db9fcc5..a348f47de 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -18,6 +18,63 @@ - Fixed subagent HUD layout and tree rendering in the TUI to align correctly with other HUD panels - Fixed session persistence corrupting Anthropic signed thinking blocks: the generic 500K-character truncation cap could shorten a large `thinking` string while leaving its cryptographic signature intact, so replaying or resuming the session sent a signature that no longer matched the text and the provider rejected it with an HTTP 400. Signed `thinking` blocks and encrypted `redactedThinking` blobs are now persisted verbatim (all-or-nothing); unsigned thinking and plain text remain truncatable for size control. +- Fixed post-rewind context to tell the agent the checkpoint completed and to make repeat `rewind` calls recover with guidance instead of a bare no-checkpoint error; the branch-scan rehydration now also restores an active checkpoint when a session resumes with the latest checkpoint not yet rewound, so `rewind` can complete instead of failing with "No active checkpoint". ([#4187](https://github.com/can1357/oh-my-pi/issues/4187)) +- Fixed task.maxConcurrency being breachable when a queued spawn was cancelled: the spawn path could release a semaphore permit it never acquired, letting a later task start while the cap was saturated. +- Fixed session exit diagnostics recording signal and crash exits (SIGTERM, SIGHUP, uncaught exceptions) as a normal "dispose": the postmortem teardown now threads the real reason into session disposal. +- Fixed the subagent yield-label guard ignoring JTD discriminator (oneOf) output schemas, which let stale incremental labels pass into successful results when final validation was skipped after retries. +- Fixed grep/ast_grep search scopes rejecting `www.` and collapsed-scheme (`https:/host`) URL spellings that the read tool accepts; unresolvable URL-shaped scopes now fail with an explicit external-URL error instead of "Path not found". +- Fixed model discovery ignoring `NODE_EXTRA_CA_CERTS`: the model registry's default fetch now applies the extra-CA wrapper, so `/models` probes work behind private-CA gateways like provider chat requests. +- Fixed ctrl+p role-model cycling getting stuck on one transition and skipping every other role: a session-branch traversal regression returned entries leaf-to-root, so the cycle (and session model restore) read the oldest recorded model change instead of the newest. +- Fixed ctrl+p cycling from a stale slot after the model was switched through another surface (alt+m, /model, retry fallback): the recorded role is now trusted only while its resolved model is still the active model, falling back to matching by model. +- Fixed the apply_patch tool to prevent silently overwriting pre-existing files during creation or renaming, rejecting upfront with an error instead. +- Fixed multi-file apply_patch to stop at the first failing file, surface applied vs. skipped paths, and correctly report the error to the agent loop. +- Fixed process termination (SIGTERM, SIGHUP, uncaught exceptions) skipping editor draft saves, session shutdown events, and background job cleanup. +- Fixed /quit and /exit commands blocking session closure by introducing a shutdown budget and backgrounding remaining tasks. +- Fixed git and GitHub CLI subprocesses hanging on interactive prompts by forcing non-interactive environments, adding timeouts, and capping output. +- Fixed RPC mode abort_bash being blocked by running bash commands by dispatching bash in the background. +- Fixed task.maxConcurrency and task.maxRecursionDepth limits being bypassed by sub-spawn paths, ensuring limits are dynamically resized and respected. +- Fixed the edit tool inflating session files by pruning extremely large file snapshots from tool-result details. +- Fixed edit-tool Markdown list guidance so hashline parser errors and the model-facing prompt teach `+- item` escaping instead of steering agents toward full-file `write` fallbacks. ([#4179](https://github.com/can1357/oh-my-pi/issues/4179)) +- Fixed workstation OS detection rendering "Kernel: unknown" on macOS 15+. +- Fixed /copy code and /copy cmd commands being treated as normal prompts instead of copying the requested blocks. +- Fixed interactive bash status line not updating after directory changes (cd). +- Fixed session title refreshes ignoring user TITLE_SYSTEM.md overrides during replans, and prevented auto-generated titles from incorrectly preserving all-caps text from user messages. +- Fixed the live todo HUD going stale during long tool-use loops by adding mid-run reminders for incomplete items. +- Fixed /shake and mid-stream chat rebuilds erasing active LLM output. +- Fixed RpcClient failing to restart after being stopped or failing on initial startup. +- Fixed /collab web guests being unable to answer ask tool questions by routing host UI requests through writable collab peers. +- Fixed search and AST tools accepting external read URLs by materializing fetched URL text through the read cache before path resolution. +- Fixed Tavily web search to retry without recency filters if no content is returned. +- Fixed extension validation failures for omp install pi-lean-ctx by exposing legacy tool factories. +- Fixed visibility of the focused option in the multi-select ask picker on certain color themes. +- Fixed TUI row overlapping and duplication in the eval tool's live subagent progress tree under heavy concurrency. +- Fixed session resumes after silent exits by recording pre-tool start markers and shutdown diagnostics. +- Fixed status-line redraw crashes when tool-call arguments contain BigInt values. +- Fixed terminal scrollback rows retaining old colors after theme switches. +- Fixed Esc key behavior in the TUI to clear unrecoverable input instead of preserving drafts. +- Fixed marketplace-installed plugins appearing redundantly in both the npm list and the extension status provider. +- Fixed parent and peer IRC message delivery delays. +- Fixed provider/model:auto entries in modelRoles collapsing to inherit and losing their auto state on reload. +- Fixed plan execution prompts to avoid embedding the full plan, referencing the local plan file instead. +- Fixed subagent live progress leaking raw tool output into the parent TUI. +- Fixed eval subagents with custom output schemas receiving stale incremental yield labels. +- Fixed hidden-thinking live status rows rendering as glyph-only lines by adding a persistent label. +- Fixed /compact summary divider placement to keep it in the live scrollable region. +- Fixed the time_spent status-line segment ticking continuously during idle sessions. +- Fixed browser tool schema validation to require the code argument for run calls. +- Improved robustness of MCP authentication error detection and header-based server discovery. +- Fixed reliable detection of 401/403 authorization failures during Smithery commands and HTTP RPCs. +- Improved streaming preview responsiveness for write, edit, and eval tools by decoding streamed string arguments incrementally. +- Added retry-path diagnostics for assistant-tail removal and scheduled continuations after transient provider errors. +- Fixed CJK history rendering issues across repeated compactions. +- Fixed user-invoked skills failing to identify themselves or resolve relative paths across various execution paths. +- Fixed type errors introduced by the merge sweep: restored the ask row-budget priority field, narrowed dereferenced schema property access, and updated stale test API usage. +- Fixed git clone and fetch being killed by the 5-minute local-command timeout; network transfers now use a separate 30-minute deadline, overridable per call. +- Fixed the TUI collab guest (omp join) silently dropping host ask/selector UI requests; they now present through the standard dialog flow and round-trip responses, with cancellation and resync replay handled. +- Fixed transcript rebuilds (theme change, /shake, focus replay) showing stale streamed write/edit/eval content by sharing the partial-JSON decode between the live streaming path and every rebuild path. +- Fixed an explicitly configured compaction.reserveTokens equal to the built-in default being silently replaced by the proportional small-window fallback; the setting now defaults to unset and explicit values are always honored. +- Fixed user-configured LiteLLM discovery providers keeping stale reseller display-name suffixes for up to 24 hours after upgrade by invalidating the warm model cache. +### Fixed - Fixed several issues with the `apply_patch` and edit tools, including preventing dirty buffers on early aborts, rejecting overwrites of pre-existing files, stopping at the first failing file in multi-file operations, and pruning extremely large file snapshots to prevent session inflation. - Fixed process termination (SIGTERM, SIGHUP, uncaught exceptions) to ensure editor drafts are saved, sessions shut down cleanly, and background jobs are cleaned up. diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 91425bbff..fbb2c6fad 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -430,7 +430,9 @@ function completedRewindFromEntry(entry: SessionEntry): CompletedRewindState | u return report.length > 0 ? { report, startedAt, rewoundAt } : undefined; } -function isSuccessfulCheckpointEntry(entry: SessionEntry): boolean { +function isSuccessfulCheckpointEntry(entry: SessionEntry): entry is SessionMessageEntry & { + message: { role: "toolResult"; toolName: "checkpoint"; isError?: false }; +} { return ( entry.type === "message" && entry.message.role === "toolResult" && @@ -439,6 +441,16 @@ function isSuccessfulCheckpointEntry(entry: SessionEntry): boolean { ); } +function checkpointStartedAtFromEntry(entry: SessionEntry): string | undefined { + if (!isSuccessfulCheckpointEntry(entry)) return undefined; + const details = entry.message.details; + if (details && typeof details === "object") { + const startedAt = stringProperty(details, "startedAt"); + if (startedAt) return startedAt; + } + return entry.timestamp; +} + // A side-channel assistant response is signed for the hidden prompt/history that // produced it. If we persist that response under a different user turn, native // replay anchors become invalid; keep only visible, non-cryptographic content. @@ -2166,7 +2178,7 @@ export class AgentSession { this.#advisorEnabled = this.settings.get("advisor.enabled") as boolean; if (this.#advisorEnabled) this.#buildAdvisorRuntime(); - this.#rehydrateLastCompletedRewind(); + this.#rehydrateCheckpointRewindState(); // Always subscribe to agent events for internal handling // (session persistence, hooks, auto-compaction, retry logic) @@ -6714,14 +6726,54 @@ export class AgentSession { this.agent.setTools(activeTools); } - #rehydrateLastCompletedRewind(): void { + #clearCheckpointRuntimeState(): void { + this.#checkpointState = undefined; + this.#pendingRewindReport = undefined; + this.#lastCompletedRewind = undefined; + this.#rewoundToolResultIds.clear(); + } + + /** + * Rebuild checkpoint/rewind runtime state from the current branch. Handles two + * cases surfaced by session resume, `switchSession()` reloading the same file, + * and tree navigation: + * - The branch's most recent checkpoint has already been rewound → restore + * `#lastCompletedRewind` so a repeat `rewind` call receives the + * "checkpoint already completed" recovery guidance. + * - The branch's most recent checkpoint has NOT been rewound (e.g. the run + * was aborted between `checkpoint` and `rewind`) → restore + * `#checkpointState` so the next `rewind` call can complete the + * checkpoint instead of failing with "No active checkpoint". + */ + #rehydrateCheckpointRewindState(): void { + this.#clearCheckpointRuntimeState(); let completed: CompletedRewindState | undefined; + let pending: { entryId: string; startedAt: string; messageCount: number } | undefined; + let messageCount = 0; for (const entry of this.sessionManager.getBranch()) { + if (entry.type === "message") messageCount++; if (isSuccessfulCheckpointEntry(entry)) { completed = undefined; + pending = { + entryId: entry.id, + startedAt: checkpointStartedAtFromEntry(entry) ?? entry.timestamp, + messageCount, + }; continue; } - completed = completedRewindFromEntry(entry) ?? completed; + const completedFromEntry = completedRewindFromEntry(entry); + if (completedFromEntry) { + completed = completedFromEntry; + pending = undefined; + } + } + if (pending) { + this.#checkpointState = { + checkpointEntryId: pending.entryId, + startedAt: pending.startedAt, + checkpointMessageCount: pending.messageCount, + }; + return; } this.#lastCompletedRewind = completed; } @@ -8304,6 +8356,7 @@ export class AgentSession { await this.sessionManager.flush(); } await this.sessionManager.newSession(options); + this.#clearCheckpointRuntimeState(); this.setTodoPhases([]); this.#freshProviderSessionId = undefined; this.#syncAgentSessionId(); @@ -9715,6 +9768,7 @@ export class AgentSession { await this.sessionManager.flush(); this.#cancelOwnAsyncJobs(); await this.sessionManager.newSession(previousSessionFile ? { parentSession: previousSessionFile } : undefined); + this.#clearCheckpointRuntimeState(); // agent.reset() clears the core steering/follow-up queues. Preserve any queued // steers/follow-ups (RPC/SDK steer()/followUp() issued during the handoff, or a // pre-loader TUI steer) so they survive into the post-handoff session instead of @@ -13970,7 +14024,14 @@ export class AgentSession { ? this.#getSessionDefaultSelectedMCPToolNames(previousSessionFile) : undefined; + // Snapshot the full checkpoint runtime state: the success path calls + // #rehydrateCheckpointRewindState(), which clears and rebuilds all four + // fields from the target branch. On rollback every one must be restored, + // or a failed switch leaks the target session's checkpoint state. + const previousCheckpointState = this.#checkpointState; + const previousPendingRewindReport = this.#pendingRewindReport; const previousLastCompletedRewind = this.#lastCompletedRewind; + const previousRewoundToolResultIds = new Set(this.#rewoundToolResultIds); this.agent.clearAllQueues(); this.#pendingNextTurnMessages = []; @@ -13991,7 +14052,7 @@ export class AgentSession { this.#didSessionMessagesChange(previousSessionContext.messages, sessionContext.messages); const fallbackSelectedMCPToolNames = this.#getSessionDefaultSelectedMCPToolNames(sessionPath); await this.#restoreMCPSelectionsForSessionContext(sessionContext, { fallbackSelectedMCPToolNames }); - this.#rehydrateLastCompletedRewind(); + this.#rehydrateCheckpointRewindState(); // Emit session_switch event to hooks if (this.#extensionRunner) { @@ -14133,7 +14194,10 @@ export class AgentSession { this.agent.replaceQueues(previousSteeringMessages, previousFollowUpMessages); this.#pendingNextTurnMessages = previousPendingNextTurnMessages; this.#scheduledHiddenNextTurnGeneration = previousScheduledHiddenNextTurnGeneration; + this.#checkpointState = previousCheckpointState; + this.#pendingRewindReport = previousPendingRewindReport; this.#lastCompletedRewind = previousLastCompletedRewind; + this.#rewoundToolResultIds = previousRewoundToolResultIds; if (previousModel) { this.agent.setModel(previousModel); } @@ -14202,6 +14266,7 @@ export class AgentSession { } else { this.sessionManager.createBranchedSession(selectedEntry.parentId); } + this.#rehydrateCheckpointRewindState(); this.#syncTodoPhasesFromBranch(); this.#freshProviderSessionId = undefined; this.#syncAgentSessionId(); @@ -14288,6 +14353,7 @@ export class AgentSession { this.#cancelOwnAsyncJobs(); this.sessionManager.createBranchedSession(leafId); + this.#rehydrateCheckpointRewindState(); this.sessionManager.appendMessage({ role: "user", content: [{ type: "text", text: question }], @@ -14484,7 +14550,7 @@ export class AgentSession { const displayContext = deobfuscateSessionContext(stateContext, this.#obfuscator); await this.#restoreMCPSelectionsForSessionContext(displayContext); this.agent.replaceMessages(displayContext.messages); - this.#rehydrateLastCompletedRewind(); + this.#rehydrateCheckpointRewindState(); this.#resetAdvisorSessionState(); this.#syncTodoPhasesFromBranch(); this.#closeCodexProviderSessionsForHistoryRewrite(); diff --git a/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts b/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts index 4c1daac2d..da59ae2fe 100644 --- a/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts +++ b/packages/coding-agent/test/agent-session-checkpoint-rewind-branch.test.ts @@ -131,6 +131,21 @@ function createToolSession(overrides: Partial = {}): ToolSession { }; } +function rewindToolForSession(session: AgentSession): RewindTool { + return new RewindTool( + createToolSession({ + getCheckpointState: () => session.getCheckpointState(), + getLastCompletedRewind: () => session.getLastCompletedRewind(), + }), + ); +} + +async function expectNoActiveCheckpointError(session: AgentSession): Promise { + await expect(rewindToolForSession(session).execute("repeat_rewind", { report: "retry" })).rejects.toThrow( + "No active checkpoint. Create a checkpoint before calling rewind.", + ); +} + describe("AgentSession checkpoint rewind branch context", () => { it("rebuilds active history through branch_summary before the post-rewind assistant turn", async () => { const report = "findings: kept checkpoint; risks: stale signed thinking"; @@ -256,6 +271,59 @@ describe("AgentSession checkpoint rewind branch context", () => { ); }); + it("clears completed rewind state when starting a new session", async () => { + const harness = await createHarness([ + { + content: [{ type: "toolCall", id: "call_checkpoint", name: "checkpoint", arguments: { goal: "inspect" } }], + stopReason: "toolUse", + }, + { + content: [{ type: "toolCall", id: "call_rewind", name: "rewind", arguments: { report: "findings" } }], + stopReason: "toolUse", + }, + { + content: ["DONE"], + stopReason: "stop", + }, + ]); + + await harness.session.prompt("investigate with a checkpoint"); + expect(harness.session.getLastCompletedRewind()).toBeDefined(); + + await harness.session.newSession(); + + expect(harness.session.getLastCompletedRewind()).toBeUndefined(); + await expectNoActiveCheckpointError(harness.session); + }); + + it("rehydrates completed rewind state from the branched path", async () => { + const harness = await createHarness([ + { + content: [{ type: "toolCall", id: "call_checkpoint", name: "checkpoint", arguments: { goal: "inspect" } }], + stopReason: "toolUse", + }, + { + content: [{ type: "toolCall", id: "call_rewind", name: "rewind", arguments: { report: "findings" } }], + stopReason: "toolUse", + }, + { + content: ["DONE"], + stopReason: "stop", + }, + ]); + + await harness.session.prompt("investigate with a checkpoint"); + expect(harness.session.getLastCompletedRewind()).toBeDefined(); + const userEntry = harness.session.sessionManager + .getEntries() + .find(entry => entry.type === "message" && entry.message.role === "user"); + if (!userEntry) throw new Error("Expected user entry for branch"); + + await harness.session.branch(userEntry.id); + + expect(harness.session.getLastCompletedRewind()).toBeUndefined(); + await expectNoActiveCheckpointError(harness.session); + }); it("tells the model to continue when rewind is repeated after completion", async () => { const tool = new RewindTool( createToolSession({ @@ -271,4 +339,84 @@ describe("AgentSession checkpoint rewind branch context", () => { "Checkpoint already completed; continue from the retained rewind report instead of calling rewind again.", ); }); + + it("rehydrates active checkpoint state when resuming a session with no rewind yet", async () => { + const startedAt = "2026-01-01T00:00:00.000Z"; + const harness = await createHarness([ + { + content: [{ type: "toolCall", id: "call_checkpoint", name: "checkpoint", arguments: { goal: "inspect" } }], + stopReason: "toolUse", + }, + { + content: [{ type: "toolCall", id: "call_rewind", name: "rewind", arguments: { report: "findings" } }], + stopReason: "toolUse", + }, + { + content: ["DONE"], + stopReason: "stop", + }, + ]); + await harness.session.prompt("investigate with a checkpoint"); + const originalCompleted = harness.session.getLastCompletedRewind(); + expect(originalCompleted).toBeDefined(); + + // Simulate "run aborted between checkpoint and rewind" by branching to the + // checkpoint entry itself — the rewind and its rewind-report entry drop off + // the active branch, leaving the checkpoint tool result as the leaf. + const branch = harness.session.sessionManager.getBranch(); + const checkpointEntry = branch.find( + entry => + entry.type === "message" && entry.message.role === "toolResult" && entry.message.toolName === "checkpoint", + ); + if (!checkpointEntry) throw new Error("Expected checkpoint tool result entry"); + harness.session.sessionManager.branch(checkpointEntry.id); + + const reloadedMock = createMockModel({ responses: [] }); + const reloadedSettings = Settings.isolated({ + "compaction.enabled": false, + "retry.enabled": false, + "todo.enabled": false, + "todo.eager": "default", + "todo.reminders": false, + }); + reloadedSettings.setModelRole("default", `${reloadedMock.provider}/${reloadedMock.id}`); + const reloadedTools = [checkpointTool as AgentTool, rewindTool as AgentTool]; + const reloadedAgent = new Agent({ + getApiKey: () => "test-key", + initialState: { + model: reloadedMock, + systemPrompt: ["Test"], + tools: reloadedTools, + messages: harness.session.sessionManager.buildSessionContext().messages, + }, + convertToLlm, + streamFn: reloadedMock.stream, + }); + const reloadedSession = new AgentSession({ + agent: reloadedAgent, + sessionManager: harness.session.sessionManager, + settings: reloadedSettings, + modelRegistry: new ModelRegistry( + harness.authStorage, + path.join(harness.tempDir.path(), "models-reloaded.yml"), + ), + toolRegistry: new Map(reloadedTools.map(tool => [tool.name, tool])), + }); + harness.extraSessions.push(reloadedSession); + + const restored = reloadedSession.getCheckpointState(); + expect(restored).toBeDefined(); + expect(restored?.checkpointEntryId).toBe(checkpointEntry.id); + expect(restored?.startedAt).toBe(startedAt); + expect(reloadedSession.getLastCompletedRewind()).toBeUndefined(); + + // The rewind tool must accept the request now that the active checkpoint + // has been re-hydrated — previously this threw "No active checkpoint". + const rewindResult = await rewindToolForSession(reloadedSession).execute("call_rewind_after_resume", { + report: "post-resume findings", + }); + expect(rewindResult.content.some(part => part.type === "text" && part.text.includes("Rewind requested"))).toBe( + true, + ); + }); });