fix(core): make goal evaluation lifecycle-safe - #6681
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: This addresses real correctness issues in the goal evaluation loop — the judge infrastructure treating timeouts/failures as "not met" verdicts, premature evaluation while background work is running, off-by-one in terminal counting, and the fragile Direction: Aligned with qwen-code's goal/background-automation roadmap. Separating evaluator failures from valid verdicts is a clear reliability improvement — a flaky judge should never burn tokens on pointless continuations. Size: 8 files changed. Core paths (
Below the 500-line threshold for concern. Documentation files add useful design rationale but could be considered optional for a fix PR — flagging for discussion, not blocking. Approach: The discriminated-union outcome type ( Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题： 这个 PR 解决的是目标判定循环中的真实正确性 bug——judge 超时/故障被当作普通的"未完成"结论、后台任务仍在运行时过早判定、终局计数偏差，以及脆弱的 方向： 与 qwen-code 的目标/后台自动化路线图对齐。将判定器故障与有效结论分离是明确的可靠性改进——不稳定的 judge 不应该浪费 token 做无意义的续跑。 规模： 8 个文件变更。核心路径（
低于 500 行的关注阈值。文档文件提供了有用的设计依据，但对于 fix PR 可能被视为可选——提出讨论，不阻断。 方案： 判别联合结果类型（ 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading diff): I would have introduced a discriminated outcome type to separate valid verdicts from infrastructure failures, added a background-work guard using the existing registries, moved Comparison with the diff: The implementation matches or exceeds my proposal. The Verified:
No critical blockers found. No AGENTS.md violations. No over-abstraction or unnecessary duplication. The changes are tightly scoped to the four stated behavioral fixes. TestingThis is a non-UI change (goal-loop control flow, types, tests). Verified by running the goal test suite before and after: Before (main branch)After (this PR)8 new tests covering: background-work deferral (agent/shell/workflow, parameterized), monitor exclusion, error-path pause, timeout pause, first-impossible termination, counting accuracy, and abort boundary. Build ( — Qwen Code · qwen3.7-max |
|
Stepping back to look at the whole picture: this is a clean, well-scoped correctness fix for the The implementation is straightforward. The discriminated outcome type is the right abstraction without being over-engineered. Every changed line serves one of the four stated fixes. The iteration cap boundary change ( The design doc and implementation plan are heavier than typical for a fix PR (334 lines of documentation vs 119 lines of production code), but they document design rationale that will help future maintainers understand the state machine. Not worth splitting out. My independent proposal before reading the diff was essentially the same approach. No simpler path was missed. Build, typecheck, and all 53 goal tests pass. Approving. ✅ 中文说明退一步看全局：这是一个干净、范围合理的 实现简洁。判别结果类型是正确的抽象，没有过度工程。每一行改动都服务于四个已声明的修复之一。迭代上限边界变更（记录前 设计文档和实施计划对于 fix PR 来说比通常更重（334 行文档 vs 119 行生产代码），但它们记录了设计依据，有助于未来维护者理解状态机。不值得拆分。 我在看 diff 之前的独立方案本质上是相同的方法。没有遗漏更简单的路径。构建、类型检查和全部 53 个目标测试通过。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| const impossible = (payload as { impossible?: unknown }).impossible === true; | ||
| const impossibleValue = (payload as { impossible?: unknown }).impossible; | ||
| if (typeof ok !== 'boolean' || typeof reason !== 'string' || !reason.trim()) { | ||
| return null; |
There was a problem hiding this comment.
[Suggestion] The tightened parseJudgeReply validation converts previously-valid responses into loop pauses. The old parser accepted {ok: true} without a reason field (using 'Goal condition reported as met.' as fallback) and silently ignored non-boolean impossible values. The new parser rejects both, producing kind: 'error' which pauses the entire goal loop.
If the fast model ever omits reason or emits "impossible": "true" (string), a previously-recoverable situation becomes a full loop pause. Consider whether a graceful degradation (e.g., fallback reason for missing reason, coercion of "true"/"false" strings to boolean) is safer than a hard reject for these specific fields, since the loop-pause path is meant for evaluator failures, not evaluator quirks.
— qwen3.7-max via Qwen Code /review
| if (verdict.kind === 'error') { | ||
| return { continue: true, systemMessage: verdict.message }; | ||
| } | ||
|
|
There was a problem hiding this comment.
[Suggestion] Error verdicts return early before recordGoalIteration, so they never increment the iterations counter. This means MAX_GOAL_ITERATIONS only counts valid verdicts, not total evaluations. If the judge endpoint is degraded (timeouts, API errors, empty responses) but the loop keeps getting continue: true, it could make an unbounded number of API calls with no ceiling.
Consider adding a separate error-count cap (e.g., max consecutive errors) to prevent unbounded token consumption during judge outages, independent from the valid-verdict iteration counter.
— qwen3.7-max via Qwen Code /review
| resolve({ | ||
| kind: 'error', | ||
| ok: false, | ||
| reason: GOAL_JUDGE_TIMEOUT_MESSAGE, |
There was a problem hiding this comment.
[Suggestion] The timeout error path sets reason: GOAL_JUDGE_TIMEOUT_MESSAGE, while judgeErrorResult() in goalJudge.ts uses reason: JUDGE_REASON_FALLBACK. No consumer currently reads reason from an error-kind outcome (only message is used for systemMessage), so the field is dead data on errors. The inconsistency means a future caller who reads reason would get different text depending on which error path fired.
Consider routing the timeout through judgeErrorResult() (accepting the generic fallback text), or dropping reason from the error variant of GoalJudgeOutcome entirely.
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| // Give the latest assistant output one final evaluation before aborting. | ||
| // The iteration cap is a safety valve for still-not-met verdicts, not a |
There was a problem hiding this comment.
[Suggestion] The off-by-one change from >= to > correctly gives the model one final evaluation at iteration 50 before aborting at 51. However, the existing MAX test starts at iterations: MAX_GOAL_ITERATIONS (50), which becomes 51 after recording and triggers abort. There is no test starting at iterations: MAX_GOAL_ITERATIONS - 1 (49) that verifies a not_met verdict records evaluation 50 and returns {decision: 'block'} (continues the loop, does NOT abort). A regression to the old >= comparison would pass all existing tests but abort one turn too early.
Consider adding a boundary test at iterations: 49.
— qwen3.7-max via Qwen Code /review
| }); | ||
|
|
||
| it('defaults to ok=false when generateContent throws', async () => { | ||
| it('returns an error when reason field is missing', async () => { |
There was a problem hiding this comment.
[Suggestion] The tightened parseJudgeReply test coverage includes 'returns an error when reason field is missing' for {ok: false} — but there is no corresponding test for {ok: true} without a reason field. This is the case more likely to occur in practice (a model confidently reporting "done" without citing evidence), and where the behavioral change is most impactful: turning a met verdict into an error that pauses the loop.
Consider adding:
it('returns an error when ok is true but reason is missing', async () => {
const client = makeMockClient({ reply: '{"ok": true}' });
const config = makeConfig({ client });
await expect(
judgeGoal(config, {
condition: 'x',
lastAssistantText: 'y',
signal: new AbortController().signal,
}),
).resolves.toMatchObject({ kind: 'error' });
});— qwen3.7-max via Qwen Code /review
| }); | ||
| finishGoal(config, sessionId, latest, { | ||
| finishGoal(config, sessionId, evaluated, { | ||
| kind: 'failed', |
There was a problem hiding this comment.
[Suggestion] Removing MIN_IMPOSSIBLE_GOAL_ITERATIONS (was 2) means the first impossible verdict immediately terminates the goal. The old floor gave the agent at least one continuation turn to produce counter-evidence. The design doc's rationale is sound (the old counter counted all negatives, not consecutive impossible verdicts), but the safety net for a flaky judge's impossible verdict is now zero turns instead of one.
The judge's system prompt provides a prompt-level defense ("independently confirm the condition is genuinely unachievable"), but prompt-level defenses are fragile against targeted adversarial input, especially on a fast model. Consider whether a floor of 1 (accepting impossible only after the first evaluation) would be a reasonable compromise.
— qwen3.7-max via Qwen Code /review
| condition: evaluated.condition, | ||
| iterations: evaluated.iterations, | ||
| durationMs: Date.now() - evaluated.setAt, | ||
| lastReason: verdict.reason, |
There was a problem hiding this comment.
[Suggestion] The judge's free-form reason text (up to 240 chars) flows directly into GoalTerminalEvent.lastReason and is displayed to users in /goal status output. This text originates from the judge model, which reads the full transcript including untrusted assistant output. If influenced by adversarial content, misleading text could reach the user's terminal.
The continuation prompt correctly avoids this (using only fixed text from the original goal condition). The 240-char cap limits exposure. Consider whether the lastReason display path also warrants a note that it is model-generated text, or whether truncation alone is sufficient for the threat model.
— qwen3.7-max via Qwen Code /review
| return { continue: true }; | ||
| } | ||
|
|
||
| if (hasGoalBlockingBackgroundWork(config)) { |
There was a problem hiding this comment.
[Critical] Background-work deferral bypasses the MAX_GOAL_ITERATIONS safety cap.
When hasGoalBlockingBackgroundWork returns true, the callback returns { continue: true } before reaching recordGoalIteration or the MAX_GOAL_ITERATIONS check. There is no deferral counter, timeout, or upper bound on consecutive deferrals. A model that spawns a new background shell/agent on every turn can defer evaluation indefinitely — the iteration counter stays at 0, and MAX_GOAL_ITERATIONS (50) is never reached. This defeats the explicit safety valve against unbounded token spend.
Consider adding a deferredEvaluations counter to ActiveGoal and forcing evaluation (or aborting) when it exceeds a threshold.
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| if (hasGoalBlockingBackgroundWork(config)) { | ||
| return { continue: true }; |
There was a problem hiding this comment.
[Suggestion] The deferral path returns { continue: true } with no debugLogger call, no systemMessage, and no update to lastReason. This is the only silent early return in the hook — every other path (error, max-iterations, goal-cleared) includes either a systemMessage or a debugLogger.debug call.
If a background task is stuck, the user sees the goal as "active" with iterations: 0 and zero diagnostic trail explaining why evaluation never runs. Adding at minimum debugLogger.debug('Goal evaluation deferred: background work in progress') would make this debuggable.
— qwen3.7-max via Qwen Code /review
|
|
||
| function hasGoalBlockingBackgroundWork(config: Config): boolean { | ||
| return ( | ||
| config.getBackgroundTaskRegistry().hasRunningTasks() || |
There was a problem hiding this comment.
[Suggestion] hasGoalBlockingBackgroundWork calls three registry methods with no try/catch. Other hook-callback code paths in this file (e.g., removeGoalFunctionHook) use defensive try/catch blocks. If any registry's internal state causes a throw, the exception propagates through the hook callback and the hook system reports an opaque "Goal evaluator failed" with no indication of which registry caused it.
Consider wrapping each registry call in try/catch, defaulting to false (proceed with evaluation) on error — a false-negative is far less harmful than a crashed hook.
— qwen3.7-max via Qwen Code /review
| 'Goal max iterations reached; cleared. Re-set with `/goal <condition>` if you still need it.'; | ||
| const GOAL_JUDGE_TIMEOUT_REASON = | ||
| 'Goal judge timed out; continue working toward the goal and run `/goal clear` to stop early.'; | ||
| const GOAL_JUDGE_TIMEOUT_MESSAGE = |
There was a problem hiding this comment.
[Suggestion] Both GOAL_JUDGE_TIMEOUT_MESSAGE and JUDGE_ERROR_MESSAGE say "the automatic /goal loop paused," but the loop is not actually paused — the hook fires again on the very next Stop event and retries evaluation. If the judge API is down, the user sees "loop paused" on every turn, which is misleading.
| const GOAL_JUDGE_TIMEOUT_MESSAGE = | |
| const GOAL_JUDGE_TIMEOUT_MESSAGE = | |
| 'Goal judge timed out; evaluation will retry on the next turn. The goal remains active.'; |
Apply the same fix to JUDGE_ERROR_MESSAGE in goalJudge.ts.
— qwen3.7-max via Qwen Code /review
| it('returns an error when generateContent throws', async () => { | ||
| const client = makeMockClient({ throws: new Error('boom') }); | ||
| const config = makeConfig({ client }); | ||
| const verdict = await judgeGoal(config, { |
There was a problem hiding this comment.
[Suggestion] The tightened parseJudgeReply validation rejects non-string reason values and whitespace-only reason strings, but no tests cover these paths:
{"ok": false, "reason": 42}—typeof reason !== 'string'rejects this, but no test exercises it.{"ok": false, "reason": " "}—!reason.trim()rejects this, but no test exercises it.
Both are behavioral changes from the old parser (which used fallback values). Consider adding test cases for these validation paths.
— qwen3.7-max via Qwen Code /review
| impossible?: boolean; | ||
| } | ||
|
|
||
| export interface JudgeResult { |
There was a problem hiding this comment.
[Suggestion] JudgeResult is exported from both goalJudge.ts and index.ts, but no production code anywhere in the repository imports it. The sole usage is a compile-time assertion in goalJudge.test.ts. Meanwhile GoalJudgeOutcome is the actual runtime return type.
The two exported types for the same concept create ambiguity. Consider removing JudgeResult from the public export or marking it as deprecated.
Similarly, every GoalJudgeOutcome variant carries ok, reason, and impossible alongside the discriminated kind field. The hook branches exclusively on verdict.kind and never reads verdict.ok or verdict.impossible. These legacy fields exist for structural compatibility with the dead JudgeResult type — a clean discriminated union with just kind + reason (+ message on error) would be easier to reason about.
— qwen3.7-max via Qwen Code /review
| resolve({ ok: false, reason: GOAL_JUDGE_TIMEOUT_REASON }); | ||
| resolve({ | ||
| kind: 'error', | ||
| ok: false, |
There was a problem hiding this comment.
[Suggestion] The timeout handler in judgeGoalWithTimeout resolves with an error result and calls debugLogger.debug, but never invokes reportGoalJudgeFailure. Every other error path in goalJudge.ts (empty response, parse failure, caught exception) calls reportGoalJudgeFailure to surface the failure to error analytics. Judge timeouts will be invisible in error reporting.
Consider calling reportGoalJudgeFailure(new Error('Judge timeout'), 'timeout') before resolving, or routing the timeout through a parameterized judgeErrorResult(message) factory.
— qwen3.7-max via Qwen Code /review
60a7fc7 to
72604c7
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| if (verdict.ok) { | ||
| finishGoal(config, sessionId, latest, { | ||
| if (verdict.kind === 'error') { | ||
| return { continue: true, systemMessage: verdict.message }; |
There was a problem hiding this comment.
[Suggestion] The error verdict path returns { continue: true, systemMessage }, which lets the turn end naturally. The old code's error-equivalent path (timeout, parse failure) fell through to the bottom of the callback and returned { decision: 'block', reason: continuationReasonForGoal(condition) }, forcing the model to keep generating. With the new semantics, a judge timeout or parse failure allows the model to stop — the goal stays registered but no further evaluations occur until the user sends a new message. For unreliable judge endpoints (frequent timeouts, API failures), this silently stalls the autonomous goal loop.
Consider returning { decision: 'block', reason: continuationReasonForGoal(condition) } on the error path (same as not_met) to force continuation through transient errors, or document that the pause-until-user-interacts behavior is the intended design for judge unavailability.
— qwen3.7-max via Qwen Code /review
| : JUDGE_REASON_FALLBACK; | ||
| const impossible = (payload as { impossible?: unknown }).impossible === true; | ||
| const impossibleValue = (payload as { impossible?: unknown }).impossible; | ||
| if (typeof ok !== 'boolean' || typeof reason !== 'string' || !reason.trim()) { |
There was a problem hiding this comment.
[Suggestion] The stricter validation rejects {ok: true} without a reason field — the old code accepted it with fallback 'Goal condition reported as met.'. A valid "met" verdict from the judge model that omits the reason field now becomes kind: 'error' and pauses the goal loop, converting a goal-achieved outcome into a loop stall.
Consider being lenient for the ok: true case:
| if (typeof ok !== 'boolean' || typeof reason !== 'string' || !reason.trim()) { | |
| if (typeof ok !== 'boolean') return null; | |
| if (typeof reason !== 'string' || !reason.trim()) { | |
| if (!ok) return null; | |
| } |
— qwen3.7-max via Qwen Code /review
| if (typeof ok !== 'boolean' || typeof reason !== 'string' || !reason.trim()) { | ||
| return null; | ||
| } | ||
| if (impossibleValue !== undefined && typeof impossibleValue !== 'boolean') { |
There was a problem hiding this comment.
[Suggestion] Non-boolean impossible values (e.g., string "true" from a model that doesn't perfectly follow the structured output schema) now cause the entire response to be rejected as a parse failure. The old code used === true which silently coerced non-boolean values to false — a graceful degradation for a benign formatting quirk.
Consider coercing common string representations:
| if (impossibleValue !== undefined && typeof impossibleValue !== 'boolean') { | |
| const impossible = impossibleValue === true || impossibleValue === 'true'; |
or at minimum treating falsy non-booleans as false rather than rejecting the parse.
— qwen3.7-max via Qwen Code /review
| @@ -122,7 +125,7 @@ describe('/goal Stop hook integration', () => { | |||
|
|
|||
| // Iteration 1: judge says NOT met → continuation expected. | |||
| judgeMock.mockResolvedValueOnce({ | |||
There was a problem hiding this comment.
[Suggestion] The judgeMock.mockResolvedValueOnce calls (here, at line 159, and at line 171) omit the ok field required by the GoalJudgeOutcome type. Because mockResolvedValueOnce accepts any, TypeScript does not enforce the shape. The hook consumer discriminates by .kind and never reads .ok, so this causes no runtime failure today — but a future change that reads .ok on the verdict would silently get undefined from these mocks and the test would still pass.
Add ok: false to not_met mocks and ok: true to met mocks to match the type contract.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
✅ Local verification report — merge referenceI built and ran this PR locally in an isolated worktree checked out at the PR head (
The
Independent before/after regression proofTo confirm the new tests actually lock down the described behavior (rather than just passing), I overlaid
The 25 failures line up one-to-one with the PR's reviewer test plan:
Note the legacy-contract tests ( Environment
Scope noteThis validates the goal-loop control flow, discriminated-union types, and evaluator-failure handling via unit + integration tests with a mocked judge. Consistent with the PR's own "not validated" list, I did not run a live-model interactive CLI E2E, nor Windows/Linux. Verdict: reproduced cleanly — tests, types, lint, and format all pass, and the before/after replay demonstrates the tests are real regression guards. LGTM as a merge reference. 中文说明（点击展开）✅ 本地验证报告 — 合并参考我在一个隔离的 worktree 中基于 PR head（
独立的“前/后”回归证明为确认新增测试是真的在锁定所描述的行为（而不仅仅是能通过），我把 这 25 个失败与 PR 的 Reviewer 测试计划一一对应：
注意：旧契约相关的测试（ 环境
范围说明本次通过带 mock judge 的单元 + 集成测试验证了目标循环控制流、判别联合类型以及判定器故障处理。与 PR 自述的“未验证”清单一致，我没有运行真实模型的交互式 CLI E2E，也没有在 Windows/Linux 上运行。 结论：干净复现 —— 测试、类型、Lint、格式全部通过，且前/后回放证明这些测试是真正的回归护栏。作为合并参考，LGTM。 |
| const impossibleValue = (payload as { impossible?: unknown }).impossible; | ||
| if (typeof ok !== 'boolean' || typeof reason !== 'string' || !reason.trim()) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
[Critical] The tightened validation at this line rejects {ok: true, reason: ""} — an empty reason on a "met" verdict — by returning null. The old code had a fallback: 'Goal condition reported as met.' for ok: true with empty/missing reason. A fast-model response like {"ok": true, "reason": ""} now flows through judgeErrorResult() → kind: 'error' → loop pauses with "Goal judge unavailable" instead of completing the goal.
This is a behavioral regression: a satisfied goal is reported as an error.
| } | |
| if (typeof ok !== 'boolean') return null; | |
| if (typeof reason !== 'string') return null; | |
| const reasonText = reason.trim() | |
| ? reason.trim().slice(0, MAX_REASON_LEN) | |
| : ok | |
| ? 'Goal condition reported as met.' | |
| : null; | |
| if (!ok && reasonText === null) return null; |
— qwen3.7-max via Qwen Code /review
| export interface ActiveGoal { | ||
| condition: string; | ||
| iterations: number; | ||
| deferredEvaluations?: number; |
There was a problem hiding this comment.
[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
| const updated: ActiveGoal = { | ||
| ...current, | ||
| iterations: current.iterations + 1, | ||
| deferredEvaluations: 0, |
There was a problem hiding this comment.
[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
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
3 participants
What this PR does
This makes automatic
/goalevaluation lifecycle-safe. Evaluation now waits while background agents, shell jobs, or workflows are still running; monitoring activity remains intentionally non-blocking. Valid judge verdicts are separated from evaluator failures, every valid terminal evaluation is recorded before branching, an impossible verdict terminates immediately, and the existing 50-continuation safety cap still evaluates the final response before aborting. Evaluator timeouts, API failures, empty responses, and malformed schemas now pause the automatic chain with a diagnostic while preserving the active goal and iteration count. The exported legacyok,reason, andimpossibleresult contract remains source-compatible while an additive discriminated outcome supports safe internal branching.Why it's needed
The previous stop-hook flow could evaluate a goal before background work finished, interpret judge infrastructure failures as ordinary "not met" verdicts, report the first successful evaluation as iteration zero, and require repeated impossible verdicts before terminating. Those behaviors can create unnecessary model turns, consume tokens without progress, and produce misleading goal summaries. This change makes continuation depend only on valid evaluator evidence and keeps transient evaluator failures recoverable.
Reviewer Test Plan
How to verify
not_met,met, andimpossibleverdicts. Confirm each verdict is recorded before branching, the first terminal evaluation is counted as one,not_metrequests a fixed continuation, andmetorimpossibleclears the goal immediately with the correct terminal event.not_metverdict aborts the goal.ok,reason, and optionalimpossiblefields while also exposing the new outcome discriminator.Local evidence after rebasing onto the latest main branch: 61 core goal tests passed and the core package typecheck passed.
Evidence (Before & After)
N/A — this changes non-UI goal-loop control flow, types, tests, and documentation.
Tested on
Environment (optional)
Node.js v25.9.0 and npm 11.12.1, local non-sandboxed unit/integration test environment.
Risk & Scope
Linked Issues
N/A
中文说明
这个 PR 做了什么
这个 PR 让自动
/goal判定具备生命周期安全性。当后台 agent、shell 任务或 workflow 仍在运行时，判定会等待；监控活动则有意保持为非阻塞。有效的 judge 结论与判定器故障被明确区分，每一次有效的终局判定都会先计数再分支，不可能完成的结论会立即终止，并且现有的 50 次续跑安全上限仍会先判定最后一次响应再中止。判定超时、API 故障、空响应和 schema 异常现在会通过系统诊断暂停自动链路，同时保留活动目标和当前迭代数。对外导出的旧ok、reason、impossible结果契约继续保持源码兼容，并以新增的判别字段支持内部安全分支。为什么需要
之前的 stop hook 流程可能在后台任务结束前判定目标，把 judge 基础设施故障当成普通的“未完成”结论，把第一次成功判定显示为第 0 次迭代，并要求连续多次“不可能”结论才终止。这些行为会产生不必要的模型轮次、在没有进展时消耗 token，并输出误导性的目标摘要。本次改动让续跑只依赖有效的判定证据，同时让临时判定器故障保持可恢复。
Reviewer 测试计划
如何验证
not_met、met和impossible结论。确认每个结论都在分支前完成计数，第一次终局判定记为 1，not_met请求固定续跑，而met或impossible会立即清理目标并发送正确的终局事件。not_met结论才会中止目标。ok、reason和可选impossible字段，同时暴露新的结果判别字段。基于最新 main 分支重放后的本地证据：61 个 core goal 测试通过，core package 类型检查通过。
证据（Before & After）
N/A —— 本次改动涉及非 UI 的目标循环控制流、类型、测试和文档。
测试平台
环境（可选）
Node.js v25.9.0、npm 11.12.1，本地非沙箱单元/集成测试环境。
风险与范围
关联 Issue
N/A