fix(coding-agent/goals): skipped goal accounting operations when accounting state was absent
- Added an internal accounting-state guard and used it to skip goal usage flushing when accounting was inactive. - Updated goal abort handling to return early unless accounting or pause logic was required, then paused only a cloned active goal state before committing. - Aligned related tests/types by tightening OpenAI helper typing and using Tool typings for the goal tool registry.
This commit is contained in:
@@ -26,7 +26,10 @@ function createCodexToken(accountId: string): string {
|
||||
* is exercised by its own targeted tests; these history-replay tests assert raw
|
||||
* payload shape and should stay independent of it.
|
||||
*/
|
||||
function getOpenAIReasoningModel<Provider extends string>(provider: Provider, id: string): Model<"openai-responses"> {
|
||||
function getOpenAIReasoningModel(
|
||||
provider: Parameters<typeof getBundledModel>[0],
|
||||
id: string,
|
||||
): Model<"openai-responses"> {
|
||||
const base = getBundledModel(provider, id) as Model<"openai-responses">;
|
||||
return { ...base, name: "Reasoning Mini" };
|
||||
}
|
||||
|
||||
@@ -162,6 +162,11 @@ export class GoalRuntime {
|
||||
return this.#host.now?.() ?? Date.now();
|
||||
}
|
||||
|
||||
#hasAccountingState(): boolean {
|
||||
const state = this.#host.getState();
|
||||
return Boolean(state?.enabled && isAccountingStatus(state.goal));
|
||||
}
|
||||
|
||||
async #withAccounting<T>(fn: () => Promise<T> | T): Promise<T> {
|
||||
const previous = this.#accountingTail;
|
||||
const { promise, resolve } = Promise.withResolvers<void>();
|
||||
@@ -225,31 +230,44 @@ export class GoalRuntime {
|
||||
|
||||
async onToolCompleted(toolName: string): Promise<void> {
|
||||
if (toolName === "goal") return;
|
||||
if (!this.#hasAccountingState()) return;
|
||||
await this.flushUsage("allowed");
|
||||
}
|
||||
|
||||
async onGoalToolCompleted(): Promise<void> {
|
||||
if (!this.#hasAccountingState()) return;
|
||||
await this.flushUsage("suppressed");
|
||||
}
|
||||
|
||||
async onAgentEnd(options?: { turnCompleted?: boolean; currentUsage?: GoalTokenUsage }): Promise<void> {
|
||||
if (!this.#hasAccountingState()) {
|
||||
this.#turnSnapshot = undefined;
|
||||
return;
|
||||
}
|
||||
await this.flushUsage("suppressed", options?.currentUsage);
|
||||
this.#turnSnapshot = undefined;
|
||||
}
|
||||
|
||||
async onTaskAborted(options?: { reason?: "interrupted" | "internal" }): Promise<void> {
|
||||
const state = this.#host.getState();
|
||||
const needsAccounting = state?.enabled && isAccountingStatus(state.goal);
|
||||
const needsPause = options?.reason === "interrupted" && state?.enabled && state.goal.status === "active";
|
||||
if (!needsAccounting && !needsPause) {
|
||||
this.#turnSnapshot = undefined;
|
||||
return;
|
||||
}
|
||||
await this.#withAccounting(async () => {
|
||||
await this.#flushUsageLocked("suppressed");
|
||||
this.#turnSnapshot = undefined;
|
||||
if (options?.reason !== "interrupted") return;
|
||||
const state = this.#getStateClone();
|
||||
if (!state?.enabled || state.goal.status !== "active") return;
|
||||
state.enabled = false;
|
||||
state.goal.status = "paused";
|
||||
state.goal.updatedAt = this.#now();
|
||||
const cloned = this.#getStateClone();
|
||||
if (!cloned?.enabled || cloned.goal.status !== "active") return;
|
||||
cloned.enabled = false;
|
||||
cloned.goal.status = "paused";
|
||||
cloned.goal.updatedAt = this.#now();
|
||||
this.#clearActiveAccounting();
|
||||
this.#budgetReportedFor = undefined;
|
||||
await this.#commitState(state, { persist: "goal_paused" });
|
||||
await this.#commitState(cloned, { persist: "goal_paused" });
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { Agent, type AgentTool } from "@oh-my-pi/pi-agent-core";
|
||||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { GoalTool } from "@oh-my-pi/pi-coding-agent/goals/tools/goal-tool";
|
||||
@@ -9,7 +9,7 @@ import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
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 { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
|
||||
import { createTools, type Tool, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
function createToolSession(cwd: string, settings: Settings, overrides: Partial<ToolSession> = {}): ToolSession {
|
||||
@@ -51,7 +51,7 @@ async function createGoalHarness(): Promise<GoalHarness> {
|
||||
});
|
||||
const bootstrapToolSession = createToolSession(tempDir.path(), settings);
|
||||
const initialTools = await createTools(bootstrapToolSession, ["read"]);
|
||||
const toolRegistry = new Map<string, AgentTool>(initialTools.map(tool => [tool.name, tool] as const));
|
||||
const toolRegistry = new Map<string, Tool>(initialTools.map(tool => [tool.name, tool] as const));
|
||||
|
||||
const session = new AgentSession({
|
||||
agent: new Agent({
|
||||
@@ -73,7 +73,7 @@ async function createGoalHarness(): Promise<GoalHarness> {
|
||||
getGoalModeState: () => session.getGoalModeState(),
|
||||
getGoalRuntime: () => session.goalRuntime,
|
||||
});
|
||||
toolRegistry.set("goal", new GoalTool(toolSession));
|
||||
toolRegistry.set("goal", new GoalTool(toolSession) as unknown as Tool);
|
||||
|
||||
return {
|
||||
tempDir,
|
||||
|
||||
Reference in New Issue
Block a user