fix(goal): persist iteration count across resume so MAX_GOAL_ITERATIONS bounds the whole session

[qqqys]

What this PR does

Persists the /goal iteration count across session resume so the MAX_GOAL_ITERATIONS safety cap bounds the goal's whole session lifetime instead of being re-granted on every resume.

A /goal loop is driven by a session-scoped Stop hook: each not-met judge increments iterations and blocks stopping, and the loop aborts once iterations reaches MAX_GOAL_ITERATIONS. On resume, restoreGoalFromHistory re-arms an unfinished goal through registerGoalHook, which always primed the store with iterations: 0, and findGoalToRestore only returned the condition — so the running count was dropped and the cap reset to a full fresh budget after each /resume, /branch, or resumed-session startup. An unreachable goal could therefore auto-loop another full budget every resume, making the runaway-loop guard effectively unbounded.

The running count is already persisted (continuations write a checking goal_status item carrying iterations), so the change just reads it back:

• findGoalToRestore now returns { condition, iterations } from the latest non-terminal goal_status item (set items predate any iteration, so they restore at 0).
• registerGoalHook takes an optional initialIterations (default 0, clamped at 0) and primes the store with it.
• restoreGoalFromHistory threads the restored count through.

No persistence-format change. Resume re-arm stays passive — continuation timing is unchanged; only the starting iteration count is corrected.

Why it's needed

The MAX_GOAL_ITERATIONS cap exists to stop a goal that can never be satisfied from looping forever. Because the counter reset on resume, a long-running or unsatisfiable goal that spans a resume could keep consuming a full iteration budget indefinitely — the opposite of the cap's intent. Closes #4999.

Reviewer Test Plan

How to verify

1. Set a goal that can't be satisfied this session, e.g. /goal the file /tmp/never-exists.txt contains the word DONE (never create it).
2. Let it iterate a few times (watch the count on the goal status chip).
3. Exit and qwen --resume (or /resume) the same session.
4. Before this PR: the restored goal starts again from iteration 1 with a full fresh budget. After this PR: it continues from the persisted count, and a goal at/near the cap aborts on the next not-met verdict.

Automated coverage:

• cd packages/core && npx vitest run src/goals/goalHook.test.ts src/goals/goalLoop.integration.test.ts
• cd packages/cli && npx vitest run src/ui/utils/restoreGoal.test.ts src/ui/commands/goalCommand.test.ts

New tests: iteration count carried from a checking item; restored count landing in the active goal; negative initialIterations clamped to 0; and a resumed near-cap goal hitting MAX_GOAL_ITERATIONS on the next not-met verdict (proving the cap is a true cross-resume bound).

Evidence (Before & After)

Non-UI logic change. Behavior is covered by the unit tests above; no TUI surface changes.

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	⚠️
🐧 Linux	⚠️

[qwen-code-ci-bot]

⚠️ Downgraded from Approve to Comment: CI still running.

No high-confidence issues found in the diff. One suggestion from the review that touches an unchanged file relied upon by this PR:

[Suggestion] checking items persist the stop-hook-loop counter, not the goal's cumulative counter

packages/cli/src/ui/hooks/useGeminiStream.ts:1373 writes iterations: value.iterationCount (per-recursion counter from StopHookLoop) instead of activeGoal.iterations (cumulative store counter). These are identical within a single top-level interaction but diverge if the user interrupts and sends a new message while the goal remains active — value.iterationCount resets to 1 while activeGoal.iterations keeps accumulating. On resume, the restored count would be lower than the true cumulative total, partially re-granting the MAX_GOAL_ITERATIONS budget.

Suggested fix: change line 1373 from iterations: value.iterationCount to iterations: activeGoal.iterations (the variable is already in scope two lines above).

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review

[wenshao]

Verification report — real-build testing at head 43d39ed

TL;DR: The restore plumbing in this PR is correct and all new unit tests pass, but the headline fix does not work end-to-end on a real build. The premise it rests on — "The running count is already persisted (continuations write a checking goal_status item carrying iterations)" — does not hold for the recorded transcript: checking items exist only in the live UI's in-memory history array and are never written to the session file. On a real resume, the latest non-terminal goal_status item is always the set item (which carries no iterations), so the restored count is always 0 and MAX_GOAL_ITERATIONS still resets on every resume. The headline scenario of #4999 still reproduces verbatim on this branch.

What I ran

Linux, Node v22, built from this PR's head, interactive TUI driven in tmux against a real LLM backend (glm-4.7 as both main model and goal judge via a workspace fastModel override, so judge verdicts were real not-met decisions).

Build + automated tests — all pass

• npm run build ✅
• packages/core: goalHook.test.ts + goalLoop.integration.test.ts — 24 passed ✅
• packages/cli: restoreGoal.test.ts + goalCommand.test.ts — 38 passed ✅; useGeminiStream.test.tsx goal tests — 3 passed ✅

Real session, following the PR's own reviewer test plan

1. Set an unsatisfiable-but-not-impossible goal: /goal The file /tmp/goal-pr5000/approved.flag exists. The user will create it later themselves. You must NOT create, write, or modify that file or any file. Only check whether it exists and wait. The model checks with read-only tools each turn, the judge returns not-met, and the loop iterates hands-free.
2. Let it run to Goal check · turn 5, then exited.
3. Transcript inspection (~/.qwen/projects/<project>/chats/<session>.jsonl): after 5 live iterations, the file contains exactly one goal_status record — the set item from the /goal command, with keys {type, kind, condition} and no iterations field. Zero checking records.
4. --resume <session-id>: the goal is re-armed (condition restore is pre-existing behavior and works), but /goal reports (not yet evaluated) — iterations restored as 0. The loop then ran turn 1 … turn 14, consuming a fresh budget. This is exactly the bug /goal iteration counter resets on session resume, defeating MAX_GOAL_ITERATIONS cap #4999 describes, unchanged.
5. Re-checked the transcript after 19 total live iterations across both sessions: still only the single set record.

Why the persistence is missing

handleStopHookLoopEvent adds the checking item with the raw historyManager.addItem (React state only). The only paths that record goal_status items into the transcript are slash-command outputs (set, cleared) and the terminal observer (achieved/failed/aborted via recordGoalStatusItem). All three restore paths (startup --resume/--continue, /resume, /branch) rebuild history from the recorded transcript via buildResumedHistoryItems, so findGoalToRestore can only ever see set (no count) or a terminal kind (returns null) — never a checking item. The new unit tests pass because they hand-construct history arrays containing checking items that the product never writes to disk. (#4999 makes the same conflation between the live UI history and the recorded transcript.)

What does work

• Commit 43d39ed (iterations: activeGoal.iterations instead of value.iterationCount) is a real, observable in-session improvement: after interrupting the loop at turn 7 and sending a new user message (which starts a fresh stop-hook chain), the next chip correctly showed Goal check · turn 8 — cumulative — where the per-chain counter would have restarted at turn 1. ✅
• registerGoalHook's initialIterations plumbing is correct (default 0, negatives clamped), per the unit tests. ✅
• No regressions observed in the goal flow: set, loop, judge, interrupt, condition restore on resume all behave as before. ✅

To make the fix land

The missing piece is small — persist the checking item on the continuation path, e.g. in the activeGoal branch of handleStopHookLoopEvent after addItem(...):

recordGoalStatusItem(config, item); // already exported from ui/utils/restoreGoal.ts

system/slash_command records are skipped when rebuilding model API history (buildApiHistoryFromConversation ignores type === 'system'), so this adds no model-context pollution, and growth is bounded by the 50-iteration cap. One visible side effect: resumed sessions would also render the per-iteration checking trail, consistent with the live UI. With persistence in place, commit 43d39ed is what makes the recorded count cumulative across interrupts and multiple resumes — good that it's already here.

I'd also suggest one integration-style test that goes through the real pipeline (ChatRecordingService → session file → buildResumedHistoryItems → restoreGoalFromHistory) instead of hand-built history arrays — it would have caught this gap.

Recommendation

Hold off merging as-is: the PR would close #4999 while the issue's headline scenario still reproduces on this branch. With the one-line persistence addition (plus a pipeline-level test), this becomes a complete fix and I'm happy to re-verify.

[qwen-code-ci-bot]

No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review

[wenshao]

Local runtime verification (maintainer, macOS)

Verdict: PASS — built both this PR (6108c54) and baseline main (53349a0), drove the real TUI in tmux (qwen3.7-max via an OpenAI-compatible endpoint, default approval mode), and observed the iteration count surviving resume exactly as claimed. Not a CI rerun — no unit tests were run for this report; everything below is captured from the running app.

Before (baseline main @ 53349a0)

1. ✅ /goal flag.txt …contains the word DONE (kept unsatisfiable: write attempts rejected via approval dialog), loop ran → live /goal showed:

   ● Goal active: flag.txt … contains the word DONE (6 turns)
   

2. ✅ Transcript (~/.qwen/projects/...-before/chats/*.jsonl) contained exactly one goal_status record — kind:"set", no iterations. No checking records at all.
3. ❌ (expected) --continue resume → count gone, full cap budget re-granted:

   ● Goal active: flag.txt … contains the word DONE (not yet evaluated)
   

   Bug of /goal iteration counter resets on session resume, defeating MAX_GOAL_ITERATIONS cap #4999 reproduced.

After (this PR @ 6108c54)

1. ✅ Same scenario → chips rendered strictly sequentially Goal check · turn 1 … turn 8; live /goal → (8 turns).
2. ✅ Transcript now persists every iteration: set + checking iterations=1..8.
3. ✅ Quit + --continue → count survives:

   ● Goal active: flag.txt … contains the word DONE. … (8 turns)
   

4. ✅ Next turn after resume → judge verdicts continued cumulatively: Goal check · turn 9, then 10…14 (not back to 1). Transcript appended checking 9..14 in the resumed process.
5. 🔍 Cap probe — patched the last checking record in the transcript to iterations: 50 (= MAX_GOAL_ITERATIONS) as a fixture, resumed: /goal → (50 turns), and the first not-met verdict aborted the goal instead of granting a fresh budget:

   ! Goal aborted · 50 turns · 1m 26s
    ⎿ Stop says:
       Goal max iterations reached; cleared. Re-set with /goal <condition> if you still need it.
   

   Transcript chain ends … → checking 50 → aborted 50.
6. 🔍 Post-abort resume — resumed once more: /goal shows the terminal summary (● Goal aborted · 50 turns), footer has no ◎ /goal active chip, no zombie re-arm. Terminal state round-trips correctly.

Observations (non-blocking)

• The switch from value.iterationCount to activeGoal.iterations for the checking chip is the right call beyond persistence: iterationCount is a per-process loop counter, so post-resume chips could never have been cumulative with the old code. Observed chips and /goal now always agree (1..8, then 9..14 across the resume).
• In my environment every judge call fell back to Goal judge unavailable; continue working… (the configured endpoint doesn't serve the fast model). The count/persist/restore paths are identical regardless of where the not-met verdict comes from, and the cap-abort went through the real iterations >= MAX_GOAL_ITERATIONS branch — but be aware this run exercised fallback verdicts, not real judge judgments.
• Adjacent, pre-existing (not this PR): on resume, a WriteFile that was rejected live is replayed with a ✓ marker in history (file verifiably never existed). Also, a resumed goal loop repeating the identical read-only tool call tripped the backend's anti-repetition guard once (400 InternalError.Algo.InvalidParameter: Repetitive tool calls detected) — the turn errored, goal state unaffected.
• Restore-from-set (0 turns) wasn't driven live (covered by the new unit tests); tested on macOS only, matching the author's test matrix.

Build recipe used: npm install && npm run bundle in a worktree of the PR head, then node dist/cli.js / node dist/cli.js --continue in a scratch dir.