From da76e6249615524bec9aa0058afb0e617ce00510 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 14 Aug 2026 01:24:15 +0000 Subject: [PATCH 1/3] fix(agent): removed capped empty stop tails Waited for assistant-message persistence before removing a capped empty stop from live and persisted context. Fixes #5179 --- packages/coding-agent/CHANGELOG.md | 4 ++++ packages/coding-agent/src/session/agent-session.ts | 10 +++++----- .../test/agent-session-empty-stop-guard.test.ts | 8 ++++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 32c0a7f79..0748807c1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed capped empty local-model stops remaining in active and persisted context after retries, which could strand subsequent turns. ([#5179](https://github.com/can1357/oh-my-pi/issues/5179)) + ## [16.4.4] - 2026-07-11 ### Changed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 43e469f52..50789a545 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -10980,11 +10980,11 @@ export class AgentSession { this.#clearPendingRecoveredRetryErrors(); this.#retryAttempt = 0; this.#resolveRetry(); - // Tool-use orphans corrupt Anthropic message history (tool_result without - // matching tool_use). Always remove them even when the retry cap is hit. - if (assistantMessage.stopReason === "toolUse") { - this.#discardAssistantTurn(assistantMessage); - } + // A capped empty turn carries no transcript value, while its provider + // usage can anchor later context accounting to the failed request. Wait + // for concurrent message persistence, then remove every empty stop from + // active context and the persisted branch. + await this.#dropPersistedAssistantTurn(assistantMessage); return false; } this.#discardAssistantTurn(assistantMessage); diff --git a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts index 9258161b2..b357dc042 100644 --- a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts +++ b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts @@ -247,7 +247,7 @@ describe("AgentSession empty stop guard", () => { ); expect(orphanedToolUseStops).toHaveLength(0); }); - it("caps empty stop retries at three attempts", async () => { + it("caps empty stop retries and removes the terminal empty turn", async () => { const { session, mock } = await createHarness([ recordCall("beta", "call-record-beta"), emptyStop(), @@ -261,13 +261,13 @@ describe("AgentSession empty stop guard", () => { expect(mock.calls).toHaveLength(5); expect(reminderMessages(session.agent.state.messages)).toHaveLength(3); - expect(emptyAssistantStops(session.agent.state.messages)).toHaveLength(1); + expect(emptyAssistantStops(session.agent.state.messages)).toHaveLength(0); const activeBranchMessages = session.sessionManager .getBranch() .filter(entry => entry.type === "message") .map(entry => entry.message as AgentMessage); - expect(emptyAssistantStops(activeBranchMessages)).toHaveLength(1); + expect(emptyAssistantStops(activeBranchMessages)).toHaveLength(0); }); it("emits failed auto-retry end when repeated empty stops exhaust the retry cap", async () => { @@ -331,7 +331,7 @@ describe("AgentSession empty stop guard", () => { }); expect(retryEndEvents[0]?.finalError).toContain("empty stop"); expect(reminderMessages(session.agent.state.messages)).toHaveLength(3); - expect(emptyAssistantStops(session.agent.state.messages)).toHaveLength(1); + expect(emptyAssistantStops(session.agent.state.messages)).toHaveLength(0); mock.push({ content: ["fresh unrelated success"], stopReason: "stop" }); await session.prompt("start unrelated turn after cap"); From 87970fb2b20e1e97b289cce0108c14b1dbfa4e0c Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 14 Aug 2026 01:41:54 +0000 Subject: [PATCH 2/3] fix(session): made discarded empty stops durable Empty assistant stops were reparented off the active leaf in memory only. The session loader rebuilds the active branch from the last physical journal entry, so a capped empty stop (or one killed mid-retry) resurfaced as the active leaf on reload. Added SessionManager.dropLeafEntry to physically remove the entry and rewrite the file, and route every empty-stop discard through it. Fixes #5179 --- packages/coding-agent/CHANGELOG.md | 2 +- .../coding-agent/src/session/agent-session.ts | 30 +++++++++++-------- .../src/session/session-manager.ts | 25 ++++++++++++++++ .../agent-session-empty-stop-guard.test.ts | 17 ++++++++++- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0748807c1..e4fb49599 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed capped empty local-model stops remaining in active and persisted context after retries, which could strand subsequent turns. ([#5179](https://github.com/can1357/oh-my-pi/issues/5179)) +- Fixed empty local-model stops lingering in the persisted journal after retries; discarded empty turns are now physically removed and the file rewritten, so a reload (or a mid-retry process kill) can no longer resurrect them as the active leaf and strand subsequent turns. ([#5179](https://github.com/can1357/oh-my-pi/issues/5179)) ## [16.4.4] - 2026-07-11 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 50789a545..c4d76a151 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -10963,7 +10963,8 @@ export class AgentSession { } this.#emptyStopRetryCount++; - if (this.#emptyStopRetryCount > EMPTY_STOP_MAX_RETRIES) { + const capExceeded = this.#emptyStopRetryCount > EMPTY_STOP_MAX_RETRIES; + if (capExceeded) { const attempts = this.#emptyStopRetryCount - 1; const finalError = "Assistant returned empty stop after retry cap"; logger.warn(finalError, { @@ -10980,14 +10981,18 @@ export class AgentSession { this.#clearPendingRecoveredRetryErrors(); this.#retryAttempt = 0; this.#resolveRetry(); - // A capped empty turn carries no transcript value, while its provider - // usage can anchor later context accounting to the failed request. Wait - // for concurrent message persistence, then remove every empty stop from - // active context and the persisted branch. - await this.#dropPersistedAssistantTurn(assistantMessage); - return false; } - this.#discardAssistantTurn(assistantMessage); + // An empty turn carries no transcript value, and its provider usage can + // anchor later context accounting to the failed request. It must never + // linger in the persisted journal either: the session loader rebuilds the + // active branch from the last physical entry, so a reparent that is not + // physically committed lets the empty stop resurface as the active leaf on + // reload — or if the process is killed mid retry sequence. Wait for the + // in-flight message_end persistence, remove it from active context + the + // branch, then physically drop the persisted entry and rewrite the file. + const droppedEntryId = await this.#dropPersistedAssistantTurn(assistantMessage); + if (droppedEntryId) await this.sessionManager.dropLeafEntry(droppedEntryId); + if (capExceeded) return false; this.agent.appendMessage({ role: "developer", content: [{ type: "text", text: this.#emptyStopRetryReminder() }], @@ -11126,9 +11131,9 @@ export class AgentSession { * replays the failed turn, while no-recovery paths leave the persisted entry * (and the user-visible transcript line) in place. */ - async #dropPersistedAssistantTurn(assistantMessage: AssistantMessage): Promise { + async #dropPersistedAssistantTurn(assistantMessage: AssistantMessage): Promise { await this.#waitForSessionMessagePersistence(assistantMessage); - this.#discardAssistantTurn(assistantMessage); + return this.#discardAssistantTurn(assistantMessage); } /** @@ -11186,7 +11191,7 @@ export class AgentSession { * the Gemini header-runaway interrupt, which must not replay a partial, * loop-fueling thinking block. */ - #discardAssistantTurn(assistantMessage: AssistantMessage): void { + #discardAssistantTurn(assistantMessage: AssistantMessage): string | undefined { this.#removeAssistantMessageFromActiveContext(assistantMessage); const branchEntry = this.sessionManager @@ -11200,13 +11205,14 @@ export class AgentSession { this.#isSameAssistantMessage(entry.message as AssistantMessage, assistantMessage), ); if (!branchEntry) { - return; + return undefined; } if (branchEntry.parentId === null) { this.sessionManager.resetLeaf(); } else { this.sessionManager.branch(branchEntry.parentId); } + return branchEntry.id; } #isSameAssistantMessage(left: AssistantMessage, right: AssistantMessage): boolean { diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index be61be344..cbcbc4252 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -1734,6 +1734,31 @@ export class SessionManager { this.#index.setLeaf(null); } + /** + * Physically remove a childless entry from the journal and rewrite the file. + * + * {@link branch}/{@link resetLeaf} only move the in-memory leaf; the session + * loader reconstructs the active branch from the *last physical entry* in the + * file (`collectActiveBranchIds`), so an in-memory-only reparent is lost on + * reload when nothing is appended after it. A terminally discarded turn (e.g. + * an empty stop that exhausts the retry cap, with no continuation) must be + * removed durably or it resurfaces as the active leaf on the next load. + * + * No-ops when the entry is unknown or still has children — removing it would + * orphan its subtree. After removal the leaf is reparented to the entry's + * parent, matching the reload path. + */ + async dropLeafEntry(entryId: string): Promise { + const entry = this.#index.get(entryId); + if (!entry) return; + if (this.#index.childrenOf(entryId).length > 0) return; + const parentId = entry.parentId; + this.#entries = this.#entries.filter(candidate => candidate.id !== entryId); + this.#index.rebuild(this.#entries); + this.#index.setLeaf(parentId); + await this.rewriteEntries(); + } + /** Like branch(), but also records a branch_summary of the abandoned path. */ branchWithSummary(branchFromId: string | null, summary: string, details?: unknown, fromExtension?: boolean): string { if (branchFromId !== null && !this.#index.has(branchFromId)) throw new Error(`Entry ${branchFromId} not found`); diff --git a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts index b357dc042..3a433045e 100644 --- a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts +++ b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts @@ -182,6 +182,10 @@ describe("AgentSession empty stop guard", () => { .filter(entry => entry.type === "message") .map(entry => entry.message as AgentMessage); expect(emptyAssistantStops(activeBranchMessages)).toHaveLength(0); + // A discarded empty stop is physically removed from the journal, not just + // reparented off the active branch: it must never be able to resurface as + // the active leaf on reload (the loader rebuilds from the last physical + // entry) if the process is killed before the recovery turn lands. expect( emptyAssistantStops( session.sessionManager @@ -189,7 +193,7 @@ describe("AgentSession empty stop guard", () => { .filter(entry => entry.type === "message") .map(entry => entry.message as AgentMessage), ), - ).toHaveLength(1); + ).toHaveLength(0); }); it("retries a tool-use stop that has no tool call or text", async () => { @@ -268,6 +272,17 @@ describe("AgentSession empty stop guard", () => { .filter(entry => entry.type === "message") .map(entry => entry.message as AgentMessage); expect(emptyAssistantStops(activeBranchMessages)).toHaveLength(0); + + // The session loader reconstructs the active branch from the last physical + // journal entry, so a capped empty stop must be physically removed — not + // just reparented in memory — or it resurfaces as the active leaf on reload. + const journalMessages = session.sessionManager + .getEntries() + .filter(entry => entry.type === "message") + .map(entry => entry.message as AgentMessage); + expect(emptyAssistantStops(journalMessages)).toHaveLength(0); + const lastJournalEntry = session.sessionManager.getEntries().at(-1); + expect(lastJournalEntry?.type === "message" && lastJournalEntry.message.role === "toolResult").toBe(true); }); it("emits failed auto-retry end when repeated empty stops exhaust the retry cap", async () => { From 6c119503664b5dcb11722eac61e17a39ff76985c Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 14 Aug 2026 01:54:52 +0000 Subject: [PATCH 3/3] fix(session): preserved discard metadata children Reparent service-tier metadata before removing discarded empty turns. Persist a branch marker for content-bearing children so reload never selects the discarded subtree. Fixes #5179 --- packages/coding-agent/CHANGELOG.md | 2 +- .../coding-agent/src/session/agent-session.ts | 15 +++-- .../src/session/session-manager.ts | 39 +++++++------ .../agent-session-empty-stop-guard.test.ts | 8 +-- .../session-manager-immediate-persist.test.ts | 55 +++++++++++++++++++ 5 files changed, 89 insertions(+), 30 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e4fb49599..6e86b36f6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed empty local-model stops lingering in the persisted journal after retries; discarded empty turns are now physically removed and the file rewritten, so a reload (or a mid-retry process kill) can no longer resurrect them as the active leaf and strand subsequent turns. ([#5179](https://github.com/can1357/oh-my-pi/issues/5179)) +- Fixed empty local-model stops lingering on the persisted active branch after retries; discarded turns now durably select their parent, preserve safe metadata children, and cannot resurface after reload or a mid-retry process kill. ([#5179](https://github.com/can1357/oh-my-pi/issues/5179)) ## [16.4.4] - 2026-07-11 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index c4d76a151..83f7fae9b 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -10983,15 +10983,14 @@ export class AgentSession { this.#resolveRetry(); } // An empty turn carries no transcript value, and its provider usage can - // anchor later context accounting to the failed request. It must never - // linger in the persisted journal either: the session loader rebuilds the - // active branch from the last physical entry, so a reparent that is not - // physically committed lets the empty stop resurface as the active leaf on - // reload — or if the process is killed mid retry sequence. Wait for the - // in-flight message_end persistence, remove it from active context + the - // branch, then physically drop the persisted entry and rewrite the file. + // anchor later context accounting to the failed request. It must not + // remain on the persisted active branch: the loader rebuilds that branch + // from the last physical entry, so an in-memory-only reparent lets the + // empty stop resurface on reload (or a mid-retry process kill). Wait for + // message_end persistence, remove it from active context + the branch, + // then durably persist the selected path. const droppedEntryId = await this.#dropPersistedAssistantTurn(assistantMessage); - if (droppedEntryId) await this.sessionManager.dropLeafEntry(droppedEntryId); + if (droppedEntryId) await this.sessionManager.discardEntryDurably(droppedEntryId); if (capExceeded) return false; this.agent.appendMessage({ role: "developer", diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index cbcbc4252..4680609cb 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -78,6 +78,7 @@ const JSONL_SUFFIX_LENGTH = ".jsonl".length; const DRAFT_ONLY_SESSION_MARKER = ".draft-only-session"; const SUPERSEDED_COMPACTION_SUMMARY = "[Superseded compaction summary elided after a newer compaction]"; const SUPERSEDED_COMPACTION_SHORT_SUMMARY = "Superseded compaction elided"; +const DISCARDED_ENTRY_BRANCH_MARKER = "discarded-entry-branch"; function mintSessionId(): string { return Bun.randomUUIDv7(); @@ -1735,27 +1736,31 @@ export class SessionManager { } /** - * Physically remove a childless entry from the journal and rewrite the file. + * Durably move the active branch past a discarded entry. * - * {@link branch}/{@link resetLeaf} only move the in-memory leaf; the session - * loader reconstructs the active branch from the *last physical entry* in the - * file (`collectActiveBranchIds`), so an in-memory-only reparent is lost on - * reload when nothing is appended after it. A terminally discarded turn (e.g. - * an empty stop that exhausts the retry cap, with no continuation) must be - * removed durably or it resurfaces as the active leaf on the next load. - * - * No-ops when the entry is unknown or still has children — removing it would - * orphan its subtree. After removal the leaf is reparented to the entry's - * parent, matching the reload path. + * The loader reconstructs the active branch from the last physical journal + * entry, so changing the in-memory leaf alone is lost on reload. Known + * metadata children are chained onto the discarded entry's parent before the + * entry is removed. If any child may carry content, the subtree is preserved + * off-branch instead. Both paths append a metadata-only branch marker and + * rewrite the journal, making the selected path durable. */ - async dropLeafEntry(entryId: string): Promise { + async discardEntryDurably(entryId: string): Promise { const entry = this.#index.get(entryId); if (!entry) return; - if (this.#index.childrenOf(entryId).length > 0) return; - const parentId = entry.parentId; - this.#entries = this.#entries.filter(candidate => candidate.id !== entryId); - this.#index.rebuild(this.#entries); - this.#index.setLeaf(parentId); + const children = this.#index.childrenOf(entryId); + const canReparentChildren = children.every(child => child.type === "service_tier_change"); + let leafId = entry.parentId; + if (canReparentChildren) { + for (const child of children) { + child.parentId = leafId; + leafId = child.id; + } + this.#entries = this.#entries.filter(candidate => candidate.id !== entryId); + this.#index.rebuild(this.#entries); + } + this.#index.setLeaf(leafId); + this.appendCustomEntry(DISCARDED_ENTRY_BRANCH_MARKER, { discardedEntryId: entryId }); await this.rewriteEntries(); } diff --git a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts index 3a433045e..00bc32d0b 100644 --- a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts +++ b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts @@ -273,16 +273,16 @@ describe("AgentSession empty stop guard", () => { .map(entry => entry.message as AgentMessage); expect(emptyAssistantStops(activeBranchMessages)).toHaveLength(0); - // The session loader reconstructs the active branch from the last physical - // journal entry, so a capped empty stop must be physically removed — not - // just reparented in memory — or it resurfaces as the active leaf on reload. + // The loader reconstructs the active branch from the last physical journal + // entry. The empty stop is removed from history and a marker durably + // selects its parent, so reload cannot reactivate the discarded turn. const journalMessages = session.sessionManager .getEntries() .filter(entry => entry.type === "message") .map(entry => entry.message as AgentMessage); expect(emptyAssistantStops(journalMessages)).toHaveLength(0); const lastJournalEntry = session.sessionManager.getEntries().at(-1); - expect(lastJournalEntry?.type === "message" && lastJournalEntry.message.role === "toolResult").toBe(true); + expect(lastJournalEntry).toMatchObject({ type: "custom", customType: "discarded-entry-branch" }); }); it("emits failed auto-retry end when repeated empty stops exhaust the retry cap", async () => { diff --git a/packages/coding-agent/test/session-manager-immediate-persist.test.ts b/packages/coding-agent/test/session-manager-immediate-persist.test.ts index 6aea97787..afbc82ba8 100644 --- a/packages/coding-agent/test/session-manager-immediate-persist.test.ts +++ b/packages/coding-agent/test/session-manager-immediate-persist.test.ts @@ -125,4 +125,59 @@ describe("SessionManager immediate JSONL persistence", () => { expect(messageRole(entries[1] ?? {})).toBe("user"); expect(messageContent(entries[1] ?? {})).toBe("persist me"); }); + + it("reparents metadata children when durably discarding an entry", async () => { + const cwd = makeTempDir("@pi-discard-metadata-cwd-"); + const sessionDir = path.join(cwd, "sessions"); + const manager = SessionManager.create(cwd, sessionDir); + const sessionFile = manager.getSessionFile(); + if (!sessionFile) throw new Error("Expected a persisted session file path"); + + const priorId = manager.appendMessage(assistantMessage("prior turn")); + const discardedId = manager.appendMessage(assistantMessage("")); + const serviceTierId = manager.appendServiceTierChange(null); + await manager.discardEntryDurably(discardedId); + await manager.close(); + + const reloaded = await SessionManager.open(sessionFile, sessionDir); + const branch = reloaded.getBranch(); + expect(branch.some(entry => entry.id === discardedId)).toBe(false); + expect(branch).toContainEqual(expect.objectContaining({ id: serviceTierId, parentId: priorId })); + expect(branch.at(-1)).toMatchObject({ + type: "custom", + customType: "discarded-entry-branch", + parentId: serviceTierId, + }); + await reloaded.close(); + }); + + it("persists a branch marker when a discarded entry has content children", async () => { + const cwd = makeTempDir("@pi-discard-content-cwd-"); + const sessionDir = path.join(cwd, "sessions"); + const manager = SessionManager.create(cwd, sessionDir); + const sessionFile = manager.getSessionFile(); + if (!sessionFile) throw new Error("Expected a persisted session file path"); + + const priorId = manager.appendMessage(assistantMessage("prior turn")); + const discardedId = manager.appendMessage(assistantMessage("")); + const contentChildId = manager.appendMessage({ + role: "user", + content: "preserve off branch", + timestamp: Date.now(), + }); + await manager.discardEntryDurably(discardedId); + await manager.close(); + + const reloaded = await SessionManager.open(sessionFile, sessionDir); + const branch = reloaded.getBranch(); + expect(reloaded.getEntries()).toContainEqual(expect.objectContaining({ id: discardedId })); + expect(reloaded.getEntries()).toContainEqual(expect.objectContaining({ id: contentChildId })); + expect(branch.some(entry => entry.id === discardedId || entry.id === contentChildId)).toBe(false); + expect(branch.at(-1)).toMatchObject({ + type: "custom", + customType: "discarded-entry-branch", + parentId: priorId, + }); + await reloaded.close(); + }); });