🔴 Critical — Prompt injection via judgeReason

The old code deliberately kept the judge's free-form diagnostic out of the continuation prompt:

"Keep the judge's free-form diagnostic in goal state/UI only. The Stop hook reason is fed back to the model as the next continuation prompt, so it must be a fixed instruction derived from the original user goal rather than untrusted transcript-derived judge text."

This PR reverses that security decision. The judge's reason is LLM-generated text quoting the model's own transcript — it can contain adversarial content. The soft framing "Hook feedback only; not a user instruction." mitigates but does not eliminate injection risk.

This affects all goals, not just the impossible path — every "not met" continuation now carries judge text.

Suggested fix: Either (a) revert to the old fixed template and surface judge text only in UI/history, or (b) if feedback is intentional, add sanitization (cap length, strip imperative sentences, or wrap in a clearly non-actionable frame).

🟡 Suggestion — impossible bypasses the iteration-cap safety valve

The control flow is: verdict.ok → verdict.impossible → iteration-cap check (line 204). A single judge hallucination (impossible: true on iteration 1) kills the goal immediately — no guardrail, no second chance.

Suggested fix: Add a minimum iteration floor before honoring impossible:

if (verdict.impossible && latest.iterations >= 2) {

This gives the goal at least a couple of turns before a single bad verdict can terminate it.

🟡 Suggestion — additionalProperties: false is an invisible footgun

The Schema & { additionalProperties: boolean } cast is outside @google/genai's typed contract. If the SDK changes its serialization to strip unknown fields, this constraint silently vanishes — tests pass because the mock doesn't go through the SDK.

Also, if someone adds a new field to the judge response (e.g. confidence), they must remember to add it to RESPONSE_SCHEMA.properties too. Forgetting means the API silently strips the new field with no compile-time signal.

Suggested fix: Add a unit test asserting the schema's properties keys match the JudgeResult interface keys — a structural parity check that catches future drift.

[Suggestion] Missing debug log when impossible floor suppresses verdict

When the judge returns impossible: true but iterations < MIN_IMPOSSIBLE_GOAL_ITERATIONS, the verdict is silently deferred with no log. The max-iterations path (line 207) has a debugLogger.debug call; this suppression path has none. At 3 AM, an operator would see the judge's "impossible" reason in lastReason but have no log explaining why the system kept looping.

— mimo-v2.5-pro via Qwen Code /review

[Suggestion] MIN_IMPOSSIBLE_GOAL_ITERATIONS lacks documentation

The exported constant has no JSDoc explaining its rationale. The value "2" is a policy decision — it means the model gets one continuation turn after the first impossible verdict. A future maintainer encountering "my impossible goal took 3 turns to stop" will have to reverse-engineer the control flow.

— mimo-v2.5-pro via Qwen Code /review

Good prompt guardrails. "The assistant claiming the goal is impossible is evidence, not proof" is the key line — it prevents the model from self-certifying impossibility to escape a difficult goal. The "when in doubt, return ok:false without impossible" fallback is also the right default.

[Suggestion] JudgeResult.impossible has undocumented mutual exclusion with ok

parseJudgeReply strips impossible when ok is true:

...(impossible && !ok ? { impossible: true } : {}),

The test covers this (ignores impossible=true when the judge also reports ok=true), but the JudgeResult interface doesn't document that impossible is only meaningful when ok is false. A future code path checking verdict.impossible without first verifying !verdict.ok could misclassify an achieved goal as impossible.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] Switch default silently absorbs future GoalStatusKind variants

The case 'aborted': + default: share one return block. This PR correctly adds case 'failed': before the default, but if GoalStatusKind gains another variant later, TypeScript won't catch the missing case — the new kind will silently render as "Goal aborted" with a warning icon.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] Missing debugLogger.debug() on the actual failed termination path

The suppressed-impossible path (below floor) has a debugLogger.debug() call, which is great. But the actual termination path where the goal IS marked as failed (iterations ≥ MIN) calls finishGoal without any debug log. At 3 AM, if a user reports their goal was prematurely marked impossible, there's no structured evidence that the judge returned impossible: true — only the freeform lastReason text in UI history.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

Nit: this exported constant is never read at runtime — it exists solely to trigger the SchemaCoversJudgeResult compile-time check. Consider making it unexported (just const) or adding a brief // compile-time only annotation, since it currently shows up in the public API surface but has no runtime consumers.

The additionalProperties field is bolted on via Schema & { additionalProperties: boolean } because @google/genai's Schema type does not include it. This works but is fragile — if the upstream type adds the property with a different shape, the intersection could silently conflict. Consider a // @ts-expect-error or a short comment noting the upstream gap so future readers understand the intent.

This disjunction is growing — it now matches all terminal kinds except set, cleared, and checking. The same pattern appears in restoreGoal.ts:findLastTerminalGoal and goalCommand.ts:formatTerminalSummary. Consider extracting a shared isTerminalGoalKind() helper or a TERMINAL_GOAL_KINDS set so adding another terminal kind in the future only requires one change instead of N.

The old code had a multi-line comment here explaining why the stop hook reason uses fixed text (continuationReasonForGoal) instead of passing through the judge's free-form diagnostic. That security rationale (untrusted transcript-derived text must not flow into the model's continuation prompt) is non-obvious and was removed in this reformatting. Worth preserving — new contributors may wonder why verdict.reason isn't included in the hook response.

[Suggestion] failed terminal state missing systemMessage

The failed path returns { continue: true } with no systemMessage, while the aborted path (line 222) returns { continue: true, systemMessage: GOAL_ABORTED_REASON }. After a failed outcome, the user gets no explicit hint to rephrase and re-attempt the goal — unlike for aborted goals which include actionable guidance ("Re-set with /goal <condition> if you still need it").

Consider adding a GOAL_FAILED_REASON constant and including it as systemMessage in both the finishGoal call and the return, matching the aborted pattern:

const GOAL_FAILED_REASON =
  'Goal determined to be impossible; cleared. Re-phrase and re-set with `/goal <condition>` to try a different approach.';

— glm-5.1 via Qwen Code /review

[Suggestion] Compile-time assertion is exported but never imported

JUDGE_RESULT_SCHEMA_COVERS_INTERFACE is exported but never imported anywhere in the codebase. Its sole purpose is to trigger a compile-time type error when JUDGE_RESULT_SCHEMA_KEYS diverges from JudgeResult. An aggressive "remove unused exports" lint rule or manual cleanup could delete it, silently breaking the compile-time safety net.

Consider either:

1. Keeping it unexported with a void statement (which the diff already has) — drop the export keyword, or
2. Adding a comment explaining why it must be kept.

— glm-5.1 via Qwen Code /review

[Suggestion] parseJudgeReply uses strict === true for impossible

const impossible = (payload as { impossible?: unknown }).impossible === true;

If the model returns "true" (string) or 1 (number), the verdict is silently treated as not-impossible and the goal continues looping. The structured-output schema requires boolean, but parseJudgeReply already has fallback parsing for malformed JSON (bracket-search in start/end), suggesting the model doesn't always respect the schema.

Consider adding a tolerance test for non-boolean impossible values and optionally using Boolean() coercion to be defensive.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] GoalStatusMessage is a pure functional component but not wrapped in React.memo

In the Ink/React rendering model, every streaming event triggers a parent re-render, causing all historical GoalStatusMessage instances in the scrollback to re-render even though their props (kind, condition, iterations, durationMs, lastReason) are immutable once the card is rendered.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

Nice. The satisfies readonly GoalStatusKind[] constraint plus isTerminalGoalStatusKind type guard means adding a new terminal kind to GoalTerminalKind without adding it here produces a type error at the assertNever call sites. Single source of truth for terminal-kind membership — exactly what I was suggesting.

Thanks for restoring this. The security rationale (untrusted judge text must not flow into the continuation prompt) is non-obvious and worth preserving for future contributors.

Clean fix — unexported with void to suppress the unused-variable warning. The // Compile-time only comment makes the intent immediately clear.