From 2474e1a6d8f1146ae790b3f63b0e141b93fa0a3c Mon Sep 17 00:00:00 2001 From: usr_bin_roygbiv Date: Sun, 14 Jun 2026 16:11:30 -0500 Subject: [PATCH] fix(goals): avoid deactivating goal mode on wall-clock-only updates --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/src/goals/runtime.ts | 5 ++++- .../test/goals/goal-runtime.test.ts | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d39ef17b6..fca8e1ce3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ - Added `ConfigFile.tryLoadAsync()`, `ConfigFile.loadAsync()`, `ConfigFile.loadOrDefaultAsync()`, `ConfigFile.getMtimeMsAsync()`, and `ConfigFile.warmup(file)` so the rest of the codebase can migrate config reads off the sync path. ### Fixed +- Fixed a bug where goal mode was incorrectly deactivated/set to 'none' on every wall-clock-only update (when tokenDelta <= 0) during tool execution flushes, preventing OMP from writing a mode change to 'none' in the session history database while keeping in-memory/UI state fresh. - Fixed `Test & smoke (TS)` CI timeouts caused by parallel test files racing on the process-global Settings singleton. `CustomEditor` now accepts a `magicKeywordsEnabledOverride` injection point so the shimmer-gate test can assert behaviour without calling `resetSettingsForTest()` / `Settings.init()`; the "streaming tool call preview height" describe drops its gratuitous Settings reset+init. Production wiring is unchanged ([#2582](https://github.com/can1357/oh-my-pi/issues/2582)) - Fixed MCP OAuth fallback rendering to show a short terminal hyperlink and keep the raw authorization URL on one unwrapped copy line ([#2121](https://github.com/can1357/oh-my-pi/issues/2121)). diff --git a/packages/coding-agent/src/goals/runtime.ts b/packages/coding-agent/src/goals/runtime.ts index 8e7dd43ff..000ae3bd8 100644 --- a/packages/coding-agent/src/goals/runtime.ts +++ b/packages/coding-agent/src/goals/runtime.ts @@ -356,7 +356,10 @@ export class GoalRuntime { this.#wallClock.lastAccountedAt += wallSeconds * 1000; } - await this.#commitState(state, { persist: "goal" }); + // Persisting wall-clock-only accounting on every tool event bloats /goal sessions with full + // objective snapshots. Keep the in-memory/UI state fresh, but persist only token/budget changes. + const shouldPersistUsage = tokenDelta > 0 || flippedToBudgetLimited; + await this.#commitState(state, { persist: shouldPersistUsage ? "goal" : undefined }); if (state.goal.status !== "budget-limited") { this.#budgetReportedFor = undefined; diff --git a/packages/coding-agent/test/goals/goal-runtime.test.ts b/packages/coding-agent/test/goals/goal-runtime.test.ts index c4af4b8e2..73c31f541 100644 --- a/packages/coding-agent/test/goals/goal-runtime.test.ts +++ b/packages/coding-agent/test/goals/goal-runtime.test.ts @@ -118,6 +118,7 @@ describe("goal runtime", () => { harness.runtime.onTurnStart("turn-1", createUsage()); harness.advance(2_500); + harness.setUsage(createUsage({ input: 1 })); await harness.runtime.flushUsage("suppressed"); expect(harness.getState()?.goal.timeUsedSeconds).toBe(2); expect(harness.runtime.snapshot.wallClock.lastAccountedAt).toBe(2_000); @@ -130,12 +131,28 @@ describe("goal runtime", () => { expect(harness.persists).toHaveLength(1); harness.advance(700); + harness.setUsage(createUsage({ input: 2 })); await harness.runtime.flushUsage("suppressed"); expect(harness.getState()?.goal.timeUsedSeconds).toBe(3); expect(harness.runtime.snapshot.wallClock.lastAccountedAt).toBe(3_000); expect(harness.persists).toHaveLength(2); }); + it("does not persist snapshots on wall-clock-only flushes", async () => { + const harness = createHarness({ + state: { enabled: true, mode: "active", goal: createGoal() }, + }); + + harness.runtime.onTurnStart("turn-1", createUsage()); + harness.advance(2_500); + // Flush wall-clock time without any token usage changes. + await harness.runtime.flushUsage("suppressed"); + // The in-memory state should still be updated. + expect(harness.getState()?.goal.timeUsedSeconds).toBe(2); + // But it should not write/persist to the session log. + expect(harness.persists).toHaveLength(0); + }); + it("steers only once until a budget mutation resets the cycle", async () => { const harness = createHarness({ state: {