diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5f6291db0..929e5e78b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1797,6 +1797,9 @@ - Fixed agents getting stuck waiting for messages from peers that have already stopped running. - Fixed compiled Linux binary extension loading when bundled web-search header generation cannot read `header-generator` data files from the build-time path. ([#5178](https://github.com/can1357/oh-my-pi/issues/5178)) - Fixed plugin custom tool loading to skip and report invalid feature entries instead of crashing startup when a plugin dependency tree leaves one feature unresolved. ([#5189](https://github.com/can1357/oh-my-pi/issues/5189)) +### Fixed + +- 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 26363c92e..f01345318 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -7094,6 +7094,7 @@ export class AgentSession { return undefined; } + #enforceRewindBeforeYield(): boolean { if (!this.#checkpointState || this.#pendingRewindReport) { return false; diff --git a/packages/coding-agent/src/session/session-maintenance.ts b/packages/coding-agent/src/session/session-maintenance.ts index 744a1efe7..7466d6f38 100644 --- a/packages/coding-agent/src/session/session-maintenance.ts +++ b/packages/coding-agent/src/session/session-maintenance.ts @@ -247,7 +247,7 @@ export interface SessionMaintenanceHost { dropImages(): Promise<{ removed: number }>; runHandoff(customInstructions?: string, options?: SessionHandoffOptions): Promise; removeAssistantMessageFromActiveContext(message: AssistantMessage): void; - dropPersistedAssistantTurn(message: AssistantMessage): Promise; + dropPersistedAssistantTurn(message: AssistantMessage): Promise; runRecoveryCompactionWithRollback( reason: "overflow" | "incomplete", message: AssistantMessage, diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index ed3ff3b20..06cc08702 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -90,6 +90,7 @@ import { const JSONL_SUFFIX_LENGTH = ".jsonl".length; const DRAFT_ONLY_SESSION_MARKER = ".draft-only-session"; +const DISCARDED_ENTRY_BRANCH_MARKER = "discarded-entry-branch"; function mintSessionId(): string { return Bun.randomUUIDv7(); @@ -2465,6 +2466,35 @@ export class SessionManager { this.#setLeaf(null); } + /** + * Durably move the active branch past a discarded entry. + * + * 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 discardEntryDurably(entryId: string): Promise { + const entry = this.#index.get(entryId); + if (!entry) return; + 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(); + } + /** 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/src/session/turn-recovery.ts b/packages/coding-agent/src/session/turn-recovery.ts index c0ee48f73..a07cb117e 100644 --- a/packages/coding-agent/src/session/turn-recovery.ts +++ b/packages/coding-agent/src/session/turn-recovery.ts @@ -409,8 +409,8 @@ export class TurnRecovery { return this.#handleUnexpectedAssistantStop(message); } - /** Removes a persisted failed assistant turn after its persistence slot settles. */ - dropPersistedAssistantTurn(message: AssistantMessage): Promise { + /** Removes a persisted failed assistant turn after its persistence slot settles; returns the dropped branch entry id. */ + dropPersistedAssistantTurn(message: AssistantMessage): Promise { return this.#dropPersistedAssistantTurn(message); } @@ -720,10 +720,14 @@ export class TurnRecovery { // provider usage can anchor the next prompt at the full failed-request size // and re-trigger compaction at the same boundary. Remove every capped // empty output; toolUse orphans still need this for Anthropic history. - await this.dropPersistedAssistantTurn(assistantMessage); + await this.#dropAssistantTurnDurably(assistantMessage); return "terminal"; } - this.discardAssistantTurn(assistantMessage); + // The reparented leaf must be durably persisted before the retry continues: + // the loader rebuilds the active branch from the last physical entry, so an + // in-memory-only reparent lets the empty stop resurface on reload or after + // a mid-retry process kill. + await this.#dropAssistantTurnDurably(assistantMessage); this.#host.agent.appendMessage({ role: "developer", content: [{ type: "text", text: this.#emptyStopRetryReminder() }], @@ -846,9 +850,19 @@ export class TurnRecovery { * 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.#host.waitForSessionMessagePersistence(assistantMessage); - this.discardAssistantTurn(assistantMessage); + return this.discardAssistantTurn(assistantMessage); + } + + /** + * Drop the failed turn from active context and the persisted branch, then + * durably persist the reparented leaf so the empty stop cannot resurface on + * reload or after a mid-retry process kill. + */ + async #dropAssistantTurnDurably(assistantMessage: AssistantMessage): Promise { + const droppedEntryId = await this.#dropPersistedAssistantTurn(assistantMessage); + if (droppedEntryId) await this.#host.sessionManager.discardEntryDurably(droppedEntryId); } /** @@ -940,7 +954,7 @@ export class TurnRecovery { * 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 branch = this.#host.sessionManager.getBranch(); @@ -962,7 +976,7 @@ export class TurnRecovery { this.#isSameAssistantMessage(entry.message as AssistantMessage, assistantMessage), ); if (!branchEntry) { - return; + return undefined; } this.#host.withBashBranchTransition(() => { if (branchEntry.parentId === null) { @@ -971,6 +985,7 @@ export class TurnRecovery { this.#host.sessionManager.branch(branchEntry.parentId); } }); + return branchEntry.id; } #isSameAssistantMessage(left: AssistantMessage, right: AssistantMessage): boolean { 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 3078ab7f5..1def562d3 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 @@ -231,6 +231,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 @@ -238,7 +242,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 () => { @@ -392,6 +396,17 @@ describe("AgentSession empty stop guard", () => { .filter(entry => entry.type === "message") .map(entry => entry.message as AgentMessage); expect(emptyAssistantStops(activeBranchMessages)).toHaveLength(0); + + // 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).toMatchObject({ type: "custom", customType: "discarded-entry-branch" }); }); it("waits for capped empty-stop persistence before removing the active branch entry", 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 d9c15d8b3..1553efa91 100644 --- a/packages/coding-agent/test/session-manager-immediate-persist.test.ts +++ b/packages/coding-agent/test/session-manager-immediate-persist.test.ts @@ -365,4 +365,59 @@ describe("SessionManager JSONL software-crash durability", () => { writeSpy.mockRestore(); } }); + + 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(); + }); });