Skip to content

fix(core): make goal evaluation lifecycle-safe #6681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 7 commits into from
Jul 11, 2026
+497 −179
Merged

fix(core): make goal evaluation lifecycle-safe #6681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
89af831
fix(core): make goal evaluations lifecycle-safe
qqqys Jul 10, 2026
70f92f8
fix(core): reject invalid goal judge schema
qqqys Jul 10, 2026
e16b9f7
fix(core): preserve goal judge result compatibility
qqqys Jul 10, 2026
72604c7
test(core): cover goal iteration boundary
qqqys Jul 11, 2026
e81d7de
Merge branch 'main' into fix/goal-loop-correctness
wenshao Jul 11, 2026
6129633
fix(core): cap background goal deferrals
qqqys Jul 11, 2026
af93b40
fix(core): address goal evaluation review blockers
qqqys Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/core/src/goals/activeGoalStore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,13 @@ describe('activeGoalStore', () => {
).toBe(false);
expect(activeGoalEquals(makeGoal(), undefined)).toBe(false);
});

it('ignores deferred evaluation bookkeeping when comparing snapshots', () => {
expect(
activeGoalEquals(
makeGoal({ deferredEvaluations: 1 }),
makeGoal({ deferredEvaluations: 2 }),
),
).toBe(true);
});
});
22 changes: 22 additions & 0 deletions packages/core/src/goals/activeGoalStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
export interface ActiveGoal {
condition: string;
iterations: number;
deferredEvaluations?: number;

@qwen-code-ci-bot qwen-code-ci-bot Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] stableActiveGoalKey (line ~33) uses Object.keys(goal) to serialize all defined fields for equality comparison. This new deferredEvaluations field changes on every recordGoalDeferral call (up to 50 times per cycle), making activeGoalEquals return false each time. Downstream consumers — client.ts:maybeEmitActiveGoalChange (called from 5 sites in the turn loop) and useGeminiStream.ts — then emit spurious ActiveGoal stream events and trigger redundant React re-renders. The UI never displays deferredEvaluations, so every event is wasted work.

Fix: exclude deferredEvaluations from stableActiveGoalKey (it is internal bookkeeping, not user-visible state), or switch to an explicit allowlist of fields:

function stableActiveGoalKey(goal: ActiveGoal): string {
  return JSON.stringify({
    condition: goal.condition,
    iterations: goal.iterations,
    setAt: goal.setAt,
    tokensAtStart: goal.tokensAtStart,
    lastReason: goal.lastReason,
    hookId: goal.hookId,
  });
}

— qwen3.7-max via Qwen Code /review

setAt: number;
tokensAtStart: number;
lastReason?: string;
Expand All @@ -32,6 +33,7 @@ export function activeGoalEquals(
function stableActiveGoalKey(goal: ActiveGoal): string {
const comparable: Record<string, unknown> = {};
for (const key of Object.keys(goal).sort() as Array<keyof ActiveGoal>) {
if (key === 'deferredEvaluations') continue;
const value = goal[key];
if (value !== undefined) {
comparable[key] = value;
Expand Down Expand Up @@ -63,12 +65,32 @@ export function recordGoalIteration(
const updated: ActiveGoal = {
...current,
iterations: current.iterations + 1,
deferredEvaluations: 0,

@qwen-code-ci-bot qwen-code-ci-bot Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] recordGoalIteration is the only code that resets deferredEvaluations to 0, but the error handler in goalHook.ts:199 returns early (before calling recordGoalIteration) when verdict.kind === 'error'. After the deferral cap is reached and the judge errors (timeout, parse failure, empty response), deferredEvaluations stays at 50 permanently — the deferral check (current.deferredEvaluations ?? 0) < MAX_GOAL_ITERATIONS is always false, and the judge is force-invoked every subsequent turn regardless of background work status.

The deferral mechanism becomes a one-shot: once it fires and the judge errors, it never re-engages. Fix: reset deferredEvaluations in the error handler path, either by calling a dedicated reset function or by moving the reset out of recordGoalIteration.

— qwen3.7-max via Qwen Code /review

lastReason,
};
store.set(sessionId, updated);
return updated;
}

export function recordGoalDeferral(sessionId: string): ActiveGoal | undefined {
const current = store.get(sessionId);
if (!current) return undefined;
const updated: ActiveGoal = {
...current,
deferredEvaluations: (current.deferredEvaluations ?? 0) + 1,
};
store.set(sessionId, updated);
return updated;
}

export function resetGoalDeferrals(sessionId: string): ActiveGoal | undefined {
const current = store.get(sessionId);
if (!current || current.deferredEvaluations === 0) return current;
const updated: ActiveGoal = { ...current, deferredEvaluations: 0 };
store.set(sessionId, updated);
return updated;
}

/**
* Test-only escape hatch — production code must scope by sessionId.
*/
Expand Down
Loading
Loading