Merge PR #7785: fix(coding-agent): make prewalk lifecycle one-shot (@eggpeat)

This commit is contained in:
can1357
2026-08-07 13:39:51 +02:00
9 changed files with 293 additions and 38 deletions
+1
View File
@@ -75,6 +75,7 @@
- Fixed extension and custom tools inheriting a same-named built-in TUI renderer, which could replace successful result content with incorrect built-in status text ([#7770](https://github.com/can1357/oh-my-pi/issues/7770)).
- Fixed the bundled `ts-no-tiny-functions` TTSR rule never firing on one-line arrow functions in real files: the second alternative's `$` anchor only matched at the absolute end of input, so the trailing newline present in every real file suppressed the match. The condition now opens with the `(?m)` inline flag so the arrow body matches to the line end ([#6890](https://github.com/can1357/oh-my-pi/issues/6890)).
- Fixed `omp commit` exiting 0 when the commit agent failed and the mechanical fallback wrote the commit: the command now exits non-zero when the fallback was used, so callers can distinguish a degraded numstat commit from a legitimate single-commit decision ([#7835](https://github.com/can1357/oh-my-pi/issues/7835)).
- Fixed prewalk lifecycle handling so same-model/same-effort arms reject without plan injection or false success output, consumed plan nudges do not return after context rebuilds, explicit re-arms require a fresh TODO, and settings-enabled prewalk does not implicitly re-arm restored sessions.
## [17.2.9] - 2026-08-05
+3 -2
View File
@@ -1033,11 +1033,12 @@ export async function buildSessionOptions(
if (parsed.noPrewalk && (parsed.prewalk || parsed.prewalkInto !== undefined)) {
throw new Error("--no-prewalk cannot be combined with --prewalk or --prewalk-into");
}
const explicitPrewalk = parsed.prewalk === true || parsed.prewalkInto !== undefined;
const prewalkEnabled = parsed.noPrewalk
? false
: parsed.prewalk === true || parsed.prewalkInto !== undefined
: explicitPrewalk
? true
: activeSettings.get("prewalk.enabled");
: !restoringSession && activeSettings.get("prewalk.enabled");
if (prewalkEnabled) {
const rolePattern = expandRoleAlias(parsed.prewalkInto ?? DEFAULT_PREWALK_TARGET, activeSettings);
const resolved = resolveCliModel({ cliModel: rolePattern, modelRegistry, preferences: modelMatchPreferences });
@@ -304,7 +304,7 @@ import {
USER_INTERRUPT_LABEL,
} from "./messages";
import { ModelControls, type ModelControlsHost } from "./model-controls";
import { PrewalkCoordinator, type PrewalkCoordinatorHost } from "./prewalk";
import { isPrewalkPlanNudge, PrewalkCoordinator, type PrewalkCoordinatorHost } from "./prewalk";
import {
isAdvisorCard,
isDisplayableQueuedMessage,
@@ -881,8 +881,8 @@ export class AgentSession {
/**
* Arm prewalk outside the normal startup path so an explicit slash command starts immediately.
*/
armPrewalk(target: Model, thinkingLevel?: ConfiguredThinkingLevel): void {
this.#prewalk.arm(target, thinkingLevel);
armPrewalk(target: Model, thinkingLevel?: ConfiguredThinkingLevel): boolean {
return this.#prewalk.arm(target, thinkingLevel);
}
/** Validate the active plan artifact and shape an `xd://propose` result for review-mode hosts. */
@@ -2489,14 +2489,17 @@ export class AgentSession {
const persistMessageEnd = () => {
// Check if this is a hook/custom message
if (event.message.role === "hookMessage" || event.message.role === "custom") {
// Persist as CustomMessageEntry
this.sessionManager.appendCustomMessageEntry(
event.message.customType,
event.message.content,
event.message.display,
event.message.details,
event.message.attribution ?? "agent",
);
// Prewalk's plan nudge is a one-run steering instruction. Persisting it would
// resurrect the consumed prompt on resume, fork, or any context rebuild.
if (!isPrewalkPlanNudge(event.message)) {
this.sessionManager.appendCustomMessageEntry(
event.message.customType,
event.message.content,
event.message.display,
event.message.details,
event.message.attribution ?? "agent",
);
}
if (event.message.role === "custom" && event.message.customType === "ttsr-injection") {
this.#ttsr.markInjectedFromDetails(event.message.details);
}
@@ -42,6 +42,7 @@ import { formatOutputNotice } from "../tools/output-meta";
export const SKILL_PROMPT_MESSAGE_TYPE = "skill-prompt";
export const LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE = "lsp-late-diagnostic";
export const BACKGROUND_TAN_DISPATCH_MESSAGE_TYPE = "background-tan-dispatch";
export const PREWALK_PLAN_MESSAGE_TYPE = "prewalk-plan";
/**
* Logs provider-error turns so their actual cause is available outside the
+50 -15
View File
@@ -14,11 +14,16 @@ import { type ConfiguredThinkingLevel, prewalkWouldBeNoop } from "../thinking";
import type { PlanProposalHandler } from "../tools/resolve";
import { ToolError } from "../tools/tool-errors";
import type { PlanYolo, Prewalk } from "./agent-session-types";
import { PREWALK_PLAN_MESSAGE_TYPE } from "./messages";
import type { SessionManager } from "./session-manager";
const PREWALK_PLAN_MESSAGE_TYPE = "prewalk-plan";
const PREWALK_CONTINUE_MESSAGE_TYPE = "prewalk-continue";
const PREWALK_CHECKLIST_MESSAGE_TYPE = "prewalk-checklist";
/** Hidden plan steering is consumed within the live run and must not reappear after a context rebuild. */
export function isPrewalkPlanNudge(message: AgentMessage): boolean {
return message.role === "custom" && message.customType === PREWALK_PLAN_MESSAGE_TYPE;
}
const PREWALK_ACTION_TOOLS: Record<string, true> = {
edit: true,
write: true,
@@ -99,10 +104,40 @@ export class PrewalkCoordinator {
return this.#prewalk;
}
#isNoop(prewalk: Prewalk): boolean {
return prewalkWouldBeNoop(
this.#host.model(),
this.#host.configuredThinkingLevel(),
prewalk.target,
prewalk.thinkingLevel,
);
}
#clearPrewalkState(): void {
this.#prewalk = undefined;
this.#planInjected = false;
this.#continuePending = false;
this.#todoSeen = false;
}
#disarmNoop(prewalk: Prewalk): void {
this.#clearPrewalkState();
this.#host.emitNotice(
"info",
`Prewalk: target ${prewalk.target.provider}/${prewalk.target.id} already matches the active model and thinking level; nothing to switch.`,
"prewalk",
);
}
/** Advances the one-way prewalk switch at a completed assistant-turn boundary. */
async advanceAtTurnEnd(liveMessages: AgentMessage[], context: AgentTurnEndContext | undefined): Promise<void> {
const prewalk = this.#prewalk;
if (!prewalk || context?.message.role !== "assistant") return;
if (this.#isNoop(prewalk)) {
this.#scrubPlanNudge(liveMessages);
this.#disarmNoop(prewalk);
return;
}
if (context.toolResults.some(result => result.toolName === "todo" && !result.isError)) this.#todoSeen = true;
const hasToolResults = context.toolResults.length > 0;
@@ -147,18 +182,12 @@ export class PrewalkCoordinator {
}
this.#scrubPlanNudge(liveMessages);
const target = prewalk.target;
const currentModel = this.#host.model();
if (prewalkWouldBeNoop(currentModel, this.#host.configuredThinkingLevel(), target, prewalk.thinkingLevel)) {
this.#prewalk = undefined;
this.#host.emitNotice(
"info",
`Prewalk: target ${target.provider}/${target.id} already matches the active model and thinking level; nothing to switch.`,
"prewalk",
);
if (this.#isNoop(prewalk)) {
this.#disarmNoop(prewalk);
return;
}
await this.#host.setModelTemporary(target, prewalk.thinkingLevel, { ephemeral: true });
this.#prewalk = undefined;
this.#clearPrewalkState();
this.#host.emitNotice(
"info",
`Prewalk: switched to ${target.provider}/${target.id} after first ${action.toolName} call.`,
@@ -175,18 +204,24 @@ export class PrewalkCoordinator {
}
/** Arms a prewalk immediately for an explicit slash-command request. */
arm(target: Model, thinkingLevel?: ConfiguredThinkingLevel): void {
arm(target: Model, thinkingLevel?: ConfiguredThinkingLevel): boolean {
if (this.#prewalk) {
this.#host.emitNotice(
"info",
`Prewalk: already armed for ${this.#prewalk.target.provider}/${this.#prewalk.target.id}, waiting for the first edit/write.`,
"prewalk",
);
return;
return true;
}
this.#prewalk = { target, thinkingLevel };
const candidate = { target, thinkingLevel };
if (this.#isNoop(candidate)) {
this.#disarmNoop(candidate);
return false;
}
this.#prewalk = candidate;
this.#planInjected = true;
this.#continuePending = true;
this.#todoSeen = false;
this.#host.agent.steer({
role: "custom",
customType: PREWALK_PLAN_MESSAGE_TYPE,
@@ -200,6 +235,7 @@ export class PrewalkCoordinator {
`Prewalk: armed for ${target.provider}/${target.id} — will switch at the first edit/write once the todo list exists.`,
"prewalk",
);
return true;
}
/** Lazily enables plan-yolo's plan phase before the first prompt is built. */
@@ -220,8 +256,7 @@ export class PrewalkCoordinator {
#scrubPlanNudge(liveMessages: AgentMessage[]): void {
if (!this.#planInjected) return;
const isPlanNudge = (message: AgentMessage): boolean =>
message.role === "custom" && message.customType === PREWALK_PLAN_MESSAGE_TYPE;
const isPlanNudge = isPrewalkPlanNudge;
for (let index = liveMessages.length - 1; index >= 0; index--) {
if (!isPlanNudge(liveMessages[index])) continue;
invalidateMessageCache(liveMessages[index]);
@@ -8,6 +8,7 @@ import {
INTERRUPTED_THINKING_MESSAGE_TYPE,
isCustomMessageContent,
normalizeCustomMessagePayload,
PREWALK_PLAN_MESSAGE_TYPE,
} from "./messages";
import { type CompactionEntry, EPHEMERAL_MODEL_CHANGE_ROLE, type SessionEntry } from "./session-entries";
@@ -337,6 +338,7 @@ export function buildSessionContext(
}
pushMessage(entry.message);
} else if (entry.type === "custom_message") {
if (!options?.transcript && entry.customType === PREWALK_PLAN_MESSAGE_TYPE) return;
if (!isCustomMessageContent(entry.content)) return;
const normalized = normalizeCustomMessagePayload(entry);
const attribution = entry.attribution === undefined ? undefined : normalized.attribution;
@@ -762,10 +762,12 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
if (!runtime.session.modelRegistry.hasConfiguredAuth(resolved.model)) {
return usage(`No API key for ${resolved.model.provider}/${resolved.model.id}`, runtime);
}
runtime.session.armPrewalk(resolved.model, resolved.thinkingLevel);
await runtime.output(
`Prewalk on: switching to ${resolved.model.provider}/${resolved.model.id} at the next edit/write (todo-gated).`,
);
const armed = runtime.session.armPrewalk(resolved.model, resolved.thinkingLevel);
if (armed) {
await runtime.output(
`Prewalk on: switching to ${resolved.model.provider}/${resolved.model.id} at the next edit/write (todo-gated).`,
);
}
return commandConsumed();
},
},
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
import * as path from "node:path";
import { type } from "@oh-my-pi/omptype";
import { Agent, type AgentTool, ThinkingLevel } from "@oh-my-pi/pi-agent-core";
@@ -7,10 +7,13 @@ import { createMockModel, type MockResponse } from "@oh-my-pi/pi-ai/providers/mo
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types";
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
import { convertToLlm } from "@oh-my-pi/pi-coding-agent/session/messages";
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
import { executeBuiltinSlashCommand } from "@oh-my-pi/pi-coding-agent/slash-commands/builtin-registry";
import type { TuiSlashCommandRuntime } from "@oh-my-pi/pi-coding-agent/slash-commands/types";
import { AUTO_THINKING } from "@oh-my-pi/pi-coding-agent/thinking";
import { TempDir } from "@oh-my-pi/pi-utils";
@@ -644,6 +647,14 @@ describe("AgentSession prewalk", () => {
const primary = modelOrThrow("claude-sonnet-4-5");
const target = modelOrThrow("claude-sonnet-4-6");
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
const sessionManager = SessionManager.inMemory();
sessionManager.appendCustomMessageEntry(
"prewalk-plan",
"legacy plan nudge written by an older OMP version",
false,
undefined,
"agent",
);
// No `prewalk` in the session config — this simulates a session that
// was NOT started with --prewalk, forced on via the slash command.
@@ -666,15 +677,15 @@ describe("AgentSession prewalk", () => {
});
session = new AgentSession({
agent,
sessionManager: SessionManager.inMemory(),
sessionManager,
settings: Settings.isolated({ "compaction.enabled": false }),
modelRegistry,
toolRegistry: new Map([[writeTool.name, writeTool as AgentTool]]),
});
// Arming twice back-to-back must stay a single, idempotent arm.
session.armPrewalk(target);
session.armPrewalk(target);
expect(session.armPrewalk(target)).toBe(true);
expect(session.armPrewalk(target)).toBe(true);
await session.prompt("do the task");
@@ -682,6 +693,164 @@ describe("AgentSession prewalk", () => {
// immediately — no second primary-model turn needed.
expect(requested).toEqual([`${primary.provider}/${primary.id}`, `${target.provider}/${target.id}`]);
expect(session.model?.id).toBe(target.id);
expect(
sessionManager
.buildSessionContext()
.messages.some(message => message.role === "custom" && message.customType === "prewalk-plan"),
).toBe(false);
// The seeded legacy entry must remain the only transcript copy; persisting
// the current arm's transient nudge would make this count two.
expect(
sessionManager
.buildSessionContext({ transcript: true })
.messages.filter(message => message.role === "custom" && message.customType === "prewalk-plan"),
).toHaveLength(1);
});
it("armPrewalk rejects a same-model same-effort no-op before injecting the plan nudge", async () => {
const model = modelOrThrow("claude-sonnet-4-5");
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
const planMarker = "complete plan in your NEXT reply";
const mock = createMockModel({ responses: [{ content: ["status only"] }] });
const calls: Array<{ hasNudge: boolean }> = [];
const agent = new Agent({
getApiKey: () => "test-key",
initialState: {
model,
systemPrompt: ["Test"],
tools: [],
messages: [],
thinkingLevel: Effort.Medium,
},
convertToLlm,
streamFn: (streamModel, context, options) => {
calls.push({ hasNudge: contextMessagesHaveMarker(context.messages, planMarker) });
return mock.stream(streamModel, context, options);
},
});
session = new AgentSession({
agent,
sessionManager: SessionManager.inMemory(),
settings: Settings.isolated({ "compaction.enabled": false }),
modelRegistry,
toolRegistry,
thinkingLevel: Effort.Medium,
});
const notices: string[] = [];
session.subscribe(event => {
if (event.type === "notice" && event.source === "prewalk") notices.push(event.message);
});
expect(session.armPrewalk(model, Effort.Medium)).toBe(false);
await session.prompt("report current status");
expect(calls).toEqual([{ hasNudge: false }]);
expect(notices.some(message => message.includes("nothing to switch"))).toBe(true);
});
it("/prewalk reports success only when the requested arm remains active", async () => {
const primary = modelOrThrow("claude-sonnet-4-5");
const target = modelOrThrow("claude-sonnet-4-6");
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
const settings = Settings.isolated({ "compaction.enabled": false });
const sessionManager = SessionManager.inMemory();
const agent = new Agent({
getApiKey: () => "test-key",
initialState: {
model: primary,
systemPrompt: ["Test"],
tools: [],
messages: [],
thinkingLevel: Effort.Medium,
},
convertToLlm,
});
session = new AgentSession({
agent,
sessionManager,
settings,
modelRegistry,
toolRegistry,
thinkingLevel: Effort.Medium,
});
const showStatus = vi.fn();
const ctx = {
session,
sessionManager,
settings,
collabGuest: false,
showStatus,
editor: { setText: vi.fn() },
refreshSlashCommandState: vi.fn(),
} as unknown as InteractiveModeContext;
const runtime = { ctx } satisfies TuiSlashCommandRuntime;
settings.setModelRole("smol", `${primary.provider}/${primary.id}:medium`);
expect(await executeBuiltinSlashCommand("/prewalk", runtime)).toBe(true);
expect(showStatus).not.toHaveBeenCalled();
settings.setModelRole("smol", `${target.provider}/${target.id}:medium`);
expect(await executeBuiltinSlashCommand("/prewalk", runtime)).toBe(true);
expect(showStatus).toHaveBeenCalledTimes(1);
expect(showStatus).toHaveBeenCalledWith(
`Prewalk on: switching to ${target.provider}/${target.id} at the next edit/write (todo-gated).`,
);
});
it("requires a fresh todo before a later explicit prewalk can hand off", async () => {
const primary = modelOrThrow("claude-sonnet-4-5");
const target = modelOrThrow("claude-sonnet-4-6");
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
const mock = createMockModel({
responses: [
toolCall("first-todo", "todo"),
toolCall("first-write", "write"),
{ content: ["first done"] },
toolCall("second-write-before-todo", "write"),
toolCall("second-todo", "todo"),
toolCall("second-write-after-todo", "write"),
{ content: ["second done"] },
],
});
const requested: string[] = [];
const agent = new Agent({
getApiKey: () => "test-key",
initialState: {
model: primary,
systemPrompt: ["Test"],
tools: [todoTool as AgentTool, writeTool as AgentTool],
messages: [],
thinkingLevel: Effort.Medium,
},
convertToLlm,
streamFn: (model, context, options) => {
requested.push(`${model.provider}/${model.id}`);
return mock.stream(model, context, options);
},
});
session = new AgentSession({
agent,
sessionManager: SessionManager.inMemory(),
settings: Settings.isolated({ "compaction.enabled": false }),
modelRegistry,
toolRegistry,
prewalk: { target },
});
await session.prompt("first task");
const firstRunCallCount = requested.length;
expect(session.model?.id).toBe(target.id);
session.armPrewalk(primary);
await session.prompt("second task");
expect(requested.slice(firstRunCallCount)).toEqual([
`${target.provider}/${target.id}`,
`${target.provider}/${target.id}`,
`${target.provider}/${target.id}`,
`${primary.provider}/${primary.id}`,
]);
expect(session.model?.id).toBe(primary.id);
});
it("effort-only prewalk on the same model downgrades the thinking level instead of silently skipping", async () => {
@@ -744,12 +913,13 @@ describe("AgentSession prewalk", () => {
// the post-switch checklist.
const model = modelOrThrow("claude-sonnet-4-5");
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
const planMarker = "complete plan in your NEXT reply";
const checklistMarker = "grep for every other call site";
const mock = createMockModel({
responses: [toolCall("t1", "record"), toolCall("t2", "write"), { content: ["done"] }],
});
const calls: Array<{ hasChecklist: boolean }> = [];
const calls: Array<{ hasNudge: boolean; hasChecklist: boolean }> = [];
const agent = new Agent({
getApiKey: () => "test-key",
initialState: {
@@ -761,7 +931,10 @@ describe("AgentSession prewalk", () => {
},
convertToLlm,
streamFn: (streamModel, context, options) => {
calls.push({ hasChecklist: contextMessagesHaveMarker(context.messages, checklistMarker) });
calls.push({
hasNudge: contextMessagesHaveMarker(context.messages, planMarker),
hasChecklist: contextMessagesHaveMarker(context.messages, checklistMarker),
});
return mock.stream(streamModel, context, options);
},
});
@@ -786,6 +959,8 @@ describe("AgentSession prewalk", () => {
// The no-op is announced, not silent.
expect(notices.some(message => message.includes("nothing to switch"))).toBe(true);
// The checklist steer only fires on a real switch.
// A genuine no-op must be rejected before the disruptive plan nudge is injected.
expect(calls.every(call => !call.hasNudge)).toBe(true);
expect(calls.every(call => !call.hasChecklist)).toBe(true);
});
@@ -68,4 +68,39 @@ describe("prewalk startup degradation", () => {
expect(options.prewalk?.target.provider).toBe(model.provider);
expect(options.prewalk?.target.id).toBe(model.id);
});
test("does not implicitly re-arm configured prewalk while restoring a session", async () => {
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
if (!model) throw new Error("expected claude-sonnet-4-5 to be bundled");
const settings = Settings.isolated();
settings.set("prewalk.enabled", true);
settings.setModelRole("smol", `${model.provider}/${model.id}`);
const { authStorage, modelRegistry } = await newRegistry("restoring");
authStorage.setRuntimeApiKey(model.provider, "test-key");
for (const args of [parseArgs(["--continue"]), parseArgs(["--resume=session.jsonl"])]) {
const options = await buildSessionOptions(args, [], SessionManager.inMemory(), modelRegistry, settings);
expect(options.prewalk).toBeUndefined();
}
});
test("honors an explicit prewalk flag while restoring a session", async () => {
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
if (!model) throw new Error("expected claude-sonnet-4-5 to be bundled");
const settings = Settings.isolated();
settings.setModelRole("smol", `${model.provider}/${model.id}`);
const { authStorage, modelRegistry } = await newRegistry("explicit-restore");
authStorage.setRuntimeApiKey(model.provider, "test-key");
const options = await buildSessionOptions(
parseArgs(["--continue", "--prewalk"]),
[],
SessionManager.inMemory(),
modelRegistry,
settings,
);
expect(options.prewalk?.target.provider).toBe(model.provider);
expect(options.prewalk?.target.id).toBe(model.id);
});
});